Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix replace bug #6765

Merged
merged 7 commits into from Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 19 additions & 11 deletions src/main/java/edu/harvard/iq/dataverse/EditDatafilesPage.java
Expand Up @@ -1873,7 +1873,8 @@ private void handleReplaceFileUpload(FacesEvent event, InputStream inputStream,

if (fileReplacePageHelper.handleNativeFileUpload(inputStream,null,
fileName,
contentType
contentType,
null
)){
saveEnabled = true;

Expand Down Expand Up @@ -1932,11 +1933,13 @@ the uploadFinished() method, triggered next, after the upload event

private void handleReplaceFileUpload(String fullStorageLocation,
String fileName,
String contentType){
String contentType,
String checkSum){

fileReplacePageHelper.resetReplaceFileHelper();
saveEnabled = false;
if (fileReplacePageHelper.handleNativeFileUpload(null, fullStorageLocation, fileName, contentType)){
String storageIdentifier = DataAccess.getStorarageIdFromLocation(fullStorageLocation);
if (fileReplacePageHelper.handleNativeFileUpload(null, storageIdentifier, fileName, contentType, checkSum)){
saveEnabled = true;

/**
Expand Down Expand Up @@ -2069,6 +2072,16 @@ public void handleExternalUpload() {
//get file size
long fileSize = sio.getSize();

if(StringUtils.isEmpty(contentType)) {
contentType = FileUtil.MIME_TYPE_UNDETERMINED_DEFAULT;
}

if(DataFile.ChecksumType.fromString(checksumType) != DataFile.ChecksumType.MD5 ) {
String warningMessage = "Non-MD5 checksums not yet supported in external uploads";
localWarningMessage = warningMessage;
//ToDo - methods like handleReplaceFileUpload and classes like OptionalFileParams will need to track the algorithm in addition to the value to enable this
}

/* ----------------------------
Check file size
- Max size NOT specified in db: default is unlimited
Expand All @@ -2083,7 +2096,7 @@ public void handleExternalUpload() {
// Is this a FileReplaceOperation? If so, then diverge!
// -----------------------------------------------------------
if (this.isFileReplaceOperation()){
this.handleReplaceFileUpload(storageLocation, fileName, FileUtil.MIME_TYPE_UNDETERMINED_DEFAULT);
this.handleReplaceFileUpload(storageLocation, fileName, contentType, checksumValue);
this.setFileMetadataSelectedForTagsPopup(fileReplacePageHelper.getNewFileMetadatasBeforeSave().get(0));
return;
}
Expand All @@ -2099,13 +2112,8 @@ public void handleExternalUpload() {
// for example, multiple files can be extracted from an uncompressed
// zip file.
//datafiles = ingestService.createDataFiles(workingVersion, dropBoxStream, fileName, "application/octet-stream");
if(StringUtils.isEmpty(contentType)) {
contentType = "application/octet-stream";
}
if(DataFile.ChecksumType.fromString(checksumType) != DataFile.ChecksumType.MD5 ) {
String warningMessage = "Non-MD5 checksums not yet supported in external uploads";
localWarningMessage = warningMessage;
}


datafiles = FileUtil.createDataFiles(workingVersion, null, fileName, contentType, fullStorageIdentifier, checksumValue, systemConfig);
} catch (IOException ex) {
logger.log(Level.SEVERE, "Error during ingest of file {0}", new Object[]{fileName});
Expand Down
Expand Up @@ -122,6 +122,11 @@ public static String[] getDriverIdAndStorageLocation(String storageLocation) {
}

public static String getStorarageIdFromLocation(String location) {
if(location.contains("://")) {
//It's a full location with a driverId, so strip and reapply the driver id
//NOte that this will strip the bucketname out (which s3 uses) but the S3IOStorage class knows to look at re-insert it
return location.substring(0,location.indexOf("://") +3) + location.substring(location.lastIndexOf('/')+1);
}
return location.substring(location.lastIndexOf('/')+1);
}

Expand Down
40 changes: 31 additions & 9 deletions src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java
Expand Up @@ -165,7 +165,36 @@ public void open(DataAccessOption... options) throws IOException {
if (storageIdentifier == null || "".equals(storageIdentifier)) {
throw new FileNotFoundException("Data Access: No local storage identifier defined for this datafile.");
}


//Fix new DataFiles: DataFiles that have not yet been saved may use this method when they don't have their storageidentifier in the final <driverId>://<bucketname>:<id> form
// So we fix it up here. ToDo: refactor so that storageidentifier is generated by the appropriate StorageIO class and is final from the start.
String newStorageIdentifier = null;
if (storageIdentifier.startsWith(this.driverId + "://")) {
if(!storageIdentifier.substring((this.driverId + "://").length()).contains(":")) {
//Driver id but no bucket
if(bucketName!=null) {
newStorageIdentifier=this.driverId + "://" + bucketName + ":" + storageIdentifier.substring((this.driverId + "://").length());
} else {
throw new IOException("S3AccessIO: DataFile (storage identifier " + storageIdentifier + ") is not associated with a bucket.");
}
} // else we're OK (assumes bucket name in storageidentifier matches the driver's bucketname)
} else {
if(!storageIdentifier.substring((this.driverId + "://").length()).contains(":")) {
//No driver id or bucket
newStorageIdentifier= this.driverId + "://" + bucketName + ":" + storageIdentifier;
} else {
//Just the bucketname
newStorageIdentifier= this.driverId + "://" + storageIdentifier;
}
}
if(newStorageIdentifier != null) {
//Fixup needed:
storageIdentifier = newStorageIdentifier;
dvObject.setStorageIdentifier(newStorageIdentifier);
}


if (isReadAccess) {
key = getMainFileKey();
ObjectMetadata objectMetadata = null;
Expand All @@ -189,14 +218,7 @@ public void open(DataAccessOption... options) throws IOException {

} else if (isWriteAccess) {
key = dataFile.getOwner().getAuthorityForFileStorage() + "/" + this.getDataFile().getOwner().getIdentifierForFileStorage();

if (storageIdentifier.startsWith(this.driverId + "://")) {
key += "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
} else {
key += "/" + storageIdentifier;
dvObject.setStorageIdentifier(this.driverId + "://" + bucketName + ":" + storageIdentifier);
}

key += "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
}

this.setMimeType(dataFile.getContentType());
Expand Down Expand Up @@ -784,7 +806,7 @@ String getMainFileKey() throws IOException {

if (storageIdentifier.startsWith(this.driverId + "://")) {
bucketName = storageIdentifier.substring((this.driverId + "://").length(), storageIdentifier.lastIndexOf(":"));
key = baseKey + "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
key = baseKey + "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
} else {
throw new IOException("S3AccessIO: DataFile (storage identifier " + storageIdentifier + ") does not appear to be an S3 object.");
}
Expand Down
Expand Up @@ -502,10 +502,12 @@ private boolean runAddReplacePhase1(Dataset dataset,
return false;

}

if(optionalFileParams.hasCheckSum()) {
newCheckSum = optionalFileParams.getCheckSum();
if(optionalFileParams != null) {
if(optionalFileParams.hasCheckSum()) {
newCheckSum = optionalFileParams.getCheckSum();
}
}

msgt("step_030_createNewFilesViaIngest");
if (!this.step_030_createNewFilesViaIngest()){
return false;
Expand Down Expand Up @@ -950,9 +952,7 @@ private boolean step_020_loadNewFile(String fileName, String fileContentType, St
if (storageIdentifier == null) {
this.addErrorSevere(getBundleErr("file_upload_failed"));
return false;
} else {
newStorageIdentifier = storageIdentifier;
}
}
}

newFileName = fileName;
Expand Down
Expand Up @@ -94,9 +94,10 @@ public boolean resetReplaceFileHelper(){

/**
* Handle native file replace
* @param checkSum
* @param event
*/
public boolean handleNativeFileUpload(InputStream inputStream, String fullStorageId, String fileName, String fileContentType) {
public boolean handleNativeFileUpload(InputStream inputStream, String fullStorageId, String fileName, String fileContentType, String checkSum) {

phase1Success = false;

Expand All @@ -111,15 +112,25 @@ public boolean handleNativeFileUpload(InputStream inputStream, String fullStorag
if (fileContentType == null){
throw new NullPointerException("fileContentType cannot be null");
}


OptionalFileParams ofp = null;
if(checkSum != null) {
try {
ofp = new OptionalFileParams(null);
} catch (DataFileTagException e) {
//Shouldn't happen with null input
e.printStackTrace();
}
ofp.setCheckSum(checkSum);
}
// Run 1st phase of replace
//
replaceFileHelper.runReplaceFromUI_Phase1(fileToReplace.getId(),
fileName,
fileContentType,
inputStream,
fullStorageId,
null
ofp
);

// Did it work?
Expand Down
Expand Up @@ -217,6 +217,10 @@ public String getMimeType() {
return mimeType;
}

public void setCheckSum(String checkSum) {
this.checkSum = checkSum;
}

public boolean hasCheckSum() {
return ((checkSum!=null)&&(!checkSum.isEmpty()));
}
Expand Down
4 changes: 3 additions & 1 deletion src/main/webapp/resources/js/fileupload.js
Expand Up @@ -24,12 +24,14 @@ function setupDirectUpload(enabled, theDatasetId) {
$('.ui-fileupload-cancel').hide();
//Catch files entered via upload dialog box. Since this 'select' widget is replaced by PF, we need to add a listener again when it is replaced
var fileInput=document.getElementById('datasetForm:fileUpload_input');
fileInput.addEventListener('change', function(event) {
if(fileInput !==null) {
fileInput.addEventListener('change', function(event) {
fileList=[];
for(var i=0;i<fileInput.files.length;i++) {
queueFileForDirectUpload(fileInput.files[i], datasetId);
}
}, {once:false});
}
//Add support for drag and drop. Since the fileUploadForm is not replaced by PF, catching changes with a mutationobserver isn't needed
var fileDropWidget=document.getElementById('datasetForm:fileUpload');
fileDropWidget.addEventListener('drop', function(event) {
Expand Down