Merge pull request #7337 from QualitativeDataRepository/IQSS/7149-Replace_file_in_draft

Allow replacing a draft file

Author: kcondon

doc/release-notes/7337-replace-draft-file.md

@@ -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

src/main/java/edu/harvard/iq/dataverse/EditDatafilesPage.java

@@ -36,6 +36,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;
@@ -796,8 +798,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");
@@ -810,9 +816,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) {
@@ -821,8 +827,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
@@ -851,28 +857,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;
-            }
-        }
-    }
-
 
 
     /**

src/main/java/edu/harvard/iq/dataverse/api/Datasets.java

@@ -1834,6 +1834,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

src/main/java/edu/harvard/iq/dataverse/api/Files.java

@@ -212,8 +212,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"));
             }
         }
 
@@ -357,7 +357,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"));
                 }
             }
 

src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java

@@ -21,6 +21,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;
@@ -504,6 +505,18 @@ 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();
+            ChecksumType checkSumType = optionalFileParams.getCheckSumType();
+            try {
+                optionalFileParams = new OptionalFileParams(fileToReplace);
+                optionalFileParams.setCheckSum(checksum, checkSumType);
+            } catch (DataFileTagException e) {
+                // Shouldn't happen since fileToReplace should have valid tags
+                e.printStackTrace();
+            }
+        }
 
         return this.runAddReplacePhase1(fileToReplace.getOwner(), 
                 newFileName, 
@@ -574,6 +587,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;
     }
 
@@ -1061,16 +1094,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)){
@@ -1532,7 +1555,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 {            
@@ -1554,89 +1592,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;
     }
 
 
@@ -1711,13 +1683,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
@@ -1727,26 +1692,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();
 

src/main/java/edu/harvard/iq/dataverse/datasetutility/FileReplacePageHelper.java

@@ -111,13 +111,8 @@ public boolean handleNativeFileUpload(InputStream inputStream, String fullStorag
         }
 
         OptionalFileParams ofp = null;
+        ofp = new OptionalFileParams();
         if(checkSumValue != null) {
-            try {
-                ofp = new OptionalFileParams(null);
-            } catch (DataFileTagException e) {
-                // Shouldn't happen with null input
-                e.printStackTrace();
-            }
             ofp.setCheckSum(checkSumValue, checkSumType);
         }
         // Run 1st phase of replace