#22002

Committed 01 Apr 2024 07:56PM CUT coverage: 20.716% (+0.5%) from 20.173%

Build # #22002

Build Type

push

github

Committed by

web-flow

Commit Message

Merge pull request #10453 from IQSS/develop

Merge 6.2 into master

Run Details

704 of 2679 new or added lines in 152 files covered. (26.28%)

81 existing lines in 49 files now uncovered.

17160 of 82836 relevant lines covered (20.72%)

0.21 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

0.0

/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDataFileCommand.java

package edu.harvard.iq.dataverse.engine.command.impl;

import edu.harvard.iq.dataverse.DataFile;
import edu.harvard.iq.dataverse.GlobalId;
import edu.harvard.iq.dataverse.search.IndexServiceBean;
import edu.harvard.iq.dataverse.authorization.Permission;
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
import edu.harvard.iq.dataverse.engine.command.AbstractVoidCommand;
import edu.harvard.iq.dataverse.engine.command.CommandContext;
import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
import edu.harvard.iq.dataverse.engine.command.RequiredPermissions;
import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
import edu.harvard.iq.dataverse.engine.command.exception.CommandExecutionException;
import edu.harvard.iq.dataverse.engine.command.exception.PermissionException;
import edu.harvard.iq.dataverse.pidproviders.PidProvider;
import edu.harvard.iq.dataverse.pidproviders.PidUtil;
import edu.harvard.iq.dataverse.util.FileUtil;
import edu.harvard.iq.dataverse.util.StringUtil;
import java.io.IOException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Collections;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
 * Deletes a data file, both DB entity and filesystem object.
 *
 * @author michael
 */
@RequiredPermissions(Permission.EditDataset)
public class DeleteDataFileCommand extends AbstractVoidCommand {
    private static final Logger logger = Logger.getLogger(DeleteDataFileCommand.class.getCanonicalName());

    private final DataFile doomed;
    private final boolean destroy;

    public DeleteDataFileCommand(DataFile doomed, DataverseRequest aRequest) {
        this(doomed, aRequest, false);
    }
    
    public DeleteDataFileCommand(DataFile doomed, DataverseRequest aRequest, boolean destroy) {
        super(aRequest, doomed.getOwner());
        this.doomed = doomed;
        this.destroy = destroy;
    }    

