Skip to content

Commit 441f55f

Browse files
pwscsiropwscsiro
committed
#329 Images - don't delete private image if upload fails, supporting tests
1 parent 37c0932 commit 441f55f

File tree

5 files changed

+347
-41
lines changed

5 files changed

+347
-41
lines changed

‎grails-app/services/au/org/ala/profile/hub/BiocacheService.groovy

+39-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ import au.org.ala.ws.service.WebService
44
import groovy.json.JsonSlurper
55
import org.springframework.web.multipart.MultipartFile
66

7+
import java.nio.file.Files
8+
import java.nio.file.Path
9+
10+
import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES
11+
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
12+
13+
714
class BiocacheService {
815

916
static final int DEFAULT_BIOCACHE_PAGE_SIZE = 50 // the biocache default is only 10, we want more than that
@@ -27,19 +34,43 @@ class BiocacheService {
2734
webService.get("${biocacheImageSearchUrl}?q=${imagesQuery}&fq=multimedia:Image&format=json&im=true&pageSize=${DEFAULT_BIOCACHE_PAGE_SIZE}")
2835
}
2936

30-
def uploadImage(String opusId, String profileId, String dataResourceId, file, Map metadata) {
31-
String imageId = UUID.randomUUID()
3237

33-
String filename
34-
File tempDir = new File("${grailsApplication.config.temp.file.location}")
38+
39+
String copyFileForUpload(String imageId, def file, File tempDir) {
40+
String filename = ''
41+
//Images sent directly to central service on upload
3542
if (file instanceof MultipartFile) {
3643
String extension = file.originalFilename.substring(file.originalFilename.lastIndexOf("."))
3744
filename = "${imageId}${extension}"
3845
file.transferTo(new File(tempDir, "/${filename}"))
46+
//Private and staged images are stored relative to this application, and may be sent to central image service later
3947
} else if (file instanceof File) {
4048
filename = file.getName()
41-
file.renameTo(new File(tempDir, file.getName()));
49+
// file.renameTo(new File(tempDir, file.getName()))
50+
//Make defensive copy in case fails
51+
File fileCopy = new File(tempDir, "/${filename}")
52+
Path target = fileCopy.toPath()
53+
Path source = file.toPath()
54+
Files.copy(source, target, REPLACE_EXISTING, COPY_ATTRIBUTES)
4255
}
56+
filename
57+
}
58+
59+
/**
60+
* Upload works by sending metadata to a central image service which then comes back to fetch the image based
61+
* on information in this metadata??
62+
*
63+
* @param opusId - collection
64+
* @param profileId - the item within the collection
65+
* @param dataResourceId
66+
* @param file - can be a File from disk or a MultipartFile streamed directly
67+
* @param metadata - information about the image
68+
* @return Map response from the webservice including statusCode and resp
69+
*/
70+
def uploadImage(String opusId, String profileId, String dataResourceId, file, Map metadata) {
71+
String imageId = UUID.randomUUID()
72+
File tempDir = new File("${grailsApplication.config.temp.file.location}")
73+
String filename = copyFileForUpload(imageId,file,tempDir)
4374

4475
metadata.multimedia[0].identifier = "${grailsApplication.config.grails.serverURL}/opus/${enc(opusId)}/profile/${enc(profileId)}/file/${enc(filename)}"
4576

@@ -53,7 +84,7 @@ class BiocacheService {
5384

5485
// TODO: REMOVE THIS - IT IS FOR TESTING ONLY!!!!
5586
String hostname = InetAddress.getLocalHost().hostName
56-
if ((hostname != 'nci-profiles-prod'|| hostname == "nci-profiles" || hostname == "nci-profiles-dev" || hostname == "maccy-bm")
87+
if ((hostname != 'nci-profiles-prod' || hostname == "nci-profiles" || hostname == "nci-profiles-dev" || hostname == "maccy-bm")
5788
&& grailsApplication.config.test.collectory.drid.mappings) {
5889
Map drIdMapping = new JsonSlurper().parseText(grailsApplication.config.test.collectory.drid.mappings)
5990

@@ -64,6 +95,8 @@ class BiocacheService {
6495
webService.post("${grailsApplication.config.image.upload.url}${dataResourceId}?apiKey=${grailsApplication.config.image.upload.apiKey}", metadata)
6596
}
6697

98+
99+
67100
private static enc(String str) {
68101
URLEncoder.encode(str, "utf-8")
69102
}

‎grails-app/services/au/org/ala/profile/hub/ImageService.groovy

+41-35
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package au.org.ala.profile.hub
22

33
import au.org.ala.ws.service.WebService
44

5+
6+
57
import static au.org.ala.profile.hub.Utils.*
68
import au.org.ala.images.thumb.ImageThumbnailer
79
import au.org.ala.images.thumb.ThumbDefinition
@@ -418,6 +420,7 @@ class ImageService {
418420
Map profileUpdates = [:]
419421
List<File> imageDirectories = []
420422
List<File> imageFiles = []
423+
List<File> imagesToPublish = []
421424
String localDirPath = staged ? grailsApplication.config.image.staging.dir : grailsApplication.config.image.private.dir
422425
//Check in old file directory structure
423426
File localDir = new File("${localDirPath}/${profile.uuid}")
@@ -434,7 +437,7 @@ class ImageService {
434437
//these directories and take the image files without going into the subdirectories containing
435438
//tiled images and thumbnails
436439
imageDirectories.each { file ->
437-
file.traverse(maxDepth: 1) {
440+
file.traverse(maxDepth: 0) {
438441
if (it.file) {
439442
imageFiles << it
440443
}
@@ -445,44 +448,47 @@ class ImageService {
445448

446449
imageFiles.each {
447450
String imageId = imageIdFromFile(it)
448-
Map localImage = images[imageId]
451+
def image = images[imageId]
452+
if (image) {
453+
imagesToPublish << it
454+
}
455+
}
449456

450-
if (!localImage) {
451-
log.error("Skipping publishing $it with id $imageId because it is missing image metadata.")
452-
// Clean up staged / private file
453-
deleteFileAndDirIfEmpty(imageId, it, localDir)
454-
} else {
455457

456-
List<Map> multimedia = localImage ? [
457-
[
458-
creator : localImage.creator ?: "",
459-
rights : localImage.rights ?: "",
460-
rightsHolder : localImage.rightsHolder ?: "",
461-
license : localImage.licence ?: "",
462-
title : localImage.title ?: "",
463-
description : localImage.description ?: "",
464-
dateCreated : localImage.dateCreated ?: "",
465-
originalFilename: localImage.originalFilename,
466-
contentType : localImage.contentType
467-
]
468-
] : []
469-
Map metadata = [multimedia: multimedia, scientificName: profile.scientificName]
470-
471-
def uploadResponse = biocacheService.uploadImage(opus.uuid, profile.uuid, opus.dataResourceUid, it, metadata)
472-
473-
if (uploadResponse?.resp) {
474-
// check if the local image was set as the primary, and swap the local id for the new permanent id
475-
if (profile.primaryImage == imageId) {
476-
profileUpdates.primaryImage = uploadResponse.resp.images[0]
477-
}
478-
479-
// check for any display options that were set for the local image, and swap the local id for the new permanent id
480-
Map imageDisplayOption = profile.imageSettings?.find { it.imageId == imageId }
481-
if (imageDisplayOption) {
482-
imageDisplayOption?.imageId = uploadResponse.resp.images[0]
483-
}
458+
imagesToPublish.each {
459+
String imageId = imageIdFromFile(it)
460+
List<Map> localImage = [images[imageId]]
461+
List<Map> multimedia = localImage ? [
462+
[
463+
creator : localImage.creator ?: "",
464+
rights : localImage.rights ?: "",
465+
rightsHolder : localImage.rightsHolder ?: "",
466+
license : localImage.licence ?: "",
467+
title : localImage.title ?: "",
468+
description : localImage.description ?: "",
469+
dateCreated : localImage.dateCreated ?: "",
470+
originalFilename: localImage.originalFileName,
471+
contentType : localImage.contentType
472+
]
473+
] : []
474+
Map metadata = [multimedia: multimedia, scientificName: profile.scientificName]
475+
476+
def uploadResponse = biocacheService.uploadImage(opus.uuid, profile.uuid, opus.dataResourceUid, it, metadata)
477+
478+
if (uploadResponse?.resp) {
479+
// check if the local image was set as the primary, and swap the local id for the new permanent id
480+
if (profile.primaryImage == imageId) {
481+
profileUpdates.primaryImage = uploadResponse.resp.images[0]
484482
}
485483

484+
// check for any display options that were set for the local image, and swap the local id for the new permanent id
485+
//that has been assigned by uploadImage call
486+
Map imageDisplayOption = profile.imageSettings?.find { it.imageId == imageId }
487+
if (imageDisplayOption) {
488+
imageDisplayOption?.imageId = uploadResponse.resp.images[0]
489+
}
490+
}
491+
if ((uploadResponse?.statusCode.toInteger() >= 200) && (uploadResponse?.statusCode.toInteger() <= 299)) { //don't delete the local images if the upload failed
486492
if (staged) {
487493
deleteStagedImage(opus.uuid, profile.uuid, imageId)
488494
} else {

‎test/data/puffinswim.jpg

75.1 KB
Loading

‎test/unit/au/org/ala/profile/LocalImage.groovy

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package au.org.ala.profile
2+
3+
import groovy.transform.EqualsAndHashCode
4+
import groovy.transform.ToString
5+
6+
@EqualsAndHashCode
7+
@ToString
8+
class LocalImage {
9+
10+
String imageId
11+
String originalFileName
12+
String title
13+
String description
14+
String rightsHolder
15+
String rights
16+
String licence
17+
String creator
18+
String contentType
19+
Date dateCreated
20+
}

0 commit comments

Comments
 (0)