Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
17c343f
Allow replacing a draft file
qqmyers Oct 16, 2020
279002d
per @scolopasta - set previous datafileId when needed
qqmyers Oct 19, 2020
2cd3d3a
typo
qqmyers Oct 20, 2020
5ecb64c
adding release note
djbrooke Oct 20, 2020
d0f039f
return 400 with bad json
qqmyers Oct 20, 2020
87e2d99
Merge branch 'IQSS/7149-Replace_file_in_draft' of https://github.com/…
qqmyers Oct 20, 2020
83eab44
check fail on bad json in tests, use Bundle
qqmyers Oct 20, 2020
04d36ae
adding lock sleeps
qqmyers Oct 20, 2020
77c6bdf
fix merge issue
qqmyers Oct 21, 2020
8727c20
Merge remote-tracking branch 'IQSS/develop' into IQSS/7149-Replace_fi…
qqmyers Oct 21, 2020
ff47892
debugging: only remove fmd when deleting from draft
qqmyers Oct 21, 2020
be1026f
resolved merge issues - simplifying tbd
qqmyers Oct 22, 2020
e6a4033
support null lock type in sleep
qqmyers Oct 23, 2020
d16b3cb
hopefully cleanup and documentation - no functional change
qqmyers Oct 23, 2020
72475d3
fix duplicate replace button
qqmyers Oct 28, 2020
e7f38f3
!= to .equals()
qqmyers Oct 30, 2020
64c9e88
always merge
qqmyers Oct 30, 2020
c5772fe
fix delete during upload
qqmyers Oct 30, 2020
75f5871
Merge remote-tracking branch 'IQSS/develop' into IQSS/7149-Replace_fi…
qqmyers Nov 3, 2020
c3b4b94
Merge remote-tracking branch 'IQSS/develop' into
qqmyers Nov 10, 2020
4a0279d
grammar
qqmyers Nov 10, 2020
4ba1080
Merge remote-tracking branch 'IQSS/develop' into
qqmyers Jan 8, 2021
d09e945
Merge remote-tracking branch 'IQSS/develop' into IQSS/7149-Replace_fi…
qqmyers Jan 29, 2021
5eb55ce
Merge remote-tracking branch 'IQSS/develop' into
qqmyers Feb 23, 2021
740c3f9
Merge remote-tracking branch 'IQSS/develop' into IQSS/7149-Replace_fi…
qqmyers Mar 7, 2021
518e9ee
add checksumtype support
qqmyers Mar 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/release-notes/7337-replace-draft-file.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Major Use Cases

- Users can now replace files in draft datasets. This functionality was previously only available on published datasets. Issue #7149/PR #7337
36 changes: 10 additions & 26 deletions src/main/java/edu/harvard/iq/dataverse/EditDatafilesPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import edu.harvard.iq.dataverse.util.SystemConfig;
import edu.harvard.iq.dataverse.util.BundleUtil;
import edu.harvard.iq.dataverse.util.EjbUtil;
import edu.harvard.iq.dataverse.util.FileMetadataUtil;