    @Override
    protected void executeImpl(CommandContext ctxt) throws CommandException {
        if (destroy) {
            //todo: clean this logic up!
            //for now, if called as destroy, will check for superuser acess
            if (doomed.getOwner().isReleased() && (!(getUser() instanceof AuthenticatedUser) || !getUser().isSuperuser())) {
                throw new PermissionException("Destroy can only be called by superusers.",
                        this, Collections.singleton(Permission.DeleteDatasetDraft), doomed);
            }
        } else // since this is not a destroy, we want to make sure the file is a draft
        // we'll do three sanity checks
        // 1. confirm the file is not released
        // 2. confirm the file is only attached to one version (i.e. only has one fileMetadata)
        // 3. confirm that version is not released
        if (doomed.isReleased() || doomed.getFileMetadatas().size() > 1 || doomed.getFileMetadata().getDatasetVersion().isReleased()) {
            throw new CommandException("Cannot delete file: the DataFile is published, is attached to more than one Dataset Version, or is attached to a released Dataset Version.", this);
        }

        // We need to delete a bunch of physical files, either from the file system,
        // or from some other storage medium where the datafile is stored, 
        // via its StorageIO driver.
        // First we delete the derivative files, then try to delete the data 
        // file itself; if that 
        // fails, we throw an exception and abort the command without
        // trying to remove the object from the database.
        // However, we do not attempt to do any deletes if this is a Harvested
        // file. 
        logger.log(Level.FINE, "Delete command called on an unpublished DataFile {0}", doomed.getId());

        if (!doomed.isHarvested() && !StringUtil.isEmpty(doomed.getStorageIdentifier())) {
            
            // "Package" files need to be treated as a special case, because, 
            // as of now, they are not supported by StorageIO (they only work 
            // with local filesystem as the storage mechanism). 
            
            if (FileUtil.isPackageFile(doomed)) {
                try { 
                    String datasetDirectory = doomed.getOwner().getFileSystemDirectory().toString();
                    Path datasetDirectoryPath = Paths.get(datasetDirectory, doomed.getStorageIdentifier());
                            
                        Files.walkFileTree(datasetDirectoryPath, new SimpleFileVisitor<Path>(){
                        @Override 
                        public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs)
                          throws IOException {
                          Files.delete(file);
                          return FileVisitResult.CONTINUE;
                        }

                        @Override 
                        public FileVisitResult visitFileFailed(final Path file, final IOException e) {
                          return handleException(e);
                        }

                        private FileVisitResult handleException(final IOException e) {
                          logger.warning("Failed to delete file due to"+e.getMessage());
                          return FileVisitResult.TERMINATE;
                        }

                        @Override 
                        public FileVisitResult postVisitDirectory(final Path dir, final IOException e)
                          throws IOException {
                          if(e!=null)return handleException(e);
                          Files.delete(dir);
                          return FileVisitResult.CONTINUE;
                        }
                        }
                );
                    
                } catch (IOException ioex) {
                    throw new CommandExecutionException("Failed to delete package file "+doomed.getStorageIdentifier(), ioex, this);
                }
                
                logger.info("Successfully deleted the package file "+doomed.getStorageIdentifier());
                
            } else {
                logger.fine("Skipping deleting the physical file on the storage volume (will be done outside the command)");
                /* We no longer attempt to delete the physical file from inside the command, 
                 * since commands are executed as (potentially nested) transactions, 
                 * and it is prudent to assume that this database transaction may 
                 * be reversed in the end. Meaning if we delete the file here, 
                 * we are at risk of the database entry not getting deleted, 
                 * leaving a "ghost" DataFile with no associated physical file
                 * on the storage medium. 
                 * The physical file delete must happen outside the transaction, 
                 * once the database delete has been confirmed. 
                 */
                /*
                logger.log(Level.FINE, "Storage identifier for the file: {0}", doomed.getStorageIdentifier());
                StorageIO<DataFile> storageIO = null;

                try {
                    storageIO = doomed.getStorageIO();
                } catch (IOException ioex) {
                    throw new CommandExecutionException("Failed to initialize physical access driver.", ioex, this);
                }

                if (storageIO != null) {

                    // First, delete all the derivative files:
                    // We may have a few extra files associated with this object - 
                    // preserved original that was used in the tabular data ingest, 
                    // cached R data frames, image thumbnails, etc.
                    // We need to delete these too; failures however are less 
                    // important with these. If we fail to delete any of these 
                    // auxiliary files, we'll just leave an error message in the 
                    // log file and proceed deleting the database object.
                    try {
                        storageIO.open();
                    } catch (IOException ioex) {
                        Logger.getLogger(DeleteDataFileCommand.class.getName()).log(Level.SEVERE, "Error calling storageIO.open() while deleting DataFile {0}", doomed.getStorageIdentifier());
                    }
                    // Previously, storageIO.open() and storageIO.deleteAllAuxObjects() were in the same try/catch block
                    // but this was preventing the auxillary objects from being deleted when the main file isn't present
                    // on disk/storage. The call to open() was throwing an IOException so the deleteAllAuxObjects() method
                    // never fired. Now we have them in separate try/catch blocks to go ahead and delete the auxillary
                    // objects even if the main file can't be opened.
                    try {
                        storageIO.deleteAllAuxObjects();
                    } catch (IOException ioex) {
                        Logger.getLogger(DeleteDataFileCommand.class.getName()).log(Level.SEVERE, "Error calling storageIO.deleteAllAuxObjects() while deleting DataFile {0}", doomed.getStorageIdentifier());
                    }

                    // We only want to attempt to delete the main physical file
                    // if it actually exists, on the filesystem or whereever it 
                    // is actually stored by its StorageIO:
                    boolean physicalFileExists = false;

                    try {
                        physicalFileExists = storageIO.exists();
                    } catch (IOException ioex) {
                        // We'll assume that an exception here means that the file does not
                        // exist; so we can skip trying to delete it. 
                        physicalFileExists = false;
                    }

                    
                    if (physicalFileExists) {
                        try {
                            storageIO.delete();
                        } catch (IOException ex) {
                            // This we will treat as a fatal condition:
                            throw new CommandExecutionException("Error deleting physical file object while deleting DataFile " + doomed.getId() + " from the database.", ex, this);
                        }
                    }

                    logger.log(Level.FINE, "Successfully deleted physical storage object (file) for the DataFile {0}", doomed.getId());
                    // Destroy the storageIO object - we will need to purge the 
                    // DataFile from the database (below), so we don't want to have any
                    // objects in this transaction that reference it:
                    storageIO = null;
                    
                }
                */
            }
        }
        GlobalId pid = doomed.getGlobalId();
        if (pid != null) {
            PidProvider pidProvider = PidUtil.getPidProvider(pid.getProviderId());

            try {
                if (pidProvider.alreadyRegistered(doomed)) {
                    pidProvider.deleteIdentifier(doomed);
                }
            } catch (Exception e) {
                logger.log(Level.WARNING, "Identifier deletion was not successfull:", e.getMessage());
            }
        }
        DataFile doomedAndMerged = ctxt.em().merge(doomed);
        ctxt.em().remove(doomedAndMerged);
        /**
         * @todo consider adding an em.flush here (despite the performance
         * impact) if you need to operate on the dataset below. Without the
         * flush, the dataset still thinks it has the file that was just
         * deleted.
         */
        // ctxt.em().flush();

    }
    
    @Override 
    public String describe() {
        StringBuilder sb = new StringBuilder();
        sb.append(super.describe());
        sb.append("DataFile:");
        sb.append(doomed.getId());
        sb.append(" ");
        return sb.toString();
    }

    @Override
    public boolean onSuccess(CommandContext ctxt, Object r) {
        // Adjust the storage use for the parent containers: 
        if (!doomed.isHarvested()) {
            long storedSize = doomed.getFilesize();
            // ingested tabular data files also have saved originals that 
            // are counted as "storage use"
            Long savedOriginalSize = doomed.getOriginalFileSize(); 
            if (savedOriginalSize != null) {
                // Note that DataFile.getFilesize() can return -1 (for "unknown"):
                storedSize = storedSize > 0 ? storedSize + savedOriginalSize : savedOriginalSize; 
            }
            if (storedSize > 0) {
                ctxt.storageUse().incrementStorageSizeRecursively(doomed.getOwner().getId(), (0L - storedSize));
            }
        }
        /**
         * We *could* re-index the entire dataset but it's more efficient to
         * target individual files for deletion, which should always be drafts.
         *
         * See also https://redmine.hmdc.harvard.edu/issues/3786
         */
        String indexingResult = ctxt.index().removeSolrDocFromIndex(IndexServiceBean.solrDocIdentifierFile + doomed.getId() + IndexServiceBean.draftSuffix);
        String indexingResult2 = ctxt.index().removeSolrDocFromIndex(IndexServiceBean.solrDocIdentifierFile + doomed.getId() + IndexServiceBean.draftSuffix + IndexServiceBean.discoverabilityPermissionSuffix);
        /**
        * @todo: check indexing result for success or failure. This method 
        * currently always returns true because the underlying methods 
        * (already existing) handle exceptions and don't return a boolean value.
        * Previously an indexing queuing system was proposed:
        *  https://redmine.hmdc.harvard.edu/issues/3643
        * but we are considering reworking the code such that methods throw
        * indexing exception to callers that may need to handle effects such
        * as on data integrity where related operations like database updates
        * or deletes are expected to be coordinated with indexing operations
        */

        return true;
    }
}

