diff --git a/dspace-api/src/main/java/org/dspace/app/itemimport/EmbargoMetadataException.java b/dspace-api/src/main/java/org/dspace/app/itemimport/EmbargoMetadataException.java new file mode 100644 index 000000000000..7316eaaec81c --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/app/itemimport/EmbargoMetadataException.java @@ -0,0 +1,34 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemimport; + +/** + * Thrown when a SAF package claims an embargo ({@code dc.rights.access=embargoedAccess} or + * {@code dc.date.embargoend}) that the import cannot turn into a resource policy. It is checked because the + * package has to be refused: an item archived without its embargo policy gets the collection default READ + * policy from {@code installItem} and its files become public. + */ +public class EmbargoMetadataException extends Exception { + + private static final long serialVersionUID = 1L; + + /** + * @param message what the package asks for and why it cannot be done + */ + public EmbargoMetadataException(String message) { + super(message); + } + + /** + * @param message what the package asks for and why it cannot be done + * @param cause the underlying failure + */ + public EmbargoMetadataException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java index 362872edfe14..7a6b8f069ec4 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java @@ -33,9 +33,11 @@ import java.nio.file.Path; import java.sql.SQLException; import java.text.SimpleDateFormat; +import java.time.LocalDate; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Calendar; import java.util.Date; import java.util.Enumeration; import java.util.GregorianCalendar; @@ -71,6 +73,8 @@ import org.dspace.app.itemimport.service.ItemImportService; import org.dspace.app.util.LocalSchemaFilenameFilter; import org.dspace.app.util.RelationshipUtils; +import org.dspace.app.util.SafEmbargoConstants; +import org.dspace.app.util.SafEmbargoDateParser; import org.dspace.app.util.XMLUtils; import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.ResourcePolicy; @@ -80,7 +84,6 @@ import org.dspace.content.BitstreamFormat; import org.dspace.content.Bundle; import org.dspace.content.Collection; -import org.dspace.content.DCDate; import org.dspace.content.DSpaceObject; import org.dspace.content.Item; import org.dspace.content.MetadataField; @@ -811,7 +814,20 @@ protected Item addItem(Context c, List mycollections, String path, List options = processContentsFile(c, myitem, itemPathDir, "contents"); // Check for embargo metadata and set up embargo terms if needed - processEmbargoMetadata(c, myitem); + // Runs before the workflow is started as well as before installItem: a submission that reaches the + // workflow without its policy is given the collection default READ policy the moment it is approved. + try { + processEmbargoMetadata(c, myitem); + } catch (EmbargoMetadataException e) { + // The half built submission goes with the failure: the batch import path completes its context + // in a finally block, so anything left behind here would be committed as an orphan. + if (wi != null) { + workspaceItemService.deleteAll(c, wi); + } + // The operator needs the package directory, not the item id: the package is what they fix. + throw new EmbargoMetadataException("SAF package '" + itemname + "': " + e.getMessage(), e); + } + if (useWorkflow) { // don't process handle file // start up a workflow @@ -2544,154 +2560,154 @@ private void logError(String message, Exception e) { } /** - * Process embargo metadata and set up ResourcePolicy-based embargo. - * This method checks for embargo metadata fields and directly creates ResourcePolicy - * with embargo start date for Anonymous group READ access. - * - * Handles two scenarios: - * 1. dc.rights.access="embargoedAccess" + dc.date.embargoend (standard embargo) - * 2. Only dc.date.embargoend present (special case with warning logs) + * Set up the ResourcePolicy based embargo of an item being imported, from its own metadata + * ({@code dc.rights.access}, {@code dc.date.embargoend}). A package that claims an embargo either gets its + * policy or fails the import: an item archived without one is given the collection default READ policy by + * {@code installItem}, which publishes files whose own metadata says they are closed. + * + * @param c DSpace context + * @param item item being imported, already carrying the metadata of the package + * @throws SQLException if a database error occurs + * @throws AuthorizeException if the policy may not be written + * @throws EmbargoMetadataException if the package claims an embargo that cannot be written; the item must + * not be archived then */ - protected void processEmbargoMetadata(Context c, Item item) throws SQLException, AuthorizeException { - try { - // Get embargo end date from dc.date.embargoend - List embargoEndDates = itemService.getMetadata(item, "dc", "date", "embargoend", Item.ANY); - - if (embargoEndDates.isEmpty()) { - // No embargo date found, check if there's embargoedAccess without date - List accessRights = itemService.getMetadata(item, "dc", "rights", "access", Item.ANY); - for (MetadataValue accessRight : accessRights) { - if ("embargoedAccess".equals(accessRight.getValue())) { - logError("WARNING: Item has dc.rights.access=embargoedAccess but no dc.date.embargoend. " + - "Cannot set embargo without end date."); - break; - } - } - return; // No embargo to process - } - - String embargoEndDateStr = embargoEndDates.get(0).getValue(); - if (StringUtils.isBlank(embargoEndDateStr)) { - logError("WARNING: dc.date.embargoend is empty. Cannot set embargo."); - return; - } - - // Parse and validate embargo date - DCDate embargoEndDate; - Date endDate; - try { - embargoEndDate = new DCDate(embargoEndDateStr); - endDate = embargoEndDate.toDate(); + protected void processEmbargoMetadata(Context c, Item item) + throws SQLException, AuthorizeException, EmbargoMetadataException { + if (isTest || item == null) { + // A test run creates no item, so there is no metadata to read and no file to disclose. + return; + } - if (endDate == null) { - logError("ERROR: Invalid embargo end date format: " + embargoEndDateStr); - return; - } + List embargoEndDates = itemService.getMetadata(item, "dc", "date", "embargoend", Item.ANY); + boolean hasEmbargoedAccess = hasEmbargoedAccess(item); - if (endDate.before(new Date())) { - logInfo("WARNING: Embargo end date is in the past: " + embargoEndDateStr + - ". Embargo will not be applied."); - return; - } - - // Resource policy start date = embargoend + 1 day - // The embargo end date is the last day of the embargo, - // so the file becomes accessible the day after. - Calendar cal = Calendar.getInstance(); - cal.setTime(endDate); - cal.add(Calendar.DAY_OF_MONTH, 1); - endDate = cal.getTime(); - } catch (Exception e) { - logError("ERROR: Failed to parse embargo end date: " + embargoEndDateStr + - ". Error: " + e.getMessage()); - return; + if (embargoEndDates.isEmpty()) { + if (hasEmbargoedAccess) { + throw new EmbargoMetadataException("dc.rights.access=embargoedAccess without a" + + " dc.date.embargoend. An embargo cannot be set without an end date, and archiving" + + " the item anyway would publish the files the package says are closed."); } + // No embargo metadata at all: the package says nothing about access, the collection defaults decide. + return; + } - // Check embargo scenario - List accessRights = itemService.getMetadata(item, "dc", "rights", "access", Item.ANY); - boolean hasEmbargoedAccess = false; - - for (MetadataValue accessRight : accessRights) { - if ("embargoedAccess".equals(accessRight.getValue())) { - hasEmbargoedAccess = true; - break; - } - } + if (embargoEndDates.size() > 1) { + // Two embargo end dates are a data error only the operator can resolve. + logError("WARNING: Multiple dc.date.embargoend values found. Using first value only."); + } - try { - if (hasEmbargoedAccess) { - // Scenario 1: Standard embargo (embargoedAccess + embargoend) - logInfo("Embargo: Setting standard embargo on item until " + embargoEndDateStr); - applyEmbargoToItemBitstreams(c, item, endDate, "Standard Embargo"); - } else { - // Scenario 2: Only embargo end date present (special case) - logInfo("Embargo: SPECIAL CASE - Found dc.date.embargoend without " + - "dc.rights.access=embargoedAccess"); - logInfo("Embargo: Applying embargo based on end date only until " + embargoEndDateStr); - applyEmbargoToItemBitstreams(c, item, endDate, - "Special Case Embargo - No access rights metadata"); - } - } catch (Exception e) { - logError("ERROR: Failed to apply embargo to bitstreams", e); - } + String embargoEndDateStr = embargoEndDates.get(0).getValue(); + if (StringUtils.isBlank(embargoEndDateStr)) { + throw new EmbargoMetadataException("dc.date.embargoend is present but empty. The field is an" + + " instruction to close the files, it cannot be carried out, and an item whose embargo" + + " could not be written must not be archived."); + } - } catch (Exception e) { - logError("ERROR: Failed to process embargo metadata", e); + // UTC calendar days throughout: the server time zone must not decide a day boundary, and DCDate is + // lenient enough to roll 2026-02-30 over into a real date. Same parser as itemupdate, so one day. + LocalDate embargoEndDay; + try { + embargoEndDay = SafEmbargoDateParser.parseEmbargoEndDay(embargoEndDateStr); + } catch (DateTimeParseException e) { + throw new EmbargoMetadataException("Invalid dc.date.embargoend '" + embargoEndDateStr + "'," + + " expected " + SafEmbargoDateParser.ACCEPTED_FORMATS + ". An end date nobody can read is" + + " not the same as no embargo, so the package is refused instead of archived.", e); + } + + // dc.date.embargoend is the last day of the embargo, so the test below runs on the start day: an + // embargo ending today has to keep the file closed for the rest of today. + LocalDate accessStartDay = embargoEndDay.plusDays(1); + if (!accessStartDay.isAfter(LocalDate.now(ZoneOffset.UTC))) { + logInfo("Embargo: end date " + embargoEndDateStr + " has already passed, no embargo policy" + + " is created. installItem applies the collection default policies, so the files" + + " are as accessible as the collection says."); + return; } + // ResourcePolicy.startDate is mapped @Temporal(DATE), so only the calendar day survives; midnight + // UTC is what the other DSpace embargo paths store for that same day. + Date accessStartDate = Date.from(accessStartDay.atStartOfDay(ZoneOffset.UTC).toInstant()); + + if (hasEmbargoedAccess) { + // Scenario 1: standard embargo (embargoedAccess + embargoend) + logInfo("Embargo: Setting standard embargo on item until " + embargoEndDateStr); + } else { + // Scenario 2: only the embargo end date is present + logInfo("Embargo: SPECIAL CASE - Found dc.date.embargoend without " + + "dc.rights.access=embargoedAccess"); + logInfo("Embargo: Applying embargo based on end date only until " + embargoEndDateStr); + } + // Both scenarios produce the same policy; they used to differ only in an rpName, one of which did + // not fit the 30 character rpname column. + applyEmbargoToItemBitstreams(c, item, accessStartDate, SafEmbargoConstants.EMBARGO_POLICY_NAME); } /** - * Apply embargo ResourcePolicy to all bitstreams in the item. - * Sets READ permission for Anonymous group with the embargo end date as start date. + * Whether the item carries {@code dc.rights.access=embargoedAccess}. + * + * @param item item being imported + * @return true if at least one value says the item is embargoed */ - protected void applyEmbargoToItemBitstreams(Context c, Item item, Date embargoEndDate, String policyReason) - throws SQLException, AuthorizeException { - - try { - // Get Anonymous group - Group anonymousGroup = groupService.findByName(c, Group.ANONYMOUS); - if (anonymousGroup == null) { - logError("ERROR: Cannot find Anonymous group for embargo policy"); - return; + protected boolean hasEmbargoedAccess(Item item) { + for (MetadataValue accessRight : itemService.getMetadata(item, "dc", "rights", "access", Item.ANY)) { + if ("embargoedAccess".equals(StringUtils.trimToEmpty(accessRight.getValue()))) { + return true; } + } + return false; + } - int bitstreamsProcessed = 0; - - // Only process ORIGINAL bundles to avoid affecting system bundles - List originalBundles = item.getBundles("ORIGINAL"); - if (originalBundles.isEmpty()) { - logInfo("Embargo: No ORIGINAL bundles found, no embargo applied"); - return; + /** + * Apply an embargo ResourcePolicy to all ORIGINAL bitstreams of the item. The import path creates the + * policy because the bitstreams have none until {@code installItem} copies the collection defaults, while + * {@code ItemUpdate} works on archived items and re-dates the policy that is already there. + * + * @param c DSpace context + * @param item item being imported + * @param accessStartDate day the files become publicly readable (dc.date.embargoend + 1 day, midnight UTC) + * @param policyReason value for resourcepolicy.rpname, at most 30 characters + * @throws SQLException if a database error occurs + * @throws AuthorizeException if the policy may not be written + * @throws EmbargoMetadataException if the Anonymous group is missing, i.e. the policy cannot be created + */ + protected void applyEmbargoToItemBitstreams(Context c, Item item, Date accessStartDate, String policyReason) + throws SQLException, AuthorizeException, EmbargoMetadataException { + + Group anonymousGroup = groupService.findByName(c, Group.ANONYMOUS); + if (anonymousGroup == null) { + throw new EmbargoMetadataException("Group '" + Group.ANONYMOUS + "' not found, the embargo policy" + + " cannot be created. Archiving the item anyway would leave it with the collection" + + " default policies and no embargo at all."); + } + + // Every bundle that holds the work itself, licence and metadata bundles excepted: a contents file + // can route its files into a bundle of its own with the "bundle:" marker, and leaving those + // out would archive the package the operator declared closed with public files. + int bitstreamsProcessed = 0; + for (Bundle bundle : item.getBundles()) { + if (!SafEmbargoConstants.isEmbargoed(bundle.getName())) { + continue; } + for (Bitstream bitstream : bundle.getBitstreams()) { + // Create ResourcePolicy for READ access with start date = embargo end date + 1 day + ResourcePolicy policy = resourcePolicyService.create(c, null, anonymousGroup); + policy.setdSpaceObject(bitstream); + policy.setAction(Constants.READ); + policy.setStartDate(accessStartDate); + policy.setRpName(policyReason); + // TYPE_CUSTOM grants nothing while the item is still in the workflow, since custom policies + // on a bitstream of an item that is not installed yet are skipped when access is checked. + policy.setRpType(ResourcePolicy.TYPE_CUSTOM); - for (Bundle bundle : originalBundles) { - for (Bitstream bitstream : bundle.getBitstreams()) { - try { - // Create ResourcePolicy for READ access with start date = embargo end date - ResourcePolicy policy = resourcePolicyService.create(c, null, anonymousGroup); - policy.setdSpaceObject(bitstream); - policy.setAction(Constants.READ); - policy.setStartDate(embargoEndDate); - policy.setRpName(policyReason); - - // Add policy to bitstream's existing policies - bitstream.getResourcePolicies().add(policy); - resourcePolicyService.update(c, policy); - bitstreamsProcessed++; - - } catch (Exception e) { - logError("ERROR: Failed to apply embargo policy to bitstream " + bitstream.getName(), e); - } - } + // Add policy to bitstream existing policies + bitstream.getResourcePolicies().add(policy); + resourcePolicyService.update(c, policy); + bitstreamsProcessed++; } - - logInfo("Embargo: Applied embargo policy to " + bitstreamsProcessed + - " bitstreams until " + embargoEndDate.toString()); - - } catch (Exception e) { - logError("ERROR: Failed to apply embargo to item bitstreams", e); - throw e; // Re-throw to maintain method signature contract } + + logInfo("Embargo: Applied embargo policy to " + bitstreamsProcessed + + " bitstreams, readable from " + accessStartDate.toString()); } } diff --git a/dspace-api/src/main/java/org/dspace/app/itemupdate/ItemUpdate.java b/dspace-api/src/main/java/org/dspace/app/itemupdate/ItemUpdate.java index 6ad230ab88eb..847b1db3ed32 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemupdate/ItemUpdate.java +++ b/dspace-api/src/main/java/org/dspace/app/itemupdate/ItemUpdate.java @@ -15,9 +15,11 @@ import java.io.IOException; import java.io.PrintWriter; import java.sql.SQLException; +import java.time.LocalDate; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Calendar; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -31,13 +33,16 @@ import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.dspace.app.util.SafEmbargoConstants; +import org.dspace.app.util.SafEmbargoDateParser; import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.ResourcePolicy; import org.dspace.authorize.factory.AuthorizeServiceFactory; import org.dspace.authorize.service.ResourcePolicyService; import org.dspace.content.Bitstream; import org.dspace.content.Bundle; -import org.dspace.content.DCDate; import org.dspace.content.Item; import org.dspace.content.MetadataValue; import org.dspace.content.factory.ContentServiceFactory; @@ -103,12 +108,22 @@ public class ItemUpdate { protected static final ResourcePolicyService resourcePolicyService = AuthorizeServiceFactory.getInstance() .getResourcePolicyService(); + /** API logging; the operator console output is written by {@link #pr(String)} and its siblings. */ + private static final Logger log = LogManager.getLogger(ItemUpdate.class); + private static final String EMBARGO_FIELD_RIGHTS_ACCESS = "dc.rights.access"; private static final String EMBARGO_FIELD_DATE_END = "dc.date.embargoend"; + private static final String OPEN_ACCESS = "openAccess"; private static final String EMBARGOED_ACCESS = "embargoedAccess"; - private static final String STANDARD_EMBARGO_POLICY_NAME = "Standard Embargo"; - // Must fit resourcepolicy.rpname length (varchar(30)) - private static final String SPECIAL_CASE_EMBARGO_POLICY_NAME = "Special Case Embargo"; + + /** The only supported way of changing access to an ORIGINAL bitstream this tool refuses to touch. */ + private static final String BULK_ACCESS_CONTROL_HINT = + "Use the 'dspace bulk-access-control' script to grant or restore access to it."; + + /** Derived bundles are out of reach of bulk-access-control, which walks the ORIGINAL bundle only. */ + private static final String DERIVATIVE_ACCESS_HINT = + "Change its access in the administrative UI, or let 'dspace filter-media' recreate it from the" + + " source file."; static { filterAliases.put("ORIGINAL", "org.dspace.app.itemupdate.OriginalBitstreamFilter"); @@ -140,6 +155,8 @@ public boolean accept(File dir, String n) { protected ActionManager actionMgr = new ActionManager(); protected List undoActionList = new ArrayList<>(); protected String eperson; + /** Bitstreams and items whose embargo could not be synchronised; a non-zero count fails the run. */ + protected int embargoSyncFailures = 0; /** * @param argv the command line arguments given @@ -382,6 +399,8 @@ public static void main(String[] argv) { context.restoreAuthSystemState(); } + status = exitStatus(status, iu.embargoSyncFailures); + if (isTest) { pr("***End of Test Run***"); } else { @@ -391,6 +410,22 @@ public static void main(String[] argv) { System.exit(status); } + /** + * Exit code of a run. Embargo problems are reported per item without aborting the run, but a run that + * reported them must not exit as a success: a script around this tool sees the exit code only. + * + * @param status exit code the run has produced so far + * @param embargoSyncFailures number of bitstreams/items whose embargo could not be synchronised + * @return {@code 1} when anything went wrong, the unchanged status otherwise + */ + protected static int exitStatus(int status, int embargoSyncFailures) { + if (embargoSyncFailures > 0) { + prErr(embargoSyncFailures + " embargo synchronisation problem(s) reported above."); + return 1; + } + return status; + } + /** * process an archive * @@ -470,7 +505,14 @@ protected void processArchive(Context context, String sourceDirPath, String item if (!isTest) { Item item = itarch.getItem(); if (syncEmbargoPolicies) { - this.syncEmbargoPolicies(context, item); + try { + this.syncEmbargoPolicies(context, item); + } catch (Exception embargoFailure) { + // The catch below only prints the exception, which would leave the run exiting 0. + // Synchronisation is idempotent, so re-running the SAF package repairs the item. + embargoSyncFailures++; + throw embargoFailure; + } } itemService.update(context, item); //need to update before commit context.uncacheEntity(item); @@ -604,9 +646,30 @@ protected void setEPerson(Context context, String eperson) * @param s String */ static void pr(String s) { + log.info(s); System.out.println(s); } + /** + * report something the operator has to look at which does not stop the run + * + * @param s String + */ + static void prWarn(String s) { + log.warn(s); + System.out.println("WARNING: " + s); + } + + /** + * report something that made this tool refuse to do what it was asked to do + * + * @param s String + */ + static void prErr(String s) { + log.error(s); + System.out.println("ERROR: " + s); + } + /** * print if verbose flag is set * @@ -636,119 +699,268 @@ protected static boolean containsEmbargoField(String[] targetFields) { return false; } + /** + * Bring the {@code Anonymous}/{@code READ} resource policies of the item bitstreams in line with the + * embargo metadata of the item ({@code dc.rights.access}, {@code dc.date.embargoend}). The metadata is + * validated before any policy is touched, and the surviving policy is re-dated instead of being replaced, + * so that a failure cannot leave a bitstream without any policy. + * + * @param context DSpace context + * @param item item that has just been updated from the SAF archive + * @throws SQLException if a database error occurs + * @throws AuthorizeException if the policy update is not permitted + */ protected void syncEmbargoPolicies(Context context, Item item) throws SQLException, AuthorizeException { - clearExistingSafEmbargoPolicies(context, item); + if (item.isWithdrawn()) { + prWarn("Item " + itemLabel(item) + " is withdrawn, its bitstream policies are left untouched." + + " A withdrawn item must never regain a READ policy, or the takedown would undo itself" + + " as soon as the embargo lapses."); + return; + } - List embargoEndDates = itemService.getMetadata(item, "dc", "date", "embargoend", Item.ANY); - if (embargoEndDates.size() > 1) { - ItemUpdate.pr("WARNING: Multiple dc.date.embargoend values found. Using first value only."); + if (!item.isArchived()) { + prWarn("Item " + itemLabel(item) + " is not archived (workspace or workflow submission)," + + " its bitstream policies are left untouched."); + return; + } + + List accessRights = itemService.getMetadata(item, "dc", "rights", "access", Item.ANY); + boolean hasEmbargoedAccess = false; + for (MetadataValue accessRight : accessRights) { + String value = StringUtils.trimToEmpty(accessRight.getValue()); + if (EMBARGOED_ACCESS.equals(value)) { + hasEmbargoedAccess = true; + } else if (!OPEN_ACCESS.equals(value)) { + // restrictedAccess, metadataOnlyAccess or an unknown value blocks the whole item, even next + // to an openAccess one: contradictory metadata is not resolved towards disclosure. + prWarn("Item " + itemLabel(item) + " carries " + EMBARGO_FIELD_RIGHTS_ACCESS + "='" + + accessRight.getValue() + "', which is not an embargo access right (" + + OPEN_ACCESS + ", " + EMBARGOED_ACCESS + "), its bitstream policies are left" + + " untouched."); + return; + } } + + List embargoEndDates = itemService.getMetadata(item, "dc", "date", "embargoend", Item.ANY); + if (embargoEndDates.isEmpty()) { - List accessRights = itemService.getMetadata(item, "dc", "rights", "access", Item.ANY); - for (MetadataValue accessRight : accessRights) { - if (EMBARGOED_ACCESS.equals(accessRight.getValue())) { - ItemUpdate.pr("WARNING: Item has dc.rights.access=embargoedAccess but no dc.date.embargoend. " - + "Cannot set embargo without end date."); - break; - } + if (hasEmbargoedAccess) { + prWarn("Item " + itemLabel(item) + " has " + EMBARGO_FIELD_RIGHTS_ACCESS + "=" + EMBARGOED_ACCESS + + " but no " + EMBARGO_FIELD_DATE_END + ". Cannot set an embargo without an end date," + + " its bitstream policies are left untouched."); + embargoSyncFailures++; + return; } + // A missing dc.date.embargoend is no instruction, not a request to lift the embargo: it may have + // been set outside this tool, and this method runs for every item of the batch. + pr("Item " + itemLabel(item) + " has no " + EMBARGO_FIELD_DATE_END + ", resource policies left" + + " untouched. To end an embargo, set " + EMBARGO_FIELD_DATE_END + " to a date in the past."); return; } + if (embargoEndDates.size() > 1) { + prWarn("Multiple " + EMBARGO_FIELD_DATE_END + " values found. Using first value only."); + } + String embargoEndDateStr = embargoEndDates.get(0).getValue(); if (StringUtils.isBlank(embargoEndDateStr)) { - ItemUpdate.pr("WARNING: dc.date.embargoend is empty. Cannot set embargo."); + prErr(EMBARGO_FIELD_DATE_END + " is empty on item " + itemLabel(item) + ", its bitstream policies" + + " are left untouched."); + embargoSyncFailures++; return; } - DCDate embargoEndDate = new DCDate(embargoEndDateStr); - Date endDate = embargoEndDate.toDate(); - if (endDate == null) { - ItemUpdate.pr("ERROR: Invalid embargo end date format: " + embargoEndDateStr); + LocalDate embargoEndDay; + try { + // Strict parsing: DCDate rolls 2026-02-30 over into 2026-03-02 and would turn a typo into a real + // embargo date. The import path uses the same parser, so a package means one day in both tools. + embargoEndDay = SafEmbargoDateParser.parseEmbargoEndDay(embargoEndDateStr); + } catch (DateTimeParseException e) { + prErr("Invalid " + EMBARGO_FIELD_DATE_END + " '" + embargoEndDateStr + "' on item " + + itemLabel(item) + ", expected " + SafEmbargoDateParser.ACCEPTED_FORMATS + ". Its" + + " bitstream policies are left untouched."); + embargoSyncFailures++; return; } - if (endDate.before(new Date())) { - ItemUpdate.pr("WARNING: Embargo end date is in the past: " + embargoEndDateStr - + ". Embargo will not be applied."); - return; - } + // dc.date.embargoend is the last day of the embargo, so access starts the day after at midnight UTC: + // startDate is mapped @Temporal(DATE) and the other DSpace embargo paths store that same day. + LocalDate accessStartDay = embargoEndDay.plusDays(1); + Date accessStartDate = Date.from(accessStartDay.atStartOfDay(ZoneOffset.UTC).toInstant()); - Calendar cal = Calendar.getInstance(); - cal.setTime(endDate); - cal.add(Calendar.DAY_OF_MONTH, 1); - Date accessStartDate = cal.getTime(); + int failuresBefore = embargoSyncFailures; + applyEmbargoToItemBitstreams(context, item, accessStartDate); - List accessRights = itemService.getMetadata(item, "dc", "rights", "access", Item.ANY); - boolean hasEmbargoedAccess = false; - for (MetadataValue accessRight : accessRights) { - if (EMBARGOED_ACCESS.equals(accessRight.getValue())) { - hasEmbargoedAccess = true; - break; - } + if (embargoSyncFailures == failuresBefore && !accessStartDay.isAfter(LocalDate.now(ZoneOffset.UTC))) { + // An expired embargo is a publication, not a deletion: the start date that has already passed + // makes the policy effective immediately. Reported once every bitstream is done, so that a + // bitstream the tool had to refuse is never announced as public. + pr("Embargo of item " + itemLabel(item) + " already expired on " + embargoEndDay + + ", its bitstreams are public since " + accessStartDay + "."); } - - String policyReason = hasEmbargoedAccess ? STANDARD_EMBARGO_POLICY_NAME - : SPECIAL_CASE_EMBARGO_POLICY_NAME; - applyEmbargoToItemBitstreams(context, item, accessStartDate, policyReason); } - protected void clearExistingSafEmbargoPolicies(Context context, Item item) throws SQLException, AuthorizeException { + /** + * Write the target embargo state onto the {@code Anonymous}/{@code READ} policy of every bitstream in + * {@link SafEmbargoConstants#EMBARGOED_BUNDLE_NAMES}, leaving exactly one such policy per bitstream, as a + * second undated one would defeat the embargo. Where there is no such policy none is created: the bitstream + * was not public, and creating one would widen access instead of re-dating it. The embargo belongs to the + * item, so the derived bundles get the same start date as the file they were derived from. + * + * @param context DSpace context + * @param item item whose bitstreams are synchronised + * @param startDate day the files become publicly readable; a day in the past takes effect immediately + * @throws SQLException if a database error occurs + * @throws AuthorizeException if the policy update is not permitted + */ + protected void applyEmbargoToItemBitstreams(Context context, Item item, Date startDate) + throws SQLException, AuthorizeException { Group anonymousGroup = groupService.findByName(context, Group.ANONYMOUS); if (anonymousGroup == null) { + prErr("Group '" + Group.ANONYMOUS + "' not found, no embargo policy was synchronised for item " + + itemLabel(item) + "."); + embargoSyncFailures++; return; } - List originalBundles = item.getBundles(Constants.CONTENT_BUNDLE_NAME); - for (Bundle bundle : originalBundles) { + for (Bundle bundle : item.getBundles()) { + String bundleName = bundle.getName(); + if (!SafEmbargoConstants.isEmbargoed(bundleName)) { + continue; + } for (Bitstream bitstream : bundle.getBitstreams()) { - List readPolicies = resourcePolicyService.find(context, bitstream, Constants.READ); - for (ResourcePolicy policy : readPolicies) { - if (policy.getGroup() != null - && anonymousGroup.equals(policy.getGroup()) - && policy.getStartDate() != null - && (STANDARD_EMBARGO_POLICY_NAME.equals(policy.getRpName()) - || SPECIAL_CASE_EMBARGO_POLICY_NAME.equals(policy.getRpName()))) { - resourcePolicyService.delete(context, policy); - } - } + applyEmbargoToBitstream(context, item, bundleName, bitstream, anonymousGroup, startDate); } } } - protected void applyEmbargoToItemBitstreams(Context context, Item item, Date startDate, String policyReason) - throws SQLException, AuthorizeException { - Group anonymousGroup = groupService.findByName(context, Group.ANONYMOUS); - if (anonymousGroup == null) { + /** + * Synchronise the {@code Anonymous}/{@code READ} policy of a single bitstream, reporting the bundle it + * belongs to so that the operator sees which file of the item a problem is about. + * + * @param context DSpace context + * @param item item the bitstream belongs to + * @param bundleName bundle the bitstream lives in + * @param bitstream bitstream to synchronise + * @param anonymousGroup the {@code Anonymous} group + * @param startDate day the file becomes publicly readable + * @throws SQLException if a database error occurs + * @throws AuthorizeException if the policy update is not permitted + */ + protected void applyEmbargoToBitstream(Context context, Item item, String bundleName, Bitstream bitstream, + Group anonymousGroup, Date startDate) throws SQLException, AuthorizeException { + // Located by (group, action) rather than by rpName: policies written under earlier names have to be + // adopted and normalised instead of being left behind next to a new one. + List anonymousReadPolicies = new ArrayList<>(); + for (ResourcePolicy policy : resourcePolicyService.find(context, bitstream, Constants.READ)) { + if (anonymousGroup.equals(policy.getGroup())) { + anonymousReadPolicies.add(policy); + } + } + + if (anonymousReadPolicies.isEmpty()) { + prErr("Bitstream " + bitstreamLabel(bundleName, bitstream) + " of item " + itemLabel(item) + + " has no " + Group.ANONYMOUS + " READ policy, so there is nothing to re-date and its" + + " embargo could not be synchronised. No policy is created: that would grant access" + + " nobody ever granted. " + accessChangeHint(bundleName)); + embargoSyncFailures++; return; } - List originalBundles = item.getBundles(Constants.CONTENT_BUNDLE_NAME); - for (Bundle bundle : originalBundles) { - for (Bitstream bitstream : bundle.getBitstreams()) { - removeImmediateAnonymousReadPolicies(context, bitstream, anonymousGroup); - - ResourcePolicy policy = resourcePolicyService.create(context, null, anonymousGroup); - policy.setdSpaceObject(bitstream); - policy.setAction(Constants.READ); - policy.setStartDate(startDate); - policy.setRpName(policyReason); - bitstream.getResourcePolicies().add(policy); - resourcePolicyService.update(context, policy); + // A policy with an end date is a lease, not an embargo: re-dating it would either close the file for + // good or drop the end date, and either way decide access for the operator. + ResourcePolicy leasePolicy = firstPolicyWithEndDate(anonymousReadPolicies); + if (leasePolicy != null) { + prErr("Bitstream " + bitstreamLabel(bundleName, bitstream) + " of item " + itemLabel(item) + + " has an " + Group.ANONYMOUS + " READ policy with an end date (policy #" + + leasePolicy.getID() + ", rpName='" + leasePolicy.getRpName() + "'), which this tool" + + " does not manage, so its embargo could not be synchronised and the bitstream is left" + + " untouched. " + accessChangeHint(bundleName)); + embargoSyncFailures++; + return; + } + + ResourcePolicy survivor = selectSurvivorPolicy(anonymousReadPolicies); + survivor.setStartDate(startDate); + survivor.setRpType(ResourcePolicy.TYPE_CUSTOM); + survivor.setRpName(SafEmbargoConstants.EMBARGO_POLICY_NAME); + resourcePolicyService.update(context, survivor); + + // The duplicates go only once the survivor is stored. Reference identity rather than equals(): + // ResourcePolicy.equals compares values, which the lines above just changed. + for (ResourcePolicy policy : anonymousReadPolicies) { + if (policy != survivor) { + resourcePolicyService.delete(context, policy); } } } - protected void removeImmediateAnonymousReadPolicies(Context context, Bitstream bitstream, Group anonymousGroup) - throws SQLException, AuthorizeException { - List readPolicies = resourcePolicyService.find(context, bitstream, Constants.READ); - for (ResourcePolicy policy : readPolicies) { - if (policy.getGroup() != null - && anonymousGroup.equals(policy.getGroup()) - && policy.getStartDate() == null) { - resourcePolicyService.delete(context, policy); + /** + * Bundle, name and id of a bitstream, so that a report identifies exactly one file of the item. + * + * @param bundleName bundle the bitstream lives in + * @param bitstream bitstream to describe + * @return label of the form {@code BUNDLE/name (uuid)} + */ + protected String bitstreamLabel(String bundleName, Bitstream bitstream) { + return bundleName + "/'" + bitstream.getName() + "' (" + bitstream.getID() + ")"; + } + + /** + * How to change access to a bitstream this tool refuses to touch. {@code bulk-access-control} walks the + * ORIGINAL bundle only, so it cannot be recommended for a derived one. + * + * @param bundleName bundle the bitstream lives in + * @return the hint matching the bundle + */ + protected String accessChangeHint(String bundleName) { + return Constants.CONTENT_BUNDLE_NAME.equals(bundleName) ? BULK_ACCESS_CONTROL_HINT + : DERIVATIVE_ACCESS_HINT; + } + + /** + * First policy of the list that expires by itself. + * + * @param anonymousReadPolicies the Anonymous READ policies of a single bitstream + * @return a policy with a non-null end date, or {@code null} when none of them has one + */ + protected ResourcePolicy firstPolicyWithEndDate(List anonymousReadPolicies) { + for (ResourcePolicy policy : anonymousReadPolicies) { + if (policy.getEndDate() != null) { + return policy; + } + } + return null; + } + + /** + * Pick the policy that has been in force the longest: the dated one with the oldest start date or, when none + * of them is dated, the first one. Adopting the newest would resurrect an obsolete embargo date. + * + * @param anonymousReadPolicies the Anonymous READ policies of a single bitstream, never empty + * @return the policy to be mutated into the embargo policy + */ + protected ResourcePolicy selectSurvivorPolicy(List anonymousReadPolicies) { + ResourcePolicy oldestDated = null; + for (ResourcePolicy policy : anonymousReadPolicies) { + if (policy.getStartDate() == null) { + continue; + } + if (oldestDated == null || policy.getStartDate().before(oldestDated.getStartDate())) { + oldestDated = policy; } } + return oldestDated == null ? anonymousReadPolicies.get(0) : oldestDated; + } + + /** + * Handle of the item, or its UUID while it has none. Only used in operator messages. + * + * @param item item to describe + * @return handle or UUID + */ + protected static String itemLabel(Item item) { + return item.getHandle() == null ? String.valueOf(item.getID()) : item.getHandle(); } } //end of class diff --git a/dspace-api/src/main/java/org/dspace/app/util/SafEmbargoConstants.java b/dspace-api/src/main/java/org/dspace/app/util/SafEmbargoConstants.java new file mode 100644 index 000000000000..84f18213217c --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/app/util/SafEmbargoConstants.java @@ -0,0 +1,58 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.util; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.dspace.core.Constants; +import org.dspace.license.service.CreativeCommonsService; + +/** + * Constants of the embargo resource policies written by the SAF batch tools, declared once so that + * {@code dspace import} and {@code dspace itemupdate} cannot drift apart. + */ +public final class SafEmbargoConstants { + + /** + * Value written to {@code resourcepolicy.rpname} on embargo policies. It is the name of the + * {@code embargoed} access condition, as written by the submission UI and {@code bulk-access-control}, and + * it fits the 30 character {@code rpname} column, which a longer name would overflow. + */ + public static final String EMBARGO_POLICY_NAME = "embargo"; + + /** Bundle holding the text extracted from a file; {@link Constants} has no name for it. */ + public static final String TEXT_BUNDLE_NAME = "TEXT"; + + /** Bundle holding the thumbnail rendered from a file; {@link Constants} has no name for it. */ + public static final String THUMBNAIL_BUNDLE_NAME = "THUMBNAIL"; + + /** + * Bundles an embargo never covers, the same three {@code DefaultEmbargoSetter} leaves world readable. + * Everything else is covered: the file, the thumbnail and the extracted full text disclose the embargoed + * work, and so does a file an operator routed into a bundle of their own with the SAF + * {@code bundle:} marker. {@code filter.*.publicPermission} is ignored on purpose. + */ + public static final List NON_EMBARGOED_BUNDLE_NAMES = Collections.unmodifiableList(Arrays.asList( + Constants.LICENSE_BUNDLE_NAME, CreativeCommonsService.CC_BUNDLE_NAME, + Constants.METADATA_BUNDLE_NAME)); + + /** + * Whether an embargo covers the bundle. + * + * @param bundleName name of the bundle + * @return false for the licence and metadata bundles, true for everything that holds the work itself + */ + public static boolean isEmbargoed(String bundleName) { + return !NON_EMBARGOED_BUNDLE_NAMES.contains(bundleName); + } + + private SafEmbargoConstants() { + } +} diff --git a/dspace-api/src/main/java/org/dspace/app/util/SafEmbargoDateParser.java b/dspace-api/src/main/java/org/dspace/app/util/SafEmbargoDateParser.java new file mode 100644 index 000000000000..9902f84c92bb --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/app/util/SafEmbargoDateParser.java @@ -0,0 +1,126 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.util; + +import java.time.LocalDate; +import java.time.Year; +import java.time.YearMonth; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DateTimeParseException; +import java.time.format.ResolverStyle; +import java.time.format.SignStyle; +import java.time.temporal.ChronoField; + +import org.apache.commons.lang3.StringUtils; + +/** + * Turns the {@code dc.date.embargoend} value of a SAF package into the UTC calendar day on which the embargo + * ends, for {@code dspace import} and {@code dspace itemupdate} alike. + * + *

Parsing is strict, unlike the {@link org.dspace.content.DCDate} both tools used before, which rolls + * {@code 2026-02-30} over into 2 March and so turns a typo into a real embargo date. The shapes + * {@code DCDate} accepted are still read, and read as the same day, so existing SAF packages keep working.

+ */ +public final class SafEmbargoDateParser { + + /** + * {@code yyyy-MM-dd'T'HH[:mm[:ss[.fff]]]['Z']}, the ISO shapes {@code DCDate} accepted. Always read as + * UTC, which is what {@code DCDate} assumed for the shapes without a trailing {@code Z}. + */ + private static final DateTimeFormatter LEGACY_TIMESTAMP = new DateTimeFormatterBuilder() + .append(DateTimeFormatter.ISO_LOCAL_DATE) + .appendLiteral('T') + .appendValue(ChronoField.HOUR_OF_DAY, 2) + .optionalStart().appendLiteral(':').appendValue(ChronoField.MINUTE_OF_HOUR, 2) + .optionalStart().appendLiteral(':').appendValue(ChronoField.SECOND_OF_MINUTE, 2) + .optionalStart().appendFraction(ChronoField.NANO_OF_SECOND, 1, 9, true) + .optionalEnd().optionalEnd().optionalEnd() + .optionalStart().appendLiteral('Z').optionalEnd() + .toFormatter().withResolverStyle(ResolverStyle.STRICT); + + /** + * {@code yyyy-M-d} with unpadded month and day, which {@code SimpleDateFormat} accepted and older SAF + * packages therefore contain. Strict all the same: {@code 2027-2-30} is rejected. + */ + private static final DateTimeFormatter UNPADDED_DATE = new DateTimeFormatterBuilder() + .appendValue(ChronoField.YEAR, 4, 10, SignStyle.EXCEEDS_PAD) + .appendLiteral('-') + .appendValue(ChronoField.MONTH_OF_YEAR) + .appendLiteral('-') + .appendValue(ChronoField.DAY_OF_MONTH) + .toFormatter().withResolverStyle(ResolverStyle.STRICT); + + /** + * {@code yyyy-M} with an unpadded month, which {@code SimpleDateFormat} accepted for the same reason. + */ + private static final DateTimeFormatter UNPADDED_YEAR_MONTH = new DateTimeFormatterBuilder() + .appendValue(ChronoField.YEAR, 4, 10, SignStyle.EXCEEDS_PAD) + .appendLiteral('-') + .appendValue(ChronoField.MONTH_OF_YEAR) + .toFormatter().withResolverStyle(ResolverStyle.STRICT); + + /** Listed in operator messages, so that the two tools describe the same set of values. */ + public static final String ACCEPTED_FORMATS = + "yyyy-MM-dd, yyyy-MM (first of the month), yyyy (1 January) or yyyy-MM-dd'T'HH[:mm[:ss]][Z]"; + + private SafEmbargoDateParser() { + } + + /** + * The UTC calendar day on which the embargo ends, i.e. the last day the files stay closed. + * + * @param value raw {@code dc.date.embargoend}, surrounding whitespace is ignored + * @return the embargo end day + * @throws DateTimeParseException if the value is none of the accepted shapes; a caller that cannot read + * the date has to refuse the package instead of assuming no embargo + */ + public static LocalDate parseEmbargoEndDay(String value) { + String trimmed = StringUtils.trimToEmpty(value); + + try { + // yyyy-MM-dd, the shape DSpace itself writes + return LocalDate.parse(trimmed); + } catch (DateTimeParseException notAnIsoDay) { + // an older shape or garbage, decided below + } + + try { + return LocalDate.parse(trimmed, LEGACY_TIMESTAMP); + } catch (DateTimeParseException notAnIsoTimestamp) { + // ditto + } + + try { + // DCDate.toDate() reported a bare month as its first day, so it keeps meaning that day + return YearMonth.parse(trimmed).atDay(1); + } catch (DateTimeParseException notAYearMonth) { + // ditto + } + + try { + // and a bare year as 1 January, for the same reason + return Year.parse(trimmed).atDay(1); + } catch (DateTimeParseException notAYear) { + // ditto + } + + try { + return LocalDate.parse(trimmed, UNPADDED_DATE); + } catch (DateTimeParseException notAnUnpaddedDay) { + // ditto + } + + try { + return YearMonth.parse(trimmed, UNPADDED_YEAR_MONTH).atDay(1); + } catch (DateTimeParseException notAnUnpaddedYearMonth) { + throw new DateTimeParseException("Unparseable embargo end date '" + value + "', expected " + + ACCEPTED_FORMATS, trimmed, notAnUnpaddedYearMonth.getErrorIndex()); + } + } +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemimport/EmbargoImportIT.java b/dspace-api/src/test/java/org/dspace/app/itemimport/EmbargoImportIT.java index 9f536bc27b2d..a2b32029de3f 100644 --- a/dspace-api/src/test/java/org/dspace/app/itemimport/EmbargoImportIT.java +++ b/dspace-api/src/test/java/org/dspace/app/itemimport/EmbargoImportIT.java @@ -8,41 +8,67 @@ package org.dspace.app.itemimport; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.File; import java.nio.file.Files; import java.nio.file.Path; +import java.sql.SQLException; import java.text.SimpleDateFormat; import java.time.LocalDate; +import java.time.YearMonth; +import java.time.ZoneOffset; +import java.util.Collections; +import java.util.Date; +import java.util.Iterator; import java.util.List; +import java.util.TimeZone; +import java.util.UUID; import java.util.stream.Collectors; +import javax.servlet.http.HttpServletRequest; import org.apache.commons.io.file.PathUtils; import org.dspace.AbstractIntegrationTestWithDatabase; +import org.dspace.app.itemimport.factory.ItemImportServiceFactory; import org.dspace.authorize.ResourcePolicy; import org.dspace.authorize.factory.AuthorizeServiceFactory; +import org.dspace.authorize.service.AuthorizeService; import org.dspace.authorize.service.ResourcePolicyService; import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; import org.dspace.builder.MetadataFieldBuilder; import org.dspace.content.Bitstream; +import org.dspace.content.Bundle; import org.dspace.content.Collection; import org.dspace.content.Item; import org.dspace.content.MetadataSchema; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.ItemService; import org.dspace.content.service.MetadataSchemaService; +import org.dspace.content.service.WorkspaceItemService; import org.dspace.core.Constants; +import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.GroupService; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; +import org.dspace.xmlworkflow.factory.XmlWorkflowServiceFactory; +import org.dspace.xmlworkflow.service.XmlWorkflowService; +import org.dspace.xmlworkflow.state.Workflow; +import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem; +import org.dspace.xmlworkflow.storedcomponents.service.XmlWorkflowItemService; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; /** * Integration tests for embargo functionality in SAF Import feature. @@ -60,15 +86,27 @@ public class EmbargoImportIT extends AbstractIntegrationTestWithDatabase { private static final String EXPECTED_POLICY_START_DATE = EMBARGO_END_FUTURE.plusDays(1).toString(); private static final String EMBARGOEND_DATE_PAST = "2020-01-01"; private static final String ITEM_TITLE = "Test Embargo Item"; + /** + * The single rpName both SAF tools write. Repeated here rather than referenced, because the value ends up + * in the database and has to fit the 30 character {@code resourcepolicy.rpname} column. + */ + private static final String EMBARGO_POLICY_NAME = "embargo"; private ItemService itemService = ContentServiceFactory.getInstance().getItemService(); + private WorkspaceItemService workspaceItemService = + ContentServiceFactory.getInstance().getWorkspaceItemService(); private ResourcePolicyService resourcePolicyService = AuthorizeServiceFactory.getInstance().getResourcePolicyService(); + private AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService(); private GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); private MetadataSchemaService metadataSchemaService = ContentServiceFactory.getInstance().getMetadataSchemaService(); + private XmlWorkflowService xmlWorkflowService = + XmlWorkflowServiceFactory.getInstance().getXmlWorkflowService(); + private XmlWorkflowItemService xmlWorkflowItemService = + XmlWorkflowServiceFactory.getInstance().getXmlWorkflowItemService(); private Collection collection; private Path tempDir; @@ -119,6 +157,7 @@ public void setUp() throws Exception { @After @Override public void destroy() throws Exception { + deleteRemainingWorkflowItems(); PathUtils.deleteDirectory(tempDir); for (Path path : Files.list(workDir).collect(Collectors.toList())) { PathUtils.delete(path); @@ -165,25 +204,32 @@ public void testStandardEmbargoImport() throws Exception { assertEquals("Should have one bitstream", 1, bitstreams.size()); Bitstream bitstream = bitstreams.get(0); - List policies = resourcePolicyService.find(context, bitstream, Constants.READ); - // Should have embargo policy for Anonymous group - ResourcePolicy embargoPolicy = policies.stream() - .filter(p -> p.getGroup() != null && p.getGroup().equals(anonymousGroup)) - .findFirst() - .orElse(null); + // installItem clones the collection's undated DEFAULT_BITSTREAM_READ onto a bitstream without an + // Anonymous READ policy, and such a second policy would open the file throughout the embargo. + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals("exactly one Anonymous READ policy may remain: " + describe(bitstream), + 1, anonymousRead.size()); - assertNotNull("Should have embargo policy for Anonymous group", embargoPolicy); + ResourcePolicy embargoPolicy = anonymousRead.get(0); assertNotNull("Embargo policy should have start date", embargoPolicy.getStartDate()); + assertEquals("the embargo policy has to be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertEquals("the embargo policy has to carry the access condition name", + EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); // Verify start date is embargoend + 1 day (file becomes accessible day after embargo ends) SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); assertEquals("Embargo policy start date should be embargoend + 1 day", EXPECTED_POLICY_START_DATE, sdf.format(embargoPolicy.getStartDate())); + + assertFalse("an embargoed file must not be downloadable by an anonymous visitor: " + describe(bitstream), + anonymousCanRead(bitstream)); } /** - * Test that no embargo is applied when embargo date is in the past + * Verifies that an embargo which has already expired produces no policy and leaves the file readable, + * which on the import path means the collection default policies applied by installItem. */ @Test public void testPastEmbargoDateNoPolicy() throws Exception { @@ -226,6 +272,256 @@ public void testPastEmbargoDateNoPolicy() throws Exception { p.getStartDate() != null); assertTrue("Should not have embargo policy for past dates", !hasEmbargoPolicy); + + // The assertion above is also satisfied by a bitstream with no policy at all, which is unreadable. + assertTrue("An expired embargo end date must leave the bitstream readable, not policy-less", + anonymousCanRead(bitstream)); + } + + /** + * Verifies that an embargo imported with {@code -w} survives approval. Approval calls {@code installItem}, + * which clones the collection's undated default onto any bitstream without an Anonymous READ policy, so + * the embargo policy has to exist before the workflow starts. + */ + @Test + public void testWorkflowEmbargoSurvivesApproval() throws Exception { + context.turnOffAuthorisationSystem(); + Collection workflowCollection = CollectionBuilder.createCollection(context, parentCommunity) + .withName("Workflow Collection") + .withWorkflowGroup(1, admin) + .build(); + context.restoreAuthSystemState(); + + Path safDir = Files.createDirectory(Path.of(tempDir.toString() + "/test")); + Path itemDir = Files.createDirectory(Path.of(safDir.toString() + "/item_000")); + + String dublinCoreContent = "\n" + + "\n" + + " " + ITEM_TITLE + "\n" + + " embargoedAccess\n" + + " " + EMBARGOEND_DATE_FUTURE + "\n" + + ""; + Files.writeString(Path.of(itemDir.toString() + "/dublin_core.xml"), dublinCoreContent); + + Path contentsFile = Files.createFile(Path.of(itemDir.toString() + "/contents")); + Files.writeString(contentsFile, "test.txt"); + Path bitstreamFile = Files.createFile(Path.of(itemDir.toString() + "/test.txt")); + Files.writeString(bitstreamFile, "TEST CONTENT FOR WORKFLOW EMBARGO"); + + // -w: the submission goes through the workflow instead of straight into the archive + String[] args = new String[] { "import", "-a", "-w", "-e", admin.getEmail(), + "-c", workflowCollection.getID().toString(), + "-s", safDir.toString(), "-m", tempDir.toString() + "/mapfile.out" }; + runDSpaceScript(args); + + // findByMetadataField only returns archived items and this one is still in the workflow, so the + // mapfile is what identifies it: " ". + Item item = itemFromMapfile(tempDir.toString() + "/mapfile.out"); + assertFalse("fixture precondition: -w must leave the item in the workflow, not in the archive", + item.isArchived()); + + Bitstream bitstream = item.getBundles("ORIGINAL").get(0).getBitstreams().get(0); + assertFalse("a submission waiting for approval must not be downloadable: " + describe(bitstream), + anonymousCanRead(bitstream)); + + approveWorkflowItem(workflowCollection, item); + + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + assertTrue("fixture precondition: approving the workflow item must archive it", item.isArchived()); + + // Without the embargo policy the bitstream would reach installItem with no Anonymous READ policy and + // get the collection default cloned onto it. + assertFalse("approving an embargoed submission published its files. dc.date.embargoend is " + + EMBARGOEND_DATE_FUTURE + ", so the file has to stay closed: " + describe(bitstream), + anonymousCanRead(bitstream)); + + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals("exactly one Anonymous READ policy may remain after approval - a second, undated one is" + + " the collection default and defeats the embargo: " + describe(bitstream), + 1, anonymousRead.size()); + + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertNotNull("the surviving Anonymous READ policy has to be dated", embargoPolicy.getStartDate()); + assertEquals("the embargo policy has to carry the access condition name", + EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertEquals("the embargo policy has to be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); + assertEquals("the embargo has to start the day after dc.date.embargoend", + EXPECTED_POLICY_START_DATE, sdf.format(embargoPolicy.getStartDate())); + } + + /** + * Verifies the branch where {@code dc.date.embargoend} arrives without + * {@code dc.rights.access=embargoedAccess}: it embargoes the files under the same, short enough rpName. + */ + @Test + public void testEmbargoEndWithoutAccessRightsStillEmbargoes() throws Exception { + Path safDir = Files.createDirectory(Path.of(tempDir.toString() + "/test")); + Path itemDir = Files.createDirectory(Path.of(safDir.toString() + "/item_000")); + + // no dc.rights.access at all - this is the branch under test + String dublinCoreContent = "\n" + + "\n" + + " " + ITEM_TITLE + "\n" + + " " + EMBARGOEND_DATE_FUTURE + "\n" + + ""; + Files.writeString(Path.of(itemDir.toString() + "/dublin_core.xml"), dublinCoreContent); + + Path contentsFile = Files.createFile(Path.of(itemDir.toString() + "/contents")); + Files.writeString(contentsFile, "test.txt"); + Path bitstreamFile = Files.createFile(Path.of(itemDir.toString() + "/test.txt")); + Files.writeString(bitstreamFile, "TEST CONTENT FOR SPECIAL CASE EMBARGO"); + + String[] args = new String[] { "import", "-a", "-e", admin.getEmail(), "-c", collection.getID().toString(), + "-s", safDir.toString(), "-m", tempDir.toString() + "/mapfile.out" }; + runDSpaceScript(args); + + Item item = itemService.findByMetadataField(context, "dc", "title", null, ITEM_TITLE).next(); + assertNotNull("Item should be created", item); + assertTrue("the item has to be archived, i.e. the import must not have been aborted by an SQL error", + item.isArchived()); + + Bitstream bitstream = item.getBundles("ORIGINAL").get(0).getBitstreams().get(0); + + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals("exactly one Anonymous READ policy may remain: " + describe(bitstream), + 1, anonymousRead.size()); + + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertNotNull("the special case branch has to write a dated policy too", embargoPolicy.getStartDate()); + assertEquals("both branches write the same rpName", EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertTrue("rpname is a varchar(30) column, so the value has to fit into it", + embargoPolicy.getRpName().length() <= 30); + assertEquals("the embargo policy has to be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); + assertEquals("the embargo has to start the day after dc.date.embargoend", + EXPECTED_POLICY_START_DATE, sdf.format(embargoPolicy.getStartDate())); + + assertFalse("a dc.date.embargoend in the future closes the file even without dc.rights.access: " + + describe(bitstream), anonymousCanRead(bitstream)); + } + + /** + * The single item the import reported in its mapfile, looked up by id. Works for workflow imports too, + * where the item is not in the archive yet and therefore invisible to {@code findByMetadataField}. + */ + private Item itemFromMapfile(String mapfilePath) throws Exception { + List lines = Files.readAllLines(Path.of(mapfilePath)); + assertEquals("the import has to report exactly one item in its mapfile, got " + lines, + 1, lines.size()); + String[] columns = lines.get(0).trim().split("\\s+"); + assertEquals("a mapfile line is ' ', got '" + lines.get(0) + "'", + 2, columns.length); + + Item item = itemService.find(context, UUID.fromString(columns[1])); + assertNotNull("the item named in the mapfile has to exist", item); + return item; + } + + /** + * A workflow item that outlives its test keeps a {@code cwf_pooltask} row referencing the workflow group, + * which then blocks {@code AbstractBuilder.cleanupObjects()} from deleting that group. + */ + private void deleteRemainingWorkflowItems() throws Exception { + if (context == null || !context.isValid()) { + return; + } + context.turnOffAuthorisationSystem(); + try { + for (XmlWorkflowItem workflowItem : xmlWorkflowItemService.findAll(context)) { + xmlWorkflowItemService.delete(context, workflowItem); + } + context.commit(); + } finally { + context.restoreAuthSystemState(); + } + } + + /** + * Claims and approves the single review task of a workflow item, which is what finally calls + * {@code installItem}. + */ + private void approveWorkflowItem(Collection workflowCollection, Item item) throws Exception { + XmlWorkflowItem workflowItem = xmlWorkflowItemService.findByItem(context, item); + assertNotNull("fixture precondition: the imported item has to be a workflow item", workflowItem); + + Workflow workflow = XmlWorkflowServiceFactory.getInstance().getWorkflowFactory() + .getWorkflow(workflowCollection); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setParameter("submit_approve", "submit_approve"); + HttpServletRequest servletRequest = request; + + EPerson previousUser = context.getCurrentUser(); + context.setCurrentUser(admin); + try { + Integer workflowItemId = workflowItem.getID(); + xmlWorkflowService.doState(context, admin, servletRequest, workflowItemId, workflow, + workflow.getStep("reviewstep").getActionConfig("claimaction")); + xmlWorkflowService.doState(context, admin, servletRequest, workflowItemId, workflow, + workflow.getStep("reviewstep").getActionConfig("reviewaction")); + } finally { + context.setCurrentUser(previousUser); + } + context.commit(); + } + + private List anonymousReadPolicies(Bitstream bitstream) throws Exception { + return resourcePolicyService.find(context, bitstream, Constants.READ).stream() + .filter(policy -> policy.getGroup() != null && policy.getGroup().equals(anonymousGroup)) + .collect(Collectors.toList()); + } + + /** + * Every resource policy of the bitstream, for failure messages: an extra undated policy and a missing one + * are otherwise indistinguishable. + */ + private String describe(Bitstream bitstream) throws Exception { + StringBuilder sb = new StringBuilder(System.lineSeparator()); + sb.append(" bitstream=").append(bitstream.getID()).append(System.lineSeparator()) + .append(" anonymousCanRead=").append(anonymousCanRead(bitstream)) + .append(System.lineSeparator()); + List policies = resourcePolicyService.find(context, bitstream, Constants.READ); + if (policies.isEmpty()) { + sb.append(" ").append(System.lineSeparator()); + } + for (ResourcePolicy policy : policies) { + sb.append(String.format(" id=%s group=%s rpType=%s rpName=%s start=%s end=%s", + policy.getID(), + policy.getGroup() == null ? "" : policy.getGroup().getName(), + policy.getRpType(), + policy.getRpName(), + policy.getStartDate(), + policy.getEndDate())) + .append(System.lineSeparator()); + } + return sb.toString(); + } + + /** + * Tells whether a visitor who is not logged in may read the bitstream, with the test's own + * turnOffAuthorisationSystem calls temporarily unwound. + */ + private boolean anonymousCanRead(Bitstream bitstream) throws Exception { + EPerson savedUser = context.getCurrentUser(); + int popped = 0; + while (context.ignoreAuthorization()) { + context.restoreAuthSystemState(); + popped++; + } + context.setCurrentUser(null); + try { + return authorizeService.authorizeActionBoolean(context, bitstream, Constants.READ); + } finally { + context.setCurrentUser(savedUser); + for (int i = 0; i < popped; i++) { + context.turnOffAuthorisationSystem(); + } + } } /** @@ -273,48 +569,355 @@ public void testNoEmbargoMetadataNoPolicy() throws Exception { } /** - * Test embargo with invalid date format + * Verifies that a {@code dc.date.embargoend} no date parser accepts is reported and leaves no readable + * file behind. The shapes {@code DCDate} did accept are still imported, see the format tests below. */ @Test public void testInvalidEmbargoDateFormat() throws Exception { - // Create SAF with invalid embargo date format + assertBrokenEmbargoPackageIsRefused("invalid-date-format", "an unparseable dc.date.embargoend"); + } + + /** + * Verifies that a date only a lenient parser would accept is refused; reading 30 February as 2 March turns + * a typo into a real embargo date. + */ + @Test + public void testLenientRollOverEmbargoDateIsRefused() throws Exception { + int year = LocalDate.now(ZoneOffset.UTC).getYear() + 1; + assertBrokenEmbargoPackageIsRefused(year + "-02-30", "a dc.date.embargoend that does not exist"); + } + + /** + * Verifies that {@code embargoedAccess} without an end date is refused: archiving it would leave an item + * whose metadata says closed while the collection default policies make the files public. + */ + @Test + public void testEmbargoedAccessWithoutEndDateIsRefused() throws Exception { + Path itemDir = safPackage("embargoedAccess", null, "TEST CONTENT NO END DATE"); + + Exception reported = runImport(itemDir.getParent()); + assertNotNull("dc.rights.access=embargoedAccess without dc.date.embargoend has to be reported to the" + + " operator instead of being archived as a public item: " + describeArchived(ITEM_TITLE), + reported); + assertNoFileOfTheItemIsPublic("embargoedAccess without an end date"); + } + + /** + * Verifies that a present but empty {@code dc.date.embargoend} is refused rather than read as "no + * embargo"; it is a broken export. + */ + @Test + public void testBlankEmbargoEndIsRefused() throws Exception { + assertBrokenEmbargoPackageIsRefused("", "an empty dc.date.embargoend"); + } + + /** + * Verifies that a package whose payload sits in a bundle of its own is embargoed too. Only the licence + * and metadata bundles stay world readable, as in {@code DefaultEmbargoSetter}; anything else holds the + * work and would otherwise be published by the collection default policies. + */ + @Test + public void testCustomBundlePayloadIsEmbargoed() throws Exception { + Path itemDir = safPackage("embargoedAccess", EMBARGOEND_DATE_FUTURE, "TEST CONTENT CUSTOM BUNDLE", + "SUPPLEMENT"); + + assertNull("a package whose payload is in a custom bundle has to import", runImport(itemDir.getParent())); + + Iterator archived = itemService.findByMetadataField(context, "dc", "title", null, ITEM_TITLE); + assertTrue("the package has to be archived", archived.hasNext()); + List bundles = archived.next().getBundles("SUPPLEMENT"); + assertFalse("fixture precondition: the payload must be in the SUPPLEMENT bundle", bundles.isEmpty()); + + for (Bitstream bitstream : bundles.get(0).getBitstreams()) { + assertFalse("the package says dc.rights.access=embargoedAccess, so a file in a bundle of its own" + + " must not be readable by an anonymous visitor either: " + describe(bitstream), + anonymousCanRead(bitstream)); + } + } + + /** + * Verifies that a refused package leaves no workspace item behind. The command line path aborts its + * context, but the batch import path completes its own in a finally block, so a half built submission + * left here would be committed as an orphan submission with its bitstreams. + * + *

{@code addItem} is driven directly because that is the unit that has to clean up after itself; going + * through the script would only exercise the caller that aborts anyway.

+ */ + @Test + public void testRefusedPackageLeavesNoWorkspaceItem() throws Exception { + Path itemDir = safPackage("embargoedAccess", "not-a-date", "TEST CONTENT ORPHAN"); + ItemImportServiceImpl importService = + (ItemImportServiceImpl) ItemImportServiceFactory.getInstance().getItemImportService(); + importService.setTest(false); + + context.turnOffAuthorisationSystem(); + try { + importService.addItem(context, Collections.singletonList(collection), + itemDir.getParent().toString(), itemDir.getFileName().toString(), null, false); + fail("an unparseable dc.date.embargoend has to be reported instead of importing the package"); + } catch (EmbargoMetadataException expected) { + // the failure the operator is given + } finally { + context.restoreAuthSystemState(); + } + + assertTrue("the refused package must leave no workspace item behind, the batch import path would" + + " commit it as an orphan submission", + workspaceItemService.findByCollection(context, collection).isEmpty()); + } + + /** + * Verifies that a bare year keeps the day {@code DCDate} mapped it to, 1 January; reading it as + * 31 December would extend embargoes that repositories already live with. + */ + @Test + public void testYearOnlyEmbargoEndIsFirstOfJanuary() throws Exception { + int year = LocalDate.now(ZoneOffset.UTC).getYear() + 1; + assertEmbargoIsAppliedFrom(String.valueOf(year), LocalDate.of(year, 1, 1).plusDays(1)); + } + + /** + * Verifies that {@code yyyy-MM} keeps the day {@code DCDate} mapped it to, the first of that month. + */ + @Test + public void testYearMonthEmbargoEndIsFirstOfMonth() throws Exception { + YearMonth yearMonth = YearMonth.from(LocalDate.now(ZoneOffset.UTC).plusYears(1)); + assertEmbargoIsAppliedFrom(yearMonth.toString(), yearMonth.atDay(1).plusDays(1)); + } + + /** + * Verifies that a full ISO timestamp is truncated to its UTC day, which is then the last closed day. + */ + @Test + public void testIsoTimestampEmbargoEndIsTruncatedToUtcDay() throws Exception { + LocalDate embargoEndDay = LocalDate.now(ZoneOffset.UTC).plusYears(1); + assertEmbargoIsAppliedFrom(embargoEndDay + "T00:00:00Z", embargoEndDay.plusDays(1)); + } + + /** + * Verifies that a policy which cannot be written stops the import instead of being logged and forgotten. + * The failure is injected because breaking the resourcepolicy table would take the rest of the suite down. + */ + @Test + public void testFailureToWriteThePolicyIsNotSwallowed() throws Exception { + Item item = importEmbargoedItem(); + + ItemImportServiceImpl service = serviceWithTestDependencies(); + ResourcePolicyService failingPolicies = mock(ResourcePolicyService.class); + when(failingPolicies.create(any(), any(), any())) + .thenThrow(new SQLException("resource policy store is down")); + service.resourcePolicyService = failingPolicies; + + try { + service.processEmbargoMetadata(context, item); + fail("an embargo policy that could not be written has to stop the import. The item is archived a" + + " few lines later in addItem and installItem then hands its bitstreams the collection" + + " undated default READ policy, so a swallowed failure here publishes the files."); + } catch (Exception expected) { + // fail closed: ItemImport rolls the import back + } + } + + /** + * Same for the other collaborator: without the {@code Anonymous} group no embargo policy can be created, + * and an item archived without one is public by collection default. + */ + @Test + public void testMissingAnonymousGroupIsNotSwallowed() throws Exception { + Item item = importEmbargoedItem(); + + ItemImportServiceImpl service = serviceWithTestDependencies(); + // a GroupService whose findByName answers null, which is the branch under test + service.groupService = mock(GroupService.class); + + try { + service.processEmbargoMetadata(context, item); + fail("without the Anonymous group the embargo policy cannot be created, and archiving the item" + + " anyway leaves it public under the collection default policies"); + } catch (Exception expected) { + // fail closed + } + } + + /** + * Writes a SAF package with the given access right and embargo end date into a fresh source directory. + * + * @param accessRight value of dc.rights.access, {@code null} to leave the field out + * @param embargoEnd value of dc.date.embargoend, {@code null} to leave the field out + * @param content payload of the single ORIGINAL bitstream + * @return the item directory; its parent is the source directory to hand to the import + */ + private Path safPackage(String accessRight, String embargoEnd, String content) throws Exception { + return safPackage(accessRight, embargoEnd, content, null); + } + + /** + * The same package with its payload routed into a bundle of its own, as the SAF + * {@code bundle:} marker does. + */ + private Path safPackage(String accessRight, String embargoEnd, String content, String bundleName) + throws Exception { Path safDir = Files.createDirectory(Path.of(tempDir.toString() + "/test")); Path itemDir = Files.createDirectory(Path.of(safDir.toString() + "/item_000")); - String dublinCoreContent = "\n" + - "\n" + - " " + ITEM_TITLE + "\n" + - " embargoedAccess\n" + - " invalid-date-format\n" + - ""; - Files.writeString(Path.of(itemDir.toString() + "/dublin_core.xml"), dublinCoreContent); + StringBuilder dublinCore = new StringBuilder("\n") + .append("\n") + .append(" ").append(ITEM_TITLE) + .append("\n"); + if (accessRight != null) { + dublinCore.append(" ").append(accessRight) + .append("\n"); + } + if (embargoEnd != null) { + dublinCore.append(" ").append(embargoEnd) + .append("\n"); + } + dublinCore.append(""); + Files.writeString(Path.of(itemDir.toString() + "/dublin_core.xml"), dublinCore.toString()); - // Add bitstream - Path contentsFile = Files.createFile(Path.of(itemDir.toString() + "/contents")); - Files.writeString(contentsFile, "test.txt"); - Path bitstreamFile = Files.createFile(Path.of(itemDir.toString() + "/test.txt")); - Files.writeString(bitstreamFile, "TEST CONTENT INVALID DATE"); + Files.writeString(Files.createFile(Path.of(itemDir.toString() + "/contents")), + bundleName == null ? "test.txt" : "test.txt\tbundle:" + bundleName); + Files.writeString(Files.createFile(Path.of(itemDir.toString() + "/test.txt")), content); + return itemDir; + } - // Perform import - should not fail but should not apply embargo + /** + * Runs {@code dspace import -a} on the source directory. + * + * @param safDir source directory holding the item directories + * @return the exception the script reported, or {@code null} when it ran through + */ + private Exception runImport(Path safDir) throws Exception { String[] args = new String[] { "import", "-a", "-e", admin.getEmail(), "-c", collection.getID().toString(), "-s", safDir.toString(), "-m", tempDir.toString() + "/mapfile.out" }; - runDSpaceScript(args); + try { + runDSpaceScript(args); + return null; + } catch (Exception reported) { + return reported; + } + } + + /** + * Both halves of "fail closed" for a package whose {@code dc.date.embargoend} cannot be used: the failure + * is reported, and no file of that package ends up readable. + */ + private void assertBrokenEmbargoPackageIsRefused(String embargoEnd, String what) throws Exception { + Path itemDir = safPackage("embargoedAccess", embargoEnd, "TEST CONTENT " + what); + + Exception reported = runImport(itemDir.getParent()); + assertNotNull(what + " has to be reported to the operator instead of being archived as a public item: " + + describeArchived(ITEM_TITLE), reported); + assertNoFileOfTheItemIsPublic(what); + } + + /** + * No ORIGINAL bitstream of an archived item with this test title may be readable by an anonymous visitor. + * A refused import leaves no item at all, which is why the loop may legitimately find nothing. + */ + private void assertNoFileOfTheItemIsPublic(String what) throws Exception { + Iterator items = itemService.findByMetadataField(context, "dc", "title", null, ITEM_TITLE); + while (items.hasNext()) { + Item item = items.next(); + for (Bundle bundle : item.getBundles("ORIGINAL")) { + for (Bitstream bitstream : bundle.getBitstreams()) { + assertFalse("the package says dc.rights.access=embargoedAccess and " + what + ", so this" + + " file must not be readable by an anonymous visitor: " + describe(bitstream), + anonymousCanRead(bitstream)); + } + } + } + } + + /** + * A {@code dc.date.embargoend} in one of the shapes {@code DCDate} accepted still produces an embargo, on + * the day {@code DCDate} mapped it to. + * + * @param embargoEnd value written into dc.date.embargoend + * @param expectedStartDay UTC day the files are expected to open, i.e. embargo end day + 1 + */ + private void assertEmbargoIsAppliedFrom(String embargoEnd, LocalDate expectedStartDay) throws Exception { + Path itemDir = safPackage("embargoedAccess", embargoEnd, "TEST CONTENT " + embargoEnd); + + assertNull("dc.date.embargoend=" + embargoEnd + " was accepted by DCDate, so the packages of this" + + " repository use it and the import must not fail on it", runImport(itemDir.getParent())); - // Verify item was created (import should not fail) Item item = itemService.findByMetadataField(context, "dc", "title", null, ITEM_TITLE).next(); - assertNotNull("Item should be created even with invalid date format", item); + assertNotNull("Item should be created", item); + Bitstream bitstream = item.getBundles("ORIGINAL").get(0).getBitstreams().get(0); + + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals("exactly one Anonymous READ policy may remain: " + describe(bitstream), + 1, anonymousRead.size()); + + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertNotNull("the embargo policy has to be dated", embargoPolicy.getStartDate()); + assertEquals("dc.date.embargoend=" + embargoEnd + " has to mean the same day it meant with DCDate," + + " and the files open the day after it", + expectedStartDay.toString(), utcDay(embargoPolicy.getStartDate())); + assertEquals("the embargo policy has to carry the access condition name", + EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertEquals("the embargo policy has to be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertFalse("an embargoed file must not be downloadable by an anonymous visitor: " + describe(bitstream), + anonymousCanRead(bitstream)); + } - // Verify no embargo policies due to invalid date - List bitstreams = item.getBundles("ORIGINAL").get(0).getBitstreams(); - Bitstream bitstream = bitstreams.get(0); - List policies = resourcePolicyService.find(context, bitstream, Constants.READ); + /** + * Imports one valid, still running embargo and returns the archived item. + */ + private Item importEmbargoedItem() throws Exception { + Path itemDir = safPackage("embargoedAccess", EMBARGOEND_DATE_FUTURE, "TEST CONTENT FOR INJECTION"); + assertNull("fixture precondition: the valid package has to import", runImport(itemDir.getParent())); - boolean hasEmbargoPolicy = policies.stream() - .anyMatch(p -> p.getGroup() != null && - p.getGroup().equals(anonymousGroup) && - p.getStartDate() != null); + Item item = itemService.findByMetadataField(context, "dc", "title", null, ITEM_TITLE).next(); + assertNotNull("fixture precondition: the item has to exist", item); + return item; + } - assertTrue("Should not have embargo policy with invalid date format", !hasEmbargoPolicy); + /** + * An {@code ItemImportServiceImpl} whose collaborators the test can replace one by one. Only the three the + * embargo code uses are wired; the service is never asked to import anything through this instance. + */ + private ItemImportServiceImpl serviceWithTestDependencies() { + ItemImportServiceImpl service = new ItemImportServiceImpl(); + service.itemService = itemService; + service.groupService = groupService; + service.resourcePolicyService = resourcePolicyService; + return service; + } + + /** + * The policy start date as the UTC calendar day it is stored as. {@code SimpleDateFormat} would otherwise + * render midnight UTC in the time zone of the build machine and report the previous day west of Greenwich. + */ + private String utcDay(Date date) { + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); + sdf.setTimeZone(TimeZone.getTimeZone("UTC")); + return sdf.format(date); + } + + /** + * Every archived item with this title and the policies of its ORIGINAL bitstreams, for failure messages: + * a refused package and a published one are otherwise indistinguishable. + */ + private String describeArchived(String title) throws Exception { + StringBuilder sb = new StringBuilder(System.lineSeparator()); + Iterator items = itemService.findByMetadataField(context, "dc", "title", null, title); + if (!items.hasNext()) { + sb.append(" ").append(System.lineSeparator()); + } + while (items.hasNext()) { + Item item = items.next(); + sb.append(" item=").append(item.getID()).append(" archived=").append(item.isArchived()) + .append(System.lineSeparator()); + for (Bundle bundle : item.getBundles("ORIGINAL")) { + for (Bitstream bitstream : bundle.getBitstreams()) { + sb.append(describe(bitstream)); + } + } + } + return sb.toString(); } /** @@ -354,19 +957,23 @@ public void testMultipleBitstreamsEmbargo() throws Exception { assertEquals("Should have two bitstreams", 2, bitstreams.size()); for (Bitstream bitstream : bitstreams) { - List policies = resourcePolicyService.find(context, bitstream, Constants.READ); - - ResourcePolicy embargoPolicy = policies.stream() - .filter(p -> p.getGroup() != null && p.getGroup().equals(anonymousGroup)) - .findFirst() - .orElse(null); + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals("exactly one Anonymous READ policy may remain on each bitstream: " + describe(bitstream), + 1, anonymousRead.size()); - assertNotNull("Each bitstream should have embargo policy", embargoPolicy); + ResourcePolicy embargoPolicy = anonymousRead.get(0); assertNotNull("Each embargo policy should have start date", embargoPolicy.getStartDate()); + assertEquals("the embargo policy has to be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertEquals("the embargo policy has to carry the access condition name", + EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); assertEquals("Each embargo start date should be embargoend + 1 day", EXPECTED_POLICY_START_DATE, sdf.format(embargoPolicy.getStartDate())); + + assertFalse("an embargoed file must not be downloadable by an anonymous visitor: " + + describe(bitstream), anonymousCanRead(bitstream)); } } } diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/AbstractEmbargoIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/AbstractEmbargoIT.java new file mode 100644 index 000000000000..474347afeaa9 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/AbstractEmbargoIT.java @@ -0,0 +1,517 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemupdate; + +import static org.junit.Assert.assertEquals; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.time.LocalDate; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; + +import org.apache.commons.io.file.PathUtils; +import org.apache.commons.io.output.TeeOutputStream; +import org.dspace.AbstractIntegrationTestWithDatabase; +import org.dspace.authorize.ResourcePolicy; +import org.dspace.authorize.factory.AuthorizeServiceFactory; +import org.dspace.authorize.service.AuthorizeService; +import org.dspace.authorize.service.ResourcePolicyService; +import org.dspace.builder.BitstreamBuilder; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.ItemBuilder; +import org.dspace.builder.MetadataFieldBuilder; +import org.dspace.builder.ResourcePolicyBuilder; +import org.dspace.content.Bitstream; +import org.dspace.content.Collection; +import org.dspace.content.Item; +import org.dspace.content.MetadataField; +import org.dspace.content.MetadataSchema; +import org.dspace.content.MetadataValue; +import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.BundleService; +import org.dspace.content.service.ItemService; +import org.dspace.content.service.MetadataFieldService; +import org.dspace.content.service.MetadataSchemaService; +import org.dspace.core.Constants; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.dspace.eperson.factory.EPersonServiceFactory; +import org.dspace.eperson.service.GroupService; +import org.dspace.handle.factory.HandleServiceFactory; +import org.dspace.handle.service.HandleService; +import org.junit.After; +import org.junit.Before; + +/** + * Fixture shared by the {@link ItemUpdate} embargo integration tests: a collection whose bitstreams inherit + * an undated {@code Anonymous}/{@code READ} policy, the metadata fields the embargo actions target, and the + * plumbing needed to drive a {@code dspace itemupdate} run against a SAF archive built on the fly. + * + *

Assertions specific to one scenario stay in the test class that makes them; only fixture building and + * policy inspection live here.

+ */ +public abstract class AbstractEmbargoIT extends AbstractIntegrationTestWithDatabase { + + /** The single normalised rpName, matching the access condition name in access-conditions.xml. */ + protected static final String EMBARGO_POLICY_NAME = "embargo"; + + /** rpName written by earlier versions; fixtures use it so that normalisation is exercised. */ + protected static final String LEGACY_EMBARGO_POLICY_NAME = "Standard Embargo"; + + /** Bundle holding the text extracted from a file. */ + protected static final String TEXT_BUNDLE = "TEXT"; + + /** Bundle holding the thumbnail rendered from a file. */ + protected static final String THUMBNAIL_BUNDLE = "THUMBNAIL"; + + /** + * Sentinel for {@link #deletePolicies(Bitstream, int)} meaning "every action", picked so it can never + * collide with a real value of {@link Constants#actionText}. + */ + protected static final int ALL_ACTIONS = -1; + + protected final ItemService itemService = ContentServiceFactory.getInstance().getItemService(); + protected final BundleService bundleService = ContentServiceFactory.getInstance().getBundleService(); + protected final HandleService handleService = HandleServiceFactory.getInstance().getHandleService(); + protected final ResourcePolicyService resourcePolicyService = + AuthorizeServiceFactory.getInstance().getResourcePolicyService(); + protected final AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService(); + protected final GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + protected final MetadataSchemaService metadataSchemaService = + ContentServiceFactory.getInstance().getMetadataSchemaService(); + protected final MetadataFieldService metadataFieldService = + ContentServiceFactory.getInstance().getMetadataFieldService(); + + protected Collection collection; + protected Group anonymousGroup; + protected Path tempDir; + + private String previousHandlePrefix; + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + context.turnOffAuthorisationSystem(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + collection = CollectionBuilder.createCollection(context, parentCommunity) + .withName("Collection") + .build(); + + // none of these exist in the test metadata registry, the update actions need them as targets + ensureMetadataFieldExists("rights", "access"); + ensureMetadataFieldExists("date", "embargoend"); + ensureMetadataFieldExists("identifier", "thesis"); + + anonymousGroup = groupService.findByName(context, Group.ANONYMOUS); + // ItemArchive resolves items through this mutable static, it is restored in destroy() + previousHandlePrefix = ItemUpdate.HANDLE_PREFIX; + ItemUpdate.HANDLE_PREFIX = handleService.getCanonicalPrefix(); + + context.restoreAuthSystemState(); + + tempDir = Files.createTempDirectory(getClass().getSimpleName()); + } + + @After + @Override + public void destroy() throws Exception { + ItemUpdate.HANDLE_PREFIX = previousHandlePrefix; + if (tempDir != null) { + PathUtils.deleteDirectory(tempDir); + } + super.destroy(); + } + + protected void ensureMetadataFieldExists(String element, String qualifier) throws Exception { + MetadataSchema dcSchema = metadataSchemaService.find(context, "dc"); + MetadataField existingField = metadataFieldService.findByElement(context, dcSchema, element, qualifier); + if (existingField == null) { + MetadataFieldBuilder.createMetadataField(context, dcSchema, element, qualifier, null).build(); + } + } + + /** + * @param metadataTriples element, qualifier and value of extra {@code dc} metadata, in groups of three + */ + protected Item createItem(String title, String... metadataTriples) throws Exception { + context.turnOffAuthorisationSystem(); + ItemBuilder builder = ItemBuilder.createItem(context, collection).withTitle(title); + for (int i = 0; i + 2 < metadataTriples.length; i += 3) { + builder.withMetadata("dc", metadataTriples[i], metadataTriples[i + 1], metadataTriples[i + 2]); + } + Item item = builder.build(); + context.restoreAuthSystemState(); + return item; + } + + /** + * A bitstream in the ORIGINAL bundle. It inherits the collection DEFAULT_BITSTREAM_READ and so carries one + * undated Anonymous READ policy, the state a freshly imported SAF item is in. + */ + protected Bitstream createOriginalBitstream(Item item, String name) throws Exception { + return createBitstreamInBundle(item, name, Constants.CONTENT_BUNDLE_NAME); + } + + protected Bitstream createBitstreamInBundle(Item item, String name, String bundleName) throws Exception { + context.turnOffAuthorisationSystem(); + Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, + new ByteArrayInputStream(("content-" + name).getBytes(StandardCharsets.UTF_8)), bundleName) + .withName(name) + .withMimeType("text/plain") + .build(); + context.restoreAuthSystemState(); + return bitstream; + } + + protected void reloadAll(List bitstreams) throws SQLException { + for (int i = 0; i < bitstreams.size(); i++) { + bitstreams.set(i, context.reloadEntity(bitstreams.get(i))); + } + } + + /** + * Tells whether a visitor who is not logged in may read the bitstream. The authorisation state is a stack + * the builders push and pop, so it is drained first - otherwise every read looks allowed. + */ + protected boolean anonymousCanRead(Bitstream bitstream) throws SQLException { + EPerson savedUser = context.getCurrentUser(); + int popped = 0; + while (context.ignoreAuthorization()) { + context.restoreAuthSystemState(); + popped++; + } + context.setCurrentUser(null); + try { + return authorizeService.authorizeActionBoolean(context, bitstream, Constants.READ); + } finally { + context.setCurrentUser(savedUser); + for (int i = 0; i < popped; i++) { + context.turnOffAuthorisationSystem(); + } + } + } + + protected List allPolicies(Bitstream bitstream) throws SQLException { + return new ArrayList<>(resourcePolicyService.find(context, bitstream)); + } + + protected List policiesForAction(Bitstream bitstream, int actionId) throws SQLException { + return new ArrayList<>(resourcePolicyService.find(context, bitstream, actionId)); + } + + protected List readPolicies(Bitstream bitstream) throws SQLException { + return policiesForAction(bitstream, Constants.READ); + } + + protected List anonymousReadPolicies(Bitstream bitstream) throws SQLException { + return readPolicies(bitstream).stream() + .filter(policy -> policy.getGroup() != null && anonymousGroup.equals(policy.getGroup())) + .collect(Collectors.toList()); + } + + /** + * Every resource policy id of the bitstream. Ids rather than counts, so a policy that was deleted and + * re-created is visible. + */ + protected Set policyIds(Bitstream bitstream) throws SQLException { + Set ids = new TreeSet<>(); + for (ResourcePolicy policy : allPolicies(bitstream)) { + ids.add(policy.getID()); + } + return ids; + } + + /** + * Deletes policies one by one, because the bulk removal helpers issue an HQL delete and leave the + * in-memory collection stale. + * + * @param actionId action to delete, or {@link #ALL_ACTIONS} for every policy regardless of action + */ + protected void deletePolicies(Bitstream bitstream, int actionId) throws Exception { + List doomed = actionId == ALL_ACTIONS + ? allPolicies(bitstream) + : policiesForAction(bitstream, actionId); + for (ResourcePolicy policy : doomed) { + resourcePolicyService.delete(context, policy); + } + } + + protected ResourcePolicy addAnonymousReadPolicy(Bitstream bitstream, Date startDate, String name) + throws Exception { + return addAnonymousReadPolicy(bitstream, startDate, name, null); + } + + protected ResourcePolicy addAnonymousReadPolicy(Bitstream bitstream, Date startDate, String name, + String policyType) throws Exception { + context.turnOffAuthorisationSystem(); + ResourcePolicyBuilder builder = ResourcePolicyBuilder.createResourcePolicy(context, null, anonymousGroup) + .withAction(Constants.READ) + .withDspaceObject(bitstream) + .withName(name); + if (startDate != null) { + builder.withStartDate(startDate); + } + if (policyType != null) { + builder.withPolicyType(policyType); + } + ResourcePolicy policy = builder.build(); + context.restoreAuthSystemState(); + return policy; + } + + /** + * Leaves the bitstream with a single Anonymous READ policy, removing the collection's undated default + * first; without that an "embargoed" fixture is not embargoed at all. + */ + protected ResourcePolicy replaceAnonymousReadPolicies(Bitstream bitstream, Date startDate, String name) + throws Exception { + return replaceAnonymousReadPolicies(bitstream, startDate, name, null); + } + + protected ResourcePolicy replaceAnonymousReadPolicies(Bitstream bitstream, Date startDate, String name, + String policyType) throws Exception { + context.turnOffAuthorisationSystem(); + deletePolicies(bitstream, Constants.READ); + context.restoreAuthSystemState(); + return addAnonymousReadPolicy(bitstream, startDate, name, policyType); + } + + protected List metadataValues(Item item, String element, String qualifier) { + return itemService.getMetadata(item, "dc", element, qualifier, Item.ANY).stream() + .map(MetadataValue::getValue) + .collect(Collectors.toList()); + } + + protected String singleMetadataValue(Item item, String element, String qualifier) { + List values = metadataValues(item, element, qualifier); + return values.isEmpty() ? null : values.get(0); + } + + /** + * Calendar day of a date. {@code ResourcePolicy.startDate} is mapped as {@code @Temporal(DATE)}, so after + * a round trip through the database it comes back as a day-granular {@code java.sql.Date}. + */ + protected LocalDate toLocalDate(Date date) { + if (date instanceof java.sql.Date) { + return ((java.sql.Date) date).toLocalDate(); + } + return date.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); + } + + /** + * The same day rendered for a failure message, {@code null} included. + */ + protected String day(Date date) { + return date == null ? "" : toLocalDate(date).toString(); + } + + /** + * Full identity and content of one policy. Comparing these rather than counts makes both + * delete-and-recreate and in-place mutation visible. + */ + protected String fingerprint(ResourcePolicy policy) { + return String.format("id=%s action=%s group=%s eperson=%s rpType=%s rpName=%s start=%s end=%s", + policy.getID(), + Constants.actionText[policy.getAction()], + policy.getGroup() == null ? "" : policy.getGroup().getName(), + policy.getEPerson() == null ? "" : policy.getEPerson().getEmail(), + policy.getRpType(), + policy.getRpName(), + day(policy.getStartDate()), + day(policy.getEndDate())); + } + + protected List policyFingerprints(Bitstream bitstream) throws SQLException { + List fingerprints = new ArrayList<>(); + for (ResourcePolicy policy : allPolicies(bitstream)) { + fingerprints.add(fingerprint(policy)); + } + Collections.sort(fingerprints); + return fingerprints; + } + + protected Date startOfDayUtc(LocalDate day) { + return Date.from(day.atStartOfDay(ZoneOffset.UTC).toInstant()); + } + + protected String pastDate() { + return LocalDate.now().minusMonths(1).toString(); + } + + /** + * A future end date, the branch that closes the file; a past one opens it again. + */ + protected String futureDate() { + return LocalDate.now().plusYears(1).toString(); + } + + /** + * Builds a SAF {@code dublin_core.xml}. {@code ItemArchive.create} resolves the item by + * {@code dc.identifier.uri == ItemUpdate.HANDLE_PREFIX + handle}. + * + * @param rightsAccess value for {@code dc.rights.access}, or {@code null} to omit the element entirely + * @param embargoEndDate {@code null} omits {@code dc.date.embargoend} entirely, the empty string writes a + * blank value + */ + protected String dublinCore(Item item, String rightsAccess, String embargoEndDate) { + return dublinCoreWithEndDates(item, rightsAccess, embargoEndDate); + } + + /** + * The same document with more than one {@code dc.date.embargoend} value, emitted in the given order. + */ + protected String dublinCoreWithEndDates(Item item, String rightsAccess, String... embargoEndDates) { + return dublinCoreWithAccessRights(item, rightsAccess == null ? Collections.emptyList() + : Collections.singletonList(rightsAccess), embargoEndDates); + } + + /** + * The same document carrying zero or more {@code dc.rights.access} values, for the contradictory metadata + * an operator can put in a package. + */ + protected String dublinCoreWithAccessRights(Item item, List accessRights, String... embargoEndDates) { + StringBuilder sb = new StringBuilder(); + sb.append("\n") + .append("\n") + .append(" ") + .append(ItemUpdate.HANDLE_PREFIX).append(item.getHandle()) + .append("\n"); + + for (String accessRight : accessRights) { + sb.append(" ") + .append(accessRight) + .append("\n"); + } + + for (String embargoEndDate : embargoEndDates) { + if (embargoEndDate == null) { + continue; + } + sb.append(" ") + // an empty XML element is dropped by the parser, a single space survives as a blank value + .append(embargoEndDate.isEmpty() ? " " : embargoEndDate) + .append("\n"); + } + + sb.append(""); + return sb.toString(); + } + + /** + * Runs itemupdate with both embargo fields as targets, the combination that triggers embargo + * synchronisation. {@code main()} is not used because it ends in {@code System.exit}. + */ + protected Run runItemUpdate(Item item, String dublinCoreContent) throws Exception { + return runItemUpdate(new ItemUpdate(), item, dublinCoreContent); + } + + /** + * The same run driven by a caller supplied {@link ItemUpdate}, so that a test can make one step of the + * synchronisation fail where a database error would. + */ + protected Run runItemUpdate(ItemUpdate itemUpdate, Item item, String dublinCoreContent) throws Exception { + Path sourceRoot = Files.createDirectory(tempDir.resolve("saf-" + System.nanoTime())); + // without this marker processArchive writes an undo archive next to the source directory + Files.createFile(sourceRoot.resolve(ItemUpdate.SUPPRESS_UNDO_FILENAME)); + + Path itemDir = Files.createDirectory(sourceRoot.resolve("item_000")); + Files.writeString(itemDir.resolve("dublin_core.xml"), dublinCoreContent, StandardCharsets.UTF_8); + + DeleteMetadataAction deleteAction = + (DeleteMetadataAction) itemUpdate.actionMgr.getUpdateAction(DeleteMetadataAction.class); + deleteAction.addTargetFields(new String[] { "dc.rights.access", "dc.date.embargoend" }); + + AddMetadataAction addAction = + (AddMetadataAction) itemUpdate.actionMgr.getUpdateAction(AddMetadataAction.class); + addAction.addTargetFields(new String[] { "dc.rights.access", "dc.date.embargoend" }); + + // ItemUpdate reports to System.out only, so the console has to be captured to assert on it; the + // stream is teed so that the failsafe -output.txt still holds the full log + ByteArrayOutputStream captured = new ByteArrayOutputStream(); + PrintStream originalOut = System.out; + PrintStream captureStream = + new PrintStream(new TeeOutputStream(originalOut, captured), true, StandardCharsets.UTF_8); + + context.turnOffAuthorisationSystem(); + System.setOut(captureStream); + try { + itemUpdate.processArchive(context, sourceRoot.toString(), null, null, true, false, true); + } finally { + captureStream.flush(); + System.setOut(originalOut); + context.restoreAuthSystemState(); + } + + context.uncacheEntity(item); + return new Run(captured.toString(StandardCharsets.UTF_8), itemUpdate.embargoSyncFailures); + } + + /** + * A run whose failure count is asserted straight away. + * + * @param expectedFailures number of embargo problems the run has to count; anything but 0 makes + * {@code ItemUpdate.main()} exit with 1 + * @return everything the run printed + */ + protected String runItemUpdateExpecting(Item item, String dublinCoreContent, int expectedFailures) + throws Exception { + Run run = runItemUpdate(item, dublinCoreContent); + assertExitCode("itemupdate run", expectedFailures, run); + return run.console; + } + + /** + * @return the number of embargo problems the run reported, which {@link ItemUpdate#exitStatus(int, int)} + * turns into the exit code of {@code dspace itemupdate} + */ + protected int runItemUpdateFailures(Item item, String dublinCoreContent) throws Exception { + return runItemUpdate(item, dublinCoreContent).embargoSyncFailures; + } + + /** + * Asserts the exit code the run would have produced; without it a skipped item looks synchronised. + */ + protected void assertExitCode(String scenario, int expectedFailures, Run run) { + assertEquals("[" + scenario + "] wrong number of reported embargo problems, so ItemUpdate.main() would" + + " exit with " + ItemUpdate.exitStatus(0, run.embargoSyncFailures) + " instead of " + + ItemUpdate.exitStatus(0, expectedFailures) + ". Console output was:" + + System.lineSeparator() + run.console, + expectedFailures, run.embargoSyncFailures); + } + + /** + * What a finished {@code itemupdate} run is judged by: its console output and its failure count. + */ + protected static final class Run { + final String console; + final int embargoSyncFailures; + + private Run(String console, int embargoSyncFailures) { + this.console = console; + this.embargoSyncFailures = embargoSyncFailures; + } + } +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoDateBoundaryIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoDateBoundaryIT.java new file mode 100644 index 000000000000..13f8139cb72c --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoDateBoundaryIT.java @@ -0,0 +1,688 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemupdate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.time.LocalDate; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.util.Date; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +import org.dspace.authorize.ResourcePolicy; +import org.dspace.content.Bitstream; +import org.dspace.content.Item; +import org.dspace.core.Constants; +import org.dspace.eperson.Group; +import org.junit.Test; + +/** + * Date-boundary behaviour of the embargo synchronisation in {@link ItemUpdate}: which calendar day the + * resulting {@code Anonymous}/{@code READ} policy starts on, that exactly one such policy is left behind, and + * that an embargo end date already in the past opens the file. All dates are derived from + * {@code LocalDate.now(ZoneOffset.UTC)} so the suite cannot expire. + */ +public class EmbargoDateBoundaryIT extends AbstractEmbargoIT { + + /** Human readable trace of every policy state observed during a test; appended to failure messages. */ + private final StringBuilder diagnostics = new StringBuilder(); + + /** + * Verifies that a future {@code dc.date.embargoend} closes the file and leaves one normalised + * {@code Anonymous}/{@code READ} policy starting the day after the embargo end date. + */ + @Test + public void futureEmbargoEndBlocksAccess() throws Exception { + LocalDate embargoEnd = utcToday().plusMonths(6); + LocalDate expectedStartDay = embargoEnd.plusDays(1); + + Item item = createItem("Future embargo thesis"); + Bitstream bitstream = createOriginalBitstream(item, "future.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", embargoEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with FUTURE dc.date.embargoend=" + embargoEnd, bitstream); + + assertEmbargoEndStored(item, embargoEnd.toString()); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy("future embargo end " + embargoEnd, bitstream); + assertNormalisedEmbargoPolicy("future embargo end " + embargoEnd, policy, expectedStartDay); + + assertFalse("dc.date.embargoend=" + embargoEnd + " lies in the future, so resource policy #" + + policy.getID() + " must not be date-valid yet." + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertFalse("dc.date.embargoend=" + embargoEnd + " lies in the future, so an anonymous visitor must NOT" + + " be able to download the ORIGINAL bitstream." + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Verifies that {@code dc.date.embargoend} is the inclusive last day: an end date of today keeps the file + * closed today and opens it tomorrow. + */ + @Test + public void embargoEndTodayStillBlocksToday() throws Exception { + LocalDate embargoEnd = utcToday(); + LocalDate expectedStartDay = embargoEnd.plusDays(1); + + Item item = createItem("Embargo ending today"); + Bitstream bitstream = createOriginalBitstream(item, "today.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", embargoEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with dc.date.embargoend=TODAY=" + embargoEnd, bitstream); + + assertEmbargoEndStored(item, embargoEnd.toString()); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy("embargo ending today " + embargoEnd, bitstream); + assertNormalisedEmbargoPolicy("embargo ending today " + embargoEnd, policy, expectedStartDay); + + assertFalse("dc.date.embargoend=" + embargoEnd + " is TODAY and the last day of an embargo is inclusive," + + " so resource policy #" + policy.getID() + " must start tomorrow (" + expectedStartDay + + ") and must not be date-valid yet." + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertFalse("dc.date.embargoend=" + embargoEnd + " is TODAY, so the ORIGINAL bitstream must still be" + + " closed for anonymous visitors today and open only from " + expectedStartDay + "." + + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Verifies that an embargo which ended yesterday publishes the file, with the policy surviving and starting + * today. + */ + @Test + public void embargoEndYesterdayOpensAccess() throws Exception { + LocalDate embargoEnd = utcToday().minusDays(1); + LocalDate expectedStartDay = embargoEnd.plusDays(1); + + // No dc.rights.access at all - the specification treats a missing value exactly like openAccess. + Item item = createItem("Embargo ended yesterday"); + Bitstream bitstream = createOriginalBitstream(item, "yesterday.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + runItemUpdateExpecting(item, dublinCore(item, null, embargoEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with dc.date.embargoend=YESTERDAY=" + embargoEnd, bitstream); + + assertEmbargoEndStored(item, embargoEnd.toString()); + + ResourcePolicy policy = + assertExactlyOneAnonymousReadPolicy("embargo ended yesterday " + embargoEnd, bitstream); + assertNormalisedEmbargoPolicy("embargo ended yesterday " + embargoEnd, policy, expectedStartDay); + + assertTrue("dc.date.embargoend=" + embargoEnd + " expired yesterday, so resource policy #" + policy.getID() + + " (start=" + policy.getStartDate() + ") must already be date-valid." + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertTrue("An embargo that ended yesterday publishes the file, so the ORIGINAL bitstream must be" + + " readable by anonymous visitors." + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Verifies that an expired {@code dc.date.embargoend} with {@code dc.rights.access=openAccess} publishes the + * file. The priming run replaces the inherited collection default with a single dated policy. + */ + @Test + public void pastEmbargoEndWithOpenAccessOpensAccess() throws Exception { + LocalDate futureEnd = utcToday().plusYears(1); + LocalDate pastEnd = utcToday().minusMonths(1); + LocalDate expectedStartDay = pastEnd.plusDays(1); + + Item item = createItem("VSB-TUO thesis published after embargo"); + Bitstream bitstream = createOriginalBitstream(item, "thesis.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + // priming run: an embargo with a future end date + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", futureEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with FUTURE dc.date.embargoend=" + futureEnd, bitstream); + assertFalse("fixture precondition: while embargoed until " + futureEnd + " the ORIGINAL bitstream must" + + " not be publicly readable." + diagnostics, + anonymousCanRead(bitstream)); + + // the embargo expires: past end date, item declared openAccess + runItemUpdateExpecting(item, dublinCore(item, "openAccess", pastEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP C - after itemupdate with PAST dc.date.embargoend=" + pastEnd + + " and dc.rights.access=openAccess", bitstream); + + assertEmbargoEndStored(item, pastEnd.toString()); + assertEquals("itemupdate did not store dc.rights.access=openAccess." + diagnostics, + "openAccess", singleMetadataValue(item, "rights", "access")); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy("expired embargo " + pastEnd + + " with dc.rights.access=openAccess", bitstream); + assertNormalisedEmbargoPolicy("expired embargo " + pastEnd, policy, expectedStartDay); + + assertTrue("Expired embargo (" + pastEnd + ") left resource policy #" + policy.getID() + + " not date-valid, so the file stays unreachable although dc.rights.access=openAccess." + + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertTrue("An expired embargo publishes the file. The ORIGINAL bitstream is still unreadable for" + + " anonymous visitors (HTTP 401) although dc.date.embargoend=" + pastEnd + + " has passed and dc.rights.access=openAccess." + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Same as {@link #pastEmbargoEndWithOpenAccessOpensAccess()} with {@code dc.rights.access=embargoedAccess}: + * the access right names the licence regime, the end date decides when the embargo lapses. + */ + @Test + public void pastEmbargoEndWithEmbargoedAccessOpensAccess() throws Exception { + LocalDate futureEnd = utcToday().plusYears(1); + LocalDate pastEnd = utcToday().minusMonths(1); + LocalDate expectedStartDay = pastEnd.plusDays(1); + + Item item = createItem("VSB-TUO thesis with lapsed embargoedAccess"); + Bitstream bitstream = createOriginalBitstream(item, "lapsed.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", futureEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with FUTURE dc.date.embargoend=" + futureEnd, bitstream); + assertFalse("fixture precondition: while embargoed until " + futureEnd + " the ORIGINAL bitstream must" + + " not be publicly readable." + diagnostics, + anonymousCanRead(bitstream)); + + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", pastEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP C - after itemupdate with PAST dc.date.embargoend=" + pastEnd + + " and dc.rights.access=embargoedAccess", bitstream); + + assertEmbargoEndStored(item, pastEnd.toString()); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy("expired embargo " + pastEnd + + " with dc.rights.access=embargoedAccess", bitstream); + assertNormalisedEmbargoPolicy("expired embargo " + pastEnd, policy, expectedStartDay); + + assertTrue("Expired embargo (" + pastEnd + ") left resource policy #" + policy.getID() + + " not date-valid." + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertTrue("dc.rights.access=embargoedAccess must not keep an ALREADY EXPIRED embargo closed; the" + + " ORIGINAL bitstream must be readable by anonymous visitors after " + pastEnd + "." + + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Verifies that the start date is midnight UTC in session and comes back from the DATE column as the + * calendar day {@code dc.date.embargoend + 1}, which is the day later requests are authorised against. + */ + @Test + public void startDateSurvivesTheDatabaseAsTheExpectedCalendarDay() throws Exception { + LocalDate futureEnd = nextIrishSummerTimeDay(); + LocalDate futureStartDay = futureEnd.plusDays(1); + assertNotEquals("fixture precondition: the JVM time zone (" + ZoneId.systemDefault() + ") must differ" + + " from UTC on " + futureStartDay + ", otherwise this test cannot tell midnight UTC" + + " apart from midnight in the server zone. The harness pins Europe/Dublin in" + + " AbstractDSpaceIntegrationTest.", + Date.from(futureStartDay.atStartOfDay(ZoneOffset.UTC).toInstant()), + Date.from(futureStartDay.atStartOfDay(ZoneId.systemDefault()).toInstant())); + + // stored day still ahead: the file stays closed after the reload + assertStoredStartDaySurvivesRoundTrip(futureEnd, false); + + // embargo ended yesterday: the stored day is today and the policy is in force + assertStoredStartDaySurvivesRoundTrip(utcToday().minusDays(1), true); + } + + /** + * One leg of {@link #startDateSurvivesTheDatabaseAsTheExpectedCalendarDay()}. Calls + * {@code syncEmbargoPolicies} directly because the in-session instant is asserted before the commit, and + * {@code processArchive} ends with {@code context.uncacheEntity(item)}. + * + * @param embargoEnd value of {@code dc.date.embargoend} + * @param expectedReadable whether an anonymous visitor must be able to download the file once the policy + * has been read back from the database + */ + private void assertStoredStartDaySurvivesRoundTrip(LocalDate embargoEnd, boolean expectedReadable) + throws Exception { + LocalDate expectedStartDay = embargoEnd.plusDays(1); + String leg = "dc.date.embargoend=" + embargoEnd; + Date expectedUtcMidnight = Date.from(expectedStartDay.atStartOfDay(ZoneOffset.UTC).toInstant()); + Date serverZoneMidnight = Date.from(expectedStartDay.atStartOfDay(ZoneId.systemDefault()).toInstant()); + + Item item = createItem("Round trip embargo " + embargoEnd, + "rights", "access", "embargoedAccess", + "date", "embargoend", embargoEnd.toString()); + Bitstream bitstream = createOriginalBitstream(item, "round-trip-" + embargoEnd + ".pdf"); + dump("STEP A [" + leg + "] - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + Integer importedPolicyId = anonymousReadPolicies(bitstream).get(0).getID(); + + context.turnOffAuthorisationSystem(); + try { + new ItemUpdate().syncEmbargoPolicies(context, item); + } finally { + context.restoreAuthSystemState(); + } + dump("STEP B [" + leg + "] - after syncEmbargoPolicies, still inside the Hibernate session", bitstream); + + ResourcePolicy inSession = assertExactlyOneAnonymousReadPolicy(leg, bitstream); + assertEquals("[" + leg + "] the inherited Anonymous/READ policy must be MUTATED in place, not deleted" + + " and recreated: a policy may never be removed before its replacement is stored," + + " otherwise a failure between the two leaves the file with zero policies (HTTP 401)." + + diagnostics, + importedPolicyId, inSession.getID()); + assertNotNull("[" + leg + "] resource policy #" + inSession.getID() + " must carry a start date." + + diagnostics, + inSession.getStartDate()); + assertEquals(leg + " must yield a start date of exactly midnight UTC on " + expectedStartDay + + " (epochMillis=" + expectedUtcMidnight.getTime() + "). Midnight in the server time" + + " zone " + ZoneId.systemDefault() + " would be epochMillis=" + + serverZoneMidnight.getTime() + ", which is what Calendar.getInstance() or" + + " java.sql.Date.valueOf(LocalDate) produce. Actual epochMillis=" + + inSession.getStartDate().getTime() + " (" + + inSession.getStartDate().toInstant().atZone(ZoneOffset.UTC) + ")." + diagnostics, + expectedUtcMidnight.getTime(), inSession.getStartDate().getTime()); + + // Read the start date back out of the DATE column instead of out of Hibernate's memory - that is the + // value the next request authorises against. + context.commit(); + context.uncacheEntities(); + // Everything the test holds is detached by now, the fixture fields included. + collection = context.reloadEntity(collection); + anonymousGroup = context.reloadEntity(anonymousGroup); + bitstream = context.reloadEntity(bitstream); + dump("STEP C [" + leg + "] - after commit + uncacheEntities, read back from the database", bitstream); + + ResourcePolicy stored = assertExactlyOneAnonymousReadPolicy(leg + ", read back from the database", + bitstream); + assertNotSame("[" + leg + "] fixture precondition: the policy has to be read back from the database," + + " but the identical instance came out of the Hibernate session, so this leg would" + + " prove nothing about the stored value." + diagnostics, + inSession, stored); + assertEquals("[" + leg + "] the policy id must survive the round trip." + diagnostics, + importedPolicyId, stored.getID()); + assertEquals("[" + leg + "] resourcepolicy.start_date is a DATE column, so the calendar day is the" + + " only part of the start date that survives - and it is the part that decides" + + " access. Expected " + expectedStartDay + " (dc.date.embargoend + 1 day), stored " + + stored.getStartDate() + "." + diagnostics, + expectedStartDay, toUtcLocalDate(stored.getStartDate())); + assertNull("[" + leg + "] this tool must never write an end date: a policy that expires by itself" + + " would close the file again on that day." + diagnostics, + stored.getEndDate()); + assertNormalisedEmbargoPolicy(leg + ", read back from the database", stored, expectedStartDay); + + if (expectedReadable) { + assertTrue("[" + leg + "] access starts on " + expectedStartDay + ", which is not after " + + utcToday() + ", so the policy read back from the database has to be date-valid." + + diagnostics, + resourcePolicyService.isDateValid(stored)); + assertTrue("[" + leg + "] the embargo has expired, so an anonymous visitor has to be able to" + + " download the ORIGINAL bitstream after the round trip." + diagnostics, + anonymousCanRead(bitstream)); + } else { + assertFalse("[" + leg + "] access starts on " + expectedStartDay + ", which is still ahead of " + + utcToday() + ", so the policy read back from the database must not be date-valid." + + diagnostics, + resourcePolicyService.isDateValid(stored)); + assertFalse("[" + leg + "] the embargo is still running, so an anonymous visitor must NOT be able" + + " to download the ORIGINAL bitstream after the round trip." + diagnostics, + anonymousCanRead(bitstream)); + } + } + + /** + * Verifies that several {@code dc.date.embargoend} values are reported to the operator while the run + * completes and follows the first value. + */ + @Test + public void multipleEmbargoEndValuesUsesFirst() throws Exception { + LocalDate firstEnd = utcToday().plusDays(30); + LocalDate secondEnd = utcToday().plusDays(400); + LocalDate expectedStartDay = firstEnd.plusDays(1); + + Item item = createItem("Two embargo end dates"); + Bitstream bitstream = createOriginalBitstream(item, "two-dates.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + String consoleOutput = runItemUpdateExpecting(item, + dublinCoreWithEndDates(item, "embargoedAccess", firstEnd.toString(), secondEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with dc.date.embargoend=" + firstEnd + " AND " + secondEnd, bitstream); + + List storedEndDates = metadataValues(item, "date", "embargoend"); + assertEquals("fixture precondition: itemupdate must have stored both dc.date.embargoend values, got " + + storedEndDates, 2, storedEndDates.size()); + assertEquals("fixture precondition: dublin_core.xml document order must be preserved, so the first stored" + + " dc.date.embargoend value is the first one written; got " + storedEndDates, + firstEnd.toString(), storedEndDates.get(0)); + + String lowerCaseOutput = consoleOutput.toLowerCase(Locale.ROOT); + assertTrue("itemupdate must warn the operator that dc.date.embargoend carries more than one value and" + + " that only the first one is used. Console output was:\n" + consoleOutput, + lowerCaseOutput.contains("multiple") && lowerCaseOutput.contains("embargoend")); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy("two dc.date.embargoend values (" + + firstEnd + ", " + secondEnd + ")", bitstream); + assertNormalisedEmbargoPolicy("two dc.date.embargoend values", policy, expectedStartDay); + + assertNotEquals("the SECOND dc.date.embargoend value (" + secondEnd + ") must be ignored, the embargo has" + + " to follow the first one (" + firstEnd + ")." + diagnostics, + secondEnd.plusDays(1), toUtcLocalDate(policy.getStartDate())); + assertFalse("both dc.date.embargoend values lie in the future, so the ORIGINAL bitstream must not be" + + " publicly readable." + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Verifies that the value shapes {@code DCDate} accepted mean the same day here as on the import path: a + * bare year or month is its first day, a timestamp is truncated to its UTC day. + */ + @Test + public void legacyEmbargoEndShapesKeepTheirDcDateDay() throws Exception { + int nextYear = utcToday().getYear() + 1; + LocalDate tomorrow = utcToday().plusDays(1); + + // a bare year is 1 January of it, not 31 December, which would extend the embargo + assertLegacyEmbargoEndClosesTheFileUntil(String.valueOf(nextYear), LocalDate.of(nextYear, 1, 1)); + // a bare month is the 1st of it, for the same reason + assertLegacyEmbargoEndClosesTheFileUntil(nextYear + "-05", LocalDate.of(nextYear, 5, 1)); + // SimpleDateFormat did not require the month to be padded, so packages contain this shape too + assertLegacyEmbargoEndClosesTheFileUntil(nextYear + "-5", LocalDate.of(nextYear, 5, 1)); + assertLegacyEmbargoEndClosesTheFileUntil(nextYear + "-5-9", LocalDate.of(nextYear, 5, 9)); + // the shape DSpace exports write; the time of day is dropped, the UTC day is the last closed day + assertLegacyEmbargoEndClosesTheFileUntil(tomorrow + "T00:00:00Z", tomorrow); + } + + /** + * Verifies that a legacy shape whose day lies in the past publishes the file: one immediately effective + * policy, an anonymous visitor who gets the file, and a run that reports no problem. + */ + @Test + public void legacyPastEmbargoEndPublishesOnPurpose() throws Exception { + LocalDate primingEnd = utcToday().plusYears(1); + int legacyPastYear = utcToday().getYear() - 5; + String legacyValue = String.valueOf(legacyPastYear); + LocalDate expectedEmbargoEnd = LocalDate.of(legacyPastYear, 1, 1); + LocalDate expectedStartDay = expectedEmbargoEnd.plusDays(1); + String scenario = "legacy dc.date.embargoend=" + legacyValue + " (DCDate day " + expectedEmbargoEnd + ")"; + + Item item = createItem("Legacy year-only embargo end in the past"); + Bitstream bitstream = createOriginalBitstream(item, "legacy-past.pdf"); + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + // The priming run is what makes this a publication rather than a no-op: it leaves the single dated + // policy that keeps the file closed, so opening it afterwards is a state change that can be observed. + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", primingEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after priming itemupdate with a FUTURE dc.date.embargoend=" + primingEnd, bitstream); + assertFalse("fixture precondition: after the priming run with dc.date.embargoend=" + primingEnd + + " the file has to be closed, otherwise the legacy value below opens nothing." + + diagnostics, + anonymousCanRead(bitstream)); + + runItemUpdateExpecting(item, dublinCore(item, "openAccess", legacyValue), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP C - after itemupdate with " + scenario, bitstream); + + assertEmbargoEndStored(item, legacyValue); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy(scenario, bitstream); + assertNormalisedEmbargoPolicy(scenario, policy, expectedStartDay); + + assertTrue("dc.date.embargoend=" + legacyValue + " is the year " + legacyPastYear + ", i.e. an embargo" + + " that ended on " + expectedEmbargoEnd + ", so resource policy #" + policy.getID() + + " (start=" + policy.getStartDate() + ") has to be date-valid already." + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertTrue("An embargo that ended in " + legacyPastYear + " publishes the file. This is the intended" + + " reading of a bare year and not a parsing accident: the ORIGINAL bitstream has to be" + + " readable by anonymous visitors." + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Verifies that values only a lenient parser would accept are refused, leaving every policy where it was + * and failing the run - both shapes would move an embargo boundary without saying so. + */ + @Test + public void unparseableLegacyLookalikeLeavesPoliciesUntouched() throws Exception { + int nextYear = utcToday().getYear() + 1; + + // trailing garbage after a year that SimpleDateFormat used to ignore + assertEmbargoEndIsRefused(nextYear + "garbage"); + // a numeric UTC offset: DCDate read this as midnight UTC, i.e. two hours off, and never said so + assertEmbargoEndIsRefused(nextYear + "-05-01T00:00:00+02:00"); + } + + /** + * One legacy value that has to close the file until the day {@code DCDate} mapped it to. + * + * @param legacyValue raw {@code dc.date.embargoend} as a legacy SAF package writes it + * @param expectedEmbargoEnd last closed day that value maps to; has to be in the future + */ + private void assertLegacyEmbargoEndClosesTheFileUntil(String legacyValue, LocalDate expectedEmbargoEnd) + throws Exception { + LocalDate expectedStartDay = expectedEmbargoEnd.plusDays(1); + String scenario = "legacy dc.date.embargoend=" + legacyValue + " (DCDate day " + expectedEmbargoEnd + ")"; + + assertTrue("test bug [" + scenario + "]: the expected embargo end day has to lie in the future, or the" + + " scenario silently turns into the expired-embargo one.", + expectedStartDay.isAfter(utcToday())); + + Item item = createItem("Legacy embargo end " + legacyValue); + Bitstream bitstream = createOriginalBitstream(item, "legacy.pdf"); + dump("STEP A [" + legacyValue + "] - fresh SAF import, before any itemupdate", bitstream); + assertFreshImportBaseline(bitstream); + + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", legacyValue), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B [" + legacyValue + "] - after itemupdate", bitstream); + + assertEmbargoEndStored(item, legacyValue); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy(scenario, bitstream); + assertNormalisedEmbargoPolicy(scenario, policy, expectedStartDay); + + assertFalse("[" + scenario + "] resource policy #" + policy.getID() + " must not be date-valid yet." + + diagnostics, + resourcePolicyService.isDateValid(policy)); + assertFalse("[" + scenario + "] the embargo ends on " + expectedEmbargoEnd + ", which is in the future," + + " so an anonymous visitor must NOT be able to download the ORIGINAL bitstream." + + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * One value that has to be refused: the policies of an embargoed file stay as they were and the run counts + * a failure. + * + * @param rejectedValue raw {@code dc.date.embargoend} that no accepted shape matches + */ + private void assertEmbargoEndIsRefused(String rejectedValue) throws Exception { + LocalDate primingEnd = utcToday().plusYears(1); + LocalDate primingStartDay = primingEnd.plusDays(1); + String scenario = "unparseable dc.date.embargoend=" + rejectedValue; + + Item item = createItem("Unparseable embargo end " + rejectedValue); + Bitstream bitstream = createOriginalBitstream(item, "unparseable.pdf"); + assertFreshImportBaseline(bitstream); + + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", primingEnd.toString()), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP A [" + rejectedValue + "] - embargoed until " + primingEnd, bitstream); + assertFalse("fixture precondition [" + scenario + "]: the file has to be closed before the unparseable" + + " value is fed in." + diagnostics, anonymousCanRead(bitstream)); + + Set idsBefore = policyIds(bitstream); + + String consoleOutput = runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", rejectedValue), 1); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B [" + rejectedValue + "] - after itemupdate with the unparseable value", bitstream); + + assertEquals("[" + scenario + "] the set of resource policy ids changed, so policies were deleted and/or" + + " re-created although an unreadable date is no instruction at all. Console output" + + " was:\n" + consoleOutput + diagnostics, + idsBefore, policyIds(bitstream)); + + ResourcePolicy policy = assertExactlyOneAnonymousReadPolicy(scenario, bitstream); + assertEquals("[" + scenario + "] the surviving policy was re-dated although the value could not be read." + + " Whatever the tool cannot parse it must not act on." + diagnostics, + primingStartDay, toUtcLocalDate(policy.getStartDate())); + assertFalse("[" + scenario + "] the embargoed file became publicly readable after an unreadable" + + " dc.date.embargoend." + diagnostics, anonymousCanRead(bitstream)); + } + + /** + * Asserts the state a fresh SAF import leaves behind: one Anonymous/READ policy without a start date, + * inherited from the collection DEFAULT_BITSTREAM_READ. + */ + private void assertFreshImportBaseline(Bitstream bitstream) throws Exception { + List defaultBitstreamReadGroups = + authorizeService.getAuthorizedGroups(context, collection, Constants.DEFAULT_BITSTREAM_READ); + assertTrue("fixture precondition: the collection must grant DEFAULT_BITSTREAM_READ to Anonymous," + + " otherwise the imported bitstream does not model the customer's repository.", + defaultBitstreamReadGroups.contains(anonymousGroup)); + + List policies = anonymousReadPolicies(bitstream); + assertEquals("fixture precondition: a freshly imported ORIGINAL bitstream must carry exactly one" + + " Anonymous/READ policy inherited from the collection default." + diagnostics, + 1, policies.size()); + assertNull("fixture precondition: the inherited Anonymous/READ policy must have no start date." + + diagnostics, + policies.get(0).getStartDate()); + assertTrue("fixture precondition: a freshly imported ORIGINAL bitstream must be publicly readable." + + diagnostics, + anonymousCanRead(bitstream)); + } + + /** + * Asserts a single Anonymous/READ policy: zero leaves the file unreachable, more than one lets an undated + * policy coexist with the dated one and neutralise the embargo. + */ + private ResourcePolicy assertExactlyOneAnonymousReadPolicy(String scenario, Bitstream bitstream) + throws Exception { + List policies = anonymousReadPolicies(bitstream); + assertEquals("After " + scenario + " the ORIGINAL bitstream must carry EXACTLY ONE Anonymous/READ policy." + + " Zero policies make the file unreachable (HTTP 401); more than one lets an undated" + + " policy neutralise the embargo. Found " + policies.size() + "." + diagnostics, + 1, policies.size()); + return policies.get(0); + } + + /** + * Asserts the normalised shape of the surviving policy: {@code TYPE_CUSTOM}, {@code rpName="embargo"} and a + * start date of {@code dc.date.embargoend + 1 day}. + */ + private void assertNormalisedEmbargoPolicy(String scenario, ResourcePolicy policy, LocalDate expectedStartDay) { + assertNotNull("After " + scenario + " resource policy #" + policy.getID() + " must carry a start date." + + diagnostics, + policy.getStartDate()); + assertEquals("After " + scenario + " resource policy #" + policy.getID() + " must start on the day AFTER" + + " dc.date.embargoend, because the end date is the inclusive last day of the embargo." + + diagnostics, + expectedStartDay, toUtcLocalDate(policy.getStartDate())); + assertEquals("After " + scenario + " resource policy #" + policy.getID() + " must be normalised to" + + " rpType=" + ResourcePolicy.TYPE_CUSTOM + "; AuthorizeServiceImpl only honours custom" + + " policies on not-yet-installed items." + diagnostics, + ResourcePolicy.TYPE_CUSTOM, policy.getRpType()); + assertEquals("After " + scenario + " resource policy #" + policy.getID() + " must be normalised to" + + " rpName=\"" + EMBARGO_POLICY_NAME + "\", replacing legacy names such as" + + " \"Standard Embargo\" or \"Special Case Embargo\"." + diagnostics, + EMBARGO_POLICY_NAME, policy.getRpName()); + } + + private void assertEmbargoEndStored(Item item, String expectedEmbargoEnd) { + assertEquals("itemupdate did not store dc.date.embargoend on the item, so the run never really reached it" + + " (ItemArchive.create may have failed to resolve it - processArchive swallows every" + + " per-item exception)." + diagnostics, + expectedEmbargoEnd, singleMetadataValue(item, "date", "embargoend")); + } + + private void dump(String label, Bitstream bitstream) throws Exception { + StringBuilder sb = new StringBuilder(); + sb.append(System.lineSeparator()) + .append(" === ").append(label).append(" ===").append(System.lineSeparator()) + .append(" bitstream=").append(bitstream.getID()).append(System.lineSeparator()) + .append(" anonymousCanRead=").append(anonymousCanRead(bitstream)).append(System.lineSeparator()); + + List policies = resourcePolicyService.find(context, bitstream, Constants.READ); + if (policies.isEmpty()) { + sb.append(" ").append(System.lineSeparator()); + } + for (ResourcePolicy policy : policies) { + sb.append(String.format(" id=%s group=%s action=%s rpType=%s rpName=%s start=%s end=%s valid=%s", + policy.getID(), + policy.getGroup() == null ? "" : policy.getGroup().getName(), + Constants.actionText[policy.getAction()], + policy.getRpType(), + policy.getRpName(), + policy.getStartDate(), + policy.getEndDate(), + resourcePolicyService.isDateValid(policy))) + .append(System.lineSeparator()); + } + + diagnostics.append(sb); + System.out.print(sb); + } + + /** + * Calendar day of a start date. {@code ResourcePolicy.startDate} is mapped as {@code @Temporal(DATE)}, so + * after a round trip through the database it comes back as a day-granular {@code java.sql.Date}. + */ + private LocalDate toUtcLocalDate(Date date) { + if (date instanceof java.sql.Date) { + return ((java.sql.Date) date).toLocalDate(); + } + return date.toInstant().atZone(ZoneOffset.UTC).toLocalDate(); + } + + /** Calendar "today" in UTC - all embargo arithmetic is done in UTC calendar days. */ + private LocalDate utcToday() { + return LocalDate.now(ZoneOffset.UTC); + } + + /** + * The next 1 July after today. Ireland is on UTC+1 then, which makes midnight UTC and midnight in the + * harness time zone distinguishable. + */ + private LocalDate nextIrishSummerTimeDay() { + LocalDate today = utcToday(); + LocalDate candidate = LocalDate.of(today.getYear(), 7, 1); + if (!candidate.isAfter(today)) { + candidate = candidate.plusYears(1); + } + return candidate; + } + +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoDerivativesIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoDerivativesIT.java new file mode 100644 index 000000000000..12b713be2415 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoDerivativesIT.java @@ -0,0 +1,422 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemupdate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.sql.SQLException; +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; + +import org.dspace.authorize.ResourcePolicy; +import org.dspace.content.Bitstream; +import org.dspace.content.Bundle; +import org.dspace.content.Item; +import org.dspace.core.Constants; +import org.junit.Test; + +/** + * Embargo synchronisation of the bundles derived from an embargoed file. While an item is under embargo + * neither its extracted full text (TEXT) nor its thumbnail (THUMBNAIL) may be readable, and both have to be + * re-opened when the embargo ends. Bundles that are not derived from the file, LICENSE above all, stay out + * of scope. + */ +public class EmbargoDerivativesIT extends AbstractEmbargoIT { + + private static final String OPEN_ACCESS = "openAccess"; + private static final String EMBARGOED_ACCESS = "embargoedAccess"; + private static final String RESTRICTED_ACCESS = "restrictedAccess"; + + private static final String LICENSE_BUNDLE = "LICENSE"; + private static final String CC_LICENSE_BUNDLE = "CC-LICENSE"; + + /** + * Verifies that a running embargo closes the thumbnail and the extracted full text as well. The TEXT + * bundle holds the whole content of the file, so leaving it public publishes the embargoed work. + */ + @Test + public void futureEmbargoAlsoClosesTextAndThumbnail() throws Exception { + LocalDate embargoEnd = LocalDate.now().plusMonths(6); + LocalDate accessStart = embargoEnd.plusDays(1); + + Item item = createItem("Embargoed Derivatives Thesis"); + List files = createOriginalWithDerivatives(item, "thesis.pdf"); + + for (Bitstream file : files) { + assertTrue("fixture precondition: [" + label(file) + "] must start publicly readable" + + describe(file), + anonymousCanRead(file)); + } + + Run run = runItemUpdate(item, dublinCore(item, EMBARGOED_ACCESS, embargoEnd.toString())); + item = context.reloadEntity(item); + reloadAll(files); + + assertExitCode("future embargo on an item with derivatives", 0, run); + for (Bitstream file : files) { + ResourcePolicy policy = onlyAnonymousReadPolicy(file, "the embargo run"); + assertNotNull("[" + label(file) + "] the surviving Anonymous READ policy carries no start date, so" + + " the embargo was never written onto this bundle." + describe(file), + policy.getStartDate()); + assertEquals("[" + label(file) + "] the embargo belongs to the item, so every bundle derived from" + + " the embargoed file has to carry the same start date." + describe(file), + accessStart, toLocalDate(policy.getStartDate())); + assertFalse("[" + label(file) + "] is publicly readable while the item is under embargo until " + + embargoEnd + ". Neither the thumbnail nor the extracted full text of an" + + " embargoed file may be reachable." + describe(file), + anonymousCanRead(file)); + } + } + + /** + * Verifies that an expired embargo re-opens the derivatives too. Closing them is only half the job; a + * published thesis whose thumbnail stays hidden is just as wrong. + */ + @Test + public void expiredEmbargoReopensTextAndThumbnail() throws Exception { + LocalDate embargoEnd = LocalDate.now().minusMonths(2); + LocalDate accessStart = embargoEnd.plusDays(1); + + Item item = createItem("Expired Derivatives Thesis"); + List files = createOriginalWithDerivatives(item, "expired.pdf"); + + // the state a finished embargo run leaves behind: one dated Anonymous READ policy on every bundle + for (Bitstream file : files) { + replaceAnonymousReadPolicies(file, startOfDayUtc(LocalDate.now().plusYears(1)), + EMBARGO_POLICY_NAME, ResourcePolicy.TYPE_CUSTOM); + } + reloadAll(files); + for (Bitstream file : files) { + assertFalse("fixture precondition: [" + label(file) + "] must start closed" + describe(file), + anonymousCanRead(file)); + } + + Run run = runItemUpdate(item, dublinCore(item, OPEN_ACCESS, embargoEnd.toString())); + item = context.reloadEntity(item); + reloadAll(files); + + assertExitCode("expired embargo on an item with derivatives", 0, run); + for (Bitstream file : files) { + ResourcePolicy policy = onlyAnonymousReadPolicy(file, "the expired embargo run"); + assertNotNull("[" + label(file) + "] the Anonymous READ policy lost its start date instead of being" + + " re-dated to the day the embargo ended." + describe(file), + policy.getStartDate()); + assertEquals("[" + label(file) + "] wrong start date after an embargo that ended on " + embargoEnd + + describe(file), + accessStart, toLocalDate(policy.getStartDate())); + assertTrue("[" + label(file) + "] is still closed although the embargo ended on " + embargoEnd + + ". An expired embargo publishes the file together with everything derived from" + + " it." + describe(file), + anonymousCanRead(file)); + } + } + + /** + * Verifies that the derivative policy is re-dated in place, the rule the ORIGINAL bundle already follows: + * a delete plus create leaves the bitstream without any policy if the run breaks in between. + */ + @Test + public void derivativePolicyIsMutatedNotRecreated() throws Exception { + LocalDate embargoEnd = LocalDate.now().plusMonths(3); + LocalDate accessStart = embargoEnd.plusDays(1); + + Item item = createItem("Mutated Derivatives Thesis"); + List files = createOriginalWithDerivatives(item, "mutate.pdf"); + List before = snapshotAll(files); + + Run run = runItemUpdate(item, dublinCore(item, EMBARGOED_ACCESS, embargoEnd.toString())); + item = context.reloadEntity(item); + reloadAll(files); + + assertExitCode("future embargo on an item with derivatives", 0, run); + for (int i = 0; i < files.size(); i++) { + Bitstream file = files.get(i); + ResourcePolicy policy = onlyAnonymousReadPolicy(file, "the embargo run"); + assertEquals("[" + label(file) + "] the set of policy ids changed, so the Anonymous READ policy was" + + " deleted and recreated instead of being re-dated." + describe(file), + before.get(i).ids, policyIds(file)); + assertNotNull("[" + label(file) + "] the policy was kept but never re-dated, so this bundle stayed" + + " outside the embargo." + describe(file), + policy.getStartDate()); + assertEquals("[" + label(file) + "] wrong embargo start date" + describe(file), + accessStart, toLocalDate(policy.getStartDate())); + assertEquals("[" + label(file) + "] the mutated policy has to be named '" + EMBARGO_POLICY_NAME + + "', otherwise the next run cannot tell it apart from a foreign one." + + describe(file), + EMBARGO_POLICY_NAME, policy.getRpName()); + } + } + + /** + * Verifies that the scope stops at the derived bundles. A licence is not derived from the embargoed file + * and has to stay readable, whatever the embargo end date says. + */ + @Test + public void licenseBundleIsNeverTouched() throws Exception { + LocalDate futureEnd = LocalDate.now().plusMonths(4); + LocalDate pastEnd = LocalDate.now().minusMonths(4); + + Item item = createItem("Licensed Thesis"); + Bitstream original = createBitstreamInBundle(item, "licensed.pdf", Constants.CONTENT_BUNDLE_NAME); + List licences = new ArrayList<>(); + licences.add(createBitstreamInBundle(item, "license.txt", LICENSE_BUNDLE)); + licences.add(createBitstreamInBundle(item, "license_rdf", CC_LICENSE_BUNDLE)); + List before = snapshotAll(licences); + + Run embargoRun = runItemUpdate(item, dublinCore(item, EMBARGOED_ACCESS, futureEnd.toString())); + item = context.reloadEntity(item); + original = context.reloadEntity(original); + reloadAll(licences); + + assertExitCode("future embargo on an item with licence bundles", 0, embargoRun); + assertFalse("sanity check: this run has to embargo the ORIGINAL bitstream, otherwise nothing happened" + + " at all and the licence bundles are untested." + describe(original), + anonymousCanRead(original)); + assertUntouched("future embargo end date", before, licences); + + Run expiredRun = runItemUpdate(item, dublinCore(item, OPEN_ACCESS, pastEnd.toString())); + item = context.reloadEntity(item); + original = context.reloadEntity(original); + reloadAll(licences); + + assertExitCode("expired embargo on an item with licence bundles", 0, expiredRun); + assertTrue("sanity check: the expired embargo has to publish the ORIGINAL bitstream" + + describe(original), + anonymousCanRead(original)); + assertUntouched("expired embargo end date", before, licences); + for (Bitstream licence : licences) { + assertTrue("[" + label(licence) + "] the licence text must stay publicly readable" + + describe(licence), + anonymousCanRead(licence)); + } + } + + /** + * Verifies that a file an operator routed into a bundle of its own follows the embargo. SAF packages can + * do that with the {@code bundle:} marker, and {@code DefaultEmbargoSetter} covers every bundle but + * the licence and metadata ones, so leaving it public would disclose the embargoed work. + */ + @Test + public void customBundleFollowsTheEmbargo() throws Exception { + LocalDate futureEnd = LocalDate.now().plusMonths(4); + LocalDate pastEnd = LocalDate.now().minusMonths(4); + + Item item = createItem("Thesis with a supplement"); + Bitstream original = createBitstreamInBundle(item, "thesis.pdf", Constants.CONTENT_BUNDLE_NAME); + Bitstream supplement = createBitstreamInBundle(item, "dataset.csv", "SUPPLEMENT"); + + Run embargoRun = runItemUpdate(item, dublinCore(item, EMBARGOED_ACCESS, futureEnd.toString())); + item = context.reloadEntity(item); + original = context.reloadEntity(original); + supplement = context.reloadEntity(supplement); + + assertExitCode("future embargo on an item with a custom bundle", 0, embargoRun); + assertFalse("sanity check: the ORIGINAL bitstream has to be closed" + describe(original), + anonymousCanRead(original)); + assertFalse("a file in a bundle of its own is part of the work and must be closed by the embargo" + + describe(supplement), + anonymousCanRead(supplement)); + + Run expiredRun = runItemUpdate(item, dublinCore(item, OPEN_ACCESS, pastEnd.toString())); + item = context.reloadEntity(item); + original = context.reloadEntity(original); + supplement = context.reloadEntity(supplement); + + assertExitCode("expired embargo on an item with a custom bundle", 0, expiredRun); + assertTrue("sanity check: the expired embargo has to publish the ORIGINAL bitstream" + + describe(original), + anonymousCanRead(original)); + assertTrue("the custom bundle has to open together with the file it belongs to" + describe(supplement), + anonymousCanRead(supplement)); + } + + /** + * Verifies that a derivative without an {@code Anonymous}/{@code READ} policy does not get one. Creating + * it would grant access nobody granted, so the run reports the bitstream instead. + */ + @Test + public void derivativeWithoutAnonymousReadIsNotPublished() throws Exception { + LocalDate embargoEnd = LocalDate.now().plusMonths(5); + + Item item = createItem("Closed Text Thesis"); + Bitstream original = createBitstreamInBundle(item, "closed.pdf", Constants.CONTENT_BUNDLE_NAME); + Bitstream text = createBitstreamInBundle(item, "closed.pdf.txt", TEXT_BUNDLE); + + context.turnOffAuthorisationSystem(); + deletePolicies(text, Constants.READ); + context.restoreAuthSystemState(); + text = context.reloadEntity(text); + + assertTrue("fixture precondition: the TEXT bitstream must carry no READ policy" + describe(text), + readPolicies(text).isEmpty()); + + Run run = runItemUpdate(item, dublinCore(item, EMBARGOED_ACCESS, embargoEnd.toString())); + item = context.reloadEntity(item); + original = context.reloadEntity(original); + text = context.reloadEntity(text); + + assertTrue("[TEXT] a READ policy was created for a bitstream that had none. Synchronising an embargo" + + " re-dates existing access, it never grants access nobody granted." + describe(text), + readPolicies(text).isEmpty()); + assertFalse("[TEXT] became publicly readable although it carried no policy before" + describe(text), + anonymousCanRead(text)); + assertTrue("the run has to report the TEXT bitstream " + text.getID() + " it could not synchronise," + + " otherwise the operator never learns that the extracted text was left behind." + + " Console output was:" + System.lineSeparator() + run.console, + run.console.contains(text.getID().toString())); + assertFalse("a derivative that cannot be synchronised must not stop the ORIGINAL bitstream from being" + + " embargoed" + describe(original), + anonymousCanRead(original)); + } + + /** + * Verifies that the guards which keep itemupdate away from an item cover the derived bundles too: an + * access right the tool does not manage and a withdrawn item both leave every policy where it was. + */ + @Test + public void guardsAlsoProtectDerivatives() throws Exception { + Item restricted = createItem("Restricted Derivatives Thesis"); + List restrictedFiles = createOriginalWithDerivatives(restricted, "restricted.pdf"); + List restrictedBefore = snapshotAll(restrictedFiles); + + for (String embargoEnd : Arrays.asList(LocalDate.now().plusYears(1).toString(), + LocalDate.now().minusYears(1).toString())) { + Run run = runItemUpdate(restricted, dublinCore(restricted, RESTRICTED_ACCESS, embargoEnd)); + restricted = context.reloadEntity(restricted); + reloadAll(restrictedFiles); + + assertTrue("itemupdate has to refuse the item because of dc.rights.access=" + RESTRICTED_ACCESS + + ". Nothing in the console output says so, so some other guard stopped the run." + + " Console output was:" + System.lineSeparator() + run.console, + run.console.contains(RESTRICTED_ACCESS)); + assertUntouched(RESTRICTED_ACCESS + " with embargo end " + embargoEnd, restrictedBefore, + restrictedFiles); + } + + Item withdrawn = createItem("Withdrawn Derivatives Thesis"); + List withdrawnFiles = createOriginalWithDerivatives(withdrawn, "withdrawn.pdf"); + + context.turnOffAuthorisationSystem(); + itemService.withdraw(context, withdrawn); + context.restoreAuthSystemState(); + withdrawn = context.reloadEntity(withdrawn); + reloadAll(withdrawnFiles); + + assertTrue("fixture precondition: the item must be withdrawn", withdrawn.isWithdrawn()); + List withdrawnBefore = snapshotAll(withdrawnFiles); + + Run run = runItemUpdate(withdrawn, + dublinCore(withdrawn, EMBARGOED_ACCESS, LocalDate.now().plusYears(1).toString())); + withdrawn = context.reloadEntity(withdrawn); + reloadAll(withdrawnFiles); + + assertTrue("itemupdate has to refuse the item because it is withdrawn. Console output was:" + + System.lineSeparator() + run.console, + run.console.contains("is withdrawn")); + assertUntouched("withdrawn item with a future embargo end date", withdrawnBefore, withdrawnFiles); + for (Bitstream file : withdrawnFiles) { + assertFalse("[" + label(file) + "] a withdrawn file must not be publicly readable, the takedown" + + " covers everything derived from it." + describe(file), + anonymousCanRead(file)); + } + } + + /** + * Compares policy identity (ids) and policy content (fingerprints) of every bitstream: the ids catch + * delete-and-recreate, the fingerprints catch in-place mutation. + */ + private void assertUntouched(String scenario, List before, List bitstreams) + throws SQLException { + for (int i = 0; i < bitstreams.size(); i++) { + Bitstream bitstream = bitstreams.get(i); + assertEquals("[" + scenario + "][" + label(bitstream) + "] the set of resource policy ids changed:" + + " policies were deleted and/or recreated although this bundle is out of scope." + + describe(bitstream), + before.get(i).ids, policyIds(bitstream)); + assertEquals("[" + scenario + "][" + label(bitstream) + "] a surviving resource policy was modified" + + " in place although this bundle is out of scope." + describe(bitstream), + before.get(i).fingerprints, policyFingerprints(bitstream)); + } + } + + /** + * Identity and content of every policy of one bitstream, taken before a run. + */ + private static final class Snapshot { + private final Set ids; + private final List fingerprints; + + private Snapshot(Set ids, List fingerprints) { + this.ids = ids; + this.fingerprints = fingerprints; + } + } + + private List snapshotAll(List bitstreams) throws SQLException { + List snapshots = new ArrayList<>(); + for (Bitstream bitstream : bitstreams) { + snapshots.add(new Snapshot(policyIds(bitstream), policyFingerprints(bitstream))); + } + return snapshots; + } + + /** + * The state filter-media leaves behind on a public item: the file plus its extracted text and its + * thumbnail, each with the one undated Anonymous READ policy inherited from the collection. + */ + private List createOriginalWithDerivatives(Item item, String fileName) throws Exception { + List files = new ArrayList<>(); + files.add(createBitstreamInBundle(item, fileName, Constants.CONTENT_BUNDLE_NAME)); + files.add(createBitstreamInBundle(item, fileName + ".txt", TEXT_BUNDLE)); + files.add(createBitstreamInBundle(item, fileName + ".jpg", THUMBNAIL_BUNDLE)); + return files; + } + + private ResourcePolicy onlyAnonymousReadPolicy(Bitstream bitstream, String what) throws SQLException { + List policies = anonymousReadPolicies(bitstream); + assertEquals("[" + label(bitstream) + "] exactly one Anonymous READ policy must remain after " + what + + ", a second one would defeat the embargo." + describe(bitstream), + 1, policies.size()); + return policies.get(0); + } + + /** + * Renders the current policies of the bitstream for failure messages, so a red build shows which policy + * moved. + */ + private String describe(Bitstream bitstream) throws SQLException { + StringBuilder sb = new StringBuilder(System.lineSeparator()) + .append(" bitstream=").append(label(bitstream)).append(" (").append(bitstream.getID()) + .append(")").append(System.lineSeparator()) + .append(" anonymousCanRead=").append(anonymousCanRead(bitstream)) + .append(System.lineSeparator()); + List fingerprints = policyFingerprints(bitstream); + if (fingerprints.isEmpty()) { + sb.append(" ").append(System.lineSeparator()); + } + for (String fingerprint : fingerprints) { + sb.append(" ").append(fingerprint).append(System.lineSeparator()); + } + return sb.toString(); + } + + /** + * Bundle and file name of a bitstream, so a failure message says which bundle is wrong. + */ + private String label(Bitstream bitstream) throws SQLException { + List bundles = bitstream.getBundles(); + return (bundles.isEmpty() ? "" : bundles.get(0).getName()) + "/" + bitstream.getName(); + } + +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoLifecycleIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoLifecycleIT.java new file mode 100644 index 000000000000..58abbba37d19 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoLifecycleIT.java @@ -0,0 +1,661 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemupdate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.UUID; + +import org.dspace.authorize.ResourcePolicy; +import org.dspace.content.Bitstream; +import org.dspace.content.Bundle; +import org.dspace.content.DSpaceObject; +import org.dspace.content.Item; +import org.dspace.core.Constants; +import org.junit.Test; + +/** + * Life cycle of an embargo driven by {@link ItemUpdate}: setting it, re-running the same SAF archive, and + * ending it with a {@code dc.date.embargoend} in the past, including on policies written by earlier versions. + * Covers the invariants that one {@code Anonymous}/{@code READ} policy survives every run, that it is mutated + * rather than recreated, and that the bundles derived from an embargoed file follow it. + */ +public class EmbargoLifecycleIT extends AbstractEmbargoIT { + + private static final String LEGACY_SPECIAL_CASE_EMBARGO = "Special Case Embargo"; + + private static final String OPEN_ACCESS = "openAccess"; + private static final String EMBARGOED_ACCESS = "embargoedAccess"; + + /** + * Verifies that a SAF package without {@code dc.date.embargoend} leaves every policy as it was. A missing + * field says nothing about the embargo; an embargo is ended by an end date that lies in the past. + */ + @Test + public void removingEmbargoMetadataLeavesPoliciesUntouched() throws Exception { + String futureEmbargoEnd = LocalDate.now().plusMonths(6).toString(); + + Item item = createItem("Lift Embargo Thesis"); + Bitstream bitstream = createOriginalBitstream(item, "thesis.pdf"); + Integer importedPolicyId = onlyAnonymousReadPolicy(bitstream, "the fresh SAF import").getID(); + + // the operator embargoes the item + assertRunSucceeded("setting the embargo", runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, + futureEmbargoEnd))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertEquals("itemupdate did not store the future embargo end date", + futureEmbargoEnd, singleMetadataValue(item, "date", "embargoend")); + ResourcePolicy embargoed = onlyAnonymousReadPolicy(bitstream, "the embargo run"); + assertNotNull("the surviving Anonymous READ policy must be dated while the embargo runs", + embargoed.getStartDate()); + assertEquals("the imported Anonymous READ policy must be mutated in place, not deleted and recreated", + importedPolicyId, embargoed.getID()); + assertFalse("while embargoed the file must not be publicly readable" + describePolicies(bitstream), + anonymousCanRead(bitstream)); + + Set policiesBefore = policySignatures(bitstream); + + // the next SAF package does not carry dc.date.embargoend + assertRunSucceeded("running without dc.date.embargoend", + runItemUpdateFailures(item, dublinCore(item, OPEN_ACCESS, null))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertTrue("fixture precondition: itemupdate has to remove dc.date.embargoend from the item, otherwise" + + " syncEmbargoPolicies still sees an end date and this test covers nothing", + itemService.getMetadata(item, "dc", "date", "embargoend", Item.ANY).isEmpty()); + + assertEquals("a SAF package without dc.date.embargoend must leave every resource policy exactly as it" + + " was - a changed policy set means an embargo was lifted by the absence of a field" + + describePolicies(bitstream), + policiesBefore, policySignatures(bitstream)); + assertFalse("removing dc.date.embargoend published an embargoed file. Absence of the field is not an" + + " instruction; an embargo is ended by a dc.date.embargoend in the past." + + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that an embargo itemupdate never set survives a run without {@code dc.date.embargoend}. The + * survivor is found by {@code (group, action)}, so a policy from the submission UI or from + * {@code bulk-access-control} is indistinguishable from one of itemupdate's own. + */ + @Test + public void foreignEmbargoIsNeverLifted() throws Exception { + LocalDate foreignEmbargoStart = LocalDate.now().plusYears(2); + + Item item = createItem("Submission Embargo Thesis"); + Bitstream bitstream = createOriginalBitstream(item, "submission-embargo.pdf"); + + // exactly what the submission access condition / bulk-access-control leave behind + ResourcePolicy foreign = replaceAnonymousReadPolicies(bitstream, startOfDayUtc(foreignEmbargoStart), + EMBARGO_POLICY_NAME, ResourcePolicy.TYPE_CUSTOM); + bitstream = context.reloadEntity(bitstream); + + assertEquals("fixture precondition: the bitstream must carry exactly the policy the submission writes", + ResourcePolicy.TYPE_CUSTOM, + onlyAnonymousReadPolicy(bitstream, "the fixture").getRpType()); + assertFalse("fixture precondition: the foreign embargo must block anonymous access" + + describePolicies(bitstream), + anonymousCanRead(bitstream)); + + Set policiesBefore = policySignatures(bitstream); + Integer foreignPolicyId = foreign.getID(); + + // a routine metadata batch: the fields are targeted, the package carries neither of them + assertRunSucceeded("running a batch that does not mention the embargo", + runItemUpdateFailures(item, dublinCore(item, null, null))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertEquals("itemupdate lifted an embargo it never set. The policy was written by the submission" + + " access condition or by bulk-access-control and is indistinguishable from one of" + + " itemupdate's own, so the absence of dc.date.embargoend must never touch it." + + describePolicies(bitstream), + policiesBefore, policySignatures(bitstream)); + + ResourcePolicy survivor = onlyAnonymousReadPolicy(bitstream, "a batch without dc.date.embargoend"); + assertEquals("the foreign policy must still be the very same row", foreignPolicyId, survivor.getID()); + assertNotNull("the foreign embargo start date must survive untouched" + describePolicies(bitstream), + survivor.getStartDate()); + assertEquals("the foreign embargo start date must not move", + foreignEmbargoStart, toLocalDate(survivor.getStartDate())); + assertFalse("a file embargoed outside itemupdate became publicly readable" + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that re-running the identical SAF archive is a no-op: the same {@code policy_id} comes back, + * which only holds if the policy is mutated rather than deleted and recreated. + */ + @Test + public void syncIsIdempotent() throws Exception { + String futureEmbargoEnd = LocalDate.now().plusMonths(3).toString(); + LocalDate expectedStart = LocalDate.parse(futureEmbargoEnd).plusDays(1); + + Item item = createItem("Idempotent Thesis"); + Bitstream bitstream = createOriginalBitstream(item, "idempotent.pdf"); + Integer importedPolicyId = onlyAnonymousReadPolicy(bitstream, "the fresh SAF import").getID(); + + assertRunSucceeded("setting the embargo", + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, futureEmbargoEnd))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + ResourcePolicy first = onlyAnonymousReadPolicy(bitstream, "the first run"); + Integer firstId = first.getID(); + assertEquals("the imported Anonymous READ policy must be mutated in place, not deleted and recreated", + importedPolicyId, firstId); + assertNotNull("the surviving policy must be dated after the first run", first.getStartDate()); + assertEquals("embargo must start the day after dc.date.embargoend", + expectedStart, toLocalDate(first.getStartDate())); + assertEquals("the surviving policy must be renamed to the canonical access condition", + EMBARGO_POLICY_NAME, first.getRpName()); + assertEquals("the surviving policy must be TYPE_CUSTOM", ResourcePolicy.TYPE_CUSTOM, first.getRpType()); + + // exactly the same archive again + assertRunSucceeded("re-running the identical archive", + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, futureEmbargoEnd))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + ResourcePolicy second = onlyAnonymousReadPolicy(bitstream, "the second, identical run"); + assertEquals("a repeated identical run must mutate the same policy row - a changed policy_id proves" + + " the policy was deleted and recreated, which is what loses the file on the way" + + describePolicies(bitstream), + firstId, second.getID()); + assertEquals("a repeated identical run must not move the embargo start date", + expectedStart, toLocalDate(second.getStartDate())); + assertEquals("a repeated identical run must not change the policy name", + EMBARGO_POLICY_NAME, second.getRpName()); + assertEquals("a repeated identical run must not change the policy type", + ResourcePolicy.TYPE_CUSTOM, second.getRpType()); + assertFalse("the file must still be embargoed after the second run" + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that a legacy policy whose start date has passed is found by {@code (Anonymous, READ)}, reused + * and normalised. Looking the survivor up by {@code rpName} would leave the expired policy in place next + * to a new one, and the file would stay downloadable. + */ + @Test + public void legacyRpNameThenFutureEmbargoIsEnforced() throws Exception { + assertLegacyPolicyIsAdoptedAndEnforced(LEGACY_EMBARGO_POLICY_NAME, EMBARGOED_ACCESS, "legacy-standard.pdf"); + } + + /** + * Same as {@link #legacyRpNameThenFutureEmbargoIsEnforced()} for the second legacy name, written whenever + * {@code dc.rights.access} was not {@code embargoedAccess}. + */ + @Test + public void legacySpecialCaseRpNameIsAlsoPickedUp() throws Exception { + assertLegacyPolicyIsAdoptedAndEnforced(LEGACY_SPECIAL_CASE_EMBARGO, null, "legacy-special-case.pdf"); + } + + /** + * Verifies that embargoing a born-open item consumes its immediate ({@code startDate == null}) policy; + * left next to a dated one it would keep granting anonymous READ and the embargo would be a no-op. + */ + @Test + public void bornOpenItemThenFutureEmbargoIsEnforced() throws Exception { + String futureEmbargoEnd = LocalDate.now().plusMonths(4).toString(); + + Item item = createItem("Born Open Thesis"); + Bitstream bitstream = createOriginalBitstream(item, "born-open.pdf"); + + ResourcePolicy imported = onlyAnonymousReadPolicy(bitstream, "the fresh SAF import"); + Integer importedPolicyId = imported.getID(); + assertNull("fixture precondition: a born open bitstream carries an immediate policy", + imported.getStartDate()); + assertTrue("fixture precondition: a born open bitstream is publicly readable", anonymousCanRead(bitstream)); + + assertRunSucceeded("embargoing a born-open item", + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, futureEmbargoEnd))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + List after = anonymousReadPolicies(bitstream); + assertEquals("the immediate Anonymous READ policy must be consumed by the embargo, not left beside a" + + " dated one" + describePolicies(bitstream), + 1, after.size()); + + ResourcePolicy survivor = after.get(0); + assertEquals("the immediate policy must be mutated in place, not deleted and recreated", + importedPolicyId, survivor.getID()); + assertNotNull("the surviving policy must be dated", survivor.getStartDate()); + assertEquals("embargo must start the day after dc.date.embargoend", + LocalDate.parse(futureEmbargoEnd).plusDays(1), toLocalDate(survivor.getStartDate())); + assertEquals("the surviving policy must be renamed to the canonical access condition", + EMBARGO_POLICY_NAME, survivor.getRpName()); + assertEquals("the surviving policy must be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, survivor.getRpType()); + assertFalse("an immediate Anonymous READ policy left next to the embargo makes the embargo a no-op" + + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that accumulated {@code Anonymous}/{@code READ} policies collapse into one of the pre-existing + * ones; a second survivor would either defeat the embargo or resurrect an obsolete date. + */ + @Test + public void duplicateAnonymousReadPoliciesCollapseToOne() throws Exception { + String futureEmbargoEnd = LocalDate.now().plusMonths(5).toString(); + + Item item = createItem("Duplicate Policies Thesis"); + Bitstream bitstream = createOriginalBitstream(item, "duplicates.pdf"); + + Integer immediateId = onlyAnonymousReadPolicy(bitstream, "the fresh SAF import").getID(); + Integer oldestDatedId = addAnonymousReadPolicy(bitstream, startOfDayUtc(LocalDate.now().minusMonths(3)), + LEGACY_EMBARGO_POLICY_NAME).getID(); + Integer newestDatedId = addAnonymousReadPolicy(bitstream, startOfDayUtc(LocalDate.now().plusMonths(2)), + LEGACY_SPECIAL_CASE_EMBARGO).getID(); + bitstream = context.reloadEntity(bitstream); + assertEquals("fixture precondition: three Anonymous READ policies" + describePolicies(bitstream), + 3, anonymousReadPolicies(bitstream).size()); + + assertRunSucceeded("collapsing duplicate Anonymous READ policies", + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, futureEmbargoEnd))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + List after = anonymousReadPolicies(bitstream); + assertEquals("duplicate Anonymous READ policies must collapse into exactly one" + + describePolicies(bitstream), + 1, after.size()); + + ResourcePolicy survivor = after.get(0); + Integer survivorId = survivor.getID(); + assertTrue("the survivor must be one of the three pre-existing policies - a brand new policy_id proves" + + " delete and recreate" + describePolicies(bitstream), + survivorId.equals(immediateId) || survivorId.equals(oldestDatedId) + || survivorId.equals(newestDatedId)); + assertTrue("the survivor must be the oldest policy: either the immediate one (in force since forever)" + + " or the one with the oldest start date - never the newest one" + + describePolicies(bitstream), + survivorId.equals(immediateId) || survivorId.equals(oldestDatedId)); + assertEquals("embargo must start the day after dc.date.embargoend", + LocalDate.parse(futureEmbargoEnd).plusDays(1), toLocalDate(survivor.getStartDate())); + assertEquals("the surviving policy must be renamed to the canonical access condition", + EMBARGO_POLICY_NAME, survivor.getRpName()); + assertFalse("a second Anonymous READ policy would defeat the embargo" + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that an ORIGINAL bitstream which had a READ policy before a run still has one afterwards, + * whatever {@code dc.date.embargoend} contained. Each case first puts a real embargo in place, which is + * what replaces the collection default with a single dated policy. + */ + @Test + public void neverZeroReadPoliciesInvariant() throws Exception { + String initialEmbargoEnd = LocalDate.now().plusYears(1).toString(); + String impossibleCalendarDay = LocalDate.now().plusYears(1).getYear() + "-02-30"; + + List cases = new ArrayList<>(); + // { label, dc.rights.access, dc.date.embargoend (null = element absent from the SAF) } + cases.add(new String[] { "future date", OPEN_ACCESS, LocalDate.now().plusMonths(9).toString() }); + cases.add(new String[] { "today", OPEN_ACCESS, LocalDate.now().toString() }); + cases.add(new String[] { "past date", OPEN_ACCESS, LocalDate.now().minusMonths(1).toString() }); + cases.add(new String[] { "empty value", OPEN_ACCESS, "" }); + cases.add(new String[] { "unparseable value", OPEN_ACCESS, "not-a-date" }); + cases.add(new String[] { "impossible calendar day", OPEN_ACCESS, impossibleCalendarDay }); + cases.add(new String[] { "element removed", OPEN_ACCESS, null }); + + StringBuilder violations = new StringBuilder(); + + for (String[] testCase : cases) { + String label = testCase[0]; + + Item item = createItem("Invariant Thesis - " + label); + Bitstream bitstream = createOriginalBitstream(item, "invariant.pdf"); + + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, initialEmbargoEnd)); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + int readPoliciesBefore = readPolicies(bitstream).size(); + int anonymousBefore = anonymousReadPolicies(bitstream).size(); + if (readPoliciesBefore < 1 || anonymousBefore < 1) { + violations.append(System.lineSeparator()) + .append(" [").append(label).append("] the fixture was already broken by the embargo run:") + .append(describePolicies(bitstream)); + continue; + } + + runItemUpdateFailures(item, dublinCore(item, testCase[1], testCase[2])); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + int readPoliciesAfter = readPolicies(bitstream).size(); + int anonymousAfter = anonymousReadPolicies(bitstream).size(); + if (readPoliciesAfter < 1 || anonymousAfter < 1) { + violations.append(System.lineSeparator()) + .append(" [").append(label).append("] dc.date.embargoend=") + .append(testCase[2] == null ? "" : "'" + testCase[2] + "'") + .append(" reduced ").append(readPoliciesBefore).append(" READ policies (") + .append(anonymousBefore).append(" of them Anonymous) to ").append(readPoliciesAfter) + .append(" (").append(anonymousAfter).append(" Anonymous):") + .append(describePolicies(bitstream)); + } + } + + if (violations.length() > 0) { + fail("An itemupdate run must never leave an ORIGINAL bitstream without an Anonymous READ policy." + + " A bitstream with no READ policy answers HTTP 401 and no later run can publish it again." + + violations); + } + } + + /** + * Verifies that every ORIGINAL bitstream of a record reaches the same state, the primary one included, and + * that the primary bitstream flag survives. + */ + @Test + public void multipleBitstreamsAllGetSameState() throws Exception { + String futureEmbargoEnd = LocalDate.now().plusMonths(7).toString(); + String pastEmbargoEnd = LocalDate.now().minusMonths(1).toString(); + + Item item = createItem("Six File Thesis"); + List bitstreams = new ArrayList<>(); + for (int i = 0; i < 6; i++) { + bitstreams.add(createOriginalBitstream(item, "file-" + i + ".pdf")); + } + Bundle originalBundle = bundleOf(item, Constants.CONTENT_BUNDLE_NAME); + setPrimaryBitstream(originalBundle, bitstreams.get(0)); + UUID primaryBitstreamId = bitstreams.get(0).getID(); + + // embargo every file of the record + assertRunSucceeded("embargoing every ORIGINAL bitstream", + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, futureEmbargoEnd))); + item = context.reloadEntity(item); + reloadAll(bitstreams); + originalBundle = context.reloadEntity(originalBundle); + + assertUniformState(bitstreams, "the embargo run"); + for (Bitstream bitstream : bitstreams) { + ResourcePolicy policy = onlyAnonymousReadPolicy(bitstream, "the embargo run"); + assertEquals("embargo must start the day after dc.date.embargoend", + LocalDate.parse(futureEmbargoEnd).plusDays(1), toLocalDate(policy.getStartDate())); + assertEquals("every ORIGINAL bitstream must carry the canonical policy name", + EMBARGO_POLICY_NAME, policy.getRpName()); + assertEquals("every ORIGINAL bitstream must carry a TYPE_CUSTOM policy", + ResourcePolicy.TYPE_CUSTOM, policy.getRpType()); + assertFalse("every embargoed file of the record must be closed" + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + assertNotNull("the ORIGINAL bundle lost its primary bitstream", originalBundle.getPrimaryBitstream()); + assertEquals("the primary bitstream flag must survive an embargo run", + primaryBitstreamId, originalBundle.getPrimaryBitstream().getID()); + + // the embargo expires: the same archive is re-imported with a past date + assertRunSucceeded("letting the embargo expire", + runItemUpdateFailures(item, dublinCore(item, OPEN_ACCESS, pastEmbargoEnd))); + item = context.reloadEntity(item); + reloadAll(bitstreams); + originalBundle = context.reloadEntity(originalBundle); + + assertUniformState(bitstreams, "the expired embargo run"); + for (Bitstream bitstream : bitstreams) { + ResourcePolicy policy = onlyAnonymousReadPolicy(bitstream, "the expired embargo run"); + assertEquals("an expired embargo still starts the day after dc.date.embargoend", + LocalDate.parse(pastEmbargoEnd).plusDays(1), toLocalDate(policy.getStartDate())); + assertTrue("an expired embargo must be effective immediately" + describePolicies(bitstream), + resourcePolicyService.isDateValid(policy)); + assertTrue("an expired embargo publishes every file of the record" + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + assertNotNull("the ORIGINAL bundle lost its primary bitstream", originalBundle.getPrimaryBitstream()); + assertEquals("the primary bitstream flag must survive an expired embargo run", + primaryBitstreamId, originalBundle.getPrimaryBitstream().getID()); + } + + /** + * Verifies that the derived bundles follow the embargo of the file they were derived from: the thumbnail + * and the extracted full text of an embargoed file must not be readable, and both re-open with it. The + * bundle objects themselves stay out of scope, only their bitstreams are synchronised. + */ + @Test + public void derivativeBundlesFollowTheEmbargo() throws Exception { + LocalDate futureEmbargoEnd = LocalDate.now().plusMonths(2); + LocalDate pastEmbargoEnd = LocalDate.now().minusMonths(2); + + Item item = createItem("Derivatives Thesis"); + Bitstream original = createOriginalBitstream(item, "thesis.pdf"); + Bitstream extractedText = createBitstreamInBundle(item, "thesis.pdf.txt", TEXT_BUNDLE); + Bitstream thumbnail = createBitstreamInBundle(item, "thesis.pdf.jpg", THUMBNAIL_BUNDLE); + List files = new ArrayList<>(List.of(original, extractedText, thumbnail)); + + Bundle originalBundle = bundleOf(item, Constants.CONTENT_BUNDLE_NAME); + Set originalBundlePolicies = policySignatures(originalBundle); + assertFalse("fixture precondition: the ORIGINAL bundle must start with policies", + originalBundlePolicies.isEmpty()); + List policyIdsBefore = new ArrayList<>(); + for (Bitstream file : files) { + policyIdsBefore.add(onlyAnonymousReadPolicy(file, "the fixture setup").getID()); + } + + // embargo run + assertRunSucceeded("embargoing the item", + runItemUpdateFailures(item, dublinCore(item, EMBARGOED_ACCESS, futureEmbargoEnd.toString()))); + item = context.reloadEntity(item); + reloadAll(files); + originalBundle = context.reloadEntity(originalBundle); + + for (int i = 0; i < files.size(); i++) { + Bitstream file = files.get(i); + ResourcePolicy policy = onlyAnonymousReadPolicy(file, "the embargo run"); + assertEquals("the Anonymous READ policy was recreated instead of being re-dated" + + describePolicies(file), + policyIdsBefore.get(i), policy.getID()); + assertEquals("every bundle derived from the embargoed file carries the item embargo" + + describePolicies(file), + EMBARGO_POLICY_NAME, policy.getRpName()); + assertEquals("wrong embargo start date" + describePolicies(file), + futureEmbargoEnd.plusDays(1), toLocalDate(policy.getStartDate())); + assertFalse("a file of an item under embargo must not be publicly readable" + + describePolicies(file), + anonymousCanRead(file)); + } + assertEquals("the ORIGINAL bundle's own policies must not be touched", + originalBundlePolicies, policySignatures(originalBundle)); + + // expired embargo run + assertRunSucceeded("letting the embargo expire", + runItemUpdateFailures(item, dublinCore(item, OPEN_ACCESS, pastEmbargoEnd.toString()))); + item = context.reloadEntity(item); + reloadAll(files); + originalBundle = context.reloadEntity(originalBundle); + + for (int i = 0; i < files.size(); i++) { + Bitstream file = files.get(i); + ResourcePolicy policy = onlyAnonymousReadPolicy(file, "the expired embargo run"); + assertEquals("the Anonymous READ policy was recreated instead of being re-dated" + + describePolicies(file), + policyIdsBefore.get(i), policy.getID()); + assertEquals("wrong start date after an expired embargo" + describePolicies(file), + pastEmbargoEnd.plusDays(1), toLocalDate(policy.getStartDate())); + assertTrue("an expired embargo publishes the file together with everything derived from it" + + describePolicies(file), + anonymousCanRead(file)); + } + assertEquals("the ORIGINAL bundle's own policies must survive an expired embargo untouched", + originalBundlePolicies, policySignatures(originalBundle)); + } + + /** + * Shared body of the two legacy {@code rpName} tests: a bitstream whose only {@code Anonymous}/{@code READ} + * policy carries a legacy name and a start date that has passed is put back under embargo. + */ + private void assertLegacyPolicyIsAdoptedAndEnforced(String legacyName, String rightsAccess, String fileName) + throws Exception { + String futureEmbargoEnd = LocalDate.now().plusYears(1).toString(); + + Item item = createItem("Legacy Policy Thesis - " + legacyName); + Bitstream bitstream = createOriginalBitstream(item, fileName); + Integer legacyPolicyId = + replaceAnonymousReadPolicies(bitstream, startOfDayUtc(LocalDate.now().minusMonths(2)), legacyName) + .getID(); + bitstream = context.reloadEntity(bitstream); + + ResourcePolicy legacy = onlyAnonymousReadPolicy(bitstream, "the legacy fixture"); + assertEquals("fixture precondition: the legacy policy must be the only Anonymous READ policy", + legacyPolicyId, legacy.getID()); + assertEquals("fixture precondition: the legacy policy keeps its old name", legacyName, legacy.getRpName()); + assertTrue("fixture precondition: an expired legacy embargo leaves the file public" + + describePolicies(bitstream), + anonymousCanRead(bitstream)); + + assertRunSucceeded("adopting a legacy embargo policy", + runItemUpdateFailures(item, dublinCore(item, rightsAccess, futureEmbargoEnd))); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + List after = anonymousReadPolicies(bitstream); + assertEquals("the legacy policy must be adopted, so exactly one Anonymous READ policy may remain." + + " Looking the survivor up by rpName instead of by (Anonymous, READ) leaves the expired" + + " legacy policy next to the new one and the embargo never takes effect." + + describePolicies(bitstream), + 1, after.size()); + + ResourcePolicy survivor = after.get(0); + assertEquals("the legacy policy must be mutated in place, not deleted and recreated", + legacyPolicyId, survivor.getID()); + assertEquals("the legacy policy name must be normalised", EMBARGO_POLICY_NAME, survivor.getRpName()); + assertEquals("the normalised policy must be TYPE_CUSTOM", + ResourcePolicy.TYPE_CUSTOM, survivor.getRpType()); + assertNotNull("the normalised policy must be dated", survivor.getStartDate()); + assertEquals("embargo must start the day after dc.date.embargoend", + LocalDate.parse(futureEmbargoEnd).plusDays(1), toLocalDate(survivor.getStartDate())); + assertFalse("re-embargoing a legacy bitstream must actually block anonymous download" + + describePolicies(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Asserts that every bitstream of the record ended in the same state - same policy count, same start date, + * same name, same type, same answer to "may an anonymous visitor download it". + */ + private void assertUniformState(List bitstreams, String what) throws Exception { + String expected = stateSignature(bitstreams.get(0)); + for (Bitstream bitstream : bitstreams) { + assertEquals("all ORIGINAL bitstreams of a record must share the same state after " + what + + " (bitstream " + bitstream.getID() + ")", + expected, stateSignature(bitstream)); + } + } + + private ResourcePolicy onlyAnonymousReadPolicy(Bitstream bitstream, String what) throws Exception { + List policies = anonymousReadPolicies(bitstream); + assertEquals("exactly one Anonymous READ policy must remain after " + what + describePolicies(bitstream), + 1, policies.size()); + return policies.get(0); + } + + /** + * State of a bitstream with the policy identities left out, so two different bitstreams can be compared. + */ + private String stateSignature(Bitstream bitstream) throws Exception { + StringBuilder sb = new StringBuilder(); + List policies = anonymousReadPolicies(bitstream); + sb.append("anonymousReadPolicies=").append(policies.size()); + for (ResourcePolicy policy : policies) { + sb.append(" [start=") + .append(policy.getStartDate() == null ? "null" : toLocalDate(policy.getStartDate())) + .append(" end=").append(policy.getEndDate() == null ? "null" : toLocalDate(policy.getEndDate())) + .append(" rpName=").append(policy.getRpName()) + .append(" rpType=").append(policy.getRpType()) + .append(" valid=").append(resourcePolicyService.isDateValid(policy)) + .append(']'); + } + sb.append(" anonymousCanRead=").append(anonymousCanRead(bitstream)); + return sb.toString(); + } + + /** + * Full identity of every policy of a DSpace object, {@code policy_id} included - used to prove that a set + * of policies was not touched at all. + */ + private Set policySignatures(DSpaceObject dso) throws Exception { + Set signatures = new TreeSet<>(); + for (ResourcePolicy policy : resourcePolicyService.find(context, dso)) { + signatures.add(String.format("id=%s action=%s group=%s eperson=%s start=%s end=%s rpName=%s rpType=%s", + policy.getID(), + Constants.actionText[policy.getAction()], + policy.getGroup() == null ? "" : policy.getGroup().getName(), + policy.getEPerson() == null ? "" : policy.getEPerson().getEmail(), + policy.getStartDate(), + policy.getEndDate(), + policy.getRpName(), + policy.getRpType())); + } + return signatures; + } + + private String describePolicies(Bitstream bitstream) throws Exception { + StringBuilder sb = new StringBuilder(System.lineSeparator()); + sb.append(" bitstream=").append(bitstream.getID()).append(System.lineSeparator()) + .append(" anonymousCanRead=").append(anonymousCanRead(bitstream)) + .append(System.lineSeparator()); + + List policies = readPolicies(bitstream); + if (policies.isEmpty()) { + sb.append(" ").append(System.lineSeparator()); + } + for (ResourcePolicy policy : policies) { + sb.append(String.format(" id=%s group=%s action=%s rpType=%s rpName=%s start=%s end=%s valid=%s", + policy.getID(), + policy.getGroup() == null ? "" : policy.getGroup().getName(), + Constants.actionText[policy.getAction()], + policy.getRpType(), + policy.getRpName(), + policy.getStartDate(), + policy.getEndDate(), + resourcePolicyService.isDateValid(policy))) + .append(System.lineSeparator()); + } + return sb.toString(); + } + + private Bundle bundleOf(Item item, String bundleName) throws Exception { + List bundles = itemService.getBundles(item, bundleName); + assertFalse("fixture precondition: the item must have a " + bundleName + " bundle", bundles.isEmpty()); + return bundles.get(0); + } + + private void setPrimaryBitstream(Bundle bundle, Bitstream bitstream) throws Exception { + context.turnOffAuthorisationSystem(); + bundle.setPrimaryBitstreamID(bitstream); + bundleService.update(context, bundle); + context.restoreAuthSystemState(); + } + + /** + * A run the tool is supposed to carry out has to end with exit code 0. + */ + private void assertRunSucceeded(String what, int embargoSyncFailures) { + assertEquals("itemupdate reported an embargo synchronisation problem while " + what + ", so" + + " ItemUpdate.main() would exit with " + ItemUpdate.exitStatus(0, embargoSyncFailures) + + " although nothing was wrong with the input", + 0, embargoSyncFailures); + } + +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoPastDateIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoPastDateIT.java new file mode 100644 index 000000000000..85c4251fa09a --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoPastDateIT.java @@ -0,0 +1,117 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemupdate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; + +import org.dspace.authorize.ResourcePolicy; +import org.dspace.content.Bitstream; +import org.dspace.content.Item; +import org.dspace.core.Constants; +import org.dspace.eperson.Group; +import org.junit.Test; + +/** + * Covers embargo synchronisation in {@link ItemUpdate} when the same archive is re-imported with a + * {@code dc.date.embargoend} that has already passed: the ORIGINAL bitstreams have to stay readable + * for anonymous users instead of losing their last {@code READ} policy. + */ +public class EmbargoPastDateIT extends AbstractEmbargoIT { + + private final StringBuilder diagnostics = new StringBuilder(); + + /** + * Verifies that a future embargo followed by an expired one leaves the ORIGINAL bitstream publicly + * readable. + */ + @Test + public void pastEmbargoEndMustKeepFilesPublic() throws Exception { + String futureEmbargoEnd = LocalDate.now().plusYears(1).toString(); + String pastEmbargoEnd = LocalDate.now().minusMonths(1).toString(); + + List defaultBitstreamReadGroups = + authorizeService.getAuthorizedGroups(context, collection, Constants.DEFAULT_BITSTREAM_READ); + assertTrue("fixture precondition: collection must grant DEFAULT_BITSTREAM_READ to Anonymous", + defaultBitstreamReadGroups.contains(anonymousGroup)); + + Item item = createItem("VSB-TUO thesis"); + Bitstream bitstream = createOriginalBitstream(item, "thesis.pdf"); + + dump("STEP A - fresh SAF import, before any itemupdate", bitstream); + assertFalse("fixture precondition: imported bitstream must carry an Anonymous READ policy", + anonymousReadPolicies(bitstream).isEmpty()); + assertTrue("fixture precondition: imported bitstream must be publicly readable", + anonymousCanRead(bitstream)); + + // first run: embargo end date in the future + runItemUpdateExpecting(item, dublinCore(item, "embargoedAccess", futureEmbargoEnd), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP B - after itemupdate with FUTURE dc.date.embargoend=" + futureEmbargoEnd, bitstream); + + assertEquals("itemupdate did not store the future embargo end date", + futureEmbargoEnd, singleMetadataValue(item, "date", "embargoend")); + List embargoed = anonymousReadPolicies(bitstream); + assertEquals("future embargo must leave exactly one Anonymous READ policy", 1, embargoed.size()); + assertNotNull("the surviving Anonymous READ policy must be dated", embargoed.get(0).getStartDate()); + assertFalse("while embargoed the file must not be publicly readable", anonymousCanRead(bitstream)); + + // second run: embargo end date in the past, item declared openAccess + runItemUpdateExpecting(item, dublinCore(item, "openAccess", pastEmbargoEnd), 0); + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + dump("STEP C - after itemupdate with PAST dc.date.embargoend=" + pastEmbargoEnd + + " and dc.rights.access=openAccess", bitstream); + + List afterExpiry = anonymousReadPolicies(bitstream); + assertFalse("Expired embargo wiped every Anonymous READ policy from the ORIGINAL bitstream." + + " The file is now unreachable (HTTP 401) although dc.rights.access=openAccess." + + diagnostics, + afterExpiry.isEmpty()); + assertTrue("Expired embargo left the ORIGINAL bitstream unreadable for anonymous users." + + diagnostics, + anonymousCanRead(bitstream)); + } + + private void dump(String label, Bitstream bitstream) throws Exception { + List lines = new ArrayList<>(); + for (ResourcePolicy policy : resourcePolicyService.find(context, bitstream, Constants.READ)) { + lines.add(String.format(" id=%s group=%s action=%s rpType=%s rpName=%s start=%s end=%s valid=%s", + policy.getID(), + policy.getGroup() == null ? "" : policy.getGroup().getName(), + Constants.actionText[policy.getAction()], + policy.getRpType(), + policy.getRpName(), + policy.getStartDate(), + policy.getEndDate(), + resourcePolicyService.isDateValid(policy))); + } + if (lines.isEmpty()) { + lines.add(" "); + } + + StringBuilder sb = new StringBuilder(); + sb.append(System.lineSeparator()) + .append(" === ").append(label).append(" ===").append(System.lineSeparator()) + .append(" bitstream=").append(bitstream.getID()).append(System.lineSeparator()) + .append(" anonymousCanRead=").append(anonymousCanRead(bitstream)).append(System.lineSeparator()); + for (String line : lines) { + sb.append(line).append(System.lineSeparator()); + } + diagnostics.append(sb); + System.out.print(sb); + } + +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoSafetyIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoSafetyIT.java new file mode 100644 index 000000000000..67a51732946f --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/EmbargoSafetyIT.java @@ -0,0 +1,555 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.itemupdate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.sql.SQLException; +import java.time.LocalDate; +import java.util.Arrays; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Set; + +import org.dspace.authorize.AuthorizeException; +import org.dspace.authorize.ResourcePolicy; +import org.dspace.builder.GroupBuilder; +import org.dspace.builder.ResourcePolicyBuilder; +import org.dspace.content.Bitstream; +import org.dspace.content.Item; +import org.dspace.content.MetadataValue; +import org.dspace.core.Constants; +import org.dspace.core.Context; +import org.dspace.eperson.Group; +import org.junit.Test; + +/** + * Pins down the cases in which embargo synchronisation in {@link ItemUpdate} has to leave a bitstream alone + * rather than publish it. Every "must not touch" assertion compares policy ids and a fingerprint of every + * policy, because counts hide both delete-and-recreate and in-place mutation. + */ +public class EmbargoSafetyIT extends AbstractEmbargoIT { + + /** + * The supported way of re-opening files whose Anonymous READ policy is already gone; ItemUpdate points + * the operator at it instead of inventing a public policy. + */ + private static final String BULK_ACCESS_CONTROL_HINT = "bulk-access-control"; + + /** + * Access condition writing an {@code Anonymous}/{@code READ} policy with an END date: public now, closed + * again on that day. + */ + private static final String LEASE_POLICY_NAME = "lease"; + + /** + * Verifies that a withdrawn item is left alone: withdrawal turns every READ policy into WITHDRAWN_READ, + * and synchronising an embargo must not undo a takedown. + */ + @Test + public void withdrawnItemIsNeverRepublished() throws Exception { + Item item = createItem("Withdrawn thesis"); + Bitstream bitstream = createEmbargoedBitstream(item, "withdrawn.pdf"); + + context.turnOffAuthorisationSystem(); + itemService.withdraw(context, item); + context.restoreAuthSystemState(); + + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertTrue("fixture precondition: the item must be withdrawn", item.isWithdrawn()); + assertFalse("fixture precondition: withdrawal must leave WITHDRAWN_READ policies behind" + + describe(bitstream), + policiesForAction(bitstream, Constants.WITHDRAWN_READ).isEmpty()); + assertTrue("fixture precondition: a withdrawn bitstream must carry no READ policy" + describe(bitstream), + policiesForAction(bitstream, Constants.READ).isEmpty()); + assertFalse("fixture precondition: a withdrawn file must not be publicly readable" + describe(bitstream), + anonymousCanRead(bitstream)); + + Set idsBefore = policyIds(bitstream); + List policiesBefore = policyFingerprints(bitstream); + + Run pastRun = runItemUpdate(item, dublinCore(item, "openAccess", pastDate())); + + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + // withdraw() also clears archived, so without this check the !isArchived guard alone would satisfy + // every policy assertion below. + assertTrue("ItemUpdate has to refuse a withdrawn item because it is withdrawn. Nothing in the console" + + " output says so, so some other guard stopped the run and the withdrawal guard is" + + " untested. Console output was:" + System.lineSeparator() + pastRun.console, + pastRun.console.contains("is withdrawn")); + assertExitCode("withdrawn item, expired embargo", 0, pastRun); + assertTrue("An expired embargo on a WITHDRAWN item created an action=READ policy." + + " Withdrawal must never be undone by itemupdate, only WITHDRAWN_READ may remain." + + describe(bitstream), + policiesForAction(bitstream, Constants.READ).isEmpty()); + assertUntouched("withdrawn item", idsBefore, policiesBefore, bitstream); + assertFalse("A withdrawn file became publicly readable after an expired embargo was synchronised." + + describe(bitstream), + anonymousCanRead(bitstream)); + + // The past-date run above only reaches the early return; the future-date branch is the one that + // creates policies. + Run futureRun = + runItemUpdate(item, dublinCore(item, "embargoedAccess", futureDate())); + + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertTrue("ItemUpdate has to refuse a withdrawn item because it is withdrawn. Console output was:" + + System.lineSeparator() + futureRun.console, + futureRun.console.contains("is withdrawn")); + assertExitCode("withdrawn item, future embargo", 0, futureRun); + assertTrue("A FUTURE dc.date.embargoend on a WITHDRAWN item created an action=READ policy. A withdrawn" + + " item must never gain one - only WITHDRAWN_READ may remain - otherwise the takedown" + + " undoes itself the moment the embargo lapses." + describe(bitstream), + policiesForAction(bitstream, Constants.READ).isEmpty()); + assertUntouched("withdrawn item with a future embargo end date", idsBefore, policiesBefore, bitstream); + assertFalse("A withdrawn file became publicly readable after a future embargo end date was synchronised." + + describe(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that {@code restrictedAccess} keeps the files closed whatever the embargo end date says. + */ + @Test + public void restrictedAccessWithStaleEmbargoEndIsUntouched() throws Exception { + assertEmbargoSyncIsANoOp("dc.rights.access=restrictedAccess with a past embargo end", + Collections.singletonList("restrictedAccess"), pastDate(), 0); + } + + /** + * Verifies that {@code metadataOnlyAccess} keeps the bitstreams undisclosed. + */ + @Test + public void metadataOnlyAccessIsUntouched() throws Exception { + assertEmbargoSyncIsANoOp("dc.rights.access=metadataOnlyAccess with a past embargo end", + Collections.singletonList("metadataOnlyAccess"), pastDate(), 0); + } + + /** + * Verifies that an access right the tool does not understand means "hands off" rather than "open". + */ + @Test + public void unknownAccessRightValueIsUntouched() throws Exception { + assertEmbargoSyncIsANoOp("dc.rights.access=someAccessRightWeDoNotKnow with a past embargo end", + Collections.singletonList("someAccessRightWeDoNotKnow"), pastDate(), 0); + } + + /** + * Verifies that one value outside the allowlist blocks the whole item, even next to {@code openAccess}: + * contradictory metadata is not resolved in favour of disclosure. + */ + @Test + public void mixedAccessRightsWithOneDisallowedIsUntouched() throws Exception { + assertEmbargoSyncIsANoOp("dc.rights.access=openAccess + restrictedAccess with a past embargo end", + Arrays.asList("openAccess", "restrictedAccess"), pastDate(), 0); + } + + /** + * Verifies that a bitstream readable by a named group only gains no Anonymous policy: with nothing to + * mutate, creating one would widen access nobody granted. + */ + @Test + public void bitstreamWithoutAnonymousReadIsNotPublished() throws Exception { + Item item = createItem("Group restricted thesis"); + Bitstream bitstream = createOriginalBitstream(item, "group-restricted.pdf"); + + context.turnOffAuthorisationSystem(); + Group reviewers = GroupBuilder.createGroup(context) + .withName("Thesis reviewers " + System.nanoTime()) + .build(); + deletePolicies(bitstream, Constants.READ); + ResourcePolicyBuilder.createResourcePolicy(context, null, reviewers) + .withAction(Constants.READ) + .withDspaceObject(bitstream) + .withName("Reviewers only") + .build(); + context.restoreAuthSystemState(); + + bitstream = context.reloadEntity(bitstream); + + assertTrue("fixture precondition: no Anonymous READ policy may be left on the bitstream" + + describe(bitstream), anonymousReadPolicies(bitstream).isEmpty()); + assertFalse("fixture precondition: a group restricted file must not be publicly readable" + + describe(bitstream), anonymousCanRead(bitstream)); + + Set idsBefore = policyIds(bitstream); + List policiesBefore = policyFingerprints(bitstream); + + Run pastRun = runItemUpdate(item, dublinCore(item, "openAccess", pastDate())); + + bitstream = context.reloadEntity(bitstream); + + assertTrue("An expired embargo created an Anonymous READ policy on a bitstream that only ever granted" + + " READ to a named group. Access may be re-dated, never widened." + + describe(bitstream), + anonymousReadPolicies(bitstream).isEmpty()); + assertUntouched("bitstream readable by a named group only", idsBefore, policiesBefore, bitstream); + assertFalse("A group restricted file became publicly readable after an expired embargo was synchronised." + + describe(bitstream), anonymousCanRead(bitstream)); + + // A past date only reaches the early return; the future-date branch is where a group-restricted file + // could gain an Anonymous policy. + Run futureRun = runItemUpdate(item, + dublinCore(item, "embargoedAccess", futureDate())); + + bitstream = context.reloadEntity(bitstream); + + assertTrue("A FUTURE dc.date.embargoend created an Anonymous READ policy on a bitstream that only ever" + + " granted READ to a named group. The embargo would lapse into public access nobody" + + " ever granted." + describe(bitstream), + anonymousReadPolicies(bitstream).isEmpty()); + assertUntouched("bitstream readable by a named group only, future embargo end date", + idsBefore, policiesBefore, bitstream); + assertFalse("A group restricted file became publicly readable after a future embargo end date was" + + " synchronised." + describe(bitstream), anonymousCanRead(bitstream)); + + assertTrue("There is no Anonymous READ policy to re-date here, so ItemUpdate has to report the bitstream" + + " it could not synchronise and name '" + BULK_ACCESS_CONTROL_HINT + "' as the supported" + + " way to change access. Console output of the expired-embargo run was:" + + System.lineSeparator() + pastRun.console, + pastRun.console.contains(BULK_ACCESS_CONTROL_HINT)); + assertTrue("ItemUpdate stayed silent about a bitstream it could not synchronise. Console output of the" + + " future-embargo run was:" + System.lineSeparator() + futureRun.console, + futureRun.console.contains(BULK_ACCESS_CONTROL_HINT)); + + // An unsynchronised bitstream that leaves the exit code at 0 is invisible to the caller. + assertExitCode("bitstream without Anonymous READ, expired embargo", 1, pastRun); + assertExitCode("bitstream without Anonymous READ, future embargo", 1, futureRun); + } + + /** + * Verifies that a bitstream left without any resource policy stays that way: what it used to grant cannot + * be reconstructed, so the run reports the damage instead of guessing. + */ + @Test + public void alreadyBrokenBitstreamWithZeroPoliciesStaysZero() throws Exception { + Item item = createItem("Already broken thesis"); + Bitstream bitstream = createOriginalBitstream(item, "already-broken.pdf"); + + context.turnOffAuthorisationSystem(); + deletePolicies(bitstream, ALL_ACTIONS); + context.restoreAuthSystemState(); + + bitstream = context.reloadEntity(bitstream); + + assertTrue("fixture precondition: the bitstream must carry no resource policy at all" + + describe(bitstream), allPolicies(bitstream).isEmpty()); + assertFalse("fixture precondition: a bitstream without policies must not be readable" + + describe(bitstream), anonymousCanRead(bitstream)); + + Run run = runItemUpdate(item, dublinCore(item, "openAccess", pastDate())); + + bitstream = context.reloadEntity(bitstream); + + assertTrue("ItemUpdate invented resource policies for a bitstream that had none. A bitstream with zero" + + " policies carries no evidence of who was allowed to read it, so re-creating a policy is" + + " a guess - and the only wrong guess leaks the file." + + describe(bitstream), + allPolicies(bitstream).isEmpty()); + assertFalse("A bitstream with zero policies became publicly readable." + describe(bitstream), + anonymousCanRead(bitstream)); + assertTrue("ItemUpdate stayed silent about a bitstream it could not synchronise. It has to report the" + + " failure and name '" + BULK_ACCESS_CONTROL_HINT + "' as the supported way to restore" + + " access. Console output was:\n" + run.console, + run.console.contains(BULK_ACCESS_CONTROL_HINT)); + assertExitCode("bitstream with zero policies", 1, run); + } + + /** + * Verifies that an {@code Anonymous}/{@code READ} policy with an END date is left alone. It is a lease + * ("public now, closed again on that day"), which {@code dc.date.embargoend} says nothing about. + */ + @Test + public void leasedAnonymousReadPolicyIsUntouched() throws Exception { + Item item = createItem("Leased thesis"); + Bitstream bitstream = createLeasedBitstream(item, "leased.pdf"); + + assertTrue("fixture precondition: a lease that has not expired yet makes the file publicly readable" + + describe(bitstream), anonymousCanRead(bitstream)); + + Set idsBefore = policyIds(bitstream); + List policiesBefore = policyFingerprints(bitstream); + + // The future-date branch is the one that writes policies, so it is where the end date would be lost. + Run futureRun = runItemUpdate(item, + dublinCore(item, "embargoedAccess", futureDate())); + + bitstream = context.reloadEntity(bitstream); + + assertUntouched("leased Anonymous READ policy, future embargo end", idsBefore, policiesBefore, bitstream); + assertTrue("ItemUpdate has to name '" + BULK_ACCESS_CONTROL_HINT + "' as the supported way to change an" + + " access condition it does not manage. Console output was:" + System.lineSeparator() + + futureRun.console, + futureRun.console.contains(BULK_ACCESS_CONTROL_HINT)); + assertExitCode("leased Anonymous READ policy, future embargo end", 1, futureRun); + + // An expired end date reaches the same mutation, only with a start date that has already passed. + Run pastRun = runItemUpdate(item, + dublinCore(item, "openAccess", pastDate())); + + bitstream = context.reloadEntity(bitstream); + + assertUntouched("leased Anonymous READ policy, expired embargo end", idsBefore, policiesBefore, bitstream); + assertExitCode("leased Anonymous READ policy, expired embargo end", 1, pastRun); + } + + /** + * Verifies that a lease sitting next to a dated embargo policy is not deleted: the refusal has to consider + * every {@code Anonymous}/{@code READ} policy, not only the one that would be mutated. + */ + @Test + public void leaseNextToADatedEmbargoPolicyIsNotDeleted() throws Exception { + Item item = createItem("Leased and embargoed thesis"); + Bitstream bitstream = createEmbargoedBitstream(item, "leased-and-embargoed.pdf"); + + context.turnOffAuthorisationSystem(); + addLeasePolicy(bitstream); + context.restoreAuthSystemState(); + bitstream = context.reloadEntity(bitstream); + + assertEquals("fixture precondition: the bitstream has to carry the dated embargo policy AND the lease" + + describe(bitstream), 2, anonymousReadPolicies(bitstream).size()); + + Set idsBefore = policyIds(bitstream); + List policiesBefore = policyFingerprints(bitstream); + + Run run = runItemUpdate(item, + dublinCore(item, "embargoedAccess", futureDate())); + + bitstream = context.reloadEntity(bitstream); + + assertUntouched("lease next to a dated embargo policy", idsBefore, policiesBefore, bitstream); + assertExitCode("lease next to a dated embargo policy", 1, run); + } + + /** + * Verifies that a blank end date changes nothing: validation runs before anything is mutated. + */ + @Test + public void blankEmbargoEndLeavesPoliciesUntouched() throws Exception { + Item item = assertEmbargoSyncIsANoOp("blank dc.date.embargoend", + Collections.singletonList("openAccess"), "", 1); + + List storedEndDates = itemService.getMetadata(item, "dc", "date", "embargoend", Item.ANY); + assertEquals("fixture precondition: the blank dc.date.embargoend has to be stored as an empty value." + + " If it were dropped the item would look like 'the operator removed the embargo' and" + + " this test would silently stop covering the blank value case.", + 1, storedEndDates.size()); + assertTrue("fixture precondition: the stored dc.date.embargoend has to be blank, not a real date", + storedEndDates.get(0).getValue().trim().isEmpty()); + } + + /** + * Verifies that parsing is strict: a lenient parser rolls 30 February over into 2 March and turns an + * unreadable value into a real embargo date. + */ + @Test + public void invalidEmbargoEndLeavesPoliciesUntouched() throws Exception { + // dynamic year, but 30 February and month 13 do not exist in any year + int year = LocalDate.now().plusYears(1).getYear(); + List invalidEndDates = Arrays.asList("abc", year + "-02-30", year + "-13-01"); + + for (String invalidEndDate : invalidEndDates) { + assertEmbargoSyncIsANoOp("unparseable dc.date.embargoend=" + invalidEndDate, + Collections.singletonList("openAccess"), invalidEndDate, 1); + } + } + + /** + * Verifies that {@code embargoedAccess} without an end date is refused rather than half-applied. + */ + @Test + public void embargoedAccessWithoutEndDateLeavesPoliciesUntouched() throws Exception { + assertEmbargoSyncIsANoOp("dc.rights.access=embargoedAccess without dc.date.embargoend", + Collections.singletonList("embargoedAccess"), null, 1); + } + + /** + * Verifies that an item outside the archive is left alone; its bitstream policies belong to the + * submission, not to itemupdate. + */ + @Test + public void notArchivedItemIsUntouched() throws Exception { + Item item = createItem("Not archived thesis"); + Bitstream bitstream = createEmbargoedBitstream(item, "not-archived.pdf"); + + // take the item back out of the archive; the handle stays, so the SAF update still resolves it + context.turnOffAuthorisationSystem(); + item.setArchived(false); + itemService.update(context, item); + context.restoreAuthSystemState(); + + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertFalse("fixture precondition: the item must not be archived", item.isArchived()); + assertFalse("fixture precondition: the embargoed file must not be publicly readable" + + describe(bitstream), anonymousCanRead(bitstream)); + + Set idsBefore = policyIds(bitstream); + List policiesBefore = policyFingerprints(bitstream); + + Run run = runItemUpdate(item, dublinCore(item, "openAccess", pastDate())); + + bitstream = context.reloadEntity(bitstream); + + assertExitCode("item outside the archive", 0, run); + assertUntouched("item outside the archive", idsBefore, policiesBefore, bitstream); + assertFalse("A file of an item outside the archive became publicly readable." + describe(bitstream), + anonymousCanRead(bitstream)); + } + + /** + * Verifies that a synchronisation which throws after re-dating a policy is not reported as success: the + * per-item catch of {@code processArchive} prints the exception and commits what was written so far. + */ + @Test + public void embargoSyncThatDiesHalfWayIsNotReportedAsSuccess() throws Exception { + Item item = createItem("Embargo sync dies half way"); + Bitstream bitstream = createEmbargoedBitstream(item, "half-way.pdf"); + + assertFalse("fixture precondition: the embargoed file must not be publicly readable" + + describe(bitstream), anonymousCanRead(bitstream)); + + ItemUpdate itemUpdate = new ItemUpdate() { + @Override + protected void applyEmbargoToItemBitstreams(Context context, Item item, Date startDate) + throws SQLException, AuthorizeException { + super.applyEmbargoToItemBitstreams(context, item, startDate); + throw new IllegalStateException("simulated failure while deleting the duplicate policies"); + } + }; + + Run run = runItemUpdate(itemUpdate, item, + dublinCore(item, "openAccess", pastDate())); + + bitstream = context.reloadEntity(bitstream); + + assertTrue("fixture precondition: the simulated failure has to strike AFTER the surviving policy was" + + " re-dated, otherwise this test does not describe the dangerous case at all. The file" + + " is readable now, and that is the state context.complete() would commit." + + describe(bitstream), + anonymousCanRead(bitstream)); + assertExitCode("embargo synchronisation that threw after re-dating a policy", 1, run); + } + + /** + * Runs one "hands off" scenario end to end and returns the reloaded item. The fixture is the state an + * earlier run with a future end date leaves: one dated Anonymous READ policy currently blocking access. + */ + private Item assertEmbargoSyncIsANoOp(String scenario, List accessRights, String embargoEndDate, + int expectedFailures) + throws Exception { + Item item = createItem("Safety scenario: " + scenario); + Bitstream bitstream = createEmbargoedBitstream(item, "thesis.pdf"); + + assertFalse("fixture precondition [" + scenario + "]: the embargoed file must not be publicly readable" + + describe(bitstream), anonymousCanRead(bitstream)); + + Set idsBefore = policyIds(bitstream); + List policiesBefore = policyFingerprints(bitstream); + + Run run = runItemUpdate(item, dublinCoreWithAccessRights(item, accessRights, embargoEndDate)); + + item = context.reloadEntity(item); + bitstream = context.reloadEntity(bitstream); + + assertExitCode(scenario, expectedFailures, run); + assertUntouched(scenario, idsBefore, policiesBefore, bitstream); + assertFalse("[" + scenario + "] the embargoed file became publicly readable, which is a data leak" + + describe(bitstream), anonymousCanRead(bitstream)); + + return item; + } + + /** + * Compares policy identity first (ids), then policy content (fingerprints). The id comparison catches + * delete-and-recreate, the fingerprint comparison catches in-place mutation of a surviving policy. + */ + private void assertUntouched(String scenario, Set idsBefore, List policiesBefore, + Bitstream bitstream) throws SQLException { + assertEquals("[" + scenario + "] the set of resource policy ids changed: policies were deleted and/or" + + " re-created although nothing at all should have happened." + describe(bitstream), + idsBefore, policyIds(bitstream)); + assertEquals("[" + scenario + "] a surviving resource policy was modified in place although nothing at" + + " all should have happened." + describe(bitstream), + policiesBefore, policyFingerprints(bitstream)); + } + + /** + * The state an itemupdate run with a future end date leaves: a single dated legacy policy blocking access, + * and nothing else between the public and the file. + */ + private Bitstream createEmbargoedBitstream(Item item, String name) throws Exception { + Bitstream bitstream = createOriginalBitstream(item, name); + + context.turnOffAuthorisationSystem(); + deletePolicies(bitstream, Constants.READ); + ResourcePolicyBuilder.createResourcePolicy(context, null, anonymousGroup) + .withAction(Constants.READ) + .withDspaceObject(bitstream) + .withName(LEGACY_EMBARGO_POLICY_NAME) + .withStartDate(startOfDayUtc(LocalDate.now().plusYears(1))) + .build(); + context.restoreAuthSystemState(); + + return context.reloadEntity(bitstream); + } + + /** + * A bitstream whose {@code Anonymous}/{@code READ} access expires by itself, as the {@code lease} access + * condition writes it: no start date and an end date six months out. + */ + private Bitstream createLeasedBitstream(Item item, String name) throws Exception { + Bitstream bitstream = createOriginalBitstream(item, name); + + context.turnOffAuthorisationSystem(); + deletePolicies(bitstream, Constants.READ); + addLeasePolicy(bitstream); + context.restoreAuthSystemState(); + + return context.reloadEntity(bitstream); + } + + private void addLeasePolicy(Bitstream bitstream) throws Exception { + ResourcePolicyBuilder.createResourcePolicy(context, null, anonymousGroup) + .withAction(Constants.READ) + .withDspaceObject(bitstream) + .withName(LEASE_POLICY_NAME) + .withPolicyType(ResourcePolicy.TYPE_CUSTOM) + .withEndDate(startOfDayUtc(LocalDate.now().plusMonths(6))) + .build(); + } + + /** + * Renders the current policies of the bitstream for failure messages, so a red build shows which policy + * moved. + */ + private String describe(Bitstream bitstream) throws SQLException { + StringBuilder sb = new StringBuilder(System.lineSeparator()) + .append(" bitstream=").append(bitstream.getID()).append(System.lineSeparator()); + List fingerprints = policyFingerprints(bitstream); + if (fingerprints.isEmpty()) { + sb.append(" ").append(System.lineSeparator()); + } + for (String fingerprint : fingerprints) { + sb.append(" ").append(fingerprint).append(System.lineSeparator()); + } + return sb.toString(); + } + +} diff --git a/dspace-api/src/test/java/org/dspace/app/itemupdate/ItemUpdateIT.java b/dspace-api/src/test/java/org/dspace/app/itemupdate/ItemUpdateIT.java index 929cb0f06bd6..6871b4d079eb 100644 --- a/dspace-api/src/test/java/org/dspace/app/itemupdate/ItemUpdateIT.java +++ b/dspace-api/src/test/java/org/dspace/app/itemupdate/ItemUpdateIT.java @@ -13,7 +13,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -22,95 +21,29 @@ import java.time.ZoneId; import java.util.Date; import java.util.List; +import java.util.Set; -import org.apache.commons.io.file.PathUtils; -import org.dspace.AbstractIntegrationTestWithDatabase; import org.dspace.authorize.ResourcePolicy; -import org.dspace.authorize.factory.AuthorizeServiceFactory; -import org.dspace.authorize.service.ResourcePolicyService; -import org.dspace.builder.BitstreamBuilder; -import org.dspace.builder.CollectionBuilder; -import org.dspace.builder.CommunityBuilder; -import org.dspace.builder.ItemBuilder; -import org.dspace.builder.MetadataFieldBuilder; -import org.dspace.builder.ResourcePolicyBuilder; import org.dspace.content.Bitstream; -import org.dspace.content.Collection; import org.dspace.content.Item; -import org.dspace.content.MetadataField; -import org.dspace.content.MetadataSchema; import org.dspace.content.MetadataValue; -import org.dspace.content.factory.ContentServiceFactory; -import org.dspace.content.service.ItemService; -import org.dspace.content.service.MetadataFieldService; -import org.dspace.content.service.MetadataSchemaService; -import org.dspace.core.Constants; -import org.dspace.eperson.Group; -import org.dspace.eperson.factory.EPersonServiceFactory; -import org.dspace.eperson.service.GroupService; -import org.dspace.handle.factory.HandleServiceFactory; -import org.dspace.handle.service.HandleService; -import org.junit.After; -import org.junit.Before; import org.junit.Test; /** * Integration tests for {@link ItemUpdate} and {@link ItemArchive}. */ -public class ItemUpdateIT extends AbstractIntegrationTestWithDatabase { - - private static final String STANDARD_EMBARGO = "Standard Embargo"; - private static final String SPECIAL_CASE_EMBARGO = "Special Case Embargo"; - - private ItemService itemService = ContentServiceFactory.getInstance().getItemService(); - private HandleService handleService = HandleServiceFactory.getInstance().getHandleService(); - private ResourcePolicyService resourcePolicyService = - AuthorizeServiceFactory.getInstance().getResourcePolicyService(); - private GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); - private MetadataSchemaService metadataSchemaService = - ContentServiceFactory.getInstance().getMetadataSchemaService(); - private MetadataFieldService metadataFieldService = - ContentServiceFactory.getInstance().getMetadataFieldService(); - - private Collection collection; - private Group anonymousGroup; - private Path tempDir; - private String previousHandlePrefix; - - @Before - @Override - public void setUp() throws Exception { - super.setUp(); - context.turnOffAuthorisationSystem(); - - parentCommunity = CommunityBuilder.createCommunity(context) - .withName("Parent Community") - .build(); - collection = CollectionBuilder.createCollection(context, parentCommunity) - .withName("Collection") - .build(); - - ensureMetadataFieldExists("identifier", "thesis"); - ensureMetadataFieldExists("rights", "access"); - ensureMetadataFieldExists("date", "embargoend"); - - anonymousGroup = groupService.findByName(context, Group.ANONYMOUS); - previousHandlePrefix = ItemUpdate.HANDLE_PREFIX; - ItemUpdate.HANDLE_PREFIX = handleService.getCanonicalPrefix(); - - context.restoreAuthSystemState(); - - tempDir = Files.createTempDirectory("itemUpdateIT"); - } +public class ItemUpdateIT extends AbstractEmbargoIT { - @After - @Override - public void destroy() throws Exception { - ItemUpdate.HANDLE_PREFIX = previousHandlePrefix; - if (tempDir != null) { - PathUtils.deleteDirectory(tempDir); - } - super.destroy(); + /** + * Verifies the step that turns {@code embargoSyncFailures} into the process exit code, which is otherwise + * only reachable through {@code main()} and its {@code System.exit}. + */ + @Test + public void embargoSyncFailuresDecideTheExitCode() { + assertEquals("a clean run has to exit 0", 0, ItemUpdate.exitStatus(0, 0)); + assertEquals("a single unsynchronised bitstream has to fail the run", 1, ItemUpdate.exitStatus(0, 1)); + assertEquals("several problems still fail the run once", 1, ItemUpdate.exitStatus(0, 7)); + assertEquals("an already failed run stays failed", 1, ItemUpdate.exitStatus(1, 0)); } @Test @@ -174,72 +107,85 @@ public void itemArchiveCreateFailsForAmbiguousIdentifierUri() throws Exception { } @Test - public void syncEmbargoPoliciesCreatesStandardEmbargoAndRemovesImmediateAnonymousRead() throws Exception { + public void syncEmbargoPoliciesDatesTheAnonymousReadPolicyAndBlocksAccess() throws Exception { String futureDate = LocalDate.now().plusDays(14).toString(); Item item = createItem("Standard Embargo Item", "rights", "access", "embargoedAccess", "date", "embargoend", futureDate); - Bitstream bitstream = createBitstream(item, "standard.txt"); + Bitstream bitstream = createOriginalBitstream(item, "standard.txt"); - createAnonymousReadPolicy(bitstream, null, "Immediate Read"); + addAnonymousReadPolicy(bitstream, null, "Immediate Read"); ItemUpdate itemUpdate = new ItemUpdate(); itemUpdate.syncEmbargoPolicies(context, item); - - List readPolicies = resourcePolicyService.find(context, bitstream, Constants.READ); - - boolean hasImmediateAnonymousRead = readPolicies.stream() - .anyMatch(policy -> isAnonymousPolicy(policy) && policy.getStartDate() == null); - assertFalse(hasImmediateAnonymousRead); - - ResourcePolicy embargoPolicy = readPolicies.stream() - .filter(policy -> isAnonymousPolicy(policy) - && STANDARD_EMBARGO.equals(policy.getRpName()) - && policy.getStartDate() != null) - .findFirst() - .orElse(null); - - assertNotNull(embargoPolicy); + assertEquals("setting a future embargo is not a failure", 0, itemUpdate.embargoSyncFailures); + + // A second, undated policy would defeat the embargo, so the count is part of the assertion. + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals(1, anonymousRead.size()); + + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertEquals(EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertEquals(ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertNotNull(embargoPolicy.getStartDate()); + assertEquals(LocalDate.parse(futureDate).plusDays(1), toLocalDate(embargoPolicy.getStartDate())); + assertFalse(anonymousCanRead(bitstream)); } @Test - public void syncEmbargoPoliciesCreatesSpecialCasePolicyWithoutAccessRightMetadata() throws Exception { + public void syncEmbargoPoliciesAppliesEmbargoWithoutAccessRightMetadata() throws Exception { String futureDate = LocalDate.now().plusDays(21).toString(); Item item = createItem("Special Case Embargo Item", "date", "embargoend", futureDate); - Bitstream bitstream = createBitstream(item, "special.txt"); + Bitstream bitstream = createOriginalBitstream(item, "special.txt"); ItemUpdate itemUpdate = new ItemUpdate(); itemUpdate.syncEmbargoPolicies(context, item); - - List readPolicies = resourcePolicyService.find(context, bitstream, Constants.READ); - - ResourcePolicy embargoPolicy = readPolicies.stream() - .filter(policy -> isAnonymousPolicy(policy) - && SPECIAL_CASE_EMBARGO.equals(policy.getRpName()) - && policy.getStartDate() != null) - .findFirst() - .orElse(null); - - assertNotNull(embargoPolicy); + assertEquals("setting a future embargo is not a failure", 0, itemUpdate.embargoSyncFailures); + + List anonymousRead = anonymousReadPolicies(bitstream); + assertEquals(1, anonymousRead.size()); + + // Both cases share one access condition name, which also fits the 30 character + // resourcepolicy.rpname column. + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertEquals(EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertEquals(ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertNotNull(embargoPolicy.getStartDate()); + assertEquals(LocalDate.parse(futureDate).plusDays(1), toLocalDate(embargoPolicy.getStartDate())); + assertFalse(anonymousCanRead(bitstream)); } + /** + * Verifies that a blank {@code dc.date.embargoend} leaves every resource policy untouched and fails the + * run; it is a broken export, not an instruction to change anything. + */ @Test - public void syncEmbargoPoliciesClearsSafEmbargoPoliciesWhenEmbargoDateInvalid() throws Exception { + public void syncEmbargoPoliciesLeavesPoliciesUntouchedWhenEmbargoDateInvalid() throws Exception { Item item = createItem("Invalid Date Item", "date", "embargoend", ""); - Bitstream bitstream = createBitstream(item, "invalid.txt"); - createAnonymousReadPolicy(bitstream, new Date(System.currentTimeMillis() + 86_400_000L), STANDARD_EMBARGO); + Bitstream bitstream = createOriginalBitstream(item, "invalid.txt"); + ResourcePolicy legacyPolicy = replaceAnonymousReadPolicies(bitstream, + new Date(System.currentTimeMillis() + 86_400_000L), LEGACY_EMBARGO_POLICY_NAME); + bitstream = context.reloadEntity(bitstream); + + Set idsBefore = policyIds(bitstream); + assertFalse("fixture precondition: the embargoed file must not be publicly readable, otherwise the" + + " 'nothing changed' assertions below say nothing about a leak", + anonymousCanRead(bitstream)); ItemUpdate itemUpdate = new ItemUpdate(); itemUpdate.syncEmbargoPolicies(context, item); - List readPolicies = resourcePolicyService.find(context, bitstream, Constants.READ); - - boolean hasSafEmbargoPolicy = readPolicies.stream() - .anyMatch(policy -> isAnonymousPolicy(policy) - && (STANDARD_EMBARGO.equals(policy.getRpName()) - || SPECIAL_CASE_EMBARGO.equals(policy.getRpName()))); - - assertFalse(hasSafEmbargoPolicy); + // A blank end date is broken input, so the run has to exit non-zero. + assertEquals("a blank dc.date.embargoend has to fail the run", 1, itemUpdate.embargoSyncFailures); + assertEquals(1, ItemUpdate.exitStatus(0, itemUpdate.embargoSyncFailures)); + assertEquals(idsBefore, policyIds(bitstream)); + assertFalse("an unparseable dc.date.embargoend published an embargoed file", + anonymousCanRead(bitstream)); + + // The dated policy the run could not validate is still there, unchanged, under its legacy name. + ResourcePolicy reloadedLegacy = resourcePolicyService.find(context, legacyPolicy.getID()); + assertNotNull(reloadedLegacy); + assertEquals(LEGACY_EMBARGO_POLICY_NAME, reloadedLegacy.getRpName()); } @Test @@ -250,13 +196,15 @@ public void processArchiveUpdatesEmbargoMetadataAndResyncsEmbargoPolicy() throws Item item = createItem("Embargo Update Item", "rights", "access", "embargoedAccess", "date", "embargoend", oldEmbargoDate); - Bitstream bitstream = createBitstream(item, "update-embargo.txt"); + Bitstream bitstream = createOriginalBitstream(item, "update-embargo.txt"); LocalDate oldPolicyDate = LocalDate.parse(oldEmbargoDate).plusDays(1); Date oldPolicyStart = Date.from(oldPolicyDate.atStartOfDay(ZoneId.systemDefault()).toInstant()); - createAnonymousReadPolicy(bitstream, oldPolicyStart, STANDARD_EMBARGO); + ResourcePolicy legacyPolicy = addAnonymousReadPolicy(bitstream, oldPolicyStart, LEGACY_EMBARGO_POLICY_NAME); + Integer legacyPolicyId = legacyPolicy.getID(); - runEmbargoMetadataUpdate(item, dublinCoreWithEmbargo(item, "embargoedAccess", newEmbargoDate)); + assertEquals("re-dating an embargo is not a failure", 0, + runItemUpdateFailures(item, dublinCore(item, "embargoedAccess", newEmbargoDate))); Item reloadedItem = context.reloadEntity(item); Bitstream reloadedBitstream = context.reloadEntity(bitstream); @@ -266,59 +214,85 @@ public void processArchiveUpdatesEmbargoMetadataAndResyncsEmbargoPolicy() throws assertEquals(newEmbargoDate, embargoDates.get(0).getValue()); LocalDate expectedPolicyDate = LocalDate.parse(newEmbargoDate).plusDays(1); - List readPolicies = resourcePolicyService.find(context, reloadedBitstream, Constants.READ); - - boolean hasOldEmbargoPolicy = readPolicies.stream().anyMatch(policy -> isAnonymousPolicy(policy) - && STANDARD_EMBARGO.equals(policy.getRpName()) - && policy.getStartDate() != null - && toLocalDate(policy.getStartDate()).equals(oldPolicyDate)); - assertFalse(hasOldEmbargoPolicy); - - boolean hasNewEmbargoPolicy = readPolicies.stream().anyMatch(policy -> isAnonymousPolicy(policy) - && STANDARD_EMBARGO.equals(policy.getRpName()) - && policy.getStartDate() != null - && toLocalDate(policy.getStartDate()).equals(expectedPolicyDate)); - assertTrue(hasNewEmbargoPolicy); + List anonymousRead = anonymousReadPolicies(reloadedBitstream); + assertEquals(1, anonymousRead.size()); + + // The pre-existing policy is re-dated in place, so the file is covered by a policy at every moment. + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertEquals(legacyPolicyId, embargoPolicy.getID()); + assertEquals(EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertEquals(ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertNotNull(embargoPolicy.getStartDate()); + assertFalse(toLocalDate(embargoPolicy.getStartDate()).equals(oldPolicyDate)); + assertEquals(expectedPolicyDate, toLocalDate(embargoPolicy.getStartDate())); + assertFalse(anonymousCanRead(reloadedBitstream)); } + /** + * Same as {@link #syncEmbargoPoliciesLeavesPoliciesUntouchedWhenEmbargoDateInvalid()} driven through a SAF + * archive whose {@code dc.date.embargoend} is blank. + */ @Test - public void processArchiveUpdateWithBlankEmbargoDateClearsSafEmbargoPolicies() throws Exception { + public void processArchiveUpdateWithBlankEmbargoDateLeavesPoliciesUntouched() throws Exception { String oldEmbargoDate = LocalDate.now().plusDays(12).toString(); Item item = createItem("Blank Embargo Date Update", "rights", "access", "embargoedAccess", "date", "embargoend", oldEmbargoDate); - Bitstream bitstream = createBitstream(item, "blank-embargo-date.txt"); + Bitstream bitstream = createOriginalBitstream(item, "blank-embargo-date.txt"); Date oldPolicyStart = Date.from(LocalDate.parse(oldEmbargoDate).plusDays(1) .atStartOfDay(ZoneId.systemDefault()).toInstant()); - createAnonymousReadPolicy(bitstream, oldPolicyStart, STANDARD_EMBARGO); + ResourcePolicy legacyPolicy = + replaceAnonymousReadPolicies(bitstream, oldPolicyStart, LEGACY_EMBARGO_POLICY_NAME); + bitstream = context.reloadEntity(bitstream); + + Set idsBefore = policyIds(bitstream); + assertFalse("fixture precondition: the embargoed file must not be publicly readable, otherwise the" + + " 'nothing changed' assertions below say nothing about a leak", + anonymousCanRead(bitstream)); - runEmbargoMetadataUpdate(item, dublinCoreWithEmbargo(item, "embargoedAccess", "")); + assertEquals("a blank dc.date.embargoend has to fail the run", 1, + runItemUpdateFailures(item, dublinCore(item, "embargoedAccess", ""))); Bitstream reloadedBitstream = context.reloadEntity(bitstream); - List readPolicies = resourcePolicyService.find(context, reloadedBitstream, Constants.READ); - boolean hasSafEmbargoPolicy = readPolicies.stream().anyMatch(policy -> isAnonymousPolicy(policy) - && (STANDARD_EMBARGO.equals(policy.getRpName()) - || SPECIAL_CASE_EMBARGO.equals(policy.getRpName()))); - assertFalse(hasSafEmbargoPolicy); + assertEquals(idsBefore, policyIds(reloadedBitstream)); + assertFalse("a blank dc.date.embargoend published an embargoed file", + anonymousCanRead(reloadedBitstream)); + + ResourcePolicy reloadedLegacy = resourcePolicyService.find(context, legacyPolicy.getID()); + assertNotNull(reloadedLegacy); + assertEquals(LEGACY_EMBARGO_POLICY_NAME, reloadedLegacy.getRpName()); } + /** + * Verifies that a SAF package without {@code dc.date.embargoend} leaves every policy untouched. The field + * carries no instruction about the embargo; a file is opened by an end date that lies in the past. + */ @Test - public void processArchiveUpdateRemovingEmbargoMetadataClearsPoliciesAndMetadata() throws Exception { + public void processArchiveUpdateRemovingEmbargoMetadataLeavesPoliciesUntouched() throws Exception { String oldEmbargoDate = LocalDate.now().plusDays(10).toString(); Item item = createItem("Remove Embargo Metadata Update", "rights", "access", "embargoedAccess", "date", "embargoend", oldEmbargoDate); - Bitstream bitstream = createBitstream(item, "remove-embargo.txt"); + Bitstream bitstream = createOriginalBitstream(item, "remove-embargo.txt"); Date oldPolicyStart = Date.from(LocalDate.parse(oldEmbargoDate).plusDays(1) .atStartOfDay(ZoneId.systemDefault()).toInstant()); - createAnonymousReadPolicy(bitstream, oldPolicyStart, STANDARD_EMBARGO); + // The undated Anonymous READ policy from the collection default has to go, otherwise the file is + // readable throughout and the assertions below prove nothing. + ResourcePolicy legacyPolicy = + replaceAnonymousReadPolicies(bitstream, oldPolicyStart, LEGACY_EMBARGO_POLICY_NAME); + Integer legacyPolicyId = legacyPolicy.getID(); + bitstream = context.reloadEntity(bitstream); - runEmbargoMetadataUpdate(item, dublinCore(item)); + Set idsBefore = policyIds(bitstream); + assertFalse("fixture precondition: the embargoed file must not be publicly readable", + anonymousCanRead(bitstream)); + + int failures = runItemUpdateFailures(item, dublinCore(item)); Item reloadedItem = context.reloadEntity(item); Bitstream reloadedBitstream = context.reloadEntity(bitstream); @@ -326,30 +300,41 @@ public void processArchiveUpdateRemovingEmbargoMetadataClearsPoliciesAndMetadata List rightsAccess = itemService.getMetadata(reloadedItem, "dc", "rights", "access", Item.ANY); List embargoDates = itemService.getMetadata(reloadedItem, "dc", "date", "embargoend", Item.ANY); assertTrue(rightsAccess.isEmpty()); - assertTrue(embargoDates.isEmpty()); - - List readPolicies = resourcePolicyService.find(context, reloadedBitstream, Constants.READ); - boolean hasSafEmbargoPolicy = readPolicies.stream().anyMatch(policy -> isAnonymousPolicy(policy) - && (STANDARD_EMBARGO.equals(policy.getRpName()) - || SPECIAL_CASE_EMBARGO.equals(policy.getRpName()))); - assertFalse(hasSafEmbargoPolicy); + assertTrue("fixture precondition: dc.date.embargoend has to be gone from the item", embargoDates.isEmpty()); + + // Nothing happened: same policy rows, same name, same start date, same answer to "can anyone read it". + assertEquals("removing dc.date.embargoend must not add or remove a single resource policy", + idsBefore, policyIds(reloadedBitstream)); + assertEquals(1, anonymousReadPolicies(reloadedBitstream).size()); + + ResourcePolicy untouchedPolicy = anonymousReadPolicies(reloadedBitstream).get(0); + assertEquals(legacyPolicyId, untouchedPolicy.getID()); + assertNotNull("removing dc.date.embargoend must not clear the embargo start date", + untouchedPolicy.getStartDate()); + assertEquals(LEGACY_EMBARGO_POLICY_NAME, untouchedPolicy.getRpName()); + assertFalse("removing dc.date.embargoend published an embargoed file", anonymousCanRead(reloadedBitstream)); + + // "No instruction" is not a failure - the batch has to keep its exit code 0. + assertEquals("a SAF package without dc.date.embargoend is not an error", 0, failures); } @Test - public void processArchiveUpdateWithEmbargoDateAndNoRightsCreatesSpecialCasePolicy() throws Exception { + public void processArchiveUpdateWithEmbargoDateAndNoRightsAppliesEmbargo() throws Exception { String oldEmbargoDate = LocalDate.now().plusDays(8).toString(); String newEmbargoDate = LocalDate.now().plusDays(25).toString(); Item item = createItem("Special Case Update", "rights", "access", "embargoedAccess", "date", "embargoend", oldEmbargoDate); - Bitstream bitstream = createBitstream(item, "special-case-update.txt"); + Bitstream bitstream = createOriginalBitstream(item, "special-case-update.txt"); Date oldPolicyStart = Date.from(LocalDate.parse(oldEmbargoDate).plusDays(1) .atStartOfDay(ZoneId.systemDefault()).toInstant()); - createAnonymousReadPolicy(bitstream, oldPolicyStart, STANDARD_EMBARGO); + ResourcePolicy legacyPolicy = addAnonymousReadPolicy(bitstream, oldPolicyStart, LEGACY_EMBARGO_POLICY_NAME); + Integer legacyPolicyId = legacyPolicy.getID(); - runEmbargoMetadataUpdate(item, dublinCoreWithEmbargo(item, null, newEmbargoDate)); + assertEquals("an embargo end date without dc.rights.access is not a failure", 0, + runItemUpdateFailures(item, dublinCore(item, null, newEmbargoDate))); Item reloadedItem = context.reloadEntity(item); Bitstream reloadedBitstream = context.reloadEntity(bitstream); @@ -358,69 +343,16 @@ public void processArchiveUpdateWithEmbargoDateAndNoRightsCreatesSpecialCasePoli assertTrue(rightsAccess.isEmpty()); LocalDate expectedPolicyDate = LocalDate.parse(newEmbargoDate).plusDays(1); - List readPolicies = resourcePolicyService.find(context, reloadedBitstream, Constants.READ); - - boolean hasSpecialCasePolicy = readPolicies.stream().anyMatch(policy -> isAnonymousPolicy(policy) - && SPECIAL_CASE_EMBARGO.equals(policy.getRpName()) - && policy.getStartDate() != null - && toLocalDate(policy.getStartDate()).equals(expectedPolicyDate)); - assertTrue(hasSpecialCasePolicy); - - boolean hasStandardPolicy = readPolicies.stream().anyMatch(policy -> isAnonymousPolicy(policy) - && STANDARD_EMBARGO.equals(policy.getRpName())); - assertFalse(hasStandardPolicy); - } - - private void ensureMetadataFieldExists(String element, String qualifier) throws Exception { - MetadataSchema dcSchema = metadataSchemaService.find(context, "dc"); - MetadataField existingField = metadataFieldService.findByElement(context, dcSchema, element, qualifier); - if (existingField == null) { - MetadataFieldBuilder.createMetadataField(context, dcSchema, element, qualifier, null).build(); - } - } - - private Item createItem(String title, String... metadataTriples) throws Exception { - context.turnOffAuthorisationSystem(); - - ItemBuilder builder = ItemBuilder.createItem(context, collection) - .withTitle(title); - - for (int i = 0; i + 2 < metadataTriples.length; i += 3) { - builder.withMetadata("dc", metadataTriples[i], metadataTriples[i + 1], metadataTriples[i + 2]); - } - - Item item = builder.build(); - context.restoreAuthSystemState(); - return item; - } - - private Bitstream createBitstream(Item item, String name) throws Exception { - context.turnOffAuthorisationSystem(); - Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, - new ByteArrayInputStream(("content-" + name).getBytes(StandardCharsets.UTF_8))) - .withName(name) - .withMimeType("text/plain") - .build(); - context.restoreAuthSystemState(); - return bitstream; - } - - private void createAnonymousReadPolicy(Bitstream bitstream, Date startDate, String name) throws Exception { - context.turnOffAuthorisationSystem(); - ResourcePolicyBuilder builder = ResourcePolicyBuilder.createResourcePolicy(context, null, anonymousGroup) - .withAction(Constants.READ) - .withDspaceObject(bitstream) - .withName(name); - - if (startDate != null) { - builder.withStartDate(startDate); - } - builder.build(); - context.restoreAuthSystemState(); - } - - private boolean isAnonymousPolicy(ResourcePolicy policy) { - return policy.getGroup() != null && policy.getGroup().equals(anonymousGroup); + List anonymousRead = anonymousReadPolicies(reloadedBitstream); + assertEquals(1, anonymousRead.size()); + + ResourcePolicy embargoPolicy = anonymousRead.get(0); + assertEquals(legacyPolicyId, embargoPolicy.getID()); + assertEquals(EMBARGO_POLICY_NAME, embargoPolicy.getRpName()); + assertEquals(ResourcePolicy.TYPE_CUSTOM, embargoPolicy.getRpType()); + assertNotNull(embargoPolicy.getStartDate()); + assertEquals(expectedPolicyDate, toLocalDate(embargoPolicy.getStartDate())); + assertFalse(anonymousCanRead(reloadedBitstream)); } private Path createSafItemDirectory(String dublinCoreContent) throws IOException { @@ -448,30 +380,6 @@ private String dublinCore(String identifierUri, String thesisIdentifier) { return sb.toString(); } - private void runEmbargoMetadataUpdate(Item item, String dublinCoreContent) throws Exception { - Path sourceRoot = Files.createDirectory(tempDir.resolve("update-source-" + System.nanoTime())); - Files.createFile(sourceRoot.resolve(ItemUpdate.SUPPRESS_UNDO_FILENAME)); - - Path itemDir = Files.createDirectory(sourceRoot.resolve("item_000")); - Files.writeString(itemDir.resolve("dublin_core.xml"), dublinCoreContent, StandardCharsets.UTF_8); - - ItemUpdate itemUpdate = new ItemUpdate(); - DeleteMetadataAction deleteAction = - (DeleteMetadataAction) itemUpdate.actionMgr.getUpdateAction(DeleteMetadataAction.class); - deleteAction.addTargetFields(new String[] { "dc.rights.access", "dc.date.embargoend" }); - - AddMetadataAction addAction = - (AddMetadataAction) itemUpdate.actionMgr.getUpdateAction(AddMetadataAction.class); - addAction.addTargetFields(new String[] { "dc.rights.access", "dc.date.embargoend" }); - - context.turnOffAuthorisationSystem(); - itemUpdate.processArchive(context, sourceRoot.toString(), null, null, true, false, true); - context.restoreAuthSystemState(); - - // Force entity reload in caller assertions after update transaction. - context.uncacheEntity(item); - } - private String dublinCore(Item item) { String identifierUri = ItemUpdate.HANDLE_PREFIX + item.getHandle(); return "\n" @@ -480,35 +388,4 @@ private String dublinCore(Item item) { + ""; } - private String dublinCoreWithEmbargo(Item item, String rightsAccess, String embargoEndDate) { - String identifierUri = ItemUpdate.HANDLE_PREFIX + item.getHandle(); - StringBuilder sb = new StringBuilder(); - sb.append("\n") - .append("\n") - .append(" ") - .append(identifierUri) - .append("\n"); - - if (rightsAccess != null) { - sb.append(" ") - .append(rightsAccess) - .append("\n"); - } - - if (embargoEndDate != null) { - sb.append(" ") - .append(embargoEndDate.isEmpty() ? " " : embargoEndDate) - .append("\n"); - } - - sb.append(""); - return sb.toString(); - } - - private LocalDate toLocalDate(Date date) { - if (date instanceof java.sql.Date) { - return ((java.sql.Date) date).toLocalDate(); - } - return date.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); - } } \ No newline at end of file