import static edu.harvard.iq.dataverse.util.JsfHelper.JH;
import java.io.File;
import java.io.FileOutputStream;
Expand Down Expand Up @@ -934,8 +936,12 @@ private void deleteFiles(List<FileMetadata> filesForDelete) {
// and let the delete be handled in the command (by adding it to the
// filesToBeDeleted list):

// ToDo - FileMetadataUtil.removeFileMetadataFromList should handle these two
// removes so they could be put after this if clause and the else clause could
// be removed.
dataset.getEditVersion().getFileMetadatas().remove(markedForDelete);
fileMetadatas.remove(markedForDelete);

filesToBeDeleted.add(markedForDelete);
} else {
logger.fine("this is a brand-new (unsaved) filemetadata");
Expand All @@ -948,9 +954,9 @@ private void deleteFiles(List<FileMetadata> filesForDelete) {
// fileMetadatas list. (but doing both just adds a no-op and won't cause an
// error)
// 1. delete the filemetadata from the local display list:
removeFileMetadataFromList(fileMetadatas, markedForDelete);
FileMetadataUtil.removeFileMetadataFromList(fileMetadatas, markedForDelete);
// 2. delete the filemetadata from the version:
removeFileMetadataFromList(dataset.getEditVersion().getFileMetadatas(), markedForDelete);
FileMetadataUtil.removeFileMetadataFromList(dataset.getEditVersion().getFileMetadatas(), markedForDelete);
}

if (markedForDelete.getDataFile().getId() == null) {
Expand All @@ -959,8 +965,8 @@ private void deleteFiles(List<FileMetadata> filesForDelete) {
// removing it from the fileMetadatas lists (above), we also remove it from
// the newFiles list and the dataset's files, so it never gets saved.

removeDataFileFromList(dataset.getFiles(), markedForDelete.getDataFile());
removeDataFileFromList(newFiles, markedForDelete.getDataFile());
FileMetadataUtil.removeDataFileFromList(dataset.getFiles(), markedForDelete.getDataFile());
FileMetadataUtil.removeDataFileFromList(newFiles, markedForDelete.getDataFile());
FileUtil.deleteTempFile(markedForDelete.getDataFile(), dataset, ingestService);
// Also remove checksum from the list of newly uploaded checksums (perhaps odd
// to delete and then try uploading the same file again, but it seems like it
Expand Down Expand Up @@ -989,28 +995,6 @@ private void deleteFiles(List<FileMetadata> filesForDelete) {
}


private void removeFileMetadataFromList(List<FileMetadata> fmds, FileMetadata fmToDelete) {
Iterator<FileMetadata> fmit = fmds.iterator();
while (fmit.hasNext()) {
FileMetadata fmd = fmit.next();
if (fmToDelete.getDataFile().getStorageIdentifier().equals(fmd.getDataFile().getStorageIdentifier())) {
fmit.remove();
break;
}
}
}

private void removeDataFileFromList(List<DataFile> dfs, DataFile dfToDelete) {
Iterator<DataFile> dfit = dfs.iterator();
while (dfit.hasNext()) {
DataFile df = dfit.next();
if (dfToDelete.getStorageIdentifier().equals(df.getStorageIdentifier())) {
dfit.remove();
break;
}
}
}

public String saveWithTermsOfUse() {
logger.fine("saving terms of use, and the dataset version");
return save();
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/api/Datasets.java
Original file line number Diff line number Diff line change
Expand Up @@ -1772,6 +1772,9 @@ public Response addFileToDataset(@PathParam("id") String idSupplied,
} catch (DataFileTagException ex) {
return error( Response.Status.BAD_REQUEST, ex.getMessage());
}
catch (ClassCastException | com.google.gson.JsonParseException ex) {
return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.addreplace.error.parsing"));
}

// -------------------------------------
// (3) Get the file name and content type
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/edu/harvard/iq/dataverse/api/Files.java
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ public Response replaceFileInDataset(
} catch (DataFileTagException ex) {
return error(Response.Status.BAD_REQUEST, ex.getMessage());
}
} catch (ClassCastException ex) {
logger.info("Exception parsing string '" + jsonData + "': " + ex);
} catch (ClassCastException | com.google.gson.JsonParseException ex) {
return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.addreplace.error.parsing"));
}
}

Expand Down Expand Up @@ -358,7 +358,7 @@ public Response updateFileMetadata(@FormDataParam("jsonData") String jsonData,
return error(Response.Status.BAD_REQUEST, ex.getMessage());
}
} catch (ClassCastException | com.google.gson.JsonParseException ex) {
return error(Response.Status.BAD_REQUEST, "Exception parsing provided json");
return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.addreplace.error.parsing"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
import edu.harvard.iq.dataverse.engine.command.impl.AbstractCreateDatasetCommand;
import edu.harvard.iq.dataverse.engine.command.impl.CreateNewDatasetCommand;
import edu.harvard.iq.dataverse.engine.command.impl.DeleteDataFileCommand;
import edu.harvard.iq.dataverse.engine.command.impl.RestrictFileCommand;
import edu.harvard.iq.dataverse.engine.command.impl.UpdateDatasetVersionCommand;
import edu.harvard.iq.dataverse.ingest.IngestServiceBean;
Expand Down Expand Up @@ -502,6 +503,17 @@ public boolean runReplaceFromUI_Phase1(Long oldFileId,
if (!this.step_005_loadFileToReplaceById(oldFileId)){
return false;
}
//Update params to match existing file (except checksum, which should match the new file)
if(fileToReplace != null) {
String checksum = optionalFileParams.getCheckSum();
try {
optionalFileParams = new OptionalFileParams(fileToReplace);
optionalFileParams.setCheckSum(checksum);
} catch (DataFileTagException e) {
// Shouldn't happen since fileToReplace should have valid tags
e.printStackTrace();
}
}

return this.runAddReplacePhase1(fileToReplace.getOwner(),
newFileName,
Expand Down Expand Up @@ -571,6 +583,26 @@ private boolean runAddReplacePhase1(Dataset owner,
return false;
}

// if the fileToReplace hasn't been released,
if (fileToReplace != null && !fileToReplace.isReleased()) {
DataFile df = finalFileList.get(0); // step_055 uses a loop and assumes only one file
// set the replacement file's previous and root datafileIds to match (unless
// they are the defaults)
if (fileToReplace.getPreviousDataFileId() != null) {
df.setPreviousDataFileId(fileToReplace.getPreviousDataFileId());
df.setRootDataFileId(fileToReplace.getRootDataFileId());
}
// Reuse any file PID during a replace operation (if File PIDs are in use)
if (systemConfig.isFilePIDsEnabled()) {
df.setGlobalId(fileToReplace.getGlobalId());
df.setGlobalIdCreateTime(fileToReplace.getGlobalIdCreateTime());
// Should be true or fileToReplace wouldn't have an identifier (since it's not
// yet released in this if statement)
df.setIdentifierRegistered(fileToReplace.isIdentifierRegistered());
fileToReplace.setGlobalId(null);
}
}

return true;
}

Expand Down Expand Up @@ -1058,16 +1090,6 @@ private boolean step_005_loadFileToReplaceById(Long dataFileId){
if (!step_015_auto_check_permissions(existingFile.getOwner())){
return false;
};



// Is the file published?
//
if (!existingFile.isReleased()){
addError(getBundleErr("unpublished_file_cannot_be_replaced"));
return false;
}

// Is the file in the latest dataset version?
//
if (!step_007_auto_isReplacementInLatestVersion(existingFile)){
Expand Down Expand Up @@ -1528,7 +1550,22 @@ private boolean step_070_run_update_dataset_command(){
}

Command<Dataset> update_cmd;
update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, clone);
String deleteStorageLocation = null;
long deleteFileId=-1;
if(isFileReplaceOperation()) {
List<FileMetadata> filesToDelete = new ArrayList<FileMetadata>();
filesToDelete.add(fileToReplace.getFileMetadata());

if(!fileToReplace.isReleased()) {
//If file is only in draft version, also need to delete the physical file
deleteStorageLocation = fileService.getPhysicalFileToDelete(fileToReplace);
deleteFileId=fileToReplace.getId();
}
//Adding the file to the delete list for the command will delete this filemetadata and, if the file hasn't been released, the datafile itself.
update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, filesToDelete, clone);
} else {
update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, clone);
}
((UpdateDatasetVersionCommand) update_cmd).setValidateLenient(true);

try {
Expand All @@ -1550,89 +1587,23 @@ private boolean step_070_run_update_dataset_command(){
this.addErrorSevere("add.add_file_error (see logs)");
logger.severe(ex.getMessage());
return false;
}
return true;
}


/**
* Go through the working DatasetVersion and remove the
* FileMetadata of the file to replace
*
* @return
*/
private boolean step_085_auto_remove_filemetadata_to_replace_from_working_version(){

msgt("step_085_auto_remove_filemetadata_to_replace_from_working_version 1");

if (!isFileReplaceOperation()){
// Shouldn't happen!
this.addErrorSevere(getBundleErr("only_replace_operation") + " (step_085_auto_remove_filemetadata_to_replace_from_working_version");
return false;
}
msg("step_085_auto_remove_filemetadata_to_replace_from_working_version 2");

if (this.hasError()){
return false;
}


msgt("File to replace getId: " + fileToReplace.getId());

Iterator<FileMetadata> fmIt = workingVersion.getFileMetadatas().iterator();
msgt("Clear file to replace");
int cnt = 0;
while (fmIt.hasNext()) {
cnt++;

FileMetadata fm = fmIt.next();
msg(cnt + ") next file: " + fm);
msg(" getDataFile().getId(): " + fm.getDataFile().getId());
if (fm.getDataFile().getId() != null) {
if (Objects.equals(fm.getDataFile().getId(), fileToReplace.getId())) {
msg("Let's remove it!");

// If this is a tabular data file with a UNF, we'll need
// to recalculate the version UNF, once the file is removed:

boolean recalculateUNF = !StringUtils.isEmpty(fm.getDataFile().getUnf());

if (workingVersion.getId() != null) {
// If this is an existing draft (i.e., this draft version
// is already saved in the dataset, we'll also need to remove this filemetadata
// explicitly:
msg(" this is an existing draft version...");
fileService.removeFileMetadata(fm);

// remove the filemetadata from the list of filemetadatas
// attached to the datafile object as well, for a good
// measure:
fileToReplace.getFileMetadatas().remove(fm);
// (and yes, we can do .remove(fm) safely - if this released
// file is part of an existing draft, we know that the
// filemetadata object also exists in the database, and thus
// has the id, and can be identified unambiguously.
}

// and remove it from the list of filemetadatas attached
// to the version object, via the iterator:
fmIt.remove();

if (recalculateUNF) {
msg("recalculating the UNF");
ingestService.recalculateDatasetVersionUNF(workingVersion);
msg("UNF recalculated: "+workingVersion.getUNF());
}

return true;
//Sanity check
if(isFileReplaceOperation()) {
if (deleteStorageLocation != null) {
// Finalize the delete of the physical file
// (File service will double-check that the datafile no
// longer exists in the database, before proceeding to
// delete the physical file)
try {
fileService.finalizeFileDelete(deleteFileId, deleteStorageLocation);
} catch (IOException ioex) {
logger.warning("Failed to delete the physical file associated with the deleted datafile id="
+ deleteFileId + ", storage location: " + deleteStorageLocation);
}
}
}

msg("No matches found!");
addErrorSevere(getBundleErr("failed_to_remove_old_file_from_dataset"));
runMajorCleanup();
return false;
return true;
}


Expand Down Expand Up @@ -1707,13 +1678,6 @@ private boolean step_080_run_update_dataset_command_for_replace(){
return false;
}

// -----------------------------------------------------------
// Remove the "fileToReplace" from the current working version
// -----------------------------------------------------------
if (!step_085_auto_remove_filemetadata_to_replace_from_working_version()){
return false;
}

// -----------------------------------------------------------
// Set the "root file ids" and "previous file ids"
// THIS IS A KEY STEP - SPLIT IT OUT
Expand All @@ -1723,26 +1687,27 @@ private boolean step_080_run_update_dataset_command_for_replace(){
// -----------------------------------------------------------


/*
Check the root file id on fileToReplace, updating it if necessary
*/
if (fileToReplace.getRootDataFileId().equals(DataFile.ROOT_DATAFILE_ID_DEFAULT)){
if (fileToReplace.isReleased()) {
/*
* Check the root file id on fileToReplace, updating it if necessary
*/
if (fileToReplace.getRootDataFileId().equals(DataFile.ROOT_DATAFILE_ID_DEFAULT)) {

fileToReplace.setRootDataFileId(fileToReplace.getId());
fileToReplace = fileService.save(fileToReplace);
}

/*
Go through the final file list, settting the rootFileId and previousFileId
*/
for (DataFile df : finalFileList){
df.setPreviousDataFileId(fileToReplace.getId());

df.setRootDataFileId(fileToReplace.getRootDataFileId());

}
fileToReplace.setRootDataFileId(fileToReplace.getId());
fileToReplace = fileService.save(fileToReplace);
}

/*
* Go through the final file list, settting the rootFileId and previousFileId
*/
for (DataFile df : finalFileList) {
df.setPreviousDataFileId(fileToReplace.getId());

df.setRootDataFileId(fileToReplace.getRootDataFileId());

// Call the update dataset command
}
}
// Call the update dataset command which will delete the replaced filemetadata and file in needed (if file is not released)
//
return step_070_run_update_dataset_command();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,9 @@ public boolean handleNativeFileUpload(InputStream inputStream, String fullStorag
}

OptionalFileParams ofp = null;
ofp = new OptionalFileParams();

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
Expand Down
Loading