1	package edu.harvard.iq.dataverse.engine.command.impl;
2
3	import edu.harvard.iq.dataverse.DataFile;
4	import edu.harvard.iq.dataverse.GlobalId;
5	import edu.harvard.iq.dataverse.search.IndexServiceBean;
6	import edu.harvard.iq.dataverse.authorization.Permission;
7	import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
8	import edu.harvard.iq.dataverse.engine.command.AbstractVoidCommand;
9	import edu.harvard.iq.dataverse.engine.command.CommandContext;
10	import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
11	import edu.harvard.iq.dataverse.engine.command.RequiredPermissions;
12	import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
13	import edu.harvard.iq.dataverse.engine.command.exception.CommandExecutionException;
14	import edu.harvard.iq.dataverse.engine.command.exception.PermissionException;
15	import edu.harvard.iq.dataverse.pidproviders.PidProvider;
16	import edu.harvard.iq.dataverse.pidproviders.PidUtil;
17	import edu.harvard.iq.dataverse.util.FileUtil;
18	import edu.harvard.iq.dataverse.util.StringUtil;
19	import java.io.IOException;
20	import java.nio.file.FileVisitResult;
21	import java.nio.file.Files;
22	import java.nio.file.Path;
23	import java.nio.file.Paths;
24	import java.nio.file.SimpleFileVisitor;
25	import java.nio.file.attribute.BasicFileAttributes;
26	import java.util.Collections;
27	import java.util.logging.Level;
28	import java.util.logging.Logger;
29
30	/**
31	* Deletes a data file, both DB entity and filesystem object.
32	*
33	* @author michael
34	*/
35	@RequiredPermissions(Permission.EditDataset)
36	public class DeleteDataFileCommand extends AbstractVoidCommand {
37	private static final Logger logger = Logger.getLogger(DeleteDataFileCommand.class.getCanonicalName());	×
38
39	private final DataFile doomed;
40	private final boolean destroy;
41
42	public DeleteDataFileCommand(DataFile doomed, DataverseRequest aRequest) {
43	this(doomed, aRequest, false);	×
44	}	×
45
46	public DeleteDataFileCommand(DataFile doomed, DataverseRequest aRequest, boolean destroy) {
47	super(aRequest, doomed.getOwner());	×
48	this.doomed = doomed;	×
49	this.destroy = destroy;	×
50	}	×
51
52	@Override
53	protected void executeImpl(CommandContext ctxt) throws CommandException {
54	if (destroy) {	×
55	//todo: clean this logic up!
56	//for now, if called as destroy, will check for superuser acess
57	if (doomed.getOwner().isReleased() && (!(getUser() instanceof AuthenticatedUser) \|\| !getUser().isSuperuser())) {	×
58	throw new PermissionException("Destroy can only be called by superusers.",	×
59	this, Collections.singleton(Permission.DeleteDatasetDraft), doomed);	×
60	}
61	} else // since this is not a destroy, we want to make sure the file is a draft
62	// we'll do three sanity checks
63	// 1. confirm the file is not released
64	// 2. confirm the file is only attached to one version (i.e. only has one fileMetadata)
65	// 3. confirm that version is not released
66	if (doomed.isReleased() \|\| doomed.getFileMetadatas().size() > 1 \|\| doomed.getFileMetadata().getDatasetVersion().isReleased()) {	×
67	throw new CommandException("Cannot delete file: the DataFile is published, is attached to more than one Dataset Version, or is attached to a released Dataset Version.", this);	×
68	}
69
70	// We need to delete a bunch of physical files, either from the file system,
71	// or from some other storage medium where the datafile is stored,
72	// via its StorageIO driver.
73	// First we delete the derivative files, then try to delete the data
74	// file itself; if that
75	// fails, we throw an exception and abort the command without
76	// trying to remove the object from the database.
77	// However, we do not attempt to do any deletes if this is a Harvested
78	// file.
79	logger.log(Level.FINE, "Delete command called on an unpublished DataFile {0}", doomed.getId());	×
80
81	if (!doomed.isHarvested() && !StringUtil.isEmpty(doomed.getStorageIdentifier())) {	×
82
83	// "Package" files need to be treated as a special case, because,
84	// as of now, they are not supported by StorageIO (they only work
85	// with local filesystem as the storage mechanism).
86
87	if (FileUtil.isPackageFile(doomed)) {	×
88	try {
89	String datasetDirectory = doomed.getOwner().getFileSystemDirectory().toString();	×
90	Path datasetDirectoryPath = Paths.get(datasetDirectory, doomed.getStorageIdentifier());	×
91
92	Files.walkFileTree(datasetDirectoryPath, new SimpleFileVisitor<Path>(){	×
93	@Override
94	public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs)
95	throws IOException {
96	Files.delete(file);	×
97	return FileVisitResult.CONTINUE;	×
98	}
99
100	@Override
101	public FileVisitResult visitFileFailed(final Path file, final IOException e) {
102	return handleException(e);	×
103	}
104
105	private FileVisitResult handleException(final IOException e) {
106	logger.warning("Failed to delete file due to"+e.getMessage());	×
107	return FileVisitResult.TERMINATE;	×
108	}
109
110	@Override
111	public FileVisitResult postVisitDirectory(final Path dir, final IOException e)
112	throws IOException {
113	if(e!=null)return handleException(e);	×
114	Files.delete(dir);	×
115	return FileVisitResult.CONTINUE;	×
116	}
117	}
118	);
119
120	} catch (IOException ioex) {	×
121	throw new CommandExecutionException("Failed to delete package file "+doomed.getStorageIdentifier(), ioex, this);	×
122	}	×
123
124	logger.info("Successfully deleted the package file "+doomed.getStorageIdentifier());	×
125
126	} else {
127	logger.fine("Skipping deleting the physical file on the storage volume (will be done outside the command)");	×
128	/* We no longer attempt to delete the physical file from inside the command,
129	* since commands are executed as (potentially nested) transactions,
130	* and it is prudent to assume that this database transaction may
131	* be reversed in the end. Meaning if we delete the file here,
132	* we are at risk of the database entry not getting deleted,
133	* leaving a "ghost" DataFile with no associated physical file
134	* on the storage medium.
135	* The physical file delete must happen outside the transaction,
136	* once the database delete has been confirmed.
137	*/
138	/*
139	logger.log(Level.FINE, "Storage identifier for the file: {0}", doomed.getStorageIdentifier());
140	StorageIO<DataFile> storageIO = null;
141
142	try {
143	storageIO = doomed.getStorageIO();
144	} catch (IOException ioex) {
145	throw new CommandExecutionException("Failed to initialize physical access driver.", ioex, this);
146	}
147
148	if (storageIO != null) {
149
150	// First, delete all the derivative files:
151	// We may have a few extra files associated with this object -
152	// preserved original that was used in the tabular data ingest,
153	// cached R data frames, image thumbnails, etc.
154	// We need to delete these too; failures however are less
155	// important with these. If we fail to delete any of these
156	// auxiliary files, we'll just leave an error message in the
157	// log file and proceed deleting the database object.
158	try {
159	storageIO.open();
160	} catch (IOException ioex) {
161	Logger.getLogger(DeleteDataFileCommand.class.getName()).log(Level.SEVERE, "Error calling storageIO.open() while deleting DataFile {0}", doomed.getStorageIdentifier());
162	}
163	// Previously, storageIO.open() and storageIO.deleteAllAuxObjects() were in the same try/catch block
164	// but this was preventing the auxillary objects from being deleted when the main file isn't present
165	// on disk/storage. The call to open() was throwing an IOException so the deleteAllAuxObjects() method
166	// never fired. Now we have them in separate try/catch blocks to go ahead and delete the auxillary
167	// objects even if the main file can't be opened.
168	try {
169	storageIO.deleteAllAuxObjects();
170	} catch (IOException ioex) {
171	Logger.getLogger(DeleteDataFileCommand.class.getName()).log(Level.SEVERE, "Error calling storageIO.deleteAllAuxObjects() while deleting DataFile {0}", doomed.getStorageIdentifier());
172	}
173
174	// We only want to attempt to delete the main physical file
175	// if it actually exists, on the filesystem or whereever it
176	// is actually stored by its StorageIO:
177	boolean physicalFileExists = false;
178
179	try {
180	physicalFileExists = storageIO.exists();
181	} catch (IOException ioex) {
182	// We'll assume that an exception here means that the file does not
183	// exist; so we can skip trying to delete it.
184	physicalFileExists = false;
185	}
186
187
188	if (physicalFileExists) {
189	try {
190	storageIO.delete();
191	} catch (IOException ex) {
192	// This we will treat as a fatal condition:
193	throw new CommandExecutionException("Error deleting physical file object while deleting DataFile " + doomed.getId() + " from the database.", ex, this);
194	}
195	}
196
197	logger.log(Level.FINE, "Successfully deleted physical storage object (file) for the DataFile {0}", doomed.getId());
198	// Destroy the storageIO object - we will need to purge the
199	// DataFile from the database (below), so we don't want to have any
200	// objects in this transaction that reference it:
201	storageIO = null;
202
203	}
204	*/
205	}
206	}
NEW 207	GlobalId pid = doomed.getGlobalId();	×
NEW 208	if (pid != null) {	×
NEW 209	PidProvider pidProvider = PidUtil.getPidProvider(pid.getProviderId());	×
210
211	try {
NEW 212	if (pidProvider.alreadyRegistered(doomed)) {	×
NEW 213	pidProvider.deleteIdentifier(doomed);	×
214	}
NEW 215	} catch (Exception e) {	×
NEW 216	logger.log(Level.WARNING, "Identifier deletion was not successfull:", e.getMessage());	×
217	}	×
218	}
219	DataFile doomedAndMerged = ctxt.em().merge(doomed);	×
220	ctxt.em().remove(doomedAndMerged);	×
221	/**
222	* @todo consider adding an em.flush here (despite the performance
223	* impact) if you need to operate on the dataset below. Without the
224	* flush, the dataset still thinks it has the file that was just
225	* deleted.
226	*/
227	// ctxt.em().flush();
228
229	}	×
230
231	@Override
232	public String describe() {
233	StringBuilder sb = new StringBuilder();	×
234	sb.append(super.describe());	×
235	sb.append("DataFile:");	×
236	sb.append(doomed.getId());	×
237	sb.append(" ");	×
238	return sb.toString();	×
239	}
240
241	@Override
242	public boolean onSuccess(CommandContext ctxt, Object r) {
243	// Adjust the storage use for the parent containers:
244	if (!doomed.isHarvested()) {	×
245	long storedSize = doomed.getFilesize();	×
246	// ingested tabular data files also have saved originals that
247	// are counted as "storage use"
248	Long savedOriginalSize = doomed.getOriginalFileSize();	×
249	if (savedOriginalSize != null) {	×
250	// Note that DataFile.getFilesize() can return -1 (for "unknown"):
251	storedSize = storedSize > 0 ? storedSize + savedOriginalSize : savedOriginalSize;	×
252	}
253	if (storedSize > 0) {	×
254	ctxt.storageUse().incrementStorageSizeRecursively(doomed.getOwner().getId(), (0L - storedSize));	×
255	}
256	}
257	/**
258	* We could re-index the entire dataset but it's more efficient to
259	* target individual files for deletion, which should always be drafts.
260	*
261	* See also https://redmine.hmdc.harvard.edu/issues/3786
262	*/
263	String indexingResult = ctxt.index().removeSolrDocFromIndex(IndexServiceBean.solrDocIdentifierFile + doomed.getId() + IndexServiceBean.draftSuffix);	×
264	String indexingResult2 = ctxt.index().removeSolrDocFromIndex(IndexServiceBean.solrDocIdentifierFile + doomed.getId() + IndexServiceBean.draftSuffix + IndexServiceBean.discoverabilityPermissionSuffix);	×
265	/**
266	* @todo: check indexing result for success or failure. This method
267	* currently always returns true because the underlying methods
268	* (already existing) handle exceptions and don't return a boolean value.
269	* Previously an indexing queuing system was proposed:
270	* https://redmine.hmdc.harvard.edu/issues/3643
271	* but we are considering reworking the code such that methods throw
272	* indexing exception to callers that may need to handle effects such
273	* as on data integrity where related operations like database updates
274	* or deletes are expected to be coordinated with indexing operations
275	*/
276
277	return true;	×
278	}
279	}

IQSS / dataverse / #22002

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous