From 50a321f96682193bf554a8c14266cfb5cf773a1b Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 1 Oct 2025 17:15:06 +0100 Subject: [PATCH 01/45] Stash: refactoring getCompareVersionsSummary endpoint WIP --- .../harvard/iq/dataverse/api/Datasets.java | 13 +++- .../DatasetVersionSummary.java | 72 +++++++++++++++++++ .../DatasetVersionSummaryContent.java | 4 ++ ...setVersionSummaryContentDeaccessioned.java | 20 ++++++ ...tasetVersionSummaryContentDifferences.java | 4 ++ .../DatasetVersionSummaryContentSimple.java | 16 +++++ .../GetDatasetVersionSummariesCommand.java | 39 ++++++++++ .../iq/dataverse/util/json/JsonPrinter.java | 6 ++ 8 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContent.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDeaccessioned.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index 3334f2ec8bb..b2a93f7b2b0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -3138,8 +3138,15 @@ public Response getCompareVersions(@Context ContainerRequestContext crc, @PathPa @GET @AuthRequired @Path("{id}/versions/compareSummary") - public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, @PathParam("id") String id, - @Context UriInfo uriInfo, @Context HttpHeaders headers) { + public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, @PathParam("id") String id) { + return response(req -> { + try { + return ok(json(execCommand(new GetDatasetVersionSummariesCommand(req, findDatasetOrDie(id))))); + } catch (WrappedResponse wr) { + return wr.getResponse(); + } + }, getRequestUser(crc)); + /* try { Dataset dataset = findDatasetOrDie(id); User user = getRequestUser(crc); @@ -3188,7 +3195,7 @@ public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, return ok(differenceSummaries); } catch (WrappedResponse wr) { return wr.getResponse(); - } + }*/ } private JsonObject getDeaccessionJson(DatasetVersion dv) { diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java new file mode 100644 index 00000000000..4a34e406398 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java @@ -0,0 +1,72 @@ +package edu.harvard.iq.dataverse.datasetversionsummaries; + +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.DatasetVersion.VersionState; +import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummaryContentSimple.Content; + +import java.util.Optional; + +/** + * An immutable data carrier representing a summary of a DatasetVersion. + */ +public record DatasetVersionSummary( + String versionId, + String versionNote, + String contributorNames, + String publicationDate, + DatasetVersionSummaryContent content +) { + + /** + * Creates a DatasetVersionSummary from a DatasetVersion entity. + *

+ * This factory method is the preferred way to create summaries, as it encapsulates + * all the logic for determining the correct summary content. + * + * @param datasetVersion The entity to convert. + * @return An {@link Optional} containing the summary, or an empty Optional if the input is null. + */ + public static Optional from(DatasetVersion datasetVersion) { + return Optional.ofNullable(datasetVersion).map(version -> new DatasetVersionSummary( + version.getFriendlyVersionNumber(), + version.getVersionNote(), + version.getContributorNames(), + version.getPublicationDateAsString(), + determineContent(version) + )); + } + + /** + * Determines the appropriate summary content based on the version's state. + * The logic is prioritized to handle the most specific cases first. + * + * @return The determined {@link DatasetVersionSummaryContent}, or {@code null} if the version + * state does not match any known summary condition. + */ + private static DatasetVersionSummaryContent determineContent(DatasetVersion version) { + // Priority 1: The version has explicit differences calculated. + if (version.getDefaultVersionDifference() != null) { + // TODO + return new DatasetVersionSummaryContentDifferences(); + } + + // Priority 2: Deaccessioned status is a critical override. + if (version.isDeaccessioned()) { + return new DatasetVersionSummaryContentDeaccessioned(version.getDeaccessionNote(), version.getDeaccessionLink()); + } + + // Priority 3: Standard draft or released versions. + if (version.isReleased() || version.isDraft()) { + boolean isFirstVersion = version.getPriorVersionState() == null; + if (isFirstVersion) { + return new DatasetVersionSummaryContentSimple(version.isReleased() ? Content.firstPublished : Content.firstDraft); + } + + if (version.getPriorVersionState() == VersionState.DEACCESSIONED) { + return new DatasetVersionSummaryContentSimple(Content.previousVersionDeaccessioned); + } + } + + return null; + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContent.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContent.java new file mode 100644 index 00000000000..653c13aa970 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContent.java @@ -0,0 +1,4 @@ +package edu.harvard.iq.dataverse.datasetversionsummaries; + +public abstract class DatasetVersionSummaryContent { +} diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDeaccessioned.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDeaccessioned.java new file mode 100644 index 00000000000..6a6ea3fc496 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDeaccessioned.java @@ -0,0 +1,20 @@ +package edu.harvard.iq.dataverse.datasetversionsummaries; + +public class DatasetVersionSummaryContentDeaccessioned extends DatasetVersionSummaryContent { + + private final String deaccessionNote; + private final String deaccessionLink; + + public DatasetVersionSummaryContentDeaccessioned(String deaccessionNote, String deaccessionLink) { + this.deaccessionNote = deaccessionNote; + this.deaccessionLink = deaccessionLink; + } + + public String getDeaccessionNote() { + return deaccessionNote; + } + + public String getDeaccessionLink() { + return deaccessionLink; + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java new file mode 100644 index 00000000000..c20eebfc400 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java @@ -0,0 +1,4 @@ +package edu.harvard.iq.dataverse.datasetversionsummaries; + +public class DatasetVersionSummaryContentDifferences extends DatasetVersionSummaryContent { +} diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java new file mode 100644 index 00000000000..db0172349f6 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java @@ -0,0 +1,16 @@ +package edu.harvard.iq.dataverse.datasetversionsummaries; + +public class DatasetVersionSummaryContentSimple extends DatasetVersionSummaryContent { + + private Content value; + + public DatasetVersionSummaryContentSimple(Content value) { + this.value = value; + } + + public enum Content { + firstPublished, + previousVersionDeaccessioned, + firstDraft + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java new file mode 100644 index 00000000000..23ea0658f0a --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -0,0 +1,39 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; +import edu.harvard.iq.dataverse.engine.command.AbstractCommand; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; + +import java.util.EnumSet; +import java.util.List; +import java.util.Optional; + +public class GetDatasetVersionSummariesCommand extends AbstractCommand> { + + private final Dataset dataset; + + public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset dataset) { + super(request, dataset); + this.dataset = dataset; + } + + @Override + public List execute(CommandContext ctxt) throws CommandException { + return dataset.getVersions().stream() + .filter(dv -> versionRequiresSummary(ctxt, dv)) + .map(DatasetVersionSummary::from) + .flatMap(Optional::stream) + .toList(); + } + + private boolean versionRequiresSummary(CommandContext ctxt, DatasetVersion dv) { + return dv.isPublished() + || dv.isDeaccessioned() + || ctxt.permissions().hasPermissionsFor(getRequest().getUser(), dv.getDataset(), EnumSet.of(Permission.ViewUnpublishedDataset)); + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index bbc834e0cc4..c6d3a18be87 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -19,6 +19,7 @@ import edu.harvard.iq.dataverse.dataaccess.DataAccess; import edu.harvard.iq.dataverse.dataset.DatasetType; import edu.harvard.iq.dataverse.dataset.DatasetUtil; +import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; import edu.harvard.iq.dataverse.datavariable.CategoryMetadata; import edu.harvard.iq.dataverse.datavariable.DataVariable; import edu.harvard.iq.dataverse.datavariable.SummaryStatistic; @@ -1685,4 +1686,9 @@ public static JsonArrayBuilder json(List notifications, Authen return notificationsArray; } + + public static JsonArrayBuilder json(List datasetVersionSummaries) { + // TODO + return Json.createArrayBuilder(); + } } From eeb6e33bb893a92f6c3876ce050cd4c5d499649b Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 2 Oct 2025 14:41:07 +0100 Subject: [PATCH 02/45] Refactor: getCompareVersionsSummary and related layers --- .../harvard/iq/dataverse/api/Datasets.java | 72 +------------------ .../DatasetVersionSummary.java | 7 +- ...tasetVersionSummaryContentDifferences.java | 12 ++++ .../DatasetVersionSummaryContentSimple.java | 6 +- .../GetDatasetVersionSummariesCommand.java | 2 + .../iq/dataverse/util/json/JsonPrinter.java | 45 ++++++++++-- 6 files changed, 65 insertions(+), 79 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index b2a93f7b2b0..db09768e879 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -2,7 +2,6 @@ import edu.harvard.iq.dataverse.*; import edu.harvard.iq.dataverse.DatasetLock.Reason; -import edu.harvard.iq.dataverse.DatasetVersion.VersionState; import edu.harvard.iq.dataverse.actionlogging.ActionLogRecord; import edu.harvard.iq.dataverse.api.auth.AuthRequired; import edu.harvard.iq.dataverse.api.dto.RoleAssignmentDTO; @@ -3141,80 +3140,11 @@ public Response getCompareVersions(@Context ContainerRequestContext crc, @PathPa public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, @PathParam("id") String id) { return response(req -> { try { - return ok(json(execCommand(new GetDatasetVersionSummariesCommand(req, findDatasetOrDie(id))))); + return ok(jsonDatasetVersionSummaries(execCommand(new GetDatasetVersionSummariesCommand(req, findDatasetOrDie(id))))); } catch (WrappedResponse wr) { return wr.getResponse(); } }, getRequestUser(crc)); - /* - try { - Dataset dataset = findDatasetOrDie(id); - User user = getRequestUser(crc); - JsonArrayBuilder differenceSummaries = Json.createArrayBuilder(); - - for (DatasetVersion dv : dataset.getVersions()) { - //only get summaries of draft is user may view unpublished - - if (dv.isPublished() ||dv.isDeaccessioned() || permissionService.hasPermissionsFor(user, dv.getDataset(), - EnumSet.of(Permission.ViewUnpublishedDataset))) { - - JsonObjectBuilder versionBuilder = new NullSafeJsonBuilder(); - versionBuilder.add("id", dv.getId()); - versionBuilder.add("versionNumber", dv.getFriendlyVersionNumber()); - DatasetVersionDifference dvdiff = dv.getDefaultVersionDifference(); - if (dvdiff == null) { - if (dv.isReleased()) { - if (dv.getPriorVersionState() == null) { - versionBuilder.add("summary", "firstPublished"); - } - if (dv.getPriorVersionState() != null && dv.getPriorVersionState().equals(VersionState.DEACCESSIONED)) { - versionBuilder.add("summary", "previousVersionDeaccessioned"); - } - } - if (dv.isDraft()) { - if (dv.getPriorVersionState() == null) { - versionBuilder.add("summary", "firstDraft"); - } - if (dv.getPriorVersionState() != null && dv.getPriorVersionState().equals(VersionState.DEACCESSIONED)) { - versionBuilder.add("summary", "previousVersionDeaccessioned"); - } - } - if (dv.isDeaccessioned()) { - versionBuilder.add("summary", getDeaccessionJson(dv)); - } - - } else { - versionBuilder.add("summary", dvdiff.getSummaryDifferenceAsJson()); - } - versionBuilder.add("versionNote", dv.getVersionNote()); - versionBuilder.add("contributors", datasetversionService.getContributorsNames(dv)); - versionBuilder.add("publishedOn", !dv.isDraft() ? dv.getPublicationDateAsString() : ""); - differenceSummaries.add(versionBuilder); - } - } - return ok(differenceSummaries); - } catch (WrappedResponse wr) { - return wr.getResponse(); - }*/ - } - - private JsonObject getDeaccessionJson(DatasetVersion dv) { - - JsonObjectBuilder compositionBuilder = Json.createObjectBuilder(); - - if (dv.getDeaccessionNote() != null && !dv.getDeaccessionNote().isEmpty()) { - compositionBuilder.add("reason", dv.getDeaccessionNote()); - } - - if (dv.getDeaccessionLink() != null && !dv.getDeaccessionLink().isEmpty()) { - compositionBuilder.add("url", dv.getDeaccessionLink()); - } - - JsonObject json = Json.createObjectBuilder() - .add("deaccessioned", compositionBuilder) - .build(); - - return json; } private static Set getDatasetFilenames(Dataset dataset) { diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java index 4a34e406398..06da005166f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummary.java @@ -10,7 +10,8 @@ * An immutable data carrier representing a summary of a DatasetVersion. */ public record DatasetVersionSummary( - String versionId, + Long id, + String versionNumber, String versionNote, String contributorNames, String publicationDate, @@ -28,6 +29,7 @@ public record DatasetVersionSummary( */ public static Optional from(DatasetVersion datasetVersion) { return Optional.ofNullable(datasetVersion).map(version -> new DatasetVersionSummary( + version.getId(), version.getFriendlyVersionNumber(), version.getVersionNote(), version.getContributorNames(), @@ -46,8 +48,7 @@ public static Optional from(DatasetVersion datasetVersion private static DatasetVersionSummaryContent determineContent(DatasetVersion version) { // Priority 1: The version has explicit differences calculated. if (version.getDefaultVersionDifference() != null) { - // TODO - return new DatasetVersionSummaryContentDifferences(); + return new DatasetVersionSummaryContentDifferences(version.getDefaultVersionDifference()); } // Priority 2: Deaccessioned status is a critical override. diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java index c20eebfc400..57add5511da 100644 --- a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentDifferences.java @@ -1,4 +1,16 @@ package edu.harvard.iq.dataverse.datasetversionsummaries; +import edu.harvard.iq.dataverse.DatasetVersionDifference; + public class DatasetVersionSummaryContentDifferences extends DatasetVersionSummaryContent { + + private final DatasetVersionDifference datasetVersionDifference; + + public DatasetVersionSummaryContentDifferences(DatasetVersionDifference datasetVersionDifference) { + this.datasetVersionDifference = datasetVersionDifference; + } + + public DatasetVersionDifference getDatasetVersionDifference() { + return datasetVersionDifference; + } } diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java index db0172349f6..5d1075b41e6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java +++ b/src/main/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryContentSimple.java @@ -2,12 +2,16 @@ public class DatasetVersionSummaryContentSimple extends DatasetVersionSummaryContent { - private Content value; + private final Content value; public DatasetVersionSummaryContentSimple(Content value) { this.value = value; } + public Content getValue() { + return value; + } + public enum Content { firstPublished, previousVersionDeaccessioned, diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index 23ea0658f0a..156d5b4cabd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -7,12 +7,14 @@ import edu.harvard.iq.dataverse.engine.command.AbstractCommand; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import java.util.EnumSet; import java.util.List; import java.util.Optional; +@RequiredPermissions({}) public class GetDatasetVersionSummariesCommand extends AbstractCommand> { private final Dataset dataset; diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index c6d3a18be87..49d887cf27d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -19,7 +19,7 @@ import edu.harvard.iq.dataverse.dataaccess.DataAccess; import edu.harvard.iq.dataverse.dataset.DatasetType; import edu.harvard.iq.dataverse.dataset.DatasetUtil; -import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; +import edu.harvard.iq.dataverse.datasetversionsummaries.*; import edu.harvard.iq.dataverse.datavariable.CategoryMetadata; import edu.harvard.iq.dataverse.datavariable.DataVariable; import edu.harvard.iq.dataverse.datavariable.SummaryStatistic; @@ -1687,8 +1687,45 @@ public static JsonArrayBuilder json(List notifications, Authen return notificationsArray; } - public static JsonArrayBuilder json(List datasetVersionSummaries) { - // TODO - return Json.createArrayBuilder(); + public static JsonArray jsonDatasetVersionSummaries(List summaries) { + JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); + summaries.stream() + .filter(Objects::nonNull) + .map(JsonPrinter::json) + .forEach(arrayBuilder::add); + + return arrayBuilder.build(); + } + + private static JsonObjectBuilder json(DatasetVersionSummary summary) { + JsonObjectBuilder jsonObjectBuilder = jsonObjectBuilder() + .add("id", summary.id()) + .add("versionNumber", summary.versionNumber()) + .add("versionNote", summary.versionNote()) + .add("contributors", summary.contributorNames()) + .add("publishedOn", summary.publicationDate()); + + DatasetVersionSummaryContent content = summary.content(); + if (content instanceof DatasetVersionSummaryContentDeaccessioned deaccessioned) { + JsonObject deaccessionedDetailsJsonObject = jsonObjectBuilder() + .add("reason", deaccessioned.getDeaccessionNote()) + .add("url", deaccessioned.getDeaccessionLink()) + .build(); + JsonObject deaccessionedJsonObject = jsonObjectBuilder() + .add("deaccessioned", deaccessionedDetailsJsonObject) + .build(); + jsonObjectBuilder.add("summary", deaccessionedJsonObject + ); + } else if (content instanceof DatasetVersionSummaryContentSimple simple) { + jsonObjectBuilder.add("summary", simple.getValue().name()); + + } else if (content instanceof DatasetVersionSummaryContentDifferences differences) { + jsonObjectBuilder.add("summary", differences + .getDatasetVersionDifference() + .getSummaryDifferenceAsJson() + .build()); + } + + return jsonObjectBuilder; } } From 98c72cbfbe67ebe13d25b6ec65b49f05924eba54 Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 2 Oct 2025 18:51:23 +0100 Subject: [PATCH 03/45] Changed: GetDatasetVersionSummariesCommand using DatasetVersionServiceBean.findVersions --- .../harvard/iq/dataverse/DatasetVersion.java | 6 +- .../dataverse/DatasetVersionServiceBean.java | 67 +++++++++++++------ .../GetDatasetVersionSummariesCommand.java | 23 ++++--- 3 files changed, 66 insertions(+), 30 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java index 14ddf65a833..cfd4c886bf3 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java @@ -73,7 +73,11 @@ @NamedQuery(name = "DatasetVersion.findById", query = "SELECT o FROM DatasetVersion o LEFT JOIN FETCH o.fileMetadatas WHERE o.id=:id"), @NamedQuery(name = "DatasetVersion.findByDataset", - query = "SELECT o FROM DatasetVersion o WHERE o.dataset.id=:datasetId ORDER BY o.versionNumber DESC, o.minorVersionNumber DESC"), + query = "SELECT o FROM DatasetVersion o WHERE o.dataset.id=:datasetId ORDER BY o.versionNumber DESC, o.minorVersionNumber DESC"), + @NamedQuery(name = "DatasetVersion.findByDesiredStatesAndDataset", + query = "SELECT o FROM DatasetVersion o " + + "WHERE o.dataset.id = :datasetId AND o.versionState IN :states " + + "ORDER BY o.versionNumber DESC, o.minorVersionNumber DESC"), @NamedQuery(name = "DatasetVersion.findReleasedByDataset", query = "SELECT o FROM DatasetVersion o WHERE o.dataset.id=:datasetId AND o.versionState=edu.harvard.iq.dataverse.DatasetVersion.VersionState.RELEASED ORDER BY o.versionNumber DESC, o.minorVersionNumber DESC")/*, @NamedQuery(name = "DatasetVersion.findVersionElements", diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java index 7e9b778c6f3..0a60051ea10 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java @@ -171,41 +171,66 @@ public DatasetVersion findDeep(Object pk) { .setHint("eclipselink.left-join-fetch", "o.fileMetadatas.dataFile.dataFileTags") .getSingleResult(); } - + /** - * Performs the same database lookup as the one behind Dataset.getVersions(). - * Additionally, provides the arguments for selecting a partial list of - * (length-offset) versions for pagination, plus the ability to pre-select - * only the publicly-viewable versions. - * It is recommended that individual software components utilize the - * ListVersionsCommand, instead of calling this service method directly. - * @param datasetId - * @param offset for pagination through long lists of versions - * @param length for pagination through long lists of versions - * @param includeUnpublished retrieves all the versions, including drafts and deaccessioned. - * @return (partial) list of versions + * Performs the same database lookup as the one behind {@code Dataset.getVersions()}. + *

+ * Additionally, supports: + *

+ *

+ * It is recommended that individual software components utilize + * {@link edu.harvard.iq.dataverse.engine.command.impl.ListVersionsCommand}, + * instead of calling this service method directly. + * + * @param datasetId the dataset identifier + * @param offset pagination offset (nullable) + * @param length pagination length (nullable) + * @param includeAllVersions if {@code true}, retrieves all versions (drafts, released, and deaccessioned) + * @param includeDeaccessioned if {@code true}, includes deaccessioned versions + * when {@code includeAll} is {@code false} + * @return a (possibly partial) list of dataset versions */ - public List findVersions(Long datasetId, Integer offset, Integer length, boolean includeUnpublished) { - TypedQuery query; - if (includeUnpublished) { + public List findVersions(Long datasetId, + Integer offset, + Integer length, + boolean includeAllVersions, + boolean includeDeaccessioned) { + TypedQuery query; + + if (includeAllVersions) { query = em.createNamedQuery("DatasetVersion.findByDataset", DatasetVersion.class); + } else if (includeDeaccessioned) { + query = em.createNamedQuery("DatasetVersion.findByDesiredStatesAndDataset", DatasetVersion.class); + query.setParameter("states", List.of( + VersionState.RELEASED, + VersionState.DEACCESSIONED + )); } else { - query = em.createNamedQuery("DatasetVersion.findReleasedByDataset", DatasetVersion.class) - .setParameter("datasetId", datasetId); + query = em.createNamedQuery("DatasetVersion.findReleasedByDataset", DatasetVersion.class); } - + query.setParameter("datasetId", datasetId); - + if (offset != null) { query.setFirstResult(offset); } if (length != null) { query.setMaxResults(length); } - + return query.getResultList(); } - + + public List findVersions(Long datasetId, + Integer offset, + Integer length, + boolean includeAllVersions) { + return findVersions(datasetId, offset, length, includeAllVersions, false); + } + public DatasetVersion findByFriendlyVersionNumber(Long datasetId, String friendlyVersionNumber) { Long majorVersionNumber = null; Long minorVersionNumber = null; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index 156d5b4cabd..baa0da172f4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -26,16 +26,23 @@ public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset datas @Override public List execute(CommandContext ctxt) throws CommandException { - return dataset.getVersions().stream() - .filter(dv -> versionRequiresSummary(ctxt, dv)) + boolean canViewUnpublished = ctxt.permissions().hasPermissionsFor( + getRequest().getUser(), + dataset, + EnumSet.of(Permission.ViewUnpublishedDataset) + ); + + List versions = ctxt.datasetVersion().findVersions( + dataset.getId(), + null, + null, + canViewUnpublished, + true + ); + + return versions.stream() .map(DatasetVersionSummary::from) .flatMap(Optional::stream) .toList(); } - - private boolean versionRequiresSummary(CommandContext ctxt, DatasetVersion dv) { - return dv.isPublished() - || dv.isDeaccessioned() - || ctxt.permissions().hasPermissionsFor(getRequest().getUser(), dv.getDataset(), EnumSet.of(Permission.ViewUnpublishedDataset)); - } } From 5919cf479272c62684dc27ada15ab375c09ca866 Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 2 Oct 2025 19:01:28 +0100 Subject: [PATCH 04/45] Added: handling pagination optional params on getCompareVersionsSummary API endpoint --- .../java/edu/harvard/iq/dataverse/api/Datasets.java | 7 +++++-- .../impl/GetDatasetVersionSummariesCommand.java | 13 ++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index db09768e879..c2888aa0f65 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -3137,10 +3137,13 @@ public Response getCompareVersions(@Context ContainerRequestContext crc, @PathPa @GET @AuthRequired @Path("{id}/versions/compareSummary") - public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, @PathParam("id") String id) { + public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, + @PathParam("id") String id, + @QueryParam("limit") Integer limit, + @QueryParam("offset") Integer offset) { return response(req -> { try { - return ok(jsonDatasetVersionSummaries(execCommand(new GetDatasetVersionSummariesCommand(req, findDatasetOrDie(id))))); + return ok(jsonDatasetVersionSummaries(execCommand(new GetDatasetVersionSummariesCommand(req, findDatasetOrDie(id), limit, offset)))); } catch (WrappedResponse wr) { return wr.getResponse(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index baa0da172f4..d820dd7de6a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -14,14 +14,21 @@ import java.util.List; import java.util.Optional; +/** + * A command that retrieves a paginated list of {@link DatasetVersionSummary} for the versions of a given {@link Dataset}. + **/ @RequiredPermissions({}) public class GetDatasetVersionSummariesCommand extends AbstractCommand> { private final Dataset dataset; + private final Integer limit; + private final Integer offset; - public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset dataset) { + public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset dataset, Integer limit, Integer offset) { super(request, dataset); this.dataset = dataset; + this.limit = limit; + this.offset = offset; } @Override @@ -34,8 +41,8 @@ public List execute(CommandContext ctxt) throws CommandEx List versions = ctxt.datasetVersion().findVersions( dataset.getId(), - null, - null, + offset, + limit, canViewUnpublished, true ); From ea5642e33d545f3829660aeaf05578bd05b9e1c9 Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 2 Oct 2025 19:14:50 +0100 Subject: [PATCH 05/45] Added: DatasetVersionSummaryTest --- .../DatasetVersionSummaryTest.java | 195 ++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java diff --git a/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java b/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java new file mode 100644 index 00000000000..ba85ff640f4 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java @@ -0,0 +1,195 @@ +package edu.harvard.iq.dataverse.datasetversionsummaries; + +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.DatasetVersion.VersionState; +import edu.harvard.iq.dataverse.DatasetVersionDifference; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +/** + * Unit tests for the {@link DatasetVersionSummary} record and its factory methods. + */ +@ExtendWith(MockitoExtension.class) +class DatasetVersionSummaryTest { + + @Mock + private DatasetVersion versionMock; + + @Mock + private DatasetVersionDifference differenceMock; + + @Test + @DisplayName("from() should return an empty Optional when the input DatasetVersion is null") + void from_whenVersionIsNull_shouldReturnEmptyOptional() { + // Act + Optional result = DatasetVersionSummary.from(null); + + // Assert + assertThat(result).isNotPresent(); + } + + @Test + @DisplayName("from() should map all basic properties from the DatasetVersion correctly") + void from_whenVersionIsNotNull_shouldMapPropertiesCorrectly() { + // Arrange + when(versionMock.getId()).thenReturn(42L); + when(versionMock.getFriendlyVersionNumber()).thenReturn("V1.0"); + when(versionMock.getVersionNote()).thenReturn("Initial version note."); + when(versionMock.getContributorNames()).thenReturn("Contributor One, Contributor Two"); + when(versionMock.getPublicationDateAsString()).thenReturn("2025-10-02"); + when(versionMock.isReleased()).thenReturn(true); + when(versionMock.getPriorVersionState()).thenReturn(null); + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + DatasetVersionSummary summary = summaryOpt.get(); + + assertThat(summary.id()).isEqualTo(42L); + assertThat(summary.versionNumber()).isEqualTo("V1.0"); + assertThat(summary.versionNote()).isEqualTo("Initial version note."); + assertThat(summary.contributorNames()).isEqualTo("Contributor One, Contributor Two"); + assertThat(summary.publicationDate()).isEqualTo("2025-10-02"); + } + + @Test + @DisplayName("Content should be 'Differences' if a version difference is present (highest priority)") + void from_whenVersionHasDifferences_shouldCreateDifferencesContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(differenceMock); + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + assertThat(summaryOpt.get().content()).isInstanceOf(DatasetVersionSummaryContentDifferences.class); + DatasetVersionSummaryContentDifferences content = (DatasetVersionSummaryContentDifferences) summaryOpt.get().content(); + assertThat(content.getDatasetVersionDifference()).isSameAs(differenceMock); + } + + @Test + @DisplayName("Content should be 'Deaccessioned' if the version is deaccessioned") + void from_whenVersionIsDeaccessioned_shouldCreateDeaccessionedContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(null); // No differences + when(versionMock.isDeaccessioned()).thenReturn(true); + when(versionMock.getDeaccessionNote()).thenReturn("Reason for deaccession."); + when(versionMock.getDeaccessionLink()).thenReturn("http://example.com/deaccession"); + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + DatasetVersionSummaryContentDeaccessioned content = (DatasetVersionSummaryContentDeaccessioned) summaryOpt.get().content(); + + assertThat(content.getDeaccessionNote()).isEqualTo("Reason for deaccession."); + assertThat(content.getDeaccessionLink()).isEqualTo("http://example.com/deaccession"); + } + + @Test + @DisplayName("Content should be 'firstPublished' for the first released version") + void from_whenIsFirstPublishedVersion_shouldCreateFirstPublishedContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(null); + when(versionMock.isDeaccessioned()).thenReturn(false); + when(versionMock.isReleased()).thenReturn(true); + when(versionMock.getPriorVersionState()).thenReturn(null); // This makes it the "first" version + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + assertThat(summaryOpt.get().content()) + .isInstanceOf(DatasetVersionSummaryContentSimple.class) + .extracting("value") + .isEqualTo(DatasetVersionSummaryContentSimple.Content.firstPublished); + } + + @Test + @DisplayName("Content should be 'firstDraft' for the first draft version") + void from_whenIsFirstDraftVersion_shouldCreateFirstDraftContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(null); + when(versionMock.isDeaccessioned()).thenReturn(false); + when(versionMock.isDraft()).thenReturn(true); + when(versionMock.getPriorVersionState()).thenReturn(null); // This makes it the "first" version + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + assertThat(summaryOpt.get().content()) + .isInstanceOf(DatasetVersionSummaryContentSimple.class) + .extracting("value") + .isEqualTo(DatasetVersionSummaryContentSimple.Content.firstDraft); + } + + @Test + @DisplayName("Content should be 'previousVersionDeaccessioned' if the prior version was deaccessioned") + void from_whenPriorVersionWasDeaccessioned_shouldCreateAppropriateContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(null); + when(versionMock.isDeaccessioned()).thenReturn(false); + when(versionMock.isReleased()).thenReturn(true); + when(versionMock.getPriorVersionState()).thenReturn(VersionState.DEACCESSIONED); + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + assertThat(summaryOpt.get().content()) + .isInstanceOf(DatasetVersionSummaryContentSimple.class) + .extracting("value") + .isEqualTo(DatasetVersionSummaryContentSimple.Content.previousVersionDeaccessioned); + } + + @Test + @DisplayName("Content should be null for a standard released version that is not the first") + void from_whenIsStandardUpdate_shouldHaveNullContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(null); + when(versionMock.isDeaccessioned()).thenReturn(false); + when(versionMock.isReleased()).thenReturn(true); + when(versionMock.getPriorVersionState()).thenReturn(VersionState.RELEASED); // Prior version exists + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + assertThat(summaryOpt.get().content()).isNull(); + } + + @Test + @DisplayName("Content should be null for an unhandled version state like ARCHIVED") + void from_whenStateIsUnhandled_shouldHaveNullContent() { + // Arrange + when(versionMock.getDefaultVersionDifference()).thenReturn(null); + when(versionMock.isDeaccessioned()).thenReturn(false); + when(versionMock.isReleased()).thenReturn(false); + when(versionMock.isDraft()).thenReturn(false); + // Assuming VersionState is ARCHIVED, isArchived() would be true but the logic doesn't check for it. + + // Act + Optional summaryOpt = DatasetVersionSummary.from(versionMock); + + // Assert + assertThat(summaryOpt).isPresent(); + assertThat(summaryOpt.get().content()).isNull(); + } +} \ No newline at end of file From b3c4fbdb19c4780b05f5c63f8f8306608c270a69 Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 2 Oct 2025 19:21:05 +0100 Subject: [PATCH 06/45] Added: GetDatasetVersionSummariesCommandTest --- .../DatasetVersionSummaryTest.java | 2 +- ...GetDatasetVersionSummariesCommandTest.java | 172 ++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java diff --git a/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java b/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java index ba85ff640f4..430c945591c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/datasetversionsummaries/DatasetVersionSummaryTest.java @@ -192,4 +192,4 @@ void from_whenStateIsUnhandled_shouldHaveNullContent() { assertThat(summaryOpt).isPresent(); assertThat(summaryOpt.get().content()).isNull(); } -} \ No newline at end of file +} diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java new file mode 100644 index 00000000000..0161bdbfbf2 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java @@ -0,0 +1,172 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.DatasetVersionServiceBean; +import edu.harvard.iq.dataverse.PermissionServiceBean; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class GetDatasetVersionSummariesCommandTest { + + @Mock + private CommandContext contextMock; + @Mock + private DataverseRequest requestMock; + @Mock + private Dataset datasetMock; + @Mock + private PermissionServiceBean permissionsMock; + @Mock + private DatasetVersionServiceBean versionServiceMock; + @Mock + private AuthenticatedUser userMock; + @Mock + private DatasetVersion versionMock; + + private static final Long DATASET_ID = 42L; + + @BeforeEach + void setUp() { + // Mock the context to return our mock services + when(contextMock.permissions()).thenReturn(permissionsMock); + when(contextMock.datasetVersion()).thenReturn(versionServiceMock); + + // Mock the request to return a user + when(requestMock.getUser()).thenReturn(userMock); + + // Mock the dataset to have a specific ID + when(datasetMock.getId()).thenReturn(DATASET_ID); + } + + @Test + @DisplayName("execute should call findVersions with 'canViewUnpublished' as TRUE when user has permission") + void execute_should_call_findVersions_with_true_when_user_has_permission() throws CommandException { + // Arrange + when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) + .thenReturn(true); + when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) + .thenReturn(Collections.emptyList()); + + GetDatasetVersionSummariesCommand command = new GetDatasetVersionSummariesCommand(requestMock, datasetMock, null, null); + + // Act + command.execute(contextMock); + + // Assert + ArgumentCaptor canViewUnpublishedCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(versionServiceMock).findVersions( + eq(DATASET_ID), + eq(null), + eq(null), + canViewUnpublishedCaptor.capture(), + eq(true) + ); + assertThat(canViewUnpublishedCaptor.getValue()).isTrue(); + } + + @Test + @DisplayName("execute should call findVersions with 'canViewUnpublished' as FALSE when user lacks permission") + void execute_should_call_findVersions_with_false_when_user_lacks_permission() throws CommandException { + // Arrange + when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) + .thenReturn(false); + when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) + .thenReturn(Collections.emptyList()); + + GetDatasetVersionSummariesCommand command = new GetDatasetVersionSummariesCommand(requestMock, datasetMock, null, null); + + // Act + command.execute(contextMock); + + // Assert + ArgumentCaptor canViewUnpublishedCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(versionServiceMock).findVersions( + eq(DATASET_ID), + eq(null), + eq(null), + canViewUnpublishedCaptor.capture(), + eq(true) + ); + assertThat(canViewUnpublishedCaptor.getValue()).isFalse(); + } + + @Test + @DisplayName("execute should pass pagination parameters correctly to findVersions") + void execute_should_pass_pagination_parameters_correctly() throws CommandException { + // Arrange + Integer expectedLimit = 10; + Integer expectedOffset = 20; + when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) + .thenReturn(Collections.emptyList()); + + GetDatasetVersionSummariesCommand command = new GetDatasetVersionSummariesCommand(requestMock, datasetMock, expectedLimit, expectedOffset); + + // Act + command.execute(contextMock); + + // Assert + verify(versionServiceMock).findVersions( + eq(DATASET_ID), + eq(expectedOffset), + eq(expectedLimit), + anyBoolean(), + eq(true) + ); + } + + @Test + @DisplayName("execute should correctly convert DatasetVersion list to DatasetVersionSummary list") + void execute_should_convert_versions_to_summaries() throws CommandException { + // Arrange: Mock the service to return a list with one version + when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) + .thenReturn(List.of(versionMock)); + + // Arrange: Prepare a dummy summary object + DatasetVersionSummary dummySummary = new DatasetVersionSummary(1L, "V1", null, null, null, null); + + // Arrange: Mock the static 'from' method to return our dummy summary + try (MockedStatic mockedStatic = Mockito.mockStatic(DatasetVersionSummary.class)) { + mockedStatic.when(() -> DatasetVersionSummary.from(versionMock)) + .thenReturn(Optional.of(dummySummary)); + + GetDatasetVersionSummariesCommand command = new GetDatasetVersionSummariesCommand(requestMock, datasetMock, null, null); + + // Act + List result = command.execute(contextMock); + + // Assert + assertThat(result) + .isNotNull() + .hasSize(1) + .containsExactly(dummySummary); + } + } +} From 89b88b5c7d02a203d15ec877b5d8c3b71571e11d Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 6 Oct 2025 09:33:30 +0100 Subject: [PATCH 07/45] Added: pagination test cases to testSummaryDatasetVersionsDifferencesAPI IT --- .../harvard/iq/dataverse/api/DatasetsIT.java | 60 ++++++++++++++----- .../edu/harvard/iq/dataverse/api/UtilIT.java | 9 ++- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index 5dac4b1d05c..b94f9276a51 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -6374,11 +6374,8 @@ public void testCompareDatasetVersionsAPI() throws InterruptedException { compareResponse = UtilIT.compareDatasetVersions(datasetPersistentId, ":latest-published", ":draft", apiToken, true); compareResponse.prettyPrint(); compareResponse.then().assertThat().statusCode(OK.getStatusCode()); - - - } - + @Test public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedException { @@ -6477,11 +6474,10 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti Response updateTerms = UtilIT.updateDatasetJsonLDMetadata(datasetId, apiToken, jsonLDTerms, true); updateTerms.then().assertThat() .statusCode(OK.getStatusCode()); - - + Response compareResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, apiToken); - compareResponse.prettyPrint(); + compareResponse.prettyPrint(); compareResponse.then().assertThat() .body("data[1].versionNumber", equalTo("1.0")) @@ -6498,32 +6494,66 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti .statusCode(OK.getStatusCode()); //user with no privileges will only see the published version - + Response createUsernoPriv = UtilIT.createRandomUser(); assertEquals(200, createUsernoPriv.getStatusCode()); String apiTokenNoPriv = UtilIT.getApiTokenFromResponse(createUsernoPriv); - + Response compareResponse2 = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, apiTokenNoPriv); compareResponse2.prettyPrint(); compareResponse2.then().assertThat() .body("data[0].versionNumber", CoreMatchers.equalTo("1.0")) .body("data[0].summary", CoreMatchers.equalTo("firstPublished")) .statusCode(OK.getStatusCode()); - + Response deaccessionDatasetResponse = UtilIT.deaccessionDataset(datasetId, DS_VERSION_LATEST_PUBLISHED, "Test deaccession reason.", null, apiToken); deaccessionDatasetResponse.then().assertThat().statusCode(OK.getStatusCode()); - + compareResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, apiToken); - compareResponse.prettyPrint(); - + compareResponse.prettyPrint(); + compareResponse.then().assertThat() .body("data[1].versionNumber", equalTo("1.0")) .body("data[1].summary.deaccessioned.reason", equalTo("Test deaccession reason.")) .body("data[0].versionNumber", equalTo("DRAFT")) .body("data[0].summary.", equalTo("previousVersionDeaccessioned")) .statusCode(OK.getStatusCode()); - - + + // Pagination tests + + // Publish the current DRAFT as a minor version to create version 2.0 + // This will give us more versions to test pagination against. + + publishDataset = UtilIT.publishDatasetViaNativeApi(datasetPersistentId, "major", apiToken); + publishDataset.then().assertThat().statusCode(OK.getStatusCode()); + + // Make one file change to create a new DRAFT on top of version 2.0 + pathToJsonFilePostPub = "doc/sphinx-guides/source/_static/api/dataset-add-metadata-after-pub.json"; + addDataToPublishedVersion = UtilIT.addDatasetMetadataViaNative(datasetPersistentId, pathToJsonFilePostPub, apiToken); + addDataToPublishedVersion.then().assertThat().statusCode(OK.getStatusCode()); + + // TEST 1: No pagination. Should return all 3 version differences (DRAFT, 2.0, 1.0). + Response compareAllResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, apiToken); + compareAllResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.size()", is(3)) + .body("data[0].versionNumber", equalTo("DRAFT")) + .body("data[1].versionNumber", equalTo("2.0")) + .body("data[2].versionNumber", equalTo("1.0")); + + // TEST 2: Use limit=1. Should return only the latest difference (DRAFT). + Response compareLimitResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, 1, null, apiToken); + compareLimitResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.size()", is(1)) + .body("data[0].versionNumber", equalTo("DRAFT")); + + // TEST 3: Use limit=1 and offset=1. Should skip the first result and return the second (2.0). + Response compareOffsetResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, 1, 1, apiToken); + compareOffsetResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.size()", is(1)) + .body("data[0].versionNumber", equalTo("2.0")); } @Test diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index e5d3f908349..20c1a016450 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1787,14 +1787,21 @@ static Response compareDatasetVersions(String persistentId, String versionNumber + "&includeDeaccessioned=" + includeDeaccessioned); } + + static Response summaryDatasetVersionDifferences(String persistentId, String apiToken) { + return summaryDatasetVersionDifferences(persistentId, null, null, apiToken); + } - static Response summaryDatasetVersionDifferences(String persistentId, String apiToken) { + static Response summaryDatasetVersionDifferences(String persistentId, Integer limit, Integer offset, String apiToken) { return given() .header(API_TOKEN_HTTP_HEADER, apiToken) + .queryParam("limit", limit) + .queryParam("offset", offset) .get("/api/datasets/:persistentId/versions/compareSummary" + "?persistentId=" + persistentId); } + static Response getDatasetWithOwners(String persistentId, String apiToken, boolean returnOwners) { return given() .header(API_TOKEN_HTTP_HEADER, apiToken) From 93823a7b40dcfab8ddca6dd44860cd3305fbabf5 Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 6 Oct 2025 09:38:30 +0100 Subject: [PATCH 08/45] Added: pagination explanation docs to compareSummary datasets endpoint --- doc/sphinx-guides/source/api/native-api.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 19a4d79c5ae..6fe0147a683 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2107,6 +2107,16 @@ The fully expanded example above (without environment variables) looks like this curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/versions/compareSummary?persistentId=doi:10.5072/FK2/BCCP9Z" +You can control pagination of the results using the following optional query parameters. + +* ``limit``: The maximum number of version differences to return. +* ``offset``: The number of version differences to skip from the beginning of the list. Used for retrieving subsequent pages of results. + +For example, to get the second page of results, with 2 items per page, you would use ``limit=2`` and ``offset=2`` (skipping the first two results). + +.. code-block:: bash + + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/versions/compareSummary?persistentId=doi:10.5072/FK2/BCCP9Z&limit=2&offset=2" Update Metadata For a Dataset ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 393c7d4bf91f78fd93097bc20fadf649533befb6 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 7 Oct 2025 23:12:51 +0100 Subject: [PATCH 09/45] Added: findFileMetadataHistory JPACriteria-based method to DataFileServiceBean --- .../iq/dataverse/DataFileServiceBean.java | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java index 937f5693511..4b7f8351d55 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java @@ -30,6 +30,8 @@ import java.util.UUID; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; + import jakarta.ejb.EJB; import jakarta.ejb.Stateless; import jakarta.ejb.TransactionAttribute; @@ -40,6 +42,7 @@ import jakarta.persistence.PersistenceContext; import jakarta.persistence.Query; import jakarta.persistence.TypedQuery; +import jakarta.persistence.criteria.*; /** * @@ -376,6 +379,74 @@ public FileMetadata findFileMetadataByDatasetVersionIdAndDataFileId(Long dataset } } + /** + * Finds the concise history of a file, returning an entry only for each + * version where the file's metadata was created or changed. + *

+ * This method correctly handles file replacements by searching for all files + * sharing the same {@code rootDataFileId}. + * + * @param datasetId The ID of the parent dataset. + * @param dataFile The DataFile entity to find the history for. + * @param canViewUnpublishedVersions A boolean indicating if the user has permission to view non-released versions. + * @return A chronologically sorted list of the file's version history. Returns an empty list if the dataFile is null. + */ + public List findFileMetadataHistory(Long datasetId, DataFile dataFile, boolean canViewUnpublishedVersions) { + // Guard clause: Return early if there's no file to search for. + if (dataFile == null) { + return Collections.emptyList(); + } + + CriteriaBuilder cb = em.getCriteriaBuilder(); + CriteriaQuery criteriaQuery = cb.createQuery(FileMetadata.class); + Root fileMetadata = criteriaQuery.from(FileMetadata.class); + + // --- Joins --- + // Define relationships for filtering and ordering. + Join version = fileMetadata.join("datasetVersion"); + Join dataset = version.join("dataset"); + Join file = fileMetadata.join("dataFile"); + + // --- Predicates (WHERE clause) --- + // Build a list of conditions for the query. + List predicates = new ArrayList<>(); + + // Filter by the parent dataset. + predicates.add(cb.equal(dataset.get("id"), datasetId)); + + // Filter by the file's lineage, handling both new and replaced files. + if (dataFile.getRootDataFileId() < 0) { + // For a new file, match by its specific ID. + predicates.add(cb.equal(file.get("id"), dataFile.getId())); + } else { + // For a replaced file, match the entire history via its root ID. + predicates.add(cb.equal(file.get("rootDataFileId"), dataFile.getRootDataFileId())); + } + + // Add permission-based filter for version states. + if (!canViewUnpublishedVersions) { + predicates.add(version.get("versionState").in( + VersionState.RELEASED, VersionState.DEACCESSIONED)); + } + + criteriaQuery.where(predicates.toArray(new Predicate[0])); + + // --- Ordering --- + // Order results from newest to oldest version. + criteriaQuery.orderBy( + cb.desc(version.get("versionNumber")), + cb.desc(version.get("minorVersionNumber")) + ); + + // --- Execution & Transformation --- + // Execute the query and map the results to the VersionedFileMetadata list. + List results = em.createQuery(criteriaQuery).getResultList(); + + return results.stream() + .map(metadata -> new VersionedFileMetadata(metadata.getDatasetVersion(), metadata)) + .collect(Collectors.toList()); + } + public FileMetadata findMostRecentVersionFileIsIn(DataFile file) { if (file == null) { return null; From 7ac4588cbd7cc6701946d6f8d22624bfb220f785 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 7 Oct 2025 23:14:00 +0100 Subject: [PATCH 10/45] Added: GetFileVersionDifferencesCommand, pending to be refactored and test-covered --- .../GetFileVersionDifferencesCommand.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java new file mode 100644 index 00000000000..48e98021150 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -0,0 +1,55 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.*; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; +import edu.harvard.iq.dataverse.engine.command.AbstractCommand; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; + +import java.util.ArrayList; +import java.util.List; + +/** + * A command that retrieves a paginated list of {@link DatasetVersionSummary} for the versions of a given {@link Dataset}. + **/ +@RequiredPermissions({}) +public class GetFileVersionDifferencesCommand extends AbstractCommand> { + private final FileMetadata fileMetadata; + private final Integer limit; + private final Integer offset; + private final FileMetadataVersionsHelper fileMetadataVersionsHelper; + + public GetFileVersionDifferencesCommand(DataverseRequest request, FileMetadata fileMetadata, Integer limit, Integer offset, FileMetadataVersionsHelper fileMetadataVersionsHelper) { + super(request, fileMetadata.getDataFile()); + this.fileMetadata = fileMetadata; + this.limit = limit; + this.offset = offset; + this.fileMetadataVersionsHelper = fileMetadataVersionsHelper; + } + + @Override + // TODO + public List execute(CommandContext ctxt) throws CommandException { + Dataset dataset = fileMetadata.getDatasetVersion().getDataset(); + boolean hasPermission = ctxt.permissions().requestOn(getRequest(), dataset).has(Permission.ViewUnpublishedDataset); + List versionedFileMetadataList = ctxt.files().findFileMetadataHistory(dataset.getId(), fileMetadata.getDataFile(), hasPermission); + List differences = new ArrayList<>(); + for (VersionedFileMetadata versionedFileMetadata : versionedFileMetadataList) { + DatasetVersion datasetVersion = versionedFileMetadata.getDatasetVersion(); + if (versionedFileMetadata.getFileMetadata() == null) { + FileMetadata dummy = new FileMetadata(); + dummy.setDatasetVersion(datasetVersion); + dummy.setDataFile(null); + FileVersionDifference fvd = new FileVersionDifference(dummy, fileMetadataVersionsHelper.getPreviousFileMetadata(fileMetadata, datasetVersion), true); + dummy.setFileVersionDifference(fvd); + differences.add(fvd); + } else { + differences.add(new FileVersionDifference(versionedFileMetadata.getFileMetadata(), fileMetadataVersionsHelper.getPreviousFileMetadata(fileMetadata, datasetVersion), true)); + } + } + return differences; + } +} From bda189f2917fc1c8a884e52901e3d61a6379d23f Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 7 Oct 2025 23:15:22 +0100 Subject: [PATCH 11/45] Changed: Files API versionDifferences endpoint now using GetFileVersionDifferencesCommand --- .../harvard/iq/dataverse/FileMetadataVersionsHelper.java | 9 +++++---- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java index 6778794dea6..21bb8ea51e4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java @@ -61,8 +61,9 @@ public List loadFileVersionList(DataverseRequest req, FileMetadata return retList; } - public JsonObjectBuilder jsonDataFileVersions(FileMetadata fileMetadata) { + public JsonObjectBuilder jsonDataFileVersions(FileVersionDifference fileVersionDifference) { JsonObjectBuilder job = jsonObjectBuilder(); + FileMetadata fileMetadata = fileVersionDifference.getNewFileMetadata(); if (fileMetadata.getDatasetVersion() != null) { job.add("datasetVersion", fileMetadata.getDatasetVersion().getFriendlyVersionNumber()); if (fileMetadata.getDatasetVersion().getVersionNumber() != null) { @@ -78,14 +79,14 @@ public JsonObjectBuilder jsonDataFileVersions(FileMetadata fileMetadata) { .add("versionState", fileMetadata.getDatasetVersion().getVersionState().name()) .add("summary", fileMetadata.getDatasetVersion().getVersionNote()) .add("contributors", fileMetadata.getContributorNames()) - .add("publishedDate", fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) + .add("publishedDate", fileMetadata.getDataFile() != null ? (fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) : null) ; } if (fileMetadata.getDataFile() != null) { job.add("datafileId", fileMetadata.getDataFile().getId()); job.add("persistentId", (fileMetadata.getDataFile().getGlobalId() != null ? fileMetadata.getDataFile().getGlobalId().asString() : "")); } - FileVersionDifference fvd = fileMetadata.getFileVersionDifference(); + FileVersionDifference fvd = fileVersionDifference; if (fvd != null) { List groups = fvd.getDifferenceSummaryGroups(); JsonObjectBuilder fileDifferenceSummary = jsonObjectBuilder() @@ -191,7 +192,7 @@ private FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, FileMeta } //TODO: this could use some refactoring to cut down on the number of for loops! - private FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, DatasetVersion currentversion) { + public FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, DatasetVersion currentversion) { List allfiles = allRelatedFiles(fileMetadata); boolean foundCurrent = false; DatasetVersion priorVersion = null; diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 4ea9087b325..4810510c8a4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -1182,10 +1182,10 @@ public Response getFileVersionsList(@Context ContainerRequestContext crc, @PathP if (fm == null) { return notFound(BundleUtil.getStringFromBundle("files.api.fileNotFound")); } - List fileMetadataList = fileMetadataVersionsHelper.loadFileVersionList(req, fm); + List fileVersionDifferences = execCommand(new GetFileVersionDifferencesCommand(req, fm, null, null, fileMetadataVersionsHelper)); JsonArrayBuilder jab = Json.createArrayBuilder(); - for (FileMetadata fileMetadata : fileMetadataList) { - jab.add(fileMetadataVersionsHelper.jsonDataFileVersions(fileMetadata).build()); + for (FileVersionDifference fileVersionDifference : fileVersionDifferences) { + jab.add(fileMetadataVersionsHelper.jsonDataFileVersions(fileVersionDifference).build()); } return Response.ok() .entity(Json.createObjectBuilder() From 797315330a49e9aa91a3af8b63049224b669e277 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 7 Oct 2025 23:15:49 +0100 Subject: [PATCH 12/45] Added: VersionedFileMetadata class --- .../iq/dataverse/VersionedFileMetadata.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/VersionedFileMetadata.java diff --git a/src/main/java/edu/harvard/iq/dataverse/VersionedFileMetadata.java b/src/main/java/edu/harvard/iq/dataverse/VersionedFileMetadata.java new file mode 100644 index 00000000000..91478d536f6 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/VersionedFileMetadata.java @@ -0,0 +1,25 @@ +package edu.harvard.iq.dataverse; + +/** + * A Data Transfer Object (DTO) to hold a DatasetVersion and its + * corresponding FileMetadata for a specific file. The fileMetadata + * field can be null if the file does not exist in that version. + */ +public class VersionedFileMetadata { + private final DatasetVersion datasetVersion; + private final FileMetadata fileMetadata; // This can be null + + public VersionedFileMetadata(DatasetVersion datasetVersion, FileMetadata fileMetadata) { + this.datasetVersion = datasetVersion; + this.fileMetadata = fileMetadata; + } + + // Add getters for both fields + public DatasetVersion getDatasetVersion() { + return datasetVersion; + } + + public FileMetadata getFileMetadata() { + return fileMetadata; + } +} From a3b753bd1b6356c530fdb739497423cde5365473 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 7 Oct 2025 23:36:35 +0100 Subject: [PATCH 13/45] Added: handling optional pagination in findFileMetadataHistory and GetFileVersionDifferencesCommand refactored --- .../iq/dataverse/DataFileServiceBean.java | 28 +++++-- .../GetFileVersionDifferencesCommand.java | 84 +++++++++++++------ .../GetFileVersionDifferencesCommandTest.java | 2 + 3 files changed, 84 insertions(+), 30 deletions(-) create mode 100644 src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java index 4b7f8351d55..67f64ac47c1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java @@ -386,12 +386,18 @@ public FileMetadata findFileMetadataByDatasetVersionIdAndDataFileId(Long dataset * This method correctly handles file replacements by searching for all files * sharing the same {@code rootDataFileId}. * - * @param datasetId The ID of the parent dataset. - * @param dataFile The DataFile entity to find the history for. + * @param datasetId The ID of the parent dataset. + * @param dataFile The DataFile entity to find the history for. * @param canViewUnpublishedVersions A boolean indicating if the user has permission to view non-released versions. - * @return A chronologically sorted list of the file's version history. Returns an empty list if the dataFile is null. + * @param limit (Optional) The maximum number of results to return. + * @param offset (Optional) The starting point of the result list. + * @return A chronologically sorted, paginated list of the file's version history. */ - public List findFileMetadataHistory(Long datasetId, DataFile dataFile, boolean canViewUnpublishedVersions) { + public List findFileMetadataHistory(Long datasetId, + DataFile dataFile, + boolean canViewUnpublishedVersions, + Integer limit, + Integer offset) { // Guard clause: Return early if there's no file to search for. if (dataFile == null) { return Collections.emptyList(); @@ -439,8 +445,20 @@ public List findFileMetadataHistory(Long datasetId, DataF ); // --- Execution & Transformation --- + // Create the query object from the criteria definition. + TypedQuery typedQuery = em.createQuery(criteriaQuery); + + // --- Pagination --- + // Apply pagination if limit and/or offset are provided. + if (limit != null) { + typedQuery.setMaxResults(limit); + } + if (offset != null) { + typedQuery.setFirstResult(offset); + } + // Execute the query and map the results to the VersionedFileMetadata list. - List results = em.createQuery(criteriaQuery).getResultList(); + List results = typedQuery.getResultList(); return results.stream() .map(metadata -> new VersionedFileMetadata(metadata.getDatasetVersion(), metadata)) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index 48e98021150..f6d22fd3707 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -2,54 +2,88 @@ import edu.harvard.iq.dataverse.*; import edu.harvard.iq.dataverse.authorization.Permission; -import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; import edu.harvard.iq.dataverse.engine.command.AbstractCommand; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; -import java.util.ArrayList; import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; /** - * A command that retrieves a paginated list of {@link DatasetVersionSummary} for the versions of a given {@link Dataset}. - **/ + * A command that retrieves the version history for a single file, + * highlighting the differences between each version. + */ @RequiredPermissions({}) public class GetFileVersionDifferencesCommand extends AbstractCommand> { + private final FileMetadata fileMetadata; private final Integer limit; private final Integer offset; - private final FileMetadataVersionsHelper fileMetadataVersionsHelper; + private final FileMetadataVersionsHelper versionsHelper; public GetFileVersionDifferencesCommand(DataverseRequest request, FileMetadata fileMetadata, Integer limit, Integer offset, FileMetadataVersionsHelper fileMetadataVersionsHelper) { super(request, fileMetadata.getDataFile()); - this.fileMetadata = fileMetadata; + this.fileMetadata = Objects.requireNonNull(fileMetadata); + this.versionsHelper = Objects.requireNonNull(fileMetadataVersionsHelper); this.limit = limit; this.offset = offset; - this.fileMetadataVersionsHelper = fileMetadataVersionsHelper; } @Override - // TODO public List execute(CommandContext ctxt) throws CommandException { + List fileHistory = findFileHistory(ctxt); + + return fileHistory.stream() + .map(this::createDifferenceFromHistory) + .collect(Collectors.toList()); + } + + /** + * Fetches the file's version history from the database, respecting user permissions. + */ + private List findFileHistory(CommandContext ctxt) { Dataset dataset = fileMetadata.getDatasetVersion().getDataset(); - boolean hasPermission = ctxt.permissions().requestOn(getRequest(), dataset).has(Permission.ViewUnpublishedDataset); - List versionedFileMetadataList = ctxt.files().findFileMetadataHistory(dataset.getId(), fileMetadata.getDataFile(), hasPermission); - List differences = new ArrayList<>(); - for (VersionedFileMetadata versionedFileMetadata : versionedFileMetadataList) { - DatasetVersion datasetVersion = versionedFileMetadata.getDatasetVersion(); - if (versionedFileMetadata.getFileMetadata() == null) { - FileMetadata dummy = new FileMetadata(); - dummy.setDatasetVersion(datasetVersion); - dummy.setDataFile(null); - FileVersionDifference fvd = new FileVersionDifference(dummy, fileMetadataVersionsHelper.getPreviousFileMetadata(fileMetadata, datasetVersion), true); - dummy.setFileVersionDifference(fvd); - differences.add(fvd); - } else { - differences.add(new FileVersionDifference(versionedFileMetadata.getFileMetadata(), fileMetadataVersionsHelper.getPreviousFileMetadata(fileMetadata, datasetVersion), true)); - } - } - return differences; + boolean canViewUnpublished = canViewUnpublishedVersions(ctxt, dataset); + + return ctxt.files().findFileMetadataHistory(dataset.getId(), fileMetadata.getDataFile(), canViewUnpublished, limit, offset); + } + + /** + * Determines if the user has permission to view non-released versions of the dataset. + */ + private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) { + return ctxt.permissions().requestOn(getRequest(), dataset).has(Permission.ViewUnpublishedDataset); + } + + /** + * Transforms a single historical entry into a FileVersionDifference object. + */ + private FileVersionDifference createDifferenceFromHistory(VersionedFileMetadata versionedFileMetadata) { + FileMetadata current = versionedFileMetadata.getFileMetadata(); + DatasetVersion version = versionedFileMetadata.getDatasetVersion(); + + FileMetadata previous = versionsHelper.getPreviousFileMetadata(this.fileMetadata, version); + + return (current != null) + ? new FileVersionDifference(current, previous, false) + : createDifferenceForNonexistentFile(version, previous); + } + + /** + * Creates a special FileVersionDifference for versions where the file does not exist. + */ + private FileVersionDifference createDifferenceForNonexistentFile(DatasetVersion version, FileMetadata previous) { + FileMetadata placeholder = new FileMetadata(); + placeholder.setDatasetVersion(version); + // Explicitly null DataFile indicates the file does not exist. + placeholder.setDataFile(null); + + FileVersionDifference difference = new FileVersionDifference(placeholder, previous, true); + placeholder.setFileVersionDifference(difference); + + return difference; } } diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java new file mode 100644 index 00000000000..55ceb149997 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -0,0 +1,2 @@ +package edu.harvard.iq.dataverse.engine.command.impl;public class GetFileVersionDifferencesCommandTest { +} From 55b78ebdb92668c1f7ebde3f26068e66454f4b3a Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 8 Oct 2025 17:12:07 +0100 Subject: [PATCH 14/45] Added: GetFileVersionDifferencesCommandTest --- .../GetFileVersionDifferencesCommand.java | 7 +- .../GetFileVersionDifferencesCommandTest.java | 211 +++++++++++++++++- 2 files changed, 216 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index f6d22fd3707..f6d53a39348 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -8,6 +8,7 @@ import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import java.util.EnumSet; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -55,7 +56,11 @@ private List findFileHistory(CommandContext ctxt) { * Determines if the user has permission to view non-released versions of the dataset. */ private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) { - return ctxt.permissions().requestOn(getRequest(), dataset).has(Permission.ViewUnpublishedDataset); + return ctxt.permissions().hasPermissionsFor( + getRequest().getUser(), + dataset, + EnumSet.of(Permission.ViewUnpublishedDataset) + ); } /** diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java index 55ceb149997..ae75b0e47b2 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -1,2 +1,211 @@ -package edu.harvard.iq.dataverse.engine.command.impl;public class GetFileVersionDifferencesCommandTest { +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.*; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class GetFileVersionDifferencesCommandTest { + + @Mock + private CommandContext contextMock; + @Mock + private DataverseRequest requestMock; + @Mock + private FileMetadata fileMetadataMock; + @Mock + private FileMetadataVersionsHelper versionsHelperMock; + @Mock + private PermissionServiceBean permissionsMock; + @Mock + private DataFileServiceBean fileServiceMock; + @Mock + private Dataset datasetMock; + @Mock + private DatasetVersion datasetVersionMock; + @Mock + private DataFile dataFileMock; + + private static final Long DATASET_ID = 1L; + + @BeforeEach + void setUp() { + // Mock the context to return our mock services + when(contextMock.permissions()).thenReturn(permissionsMock); + when(contextMock.files()).thenReturn(fileServiceMock); + + // Mock the chain of objects from FileMetadata -> Dataset + when(fileMetadataMock.getDatasetVersion()).thenReturn(datasetVersionMock); + when(datasetVersionMock.getDataset()).thenReturn(datasetMock); + when(datasetMock.getId()).thenReturn(DATASET_ID); + when(fileMetadataMock.getDataFile()).thenReturn(dataFileMock); + } + + @Test + @DisplayName("execute should call findFileMetadataHistory with 'canViewUnpublished' as TRUE when user has permission") + void execute_should_call_history_with_true_when_user_has_permission() throws CommandException { + // Arrange + when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) + .thenReturn(true); + when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) + .thenReturn(Collections.emptyList()); + + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null, versionsHelperMock); + + // Act + command.execute(contextMock); + + // Assert + ArgumentCaptor canViewUnpublishedCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(fileServiceMock).findFileMetadataHistory( + eq(DATASET_ID), + eq(dataFileMock), + canViewUnpublishedCaptor.capture(), + eq(null), + eq(null) + ); + assertThat(canViewUnpublishedCaptor.getValue()).isTrue(); + } + + @Test + @DisplayName("execute should call findFileMetadataHistory with 'canViewUnpublished' as FALSE when user lacks permission") + void execute_should_call_history_with_false_when_user_lacks_permission() throws CommandException { + // Arrange + when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) + .thenReturn(false); + when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) + .thenReturn(Collections.emptyList()); + + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null, versionsHelperMock); + + // Act + command.execute(contextMock); + + // Assert + ArgumentCaptor canViewUnpublishedCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(fileServiceMock).findFileMetadataHistory( + eq(DATASET_ID), + eq(dataFileMock), + canViewUnpublishedCaptor.capture(), + eq(null), + eq(null) + ); + assertThat(canViewUnpublishedCaptor.getValue()).isFalse(); + } + + @Test + @DisplayName("execute should pass pagination parameters correctly to findFileMetadataHistory") + void execute_should_pass_pagination_parameters_correctly() throws CommandException { + // Arrange + Integer expectedLimit = 10; + Integer expectedOffset = 20; + when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) + .thenReturn(Collections.emptyList()); + + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, expectedLimit, expectedOffset, versionsHelperMock); + + // Act + command.execute(contextMock); + + // Assert + verify(fileServiceMock).findFileMetadataHistory( + eq(DATASET_ID), + eq(dataFileMock), + anyBoolean(), + eq(expectedLimit), + eq(expectedOffset) + ); + } + + @Test + @DisplayName("execute should correctly convert file history to FileVersionDifference list") + void execute_should_convert_history_to_differences( + // Mocks for a version with METADATA CHANGES + @Mock VersionedFileMetadata vfm_changed, @Mock FileMetadata currentFm_changed, + @Mock FileMetadata previousFm_changed, @Mock DatasetVersion version_changed, + // Mocks for a version with NO METADATA CHANGES + @Mock VersionedFileMetadata vfm_same, @Mock FileMetadata currentFm_same, + @Mock FileMetadata previousFm_same, @Mock DatasetVersion version_same, + // Mocks for underlying file objects + @Mock DataFile dataFile1, @Mock DataFile dataFile2 + ) throws CommandException { + + // Arrange: Prepare a history list with two entries. + + // --- Scenario 1: A version where file metadata (e.g., the label) has changed. --- + when(vfm_changed.getFileMetadata()).thenReturn(currentFm_changed); + when(vfm_changed.getDatasetVersion()).thenReturn(version_changed); + when(versionsHelperMock.getPreviousFileMetadata(fileMetadataMock, version_changed)).thenReturn(previousFm_changed); + // Mock the underlying data to trigger a difference in compareMetadata() + when(currentFm_changed.getLabel()).thenReturn("new_name.txt"); + when(previousFm_changed.getLabel()).thenReturn("old_name.txt"); + when(currentFm_changed.getDataFile()).thenReturn(dataFile1); + when(previousFm_changed.getDataFile()).thenReturn(dataFile1); + + // --- Scenario 2: A version where the file exists but its metadata is identical to the previous version. --- + when(vfm_same.getFileMetadata()).thenReturn(currentFm_same); + when(vfm_same.getDatasetVersion()).thenReturn(version_same); + when(versionsHelperMock.getPreviousFileMetadata(fileMetadataMock, version_same)).thenReturn(previousFm_same); + // Mock the underlying data to be identical in compareMetadata() + when(currentFm_same.isRestricted()).thenReturn(false); + when(previousFm_same.isRestricted()).thenReturn(false); + when(currentFm_same.getLabel()).thenReturn("file.txt"); + when(previousFm_same.getLabel()).thenReturn("file.txt"); + when(currentFm_same.getDescription()).thenReturn("description"); + when(previousFm_same.getDescription()).thenReturn("description"); + when(currentFm_same.getProvFreeForm()).thenReturn(null); + when(previousFm_same.getProvFreeForm()).thenReturn(null); + when(currentFm_same.getCategoriesByName()).thenReturn(Collections.emptyList()); + when(previousFm_same.getCategoriesByName()).thenReturn(Collections.emptyList()); + when(currentFm_same.getDataFile()).thenReturn(dataFile2); + when(previousFm_same.getDataFile()).thenReturn(dataFile2); + + // Mock the service to return our prepared history list. + when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) + .thenReturn(List.of(vfm_changed, vfm_same)); + + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null, versionsHelperMock); + + // Act + List result = command.execute(contextMock); + + // Assert + assertThat(result).hasSize(2); + + // --- Verify the first difference object (METADATA HAS CHANGED) --- + FileVersionDifference diff1 = result.get(0); + // Assert the correct FileMetadata objects were passed to the constructor. + assertThat(diff1.getNewFileMetadata()).isEqualTo(currentFm_changed); + assertThat(diff1.getOriginalFileMetadata()).isEqualTo(previousFm_changed); + // Assert that compareMetadata() correctly identified a difference. + assertThat(diff1.isSame()).isFalse(); + + // --- Verify the second difference object (METADATA IS THE SAME) --- + FileVersionDifference diff2 = result.get(1); + // Assert the correct FileMetadata objects were passed to the constructor. + assertThat(diff2.getNewFileMetadata()).isEqualTo(currentFm_same); + assertThat(diff2.getOriginalFileMetadata()).isEqualTo(previousFm_same); + // Assert that compareMetadata() correctly identified the metadata as identical. + assertThat(diff2.isSame()).isTrue(); + } } From 0bc91040f96b45ed67ece6d16da5d67428dce57a Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 9 Oct 2025 17:02:46 +0100 Subject: [PATCH 15/45] Changed: using JPACriteria-based method instead of javacode for filtering previous file metadata version in GetFileVersionDifferencesCommand --- .../iq/dataverse/DataFileServiceBean.java | 40 +++++- .../edu/harvard/iq/dataverse/api/Files.java | 2 +- .../GetFileVersionDifferencesCommand.java | 16 +-- .../GetFileVersionDifferencesCommandTest.java | 114 +++++++----------- 4 files changed, 89 insertions(+), 83 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java index 67f64ac47c1..0014cce282c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java @@ -398,7 +398,6 @@ public List findFileMetadataHistory(Long datasetId, boolean canViewUnpublishedVersions, Integer limit, Integer offset) { - // Guard clause: Return early if there's no file to search for. if (dataFile == null) { return Collections.emptyList(); } @@ -465,6 +464,45 @@ public List findFileMetadataHistory(Long datasetId, .collect(Collectors.toList()); } + /** + * Finds the FileMetadata for a given file in the version immediately preceding a specified version. + * + * @param fileMetadata The FileMetadata instance from the current version, used to identify the file's lineage. + * @return The FileMetadata from the immediately prior version, or {@code null} if this is the first version of the file. + */ + public FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata) { + if (fileMetadata == null || fileMetadata.getDataFile() == null) { + return null; + } + + // 1. Get the ID of the file that was replaced. + Long previousId = fileMetadata.getDataFile().getPreviousDataFileId(); + + // If there's no previous ID, this is the first version of the file. + if (previousId == null) { + return null; + } + + CriteriaBuilder cb = em.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FileMetadata.class); + Root fileMetadataRoot = cq.from(FileMetadata.class); + + // 2. Join FileMetadata to DataFile to access the ID. + Join dataFileJoin = fileMetadataRoot.join("dataFile"); + + // 3. Find the FileMetadata whose DataFile ID matches the previousId. + cq.where(cb.equal(dataFileJoin.get("id"), previousId)); + + // --- Execution --- + TypedQuery query = em.createQuery(cq); + try { + return query.getSingleResult(); + } catch (NoResultException e) { + // If no result is found, return null. + return null; + } + } + public FileMetadata findMostRecentVersionFileIsIn(DataFile file) { if (file == null) { return null; diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 4810510c8a4..7cd0ef3dd25 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -1182,7 +1182,7 @@ public Response getFileVersionsList(@Context ContainerRequestContext crc, @PathP if (fm == null) { return notFound(BundleUtil.getStringFromBundle("files.api.fileNotFound")); } - List fileVersionDifferences = execCommand(new GetFileVersionDifferencesCommand(req, fm, null, null, fileMetadataVersionsHelper)); + List fileVersionDifferences = execCommand(new GetFileVersionDifferencesCommand(req, fm, null, null)); JsonArrayBuilder jab = Json.createArrayBuilder(); for (FileVersionDifference fileVersionDifference : fileVersionDifferences) { jab.add(fileMetadataVersionsHelper.jsonDataFileVersions(fileVersionDifference).build()); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index f6d53a39348..674f6c6c42c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -23,12 +23,10 @@ public class GetFileVersionDifferencesCommand extends AbstractCommand execute(CommandContext ctxt) throws CommandEx List fileHistory = findFileHistory(ctxt); return fileHistory.stream() - .map(this::createDifferenceFromHistory) + .map(versionedFileMetadata -> createDifferenceFromHistory(ctxt, versionedFileMetadata)) .collect(Collectors.toList()); } /** - * Fetches the file's version history from the database, respecting user permissions. + * Fetches the file's version history, respecting user permissions. */ private List findFileHistory(CommandContext ctxt) { Dataset dataset = fileMetadata.getDatasetVersion().getDataset(); @@ -66,15 +64,13 @@ private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) /** * Transforms a single historical entry into a FileVersionDifference object. */ - private FileVersionDifference createDifferenceFromHistory(VersionedFileMetadata versionedFileMetadata) { + private FileVersionDifference createDifferenceFromHistory(CommandContext ctxt, VersionedFileMetadata versionedFileMetadata) { FileMetadata current = versionedFileMetadata.getFileMetadata(); - DatasetVersion version = versionedFileMetadata.getDatasetVersion(); - - FileMetadata previous = versionsHelper.getPreviousFileMetadata(this.fileMetadata, version); + FileMetadata previous = ctxt.files().getPreviousFileMetadata(current); return (current != null) ? new FileVersionDifference(current, previous, false) - : createDifferenceForNonexistentFile(version, previous); + : createDifferenceForNonexistentFile(versionedFileMetadata.getDatasetVersion(), previous); } /** diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java index ae75b0e47b2..b931fa0d8ae 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -21,8 +21,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) class GetFileVersionDifferencesCommandTest { @@ -34,8 +33,6 @@ class GetFileVersionDifferencesCommandTest { @Mock private FileMetadata fileMetadataMock; @Mock - private FileMetadataVersionsHelper versionsHelperMock; - @Mock private PermissionServiceBean permissionsMock; @Mock private DataFileServiceBean fileServiceMock; @@ -50,11 +47,9 @@ class GetFileVersionDifferencesCommandTest { @BeforeEach void setUp() { - // Mock the context to return our mock services when(contextMock.permissions()).thenReturn(permissionsMock); when(contextMock.files()).thenReturn(fileServiceMock); - // Mock the chain of objects from FileMetadata -> Dataset when(fileMetadataMock.getDatasetVersion()).thenReturn(datasetVersionMock); when(datasetVersionMock.getDataset()).thenReturn(datasetMock); when(datasetMock.getId()).thenReturn(DATASET_ID); @@ -70,7 +65,7 @@ void execute_should_call_history_with_true_when_user_has_permission() throws Com when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) .thenReturn(Collections.emptyList()); - GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null, versionsHelperMock); + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null); // Act command.execute(contextMock); @@ -96,7 +91,7 @@ void execute_should_call_history_with_false_when_user_lacks_permission() throws when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) .thenReturn(Collections.emptyList()); - GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null, versionsHelperMock); + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null); // Act command.execute(contextMock); @@ -122,7 +117,7 @@ void execute_should_pass_pagination_parameters_correctly() throws CommandExcepti when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) .thenReturn(Collections.emptyList()); - GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, expectedLimit, expectedOffset, versionsHelperMock); + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, expectedLimit, expectedOffset); // Act command.execute(contextMock); @@ -139,73 +134,50 @@ void execute_should_pass_pagination_parameters_correctly() throws CommandExcepti @Test @DisplayName("execute should correctly convert file history to FileVersionDifference list") - void execute_should_convert_history_to_differences( - // Mocks for a version with METADATA CHANGES - @Mock VersionedFileMetadata vfm_changed, @Mock FileMetadata currentFm_changed, - @Mock FileMetadata previousFm_changed, @Mock DatasetVersion version_changed, - // Mocks for a version with NO METADATA CHANGES - @Mock VersionedFileMetadata vfm_same, @Mock FileMetadata currentFm_same, - @Mock FileMetadata previousFm_same, @Mock DatasetVersion version_same, - // Mocks for underlying file objects - @Mock DataFile dataFile1, @Mock DataFile dataFile2 - ) throws CommandException { - - // Arrange: Prepare a history list with two entries. - - // --- Scenario 1: A version where file metadata (e.g., the label) has changed. --- - when(vfm_changed.getFileMetadata()).thenReturn(currentFm_changed); - when(vfm_changed.getDatasetVersion()).thenReturn(version_changed); - when(versionsHelperMock.getPreviousFileMetadata(fileMetadataMock, version_changed)).thenReturn(previousFm_changed); - // Mock the underlying data to trigger a difference in compareMetadata() - when(currentFm_changed.getLabel()).thenReturn("new_name.txt"); - when(previousFm_changed.getLabel()).thenReturn("old_name.txt"); - when(currentFm_changed.getDataFile()).thenReturn(dataFile1); - when(previousFm_changed.getDataFile()).thenReturn(dataFile1); - - // --- Scenario 2: A version where the file exists but its metadata is identical to the previous version. --- - when(vfm_same.getFileMetadata()).thenReturn(currentFm_same); - when(vfm_same.getDatasetVersion()).thenReturn(version_same); - when(versionsHelperMock.getPreviousFileMetadata(fileMetadataMock, version_same)).thenReturn(previousFm_same); - // Mock the underlying data to be identical in compareMetadata() - when(currentFm_same.isRestricted()).thenReturn(false); - when(previousFm_same.isRestricted()).thenReturn(false); - when(currentFm_same.getLabel()).thenReturn("file.txt"); - when(previousFm_same.getLabel()).thenReturn("file.txt"); - when(currentFm_same.getDescription()).thenReturn("description"); - when(previousFm_same.getDescription()).thenReturn("description"); - when(currentFm_same.getProvFreeForm()).thenReturn(null); - when(previousFm_same.getProvFreeForm()).thenReturn(null); - when(currentFm_same.getCategoriesByName()).thenReturn(Collections.emptyList()); - when(previousFm_same.getCategoriesByName()).thenReturn(Collections.emptyList()); - when(currentFm_same.getDataFile()).thenReturn(dataFile2); - when(previousFm_same.getDataFile()).thenReturn(dataFile2); - - // Mock the service to return our prepared history list. + void execute_should_convert_history_to_differences() throws CommandException { + // Arrange + // Create mock history: 3 versions of a file's metadata + FileMetadata fm3 = new FileMetadata(); // Newest + fm3.setId(3L); + FileMetadata fm2 = new FileMetadata(); // Middle + fm2.setId(2L); + FileMetadata fm1 = new FileMetadata(); // Oldest + fm1.setId(1L); + + VersionedFileMetadata vfm3 = mock(VersionedFileMetadata.class); + VersionedFileMetadata vfm2 = mock(VersionedFileMetadata.class); + VersionedFileMetadata vfm1 = mock(VersionedFileMetadata.class); + when(vfm3.getFileMetadata()).thenReturn(fm3); + when(vfm2.getFileMetadata()).thenReturn(fm2); + when(vfm1.getFileMetadata()).thenReturn(fm1); + + List history = List.of(vfm3, vfm2, vfm1); when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) - .thenReturn(List.of(vfm_changed, vfm_same)); + .thenReturn(history); + + // Mock the logic to find the direct predecessor of each version + when(fileServiceMock.getPreviousFileMetadata(fm3)).thenReturn(fm2); + when(fileServiceMock.getPreviousFileMetadata(fm2)).thenReturn(fm1); + when(fileServiceMock.getPreviousFileMetadata(fm1)).thenReturn(null); // The oldest has no predecessor - GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null, versionsHelperMock); + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null); // Act - List result = command.execute(contextMock); + List differences = command.execute(contextMock); // Assert - assertThat(result).hasSize(2); - - // --- Verify the first difference object (METADATA HAS CHANGED) --- - FileVersionDifference diff1 = result.get(0); - // Assert the correct FileMetadata objects were passed to the constructor. - assertThat(diff1.getNewFileMetadata()).isEqualTo(currentFm_changed); - assertThat(diff1.getOriginalFileMetadata()).isEqualTo(previousFm_changed); - // Assert that compareMetadata() correctly identified a difference. - assertThat(diff1.isSame()).isFalse(); - - // --- Verify the second difference object (METADATA IS THE SAME) --- - FileVersionDifference diff2 = result.get(1); - // Assert the correct FileMetadata objects were passed to the constructor. - assertThat(diff2.getNewFileMetadata()).isEqualTo(currentFm_same); - assertThat(diff2.getOriginalFileMetadata()).isEqualTo(previousFm_same); - // Assert that compareMetadata() correctly identified the metadata as identical. - assertThat(diff2.isSame()).isTrue(); + assertThat(differences).hasSize(3); + + // Check the difference for the newest version (v3 vs v2) + assertThat(differences.get(0).getNewFileMetadata()).isEqualTo(fm3); + assertThat(differences.get(0).getOriginalFileMetadata()).isEqualTo(fm2); + + // Check the difference for the middle version (v2 vs v1) + assertThat(differences.get(1).getNewFileMetadata()).isEqualTo(fm2); + assertThat(differences.get(1).getOriginalFileMetadata()).isEqualTo(fm1); + + // Check the difference for the oldest version (v1 vs null) + assertThat(differences.get(2).getNewFileMetadata()).isEqualTo(fm1); + assertThat(differences.get(2).getOriginalFileMetadata()).isNull(); } } From c7da46ed8cfda919dc2102b40b01a56757e27895 Mon Sep 17 00:00:00 2001 From: GPortas Date: Sun, 12 Oct 2025 11:58:39 +0100 Subject: [PATCH 16/45] Stash: refactoring Files {id}/versionDifferences JSON printing logic --- .../dataverse/FileMetadataVersionsHelper.java | 136 ----------------- .../edu/harvard/iq/dataverse/api/Files.java | 17 +-- .../iq/dataverse/util/json/JsonPrinter.java | 142 ++++++++++++++++++ 3 files changed, 145 insertions(+), 150 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java index 21bb8ea51e4..368bc1b0628 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java @@ -2,21 +2,13 @@ import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; -import edu.harvard.iq.dataverse.util.StringUtil; import jakarta.ejb.EJB; import jakarta.ejb.Stateless; -import jakarta.json.*; import java.util.*; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; @Stateless public class FileMetadataVersionsHelper { - private static final Logger logger = Logger.getLogger(FileMetadataVersionsHelper.class.getCanonicalName()); - @EJB DataFileServiceBean datafileService; @EJB @@ -24,9 +16,6 @@ public class FileMetadataVersionsHelper { @EJB PermissionServiceBean permissionService; - // Groups that are single element groups and therefore not arrays. - private static final List SINGLE_ELEMENT_GROUPS = List.of("File Access"); - public List loadFileVersionList(DataverseRequest req, FileMetadata fileMetadata) { List allfiles = allRelatedFiles(fileMetadata); List retList = new ArrayList<>(); @@ -61,94 +50,6 @@ public List loadFileVersionList(DataverseRequest req, FileMetadata return retList; } - public JsonObjectBuilder jsonDataFileVersions(FileVersionDifference fileVersionDifference) { - JsonObjectBuilder job = jsonObjectBuilder(); - FileMetadata fileMetadata = fileVersionDifference.getNewFileMetadata(); - if (fileMetadata.getDatasetVersion() != null) { - job.add("datasetVersion", fileMetadata.getDatasetVersion().getFriendlyVersionNumber()); - if (fileMetadata.getDatasetVersion().getVersionNumber() != null) { - job - .add("versionNumber", fileMetadata.getDatasetVersion().getVersionNumber()) - .add("versionMinorNumber", fileMetadata.getDatasetVersion().getMinorVersionNumber()); - } - - job - .add("isDraft", fileMetadata.getDatasetVersion().isDraft()) - .add("isReleased", fileMetadata.getDatasetVersion().isReleased()) - .add("isDeaccessioned", fileMetadata.getDatasetVersion().isDeaccessioned()) - .add("versionState", fileMetadata.getDatasetVersion().getVersionState().name()) - .add("summary", fileMetadata.getDatasetVersion().getVersionNote()) - .add("contributors", fileMetadata.getContributorNames()) - .add("publishedDate", fileMetadata.getDataFile() != null ? (fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) : null) - ; - } - if (fileMetadata.getDataFile() != null) { - job.add("datafileId", fileMetadata.getDataFile().getId()); - job.add("persistentId", (fileMetadata.getDataFile().getGlobalId() != null ? fileMetadata.getDataFile().getGlobalId().asString() : "")); - } - FileVersionDifference fvd = fileVersionDifference; - if (fvd != null) { - List groups = fvd.getDifferenceSummaryGroups(); - JsonObjectBuilder fileDifferenceSummary = jsonObjectBuilder() - .add("versionNote", fileMetadata.getDatasetVersion().getVersionNote()) - .add("deaccessionedReason", fileMetadata.getDatasetVersion().getDeaccessionNote()) - .add("file", getFileAction(fvd.getOriginalFileMetadata(), fvd.getNewFileMetadata())); - - if (groups != null && !groups.isEmpty()) { - List sortedGroups = groups.stream() - .sorted(Comparator.comparing(FileVersionDifference.FileDifferenceSummaryGroup::getName)) - .collect(Collectors.toList()); - String groupName = null; - final JsonArrayBuilder groupsArrayBuilder = Json.createArrayBuilder(); - final JsonObjectBuilder groupsObjectBuilder = jsonObjectBuilder(); - Map itemCounts = new HashMap<>(); - - for (FileVersionDifference.FileDifferenceSummaryGroup group : sortedGroups) { - if (!StringUtil.isEmpty(group.getName())) { - // if the group name changed then add its data to the fileDifferenceSummary and reset list for next group - if (groupName != null && groupName.compareTo(group.getName()) != 0) { - addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); - // Note: groupsArrayBuilder.build() also clears the data within it - itemCounts.clear(); - } - groupName = group.getName(); - - group.getFileDifferenceSummaryItems().forEach(item -> { - JsonObjectBuilder itemObjectBuilder = jsonObjectBuilder(); - if (item.getName().isEmpty()) { - // 'groupName': {'Added'=#, 'Changed'=#, ...} - // accumulate the counts since we can't make a separate array item - itemCounts.merge("Added", item.getAdded(), Integer::sum); - itemCounts.merge("Changed", item.getChanged(), Integer::sum); - itemCounts.merge("Deleted", item.getDeleted(), Integer::sum); - itemCounts.merge("Replaced", item.getReplaced(), Integer::sum); - } else if (SINGLE_ELEMENT_GROUPS.contains(group.getName())) { - // 'groupName': 'getNameValue' - groupsObjectBuilder.add(group.getName(), group.getFileDifferenceSummaryItems().get(0).getName()); - } else { - // 'groupName': [{name='', action=''}, {name='', action=''}] - String action = item.getAdded() > 0 ? "Added" : item.getChanged() > 0 ? "Changed" : - item.getDeleted() > 0 ? "Deleted" : item.getReplaced() > 0 ? "Replaced" : ""; - itemObjectBuilder.add("name", item.getName()); - if (!action.isEmpty()) { - itemObjectBuilder.add("action", action); - } - groupsArrayBuilder.add(itemObjectBuilder.build()); - } - }); - } - } - // process last group - addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); - } - JsonObject fileDifferenceSummaryObject = fileDifferenceSummary.build(); - if (!fileDifferenceSummaryObject.isEmpty()) { - job.add("fileDifferenceSummary", fileDifferenceSummaryObject); - } - } - return job; - } - //TODO: this could use some refactoring to cut down on the number of for loops! private FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, FileMetadata fmdIn){ @@ -228,41 +129,4 @@ private List allRelatedFiles(FileMetadata fileMetadata) { } return dataFiles; } - - private String getFileAction(FileMetadata originalFileMetadata, FileMetadata newFileMetadata) { - if (newFileMetadata.getDataFile() != null && originalFileMetadata == null) { - return "Added"; - } else if (newFileMetadata.getDataFile() == null && originalFileMetadata != null) { - return "Deleted"; - } else if (originalFileMetadata != null && - newFileMetadata.getDataFile() != null && originalFileMetadata.getDataFile() != null &&!originalFileMetadata.getDataFile().equals(newFileMetadata.getDataFile())) { - return "Replaced"; - } else { - return null; - } - } - - private void addJsonGroupObject(JsonObjectBuilder jsonObjectBuilder, String key, JsonObject jsonObjectValue, JsonArray jsonArrayValue, Map itemCounts) { - if (key != null && !key.isEmpty()) { - String sanitizedKey = key.replaceAll("\\s+", ""); - if (itemCounts.isEmpty()) { - if (jsonArrayValue.isEmpty()) { - // add the object - jsonObjectBuilder.add(sanitizedKey, jsonObjectValue.getValue("/"+key)); - } else { - // add the array - jsonObjectBuilder.add(sanitizedKey, jsonArrayValue); - } - } else { - // add the accumulated totals - JsonObjectBuilder accumulatedTotalsObjectBuilder = jsonObjectBuilder(); - itemCounts.forEach((k, v) -> { - if (v != 0) { - accumulatedTotalsObjectBuilder.add(k, v); - } - }); - jsonObjectBuilder.add(sanitizedKey, accumulatedTotalsObjectBuilder.build()); - } - } - } } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 7cd0ef3dd25..42e23656808 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -35,7 +35,7 @@ import static edu.harvard.iq.dataverse.api.ApiConstants.*; import static edu.harvard.iq.dataverse.api.Datasets.handleVersion; -import static edu.harvard.iq.dataverse.util.json.JsonPrinter.json; + import edu.harvard.iq.dataverse.util.json.JsonUtil; import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder; @@ -61,7 +61,7 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; -import static edu.harvard.iq.dataverse.util.json.JsonPrinter.jsonDT; +import static edu.harvard.iq.dataverse.util.json.JsonPrinter.*; import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; import static jakarta.ws.rs.core.Response.Status.FORBIDDEN; @@ -104,8 +104,6 @@ public class Files extends AbstractApiBean { GuestbookResponseServiceBean guestbookResponseService; @Inject DataFileServiceBean dataFileServiceBean; - @Inject - FileMetadataVersionsHelper fileMetadataVersionsHelper; private static final Logger logger = Logger.getLogger(Files.class.getName()); @@ -1182,16 +1180,7 @@ public Response getFileVersionsList(@Context ContainerRequestContext crc, @PathP if (fm == null) { return notFound(BundleUtil.getStringFromBundle("files.api.fileNotFound")); } - List fileVersionDifferences = execCommand(new GetFileVersionDifferencesCommand(req, fm, null, null)); - JsonArrayBuilder jab = Json.createArrayBuilder(); - for (FileVersionDifference fileVersionDifference : fileVersionDifferences) { - jab.add(fileMetadataVersionsHelper.jsonDataFileVersions(fileVersionDifference).build()); - } - return Response.ok() - .entity(Json.createObjectBuilder() - .add("status", STATUS_OK) - .add("data", jab.build()).build() - ).build(); + return ok(jsonFileVersionSummaries(execCommand(new GetFileVersionDifferencesCommand(req, fm, null, null)))); } catch (WrappedResponse ex) { return ex.getResponse(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index 49d887cf27d..c451aaf92a3 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -38,10 +38,13 @@ import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; import edu.harvard.iq.dataverse.util.MailUtil; +import edu.harvard.iq.dataverse.util.StringUtil; import edu.harvard.iq.dataverse.workflow.Workflow; import edu.harvard.iq.dataverse.workflow.step.WorkflowStepData; import java.util.*; + +import jakarta.inject.Inject; import jakarta.json.Json; import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonObjectBuilder; @@ -84,6 +87,9 @@ public class JsonPrinter { @EJB static InAppNotificationsJsonPrinter inAppNotificationsJsonPrinter; + + @EJB + static FileMetadataVersionsHelper fileMetadataVersionsHelper; public static void injectSettingsService(SettingsServiceBean ssb, DatasetFieldServiceBean dfsb, @@ -1728,4 +1734,140 @@ private static JsonObjectBuilder json(DatasetVersionSummary summary) { return jsonObjectBuilder; } + + public static JsonArray jsonFileVersionSummaries(List differences) { + JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); + differences.stream() + .filter(Objects::nonNull) + .map(diff -> jsonDataFileVersions(diff).build()) + .forEach(arrayBuilder::add); + + return arrayBuilder.build(); + } + + private static JsonObjectBuilder jsonDataFileVersions(FileVersionDifference fileVersionDifference) { + JsonObjectBuilder job = jsonObjectBuilder(); + FileMetadata fileMetadata = fileVersionDifference.getNewFileMetadata(); + if (fileMetadata.getDatasetVersion() != null) { + job.add("datasetVersion", fileMetadata.getDatasetVersion().getFriendlyVersionNumber()); + if (fileMetadata.getDatasetVersion().getVersionNumber() != null) { + job + .add("versionNumber", fileMetadata.getDatasetVersion().getVersionNumber()) + .add("versionMinorNumber", fileMetadata.getDatasetVersion().getMinorVersionNumber()); + } + + job + .add("isDraft", fileMetadata.getDatasetVersion().isDraft()) + .add("isReleased", fileMetadata.getDatasetVersion().isReleased()) + .add("isDeaccessioned", fileMetadata.getDatasetVersion().isDeaccessioned()) + .add("versionState", fileMetadata.getDatasetVersion().getVersionState().name()) + .add("summary", fileMetadata.getDatasetVersion().getVersionNote()) + .add("contributors", fileMetadata.getContributorNames()) + .add("publishedDate", fileMetadata.getDataFile() != null ? (fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) : null) + ; + } + if (fileMetadata.getDataFile() != null) { + job.add("datafileId", fileMetadata.getDataFile().getId()); + job.add("persistentId", (fileMetadata.getDataFile().getGlobalId() != null ? fileMetadata.getDataFile().getGlobalId().asString() : "")); + } + FileVersionDifference fvd = fileVersionDifference; + if (fvd != null) { + List groups = fvd.getDifferenceSummaryGroups(); + JsonObjectBuilder fileDifferenceSummary = jsonObjectBuilder() + .add("versionNote", fileMetadata.getDatasetVersion().getVersionNote()) + .add("deaccessionedReason", fileMetadata.getDatasetVersion().getDeaccessionNote()) + .add("file", getFileAction(fvd.getOriginalFileMetadata(), fvd.getNewFileMetadata())); + + if (groups != null && !groups.isEmpty()) { + List sortedGroups = groups.stream() + .sorted(Comparator.comparing(FileVersionDifference.FileDifferenceSummaryGroup::getName)) + .collect(Collectors.toList()); + String groupName = null; + final JsonArrayBuilder groupsArrayBuilder = Json.createArrayBuilder(); + final JsonObjectBuilder groupsObjectBuilder = jsonObjectBuilder(); + Map itemCounts = new HashMap<>(); + + for (FileVersionDifference.FileDifferenceSummaryGroup group : sortedGroups) { + if (!StringUtil.isEmpty(group.getName())) { + // if the group name changed then add its data to the fileDifferenceSummary and reset list for next group + if (groupName != null && groupName.compareTo(group.getName()) != 0) { + addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); + // Note: groupsArrayBuilder.build() also clears the data within it + itemCounts.clear(); + } + groupName = group.getName(); + + group.getFileDifferenceSummaryItems().forEach(item -> { + JsonObjectBuilder itemObjectBuilder = jsonObjectBuilder(); + if (item.getName().isEmpty()) { + // 'groupName': {'Added'=#, 'Changed'=#, ...} + // accumulate the counts since we can't make a separate array item + itemCounts.merge("Added", item.getAdded(), Integer::sum); + itemCounts.merge("Changed", item.getChanged(), Integer::sum); + itemCounts.merge("Deleted", item.getDeleted(), Integer::sum); + itemCounts.merge("Replaced", item.getReplaced(), Integer::sum); + } else if (List.of("File Access").contains(group.getName())) { + // 'groupName': 'getNameValue' + groupsObjectBuilder.add(group.getName(), group.getFileDifferenceSummaryItems().get(0).getName()); + } else { + // 'groupName': [{name='', action=''}, {name='', action=''}] + String action = item.getAdded() > 0 ? "Added" : item.getChanged() > 0 ? "Changed" : + item.getDeleted() > 0 ? "Deleted" : item.getReplaced() > 0 ? "Replaced" : ""; + itemObjectBuilder.add("name", item.getName()); + if (!action.isEmpty()) { + itemObjectBuilder.add("action", action); + } + groupsArrayBuilder.add(itemObjectBuilder.build()); + } + }); + } + } + // process last group + addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); + } + JsonObject fileDifferenceSummaryObject = fileDifferenceSummary.build(); + if (!fileDifferenceSummaryObject.isEmpty()) { + job.add("fileDifferenceSummary", fileDifferenceSummaryObject); + } + } + return job; + } + + private static String getFileAction(FileMetadata originalFileMetadata, FileMetadata newFileMetadata) { + if (newFileMetadata.getDataFile() != null && originalFileMetadata == null) { + return "Added"; + } else if (newFileMetadata.getDataFile() == null && originalFileMetadata != null) { + return "Deleted"; + } else if (originalFileMetadata != null && + newFileMetadata.getDataFile() != null && originalFileMetadata.getDataFile() != null &&!originalFileMetadata.getDataFile().equals(newFileMetadata.getDataFile())) { + return "Replaced"; + } else { + return null; + } + } + + private static void addJsonGroupObject(JsonObjectBuilder jsonObjectBuilder, String key, JsonObject jsonObjectValue, JsonArray jsonArrayValue, Map itemCounts) { + if (key != null && !key.isEmpty()) { + String sanitizedKey = key.replaceAll("\\s+", ""); + if (itemCounts.isEmpty()) { + if (jsonArrayValue.isEmpty()) { + // add the object + jsonObjectBuilder.add(sanitizedKey, jsonObjectValue.getValue("/"+key)); + } else { + // add the array + jsonObjectBuilder.add(sanitizedKey, jsonArrayValue); + } + } else { + // add the accumulated totals + JsonObjectBuilder accumulatedTotalsObjectBuilder = jsonObjectBuilder(); + itemCounts.forEach((k, v) -> { + if (v != 0) { + accumulatedTotalsObjectBuilder.add(k, v); + } + }); + jsonObjectBuilder.add(sanitizedKey, accumulatedTotalsObjectBuilder.build()); + } + } + } + } From 755eb80f840d6758559f868e8b849c853d37c4a5 Mon Sep 17 00:00:00 2001 From: GPortas Date: Sun, 12 Oct 2025 19:07:56 +0100 Subject: [PATCH 17/45] Refactor: extracted FileVersionDifferenceJsonPrinter --- .../FileVersionDifferenceJsonPrinter.java | 141 ++++++++++++++++++ .../iq/dataverse/util/json/JsonPrinter.java | 135 +---------------- 2 files changed, 144 insertions(+), 132 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java new file mode 100644 index 00000000000..3fe3a4540be --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java @@ -0,0 +1,141 @@ +package edu.harvard.iq.dataverse.util.json; + +import edu.harvard.iq.dataverse.FileMetadata; +import edu.harvard.iq.dataverse.FileVersionDifference; +import edu.harvard.iq.dataverse.util.StringUtil; +import jakarta.json.*; + +import java.util.*; + +import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; + +public class FileVersionDifferenceJsonPrinter { + + private static final String ACTION_ADDED = "Added"; + private static final String ACTION_CHANGED = "Changed"; + private static final String ACTION_DELETED = "Deleted"; + private static final String ACTION_REPLACED = "Replaced"; + private static final String GROUP_FILE_ACCESS = "File Access"; + + public static JsonObjectBuilder jsonFileVersionDifference(FileVersionDifference fileVersionDifference) { + JsonObjectBuilder job = jsonObjectBuilder(); + FileMetadata fileMetadata = fileVersionDifference.getNewFileMetadata(); + if (fileMetadata.getDatasetVersion() != null) { + job.add("datasetVersion", fileMetadata.getDatasetVersion().getFriendlyVersionNumber()); + if (fileMetadata.getDatasetVersion().getVersionNumber() != null) { + job + .add("versionNumber", fileMetadata.getDatasetVersion().getVersionNumber()) + .add("versionMinorNumber", fileMetadata.getDatasetVersion().getMinorVersionNumber()); + } + + job + .add("isDraft", fileMetadata.getDatasetVersion().isDraft()) + .add("isReleased", fileMetadata.getDatasetVersion().isReleased()) + .add("isDeaccessioned", fileMetadata.getDatasetVersion().isDeaccessioned()) + .add("versionState", fileMetadata.getDatasetVersion().getVersionState().name()) + .add("summary", fileMetadata.getDatasetVersion().getVersionNote()) + .add("contributors", fileMetadata.getContributorNames()) + .add("publishedDate", fileMetadata.getDataFile() != null ? (fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) : null) + ; + } + if (fileMetadata.getDataFile() != null) { + job.add("datafileId", fileMetadata.getDataFile().getId()); + job.add("persistentId", (fileMetadata.getDataFile().getGlobalId() != null ? fileMetadata.getDataFile().getGlobalId().asString() : "")); + } + List groups = fileVersionDifference.getDifferenceSummaryGroups(); + JsonObjectBuilder fileDifferenceSummary = jsonObjectBuilder() + .add("versionNote", fileMetadata.getDatasetVersion().getVersionNote()) + .add("deaccessionedReason", fileMetadata.getDatasetVersion().getDeaccessionNote()) + .add("file", getFileAction(fileVersionDifference.getOriginalFileMetadata(), fileVersionDifference.getNewFileMetadata())); + + if (groups != null && !groups.isEmpty()) { + List sortedGroups = groups.stream() + .sorted(Comparator.comparing(FileVersionDifference.FileDifferenceSummaryGroup::getName)) + .toList(); + String groupName = null; + final JsonArrayBuilder groupsArrayBuilder = Json.createArrayBuilder(); + final JsonObjectBuilder groupsObjectBuilder = jsonObjectBuilder(); + Map itemCounts = new HashMap<>(); + + for (FileVersionDifference.FileDifferenceSummaryGroup group : sortedGroups) { + if (!StringUtil.isEmpty(group.getName())) { + // if the group name changed then add its data to the fileDifferenceSummary and reset list for next group + if (groupName != null && groupName.compareTo(group.getName()) != 0) { + addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); + // Note: groupsArrayBuilder.build() also clears the data within it + itemCounts.clear(); + } + groupName = group.getName(); + + group.getFileDifferenceSummaryItems().forEach(item -> { + JsonObjectBuilder itemObjectBuilder = jsonObjectBuilder(); + if (item.getName().isEmpty()) { + // 'groupName': {'Added'=#, 'Changed'=#, ...} + // accumulate the counts since we can't make a separate array item + itemCounts.merge(ACTION_ADDED, item.getAdded(), Integer::sum); + itemCounts.merge(ACTION_CHANGED, item.getChanged(), Integer::sum); + itemCounts.merge(ACTION_DELETED, item.getDeleted(), Integer::sum); + itemCounts.merge(ACTION_REPLACED, item.getReplaced(), Integer::sum); + } else if (GROUP_FILE_ACCESS.equals(group.getName())) { + // 'groupName': 'getNameValue' + groupsObjectBuilder.add(group.getName(), group.getFileDifferenceSummaryItems().get(0).getName()); + } else { + // 'groupName': [{name='', action=''}, {name='', action=''}] + String action = item.getAdded() > 0 ? ACTION_ADDED : item.getChanged() > 0 ? ACTION_CHANGED : + item.getDeleted() > 0 ? ACTION_DELETED : item.getReplaced() > 0 ? ACTION_REPLACED : ""; + itemObjectBuilder.add("name", item.getName()); + if (!action.isEmpty()) { + itemObjectBuilder.add("action", action); + } + groupsArrayBuilder.add(itemObjectBuilder.build()); + } + }); + } + } + // process last group + addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); + } + JsonObject fileDifferenceSummaryObject = fileDifferenceSummary.build(); + if (!fileDifferenceSummaryObject.isEmpty()) { + job.add("fileDifferenceSummary", fileDifferenceSummaryObject); + } + return job; + } + + private static String getFileAction(FileMetadata originalFileMetadata, FileMetadata newFileMetadata) { + if (newFileMetadata.getDataFile() != null && originalFileMetadata == null) { + return ACTION_ADDED; + } else if (newFileMetadata.getDataFile() == null && originalFileMetadata != null) { + return ACTION_DELETED; + } else if (originalFileMetadata != null && + newFileMetadata.getDataFile() != null && originalFileMetadata.getDataFile() != null && !originalFileMetadata.getDataFile().equals(newFileMetadata.getDataFile())) { + return ACTION_REPLACED; + } else { + return null; + } + } + + private static void addJsonGroupObject(JsonObjectBuilder jsonObjectBuilder, String key, JsonObject jsonObjectValue, JsonArray jsonArrayValue, Map itemCounts) { + if (key != null && !key.isEmpty()) { + String sanitizedKey = key.replaceAll("\\s+", ""); + if (itemCounts.isEmpty()) { + if (jsonArrayValue.isEmpty()) { + // add the object + jsonObjectBuilder.add(sanitizedKey, jsonObjectValue.getValue("/" + key)); + } else { + // add the array + jsonObjectBuilder.add(sanitizedKey, jsonArrayValue); + } + } else { + // add the accumulated totals + JsonObjectBuilder accumulatedTotalsObjectBuilder = jsonObjectBuilder(); + itemCounts.forEach((k, v) -> { + if (v != 0) { + accumulatedTotalsObjectBuilder.add(k, v); + } + }); + jsonObjectBuilder.add(sanitizedKey, accumulatedTotalsObjectBuilder.build()); + } + } + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index c451aaf92a3..61ac9978153 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -35,16 +35,16 @@ import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.DatasetFieldWalker; + +import static edu.harvard.iq.dataverse.util.json.FileVersionDifferenceJsonPrinter.jsonFileVersionDifference; import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; import edu.harvard.iq.dataverse.util.MailUtil; -import edu.harvard.iq.dataverse.util.StringUtil; import edu.harvard.iq.dataverse.workflow.Workflow; import edu.harvard.iq.dataverse.workflow.step.WorkflowStepData; import java.util.*; -import jakarta.inject.Inject; import jakarta.json.Json; import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonObjectBuilder; @@ -88,9 +88,6 @@ public class JsonPrinter { @EJB static InAppNotificationsJsonPrinter inAppNotificationsJsonPrinter; - @EJB - static FileMetadataVersionsHelper fileMetadataVersionsHelper; - public static void injectSettingsService(SettingsServiceBean ssb, DatasetFieldServiceBean dfsb, DataverseFieldTypeInputLevelServiceBean dfils, @@ -1739,135 +1736,9 @@ public static JsonArray jsonFileVersionSummaries(List dif JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); differences.stream() .filter(Objects::nonNull) - .map(diff -> jsonDataFileVersions(diff).build()) + .map(diff -> jsonFileVersionDifference(diff).build()) .forEach(arrayBuilder::add); return arrayBuilder.build(); } - - private static JsonObjectBuilder jsonDataFileVersions(FileVersionDifference fileVersionDifference) { - JsonObjectBuilder job = jsonObjectBuilder(); - FileMetadata fileMetadata = fileVersionDifference.getNewFileMetadata(); - if (fileMetadata.getDatasetVersion() != null) { - job.add("datasetVersion", fileMetadata.getDatasetVersion().getFriendlyVersionNumber()); - if (fileMetadata.getDatasetVersion().getVersionNumber() != null) { - job - .add("versionNumber", fileMetadata.getDatasetVersion().getVersionNumber()) - .add("versionMinorNumber", fileMetadata.getDatasetVersion().getMinorVersionNumber()); - } - - job - .add("isDraft", fileMetadata.getDatasetVersion().isDraft()) - .add("isReleased", fileMetadata.getDatasetVersion().isReleased()) - .add("isDeaccessioned", fileMetadata.getDatasetVersion().isDeaccessioned()) - .add("versionState", fileMetadata.getDatasetVersion().getVersionState().name()) - .add("summary", fileMetadata.getDatasetVersion().getVersionNote()) - .add("contributors", fileMetadata.getContributorNames()) - .add("publishedDate", fileMetadata.getDataFile() != null ? (fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) : null) - ; - } - if (fileMetadata.getDataFile() != null) { - job.add("datafileId", fileMetadata.getDataFile().getId()); - job.add("persistentId", (fileMetadata.getDataFile().getGlobalId() != null ? fileMetadata.getDataFile().getGlobalId().asString() : "")); - } - FileVersionDifference fvd = fileVersionDifference; - if (fvd != null) { - List groups = fvd.getDifferenceSummaryGroups(); - JsonObjectBuilder fileDifferenceSummary = jsonObjectBuilder() - .add("versionNote", fileMetadata.getDatasetVersion().getVersionNote()) - .add("deaccessionedReason", fileMetadata.getDatasetVersion().getDeaccessionNote()) - .add("file", getFileAction(fvd.getOriginalFileMetadata(), fvd.getNewFileMetadata())); - - if (groups != null && !groups.isEmpty()) { - List sortedGroups = groups.stream() - .sorted(Comparator.comparing(FileVersionDifference.FileDifferenceSummaryGroup::getName)) - .collect(Collectors.toList()); - String groupName = null; - final JsonArrayBuilder groupsArrayBuilder = Json.createArrayBuilder(); - final JsonObjectBuilder groupsObjectBuilder = jsonObjectBuilder(); - Map itemCounts = new HashMap<>(); - - for (FileVersionDifference.FileDifferenceSummaryGroup group : sortedGroups) { - if (!StringUtil.isEmpty(group.getName())) { - // if the group name changed then add its data to the fileDifferenceSummary and reset list for next group - if (groupName != null && groupName.compareTo(group.getName()) != 0) { - addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); - // Note: groupsArrayBuilder.build() also clears the data within it - itemCounts.clear(); - } - groupName = group.getName(); - - group.getFileDifferenceSummaryItems().forEach(item -> { - JsonObjectBuilder itemObjectBuilder = jsonObjectBuilder(); - if (item.getName().isEmpty()) { - // 'groupName': {'Added'=#, 'Changed'=#, ...} - // accumulate the counts since we can't make a separate array item - itemCounts.merge("Added", item.getAdded(), Integer::sum); - itemCounts.merge("Changed", item.getChanged(), Integer::sum); - itemCounts.merge("Deleted", item.getDeleted(), Integer::sum); - itemCounts.merge("Replaced", item.getReplaced(), Integer::sum); - } else if (List.of("File Access").contains(group.getName())) { - // 'groupName': 'getNameValue' - groupsObjectBuilder.add(group.getName(), group.getFileDifferenceSummaryItems().get(0).getName()); - } else { - // 'groupName': [{name='', action=''}, {name='', action=''}] - String action = item.getAdded() > 0 ? "Added" : item.getChanged() > 0 ? "Changed" : - item.getDeleted() > 0 ? "Deleted" : item.getReplaced() > 0 ? "Replaced" : ""; - itemObjectBuilder.add("name", item.getName()); - if (!action.isEmpty()) { - itemObjectBuilder.add("action", action); - } - groupsArrayBuilder.add(itemObjectBuilder.build()); - } - }); - } - } - // process last group - addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); - } - JsonObject fileDifferenceSummaryObject = fileDifferenceSummary.build(); - if (!fileDifferenceSummaryObject.isEmpty()) { - job.add("fileDifferenceSummary", fileDifferenceSummaryObject); - } - } - return job; - } - - private static String getFileAction(FileMetadata originalFileMetadata, FileMetadata newFileMetadata) { - if (newFileMetadata.getDataFile() != null && originalFileMetadata == null) { - return "Added"; - } else if (newFileMetadata.getDataFile() == null && originalFileMetadata != null) { - return "Deleted"; - } else if (originalFileMetadata != null && - newFileMetadata.getDataFile() != null && originalFileMetadata.getDataFile() != null &&!originalFileMetadata.getDataFile().equals(newFileMetadata.getDataFile())) { - return "Replaced"; - } else { - return null; - } - } - - private static void addJsonGroupObject(JsonObjectBuilder jsonObjectBuilder, String key, JsonObject jsonObjectValue, JsonArray jsonArrayValue, Map itemCounts) { - if (key != null && !key.isEmpty()) { - String sanitizedKey = key.replaceAll("\\s+", ""); - if (itemCounts.isEmpty()) { - if (jsonArrayValue.isEmpty()) { - // add the object - jsonObjectBuilder.add(sanitizedKey, jsonObjectValue.getValue("/"+key)); - } else { - // add the array - jsonObjectBuilder.add(sanitizedKey, jsonArrayValue); - } - } else { - // add the accumulated totals - JsonObjectBuilder accumulatedTotalsObjectBuilder = jsonObjectBuilder(); - itemCounts.forEach((k, v) -> { - if (v != 0) { - accumulatedTotalsObjectBuilder.add(k, v); - } - }); - jsonObjectBuilder.add(sanitizedKey, accumulatedTotalsObjectBuilder.build()); - } - } - } - } From 79094a7815c88e09abc6f0ecd944e6f5a0c158e9 Mon Sep 17 00:00:00 2001 From: GPortas Date: Sun, 12 Oct 2025 19:19:46 +0100 Subject: [PATCH 18/45] Added: simple javadoc with TODO to FileVersionDifferenceJsonPrinter --- .../dataverse/util/json/FileVersionDifferenceJsonPrinter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java index 3fe3a4540be..e0e561fc0f8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java @@ -9,6 +9,10 @@ import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; +/** + * Utility class responsible for producing JSON representations of {@link edu.harvard.iq.dataverse.FileVersionDifference} objects. + * TODO: The logic in this class was originally extracted from {@code FileMetadataVersionHelper}. Further refactoring and testing is required. + */ public class FileVersionDifferenceJsonPrinter { private static final String ACTION_ADDED = "Added"; From 0f71d6babdc94542eb2b1017b1e95d84084fce98 Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 12:48:44 +0100 Subject: [PATCH 19/45] Added: pagination params to getFileVersionsList API endpoint --- .../edu/harvard/iq/dataverse/api/Files.java | 7 ++- .../GetFileVersionDifferencesCommand.java | 24 +++++++- src/main/java/propertyFiles/Bundle.properties | 3 + .../edu/harvard/iq/dataverse/api/FilesIT.java | 55 ++++++++++++++++++- .../edu/harvard/iq/dataverse/api/UtilIT.java | 18 +++++- 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 42e23656808..0b5e5b02b85 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -1172,7 +1172,10 @@ public Response getFileCitationByVersion(@Context ContainerRequestContext crc, @ @AuthRequired @Path("{id}/versionDifferences") @Produces(MediaType.APPLICATION_JSON) - public Response getFileVersionsList(@Context ContainerRequestContext crc, @PathParam("id") String fileIdOrPersistentId) { + public Response getFileVersionsList(@Context ContainerRequestContext crc, + @PathParam("id") String fileIdOrPersistentId, + @QueryParam("limit") Integer limit, + @QueryParam("offset") Integer offset) { try { DataverseRequest req = createDataverseRequest(getRequestUser(crc)); final DataFile df = execCommand(new GetDataFileCommand(req, findDataFileOrDie(fileIdOrPersistentId))); @@ -1180,7 +1183,7 @@ public Response getFileVersionsList(@Context ContainerRequestContext crc, @PathP if (fm == null) { return notFound(BundleUtil.getStringFromBundle("files.api.fileNotFound")); } - return ok(jsonFileVersionSummaries(execCommand(new GetFileVersionDifferencesCommand(req, fm, null, null)))); + return ok(jsonFileVersionSummaries(execCommand(new GetFileVersionDifferencesCommand(req, fm, limit, offset)))); } catch (WrappedResponse ex) { return ex.getResponse(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index 674f6c6c42c..f53af2cee1d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -6,7 +6,8 @@ import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; -import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; +import edu.harvard.iq.dataverse.util.BundleUtil; import java.util.EnumSet; import java.util.List; @@ -32,7 +33,10 @@ public GetFileVersionDifferencesCommand(DataverseRequest request, FileMetadata f } @Override - public List execute(CommandContext ctxt) throws CommandException { + public List execute(CommandContext ctxt) throws InvalidCommandArgumentsException { + validatePaginationParameter(limit, "limit"); + validatePaginationParameter(offset, "offset"); + List fileHistory = findFileHistory(ctxt); return fileHistory.stream() @@ -40,6 +44,22 @@ public List execute(CommandContext ctxt) throws CommandEx .collect(Collectors.toList()); } + /** + * Validates that a given pagination parameter is not negative. + * + * @param value The parameter's value. + * @param name The parameter's name, used for the error message. + * @throws InvalidCommandArgumentsException if the value is negative. + */ + private void validatePaginationParameter(Integer value, String name) throws InvalidCommandArgumentsException { + if (value != null && value < 0) { + throw new InvalidCommandArgumentsException( + BundleUtil.getStringFromBundle("getFileVersionDifferencesCommand.errors.negativePaginationParam", List.of(name)), + this + ); + } + } + /** * Fetches the file's version history, respecting user permissions. */ diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index a07546284e0..928348275ad 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -3236,3 +3236,6 @@ roleAssigneeServiceBean.error.dataverseRequestCannotBeNull=DataverseRequest cann #GetUserPermittedCollectionsCommand.java getUserPermittedCollectionsCommand.errors.userNotFound=User not found. getUserPermittedCollectionsCommand.errors.permissionNotValid=Permission not valid. + +#GetFileVersionDifferencesCommand.java +getFileVersionDifferencesCommand.errors.negativePaginationParam=The {0} parameter cannot be negative. diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 38d89f782dd..ec24fb1124c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1448,7 +1448,7 @@ public void testDataSizeInDataverse() throws InterruptedException { } @Test - public void GetFileVersionDifferences() { + public void getFileVersionDifferences() { // Create superuser and regular user Response createUser = UtilIT.createRandomUser(); String superUserUsername = UtilIT.getUsernameFromResponse(createUser); @@ -1525,6 +1525,58 @@ public void GetFileVersionDifferences() { .body("data[1].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); + // Test pagination: limit=1, offset=0 (should get the first item: DRAFT) + Response paginatedResponse = UtilIT.getFileVersionDifferences(dataFileId, regularApiToken, 1, 0); + paginatedResponse.prettyPrint(); + paginatedResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("status", equalTo("OK")) + .body("data.size()", is(1)) + .body("data[0].datasetVersion", equalTo("DRAFT")); + + // Test pagination: limit=1, offset=1 (should skip 1 and get the second item: 1.0) + paginatedResponse = UtilIT.getFileVersionDifferences(dataFileId, regularApiToken, 1, 1); + paginatedResponse.prettyPrint(); + paginatedResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("status", equalTo("OK")) + .body("data.size()", is(1)) + .body("data[0].datasetVersion", equalTo("1.0")); + + // Test pagination: limit=1, offset=2 (out of bounds, should get an empty list) + paginatedResponse = UtilIT.getFileVersionDifferences(dataFileId, regularApiToken, 1, 2); + paginatedResponse.prettyPrint(); + paginatedResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("status", equalTo("OK")) + .body("data.size()", is(0)); + + // Test pagination: limit=2, offset=0 (should get both items) + paginatedResponse = UtilIT.getFileVersionDifferences(dataFileId, regularApiToken, 2, 0); + paginatedResponse.prettyPrint(); + paginatedResponse.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("status", equalTo("OK")) + .body("data.size()", is(2)) + .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[1].datasetVersion", equalTo("1.0")); + + // Test invalid pagination: limit=-1 (should return a bad request error) + paginatedResponse = UtilIT.getFileVersionDifferences(dataFileId, regularApiToken, -1, 0); + paginatedResponse.prettyPrint(); + paginatedResponse.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("status", equalTo("ERROR")) + .body("message", containsString("The limit parameter cannot be negative.")); + + // Test invalid pagination: offset=-1 (should return a bad request error) + paginatedResponse = UtilIT.getFileVersionDifferences(dataFileId, regularApiToken, 1, -1); + paginatedResponse.prettyPrint(); + paginatedResponse.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("status", equalTo("ERROR")) + .body("message", containsString("The offset parameter cannot be negative.")); + // test replace file pathToFile = "src/test/resources/images/coffeeshop.png"; Response replaceFileResponse = UtilIT.replaceFile(dataFileId, pathToFile, superUserApiToken); @@ -1567,6 +1619,7 @@ public void GetFileVersionDifferences() { .body("data[1].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); } + @Test public void testGetFileInfo() { Response createUser = UtilIT.createRandomUser(); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index f9d11a46917..7ebebe0148c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1339,9 +1339,21 @@ static Response getFileData(String fileId, String apiToken, String datasetVersio } static Response getFileVersionDifferences(String fileId, String apiToken) { - return given() - .header(API_TOKEN_HTTP_HEADER, apiToken) - .get("/api/files/" + fileId + "/versionDifferences"); + return getFileVersionDifferences(fileId, apiToken, null, null); + } + + static Response getFileVersionDifferences(String fileId, String apiToken, Integer limit, Integer offset) { + RequestSpecification request = given() + .header(API_TOKEN_HTTP_HEADER, apiToken); + + if (limit != null) { + request.queryParam("limit", limit); + } + if (offset != null) { + request.queryParam("offset", offset); + } + + return request.get("/api/files/" + fileId + "/versionDifferences"); } static Response testIngest(String fileName, String fileType) { From 6c1da739e572dfb15a157f8fed1dea3e4e2aba7c Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 12:54:45 +0100 Subject: [PATCH 20/45] Added: test cases producing InvalidCommandArgumentsException to GetFileVersionDifferencesCommandTest --- .../GetFileVersionDifferencesCommandTest.java | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java index b931fa0d8ae..861fa55a5c0 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -5,6 +5,7 @@ import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -18,6 +19,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; @@ -47,19 +49,25 @@ class GetFileVersionDifferencesCommandTest { @BeforeEach void setUp() { + when(fileMetadataMock.getDataFile()).thenReturn(dataFileMock); + } + + /** + * Helper method to set up mocks required for tests that execute the full command logic, + */ + private void setupMocksForSuccessfulExecution() { when(contextMock.permissions()).thenReturn(permissionsMock); when(contextMock.files()).thenReturn(fileServiceMock); - when(fileMetadataMock.getDatasetVersion()).thenReturn(datasetVersionMock); when(datasetVersionMock.getDataset()).thenReturn(datasetMock); when(datasetMock.getId()).thenReturn(DATASET_ID); - when(fileMetadataMock.getDataFile()).thenReturn(dataFileMock); } @Test @DisplayName("execute should call findFileMetadataHistory with 'canViewUnpublished' as TRUE when user has permission") void execute_should_call_history_with_true_when_user_has_permission() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) .thenReturn(true); when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) @@ -86,6 +94,7 @@ void execute_should_call_history_with_true_when_user_has_permission() throws Com @DisplayName("execute should call findFileMetadataHistory with 'canViewUnpublished' as FALSE when user lacks permission") void execute_should_call_history_with_false_when_user_lacks_permission() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) .thenReturn(false); when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) @@ -112,6 +121,7 @@ void execute_should_call_history_with_false_when_user_lacks_permission() throws @DisplayName("execute should pass pagination parameters correctly to findFileMetadataHistory") void execute_should_pass_pagination_parameters_correctly() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); Integer expectedLimit = 10; Integer expectedOffset = 20; when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) @@ -136,6 +146,7 @@ void execute_should_pass_pagination_parameters_correctly() throws CommandExcepti @DisplayName("execute should correctly convert file history to FileVersionDifference list") void execute_should_convert_history_to_differences() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); // Create mock history: 3 versions of a file's metadata FileMetadata fm3 = new FileMetadata(); // Newest fm3.setId(3L); @@ -180,4 +191,26 @@ void execute_should_convert_history_to_differences() throws CommandException { assertThat(differences.get(2).getNewFileMetadata()).isEqualTo(fm1); assertThat(differences.get(2).getOriginalFileMetadata()).isNull(); } + + @Test + @DisplayName("execute should throw InvalidCommandArgumentsException for negative limit") + void execute_should_throw_exception_for_negative_limit() { + // Arrange + Integer invalidLimit = -1; + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, invalidLimit, 0); + + // Act & Assert + assertThrows(InvalidCommandArgumentsException.class, () -> command.execute(contextMock)); + } + + @Test + @DisplayName("execute should throw InvalidCommandArgumentsException for negative offset") + void execute_should_throw_exception_for_negative_offset() { + // Arrange + Integer invalidOffset = -1; + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, 10, invalidOffset); + + // Act & Assert + assertThrows(InvalidCommandArgumentsException.class, () -> command.execute(contextMock)); + } } From a3fe9c509a026dbeb3845dc1329272b8837b34dc Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 12:58:01 +0100 Subject: [PATCH 21/45] Added: pagination params validation to GetDatasetVersionSummariesCommand --- .../GetDatasetVersionSummariesCommand.java | 21 ++++++++++ ...GetDatasetVersionSummariesCommandTest.java | 40 ++++++++++++++----- .../GetFileVersionDifferencesCommandTest.java | 2 +- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index d820dd7de6a..09220d9c80a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -9,6 +9,8 @@ import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; +import edu.harvard.iq.dataverse.util.BundleUtil; import java.util.EnumSet; import java.util.List; @@ -33,6 +35,9 @@ public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset datas @Override public List execute(CommandContext ctxt) throws CommandException { + validatePaginationParameter(limit, "limit"); + validatePaginationParameter(offset, "offset"); + boolean canViewUnpublished = ctxt.permissions().hasPermissionsFor( getRequest().getUser(), dataset, @@ -52,4 +57,20 @@ public List execute(CommandContext ctxt) throws CommandEx .flatMap(Optional::stream) .toList(); } + + /** + * Validates that a given pagination parameter is not negative. + * + * @param value The parameter's value. + * @param name The parameter's name, used for the error message. + * @throws InvalidCommandArgumentsException if the value is negative. + */ + private void validatePaginationParameter(Integer value, String name) throws InvalidCommandArgumentsException { + if (value != null && value < 0) { + throw new InvalidCommandArgumentsException( + BundleUtil.getStringFromBundle("getFileVersionDifferencesCommand.errors.negativePaginationParam", List.of(name)), + this + ); + } + } } diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java index 0161bdbfbf2..e47deb86c8f 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java @@ -10,7 +10,7 @@ import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; -import org.junit.jupiter.api.BeforeEach; +import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -26,6 +26,7 @@ import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; @@ -53,16 +54,13 @@ class GetDatasetVersionSummariesCommandTest { private static final Long DATASET_ID = 42L; - @BeforeEach - void setUp() { - // Mock the context to return our mock services + /** + * Helper method to set up mocks required for tests that execute the full command logic. + */ + private void setupMocksForSuccessfulExecution() { when(contextMock.permissions()).thenReturn(permissionsMock); when(contextMock.datasetVersion()).thenReturn(versionServiceMock); - - // Mock the request to return a user when(requestMock.getUser()).thenReturn(userMock); - - // Mock the dataset to have a specific ID when(datasetMock.getId()).thenReturn(DATASET_ID); } @@ -70,6 +68,7 @@ void setUp() { @DisplayName("execute should call findVersions with 'canViewUnpublished' as TRUE when user has permission") void execute_should_call_findVersions_with_true_when_user_has_permission() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) .thenReturn(true); when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) @@ -96,6 +95,7 @@ void execute_should_call_findVersions_with_true_when_user_has_permission() throw @DisplayName("execute should call findVersions with 'canViewUnpublished' as FALSE when user lacks permission") void execute_should_call_findVersions_with_false_when_user_lacks_permission() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); when(permissionsMock.hasPermissionsFor(requestMock.getUser(), datasetMock, EnumSet.of(Permission.ViewUnpublishedDataset))) .thenReturn(false); when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) @@ -122,6 +122,7 @@ void execute_should_call_findVersions_with_false_when_user_lacks_permission() th @DisplayName("execute should pass pagination parameters correctly to findVersions") void execute_should_pass_pagination_parameters_correctly() throws CommandException { // Arrange + setupMocksForSuccessfulExecution(); Integer expectedLimit = 10; Integer expectedOffset = 20; when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) @@ -145,7 +146,8 @@ void execute_should_pass_pagination_parameters_correctly() throws CommandExcepti @Test @DisplayName("execute should correctly convert DatasetVersion list to DatasetVersionSummary list") void execute_should_convert_versions_to_summaries() throws CommandException { - // Arrange: Mock the service to return a list with one version + // Arrange + setupMocksForSuccessfulExecution(); when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) .thenReturn(List.of(versionMock)); @@ -169,4 +171,24 @@ void execute_should_convert_versions_to_summaries() throws CommandException { .containsExactly(dummySummary); } } + + @Test + @DisplayName("execute should throw InvalidCommandArgumentsException for negative limit") + void execute_should_throw_exception_for_negative_limit() { + // Arrange + GetDatasetVersionSummariesCommand command = new GetDatasetVersionSummariesCommand(requestMock, datasetMock, -1, 0); + + // Act & Assert + assertThrows(InvalidCommandArgumentsException.class, () -> command.execute(contextMock)); + } + + @Test + @DisplayName("execute should throw InvalidCommandArgumentsException for negative offset") + void execute_should_throw_exception_for_negative_offset() { + // Arrange + GetDatasetVersionSummariesCommand command = new GetDatasetVersionSummariesCommand(requestMock, datasetMock, 10, -1); + + // Act & Assert + assertThrows(InvalidCommandArgumentsException.class, () -> command.execute(contextMock)); + } } diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java index 861fa55a5c0..9b672fd2ca5 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -53,7 +53,7 @@ void setUp() { } /** - * Helper method to set up mocks required for tests that execute the full command logic, + * Helper method to set up mocks required for tests that execute the full command logic. */ private void setupMocksForSuccessfulExecution() { when(contextMock.permissions()).thenReturn(permissionsMock); From 6cecbf4342cfb32a9228a837814899a26e385d74 Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 13:03:06 +0100 Subject: [PATCH 22/45] Refactor: AbstractPaginatedCommand --- .../impl/AbstractPaginatedCommand.java | 64 +++++++++++++++++++ .../GetDatasetVersionSummariesCommand.java | 32 +--------- .../GetFileVersionDifferencesCommand.java | 32 +--------- 3 files changed, 70 insertions(+), 58 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java new file mode 100644 index 00000000000..ef32c5b613e --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java @@ -0,0 +1,64 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.DvObject; +import edu.harvard.iq.dataverse.engine.command.AbstractCommand; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; +import edu.harvard.iq.dataverse.util.BundleUtil; + +import java.util.List; + +/** + * An abstract command for operations that support pagination. + * It handles the common logic for 'limit' and 'offset' parameters, + * including validation. + * + * @param The return type of the command's result. + */ +public abstract class AbstractPaginatedCommand extends AbstractCommand { + + protected final Integer limit; + protected final Integer offset; + + public AbstractPaginatedCommand(DataverseRequest request, DvObject dvObject, Integer limit, Integer offset) { + super(request, dvObject); + this.limit = limit; + this.offset = offset; + } + + @Override + public T execute(CommandContext ctxt) throws CommandException { + validatePaginationParameter(limit, "limit"); + validatePaginationParameter(offset, "offset"); + return executeCommand(ctxt); + } + + /** + * The core logic of the command, executed after pagination parameters have been validated. + * Subclasses must implement this method. + * + * @param ctxt The command context. + * @return The result of the command. + * @throws CommandException if an error occurs during command execution. + */ + public abstract T executeCommand(CommandContext ctxt) throws CommandException; + + /** + * Validates that a given pagination parameter is not negative. + * + * @param value The parameter's value. + * @param name The parameter's name, used for the error message. + * @throws InvalidCommandArgumentsException if the value is negative. + */ + private void validatePaginationParameter(Integer value, String name) throws InvalidCommandArgumentsException { + if (value != null && value < 0) { + // Both original commands used this specific bundle key. A more generic one could be introduced later. + throw new InvalidCommandArgumentsException( + BundleUtil.getStringFromBundle("getFileVersionDifferencesCommand.errors.negativePaginationParam", List.of(name)), + this + ); + } + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index 09220d9c80a..802c4189e45 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -4,13 +4,10 @@ import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; -import edu.harvard.iq.dataverse.engine.command.AbstractCommand; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; -import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; -import edu.harvard.iq.dataverse.util.BundleUtil; import java.util.EnumSet; import java.util.List; @@ -20,24 +17,17 @@ * A command that retrieves a paginated list of {@link DatasetVersionSummary} for the versions of a given {@link Dataset}. **/ @RequiredPermissions({}) -public class GetDatasetVersionSummariesCommand extends AbstractCommand> { +public class GetDatasetVersionSummariesCommand extends AbstractPaginatedCommand> { private final Dataset dataset; - private final Integer limit; - private final Integer offset; public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset dataset, Integer limit, Integer offset) { - super(request, dataset); + super(request, dataset, limit, offset); this.dataset = dataset; - this.limit = limit; - this.offset = offset; } @Override - public List execute(CommandContext ctxt) throws CommandException { - validatePaginationParameter(limit, "limit"); - validatePaginationParameter(offset, "offset"); - + public List executeCommand(CommandContext ctxt) throws CommandException { boolean canViewUnpublished = ctxt.permissions().hasPermissionsFor( getRequest().getUser(), dataset, @@ -57,20 +47,4 @@ public List execute(CommandContext ctxt) throws CommandEx .flatMap(Optional::stream) .toList(); } - - /** - * Validates that a given pagination parameter is not negative. - * - * @param value The parameter's value. - * @param name The parameter's name, used for the error message. - * @throws InvalidCommandArgumentsException if the value is negative. - */ - private void validatePaginationParameter(Integer value, String name) throws InvalidCommandArgumentsException { - if (value != null && value < 0) { - throw new InvalidCommandArgumentsException( - BundleUtil.getStringFromBundle("getFileVersionDifferencesCommand.errors.negativePaginationParam", List.of(name)), - this - ); - } - } } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index f53af2cee1d..62bfb24b801 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -2,12 +2,9 @@ import edu.harvard.iq.dataverse.*; import edu.harvard.iq.dataverse.authorization.Permission; -import edu.harvard.iq.dataverse.engine.command.AbstractCommand; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; -import edu.harvard.iq.dataverse.engine.command.exception.InvalidCommandArgumentsException; -import edu.harvard.iq.dataverse.util.BundleUtil; import java.util.EnumSet; import java.util.List; @@ -19,24 +16,17 @@ * highlighting the differences between each version. */ @RequiredPermissions({}) -public class GetFileVersionDifferencesCommand extends AbstractCommand> { +public class GetFileVersionDifferencesCommand extends AbstractPaginatedCommand> { private final FileMetadata fileMetadata; - private final Integer limit; - private final Integer offset; public GetFileVersionDifferencesCommand(DataverseRequest request, FileMetadata fileMetadata, Integer limit, Integer offset) { - super(request, fileMetadata.getDataFile()); + super(request, fileMetadata.getDataFile(), limit, offset); this.fileMetadata = Objects.requireNonNull(fileMetadata); - this.limit = limit; - this.offset = offset; } @Override - public List execute(CommandContext ctxt) throws InvalidCommandArgumentsException { - validatePaginationParameter(limit, "limit"); - validatePaginationParameter(offset, "offset"); - + public List executeCommand(CommandContext ctxt) { List fileHistory = findFileHistory(ctxt); return fileHistory.stream() @@ -44,22 +34,6 @@ public List execute(CommandContext ctxt) throws InvalidCo .collect(Collectors.toList()); } - /** - * Validates that a given pagination parameter is not negative. - * - * @param value The parameter's value. - * @param name The parameter's name, used for the error message. - * @throws InvalidCommandArgumentsException if the value is negative. - */ - private void validatePaginationParameter(Integer value, String name) throws InvalidCommandArgumentsException { - if (value != null && value < 0) { - throw new InvalidCommandArgumentsException( - BundleUtil.getStringFromBundle("getFileVersionDifferencesCommand.errors.negativePaginationParam", List.of(name)), - this - ); - } - } - /** * Fetches the file's version history, respecting user permissions. */ From 98a03c3b3c6bcd77ec283da523f0e51ab2d4877e Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 13:16:32 +0100 Subject: [PATCH 23/45] Added: invalid pagination params test cases to testSummaryDatasetVersionsDifferencesAPI IT --- .../impl/AbstractPaginatedCommand.java | 3 +-- src/main/java/propertyFiles/Bundle.properties | 4 ++-- .../harvard/iq/dataverse/api/DatasetsIT.java | 20 ++++++++++++++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java index ef32c5b613e..0a9f40d7ec2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractPaginatedCommand.java @@ -54,9 +54,8 @@ public T execute(CommandContext ctxt) throws CommandException { */ private void validatePaginationParameter(Integer value, String name) throws InvalidCommandArgumentsException { if (value != null && value < 0) { - // Both original commands used this specific bundle key. A more generic one could be introduced later. throw new InvalidCommandArgumentsException( - BundleUtil.getStringFromBundle("getFileVersionDifferencesCommand.errors.negativePaginationParam", List.of(name)), + BundleUtil.getStringFromBundle("abstractPaginatedCommand.errors.negativePaginationParam", List.of(name)), this ); } diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 928348275ad..1016bb96788 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -3237,5 +3237,5 @@ roleAssigneeServiceBean.error.dataverseRequestCannotBeNull=DataverseRequest cann getUserPermittedCollectionsCommand.errors.userNotFound=User not found. getUserPermittedCollectionsCommand.errors.permissionNotValid=Permission not valid. -#GetFileVersionDifferencesCommand.java -getFileVersionDifferencesCommand.errors.negativePaginationParam=The {0} parameter cannot be negative. +#AbstractPaginatedCommand.java +abstractPaginatedCommand.errors.negativePaginationParam=The {0} parameter cannot be negative. diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index fcd604db8d8..31f4669cd42 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -6586,7 +6586,7 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti addDataToPublishedVersion = UtilIT.addDatasetMetadataViaNative(datasetPersistentId, pathToJsonFilePostPub, apiToken); addDataToPublishedVersion.then().assertThat().statusCode(OK.getStatusCode()); - // TEST 1: No pagination. Should return all 3 version differences (DRAFT, 2.0, 1.0). + // Test pagination: No pagination. Should return all 3 version differences (DRAFT, 2.0, 1.0). Response compareAllResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, apiToken); compareAllResponse.then().assertThat() .statusCode(OK.getStatusCode()) @@ -6595,19 +6595,33 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti .body("data[1].versionNumber", equalTo("2.0")) .body("data[2].versionNumber", equalTo("1.0")); - // TEST 2: Use limit=1. Should return only the latest difference (DRAFT). + // Test pagination: Use limit=1. Should return only the latest difference (DRAFT). Response compareLimitResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, 1, null, apiToken); compareLimitResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.size()", is(1)) .body("data[0].versionNumber", equalTo("DRAFT")); - // TEST 3: Use limit=1 and offset=1. Should skip the first result and return the second (2.0). + // Test pagination: Use limit=1 and offset=1. Should skip the first result and return the second (2.0). Response compareOffsetResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, 1, 1, apiToken); compareOffsetResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.size()", is(1)) .body("data[0].versionNumber", equalTo("2.0")); + + // Test invalid pagination: limit=-1 (should return a bad request error) + compareAllResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, -1, 1, apiToken); + compareAllResponse.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("status", equalTo("ERROR")) + .body("message", containsString("The limit parameter cannot be negative.")); + + // Test invalid pagination: offset=-1 (should return a bad request error) + compareAllResponse = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, 1, -1, apiToken); + compareAllResponse.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("status", equalTo("ERROR")) + .body("message", containsString("The offset parameter cannot be negative.")); } @Test From 354627912c2277872593375cf5fd8026ca1f4b2c Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 13:31:09 +0100 Subject: [PATCH 24/45] Added: docs for pagination in versionDifferences Files API endpoint --- doc/sphinx-guides/source/api/native-api.rst | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 6fe0147a683..91b0fbd9b44 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4288,8 +4288,19 @@ The fully expanded example above (without environment variables) looks like this .. code-block:: bash - curl -X GET "https://demo.dataverse.org/api/files/1234/versionDifferences" - curl -X GET "https://demo.dataverse.org/api/files/:persistentId/versionDifferences?persistentId=doi:10.5072/FK2/J8SJZB" + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X GET "https://demo.dataverse.org/api/files/1234/versionDifferences" + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X GET "https://demo.dataverse.org/api/files/:persistentId/versionDifferences?persistentId=doi:10.5072/FK2/J8SJZB" + +You can control pagination of the results using the following optional query parameters. + +* ``limit``: The maximum number of version differences to return. +* ``offset``: The number of version differences to skip from the beginning of the list. Used for retrieving subsequent pages of results. + +For example, to get the second page of results, with 2 items per page, you would use ``limit=2`` and ``offset=2`` (skipping the first two results). + +.. code-block:: bash + + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X GET "https://demo.dataverse.org/api/files/1234/versionDifferences?limit=2&offset=2" Adding Files ~~~~~~~~~~~~ From ddfefd4c3249ee5be26aaebc0d8cdfdff63cc7e7 Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 13:31:59 +0100 Subject: [PATCH 25/45] Fixed: typo in docs for versions/compareSummary --- doc/sphinx-guides/source/api/native-api.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 91b0fbd9b44..5d005d983d7 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2099,13 +2099,13 @@ be available to users who have permission to view unpublished drafts. The api to export SERVER_URL=https://demo.dataverse.org export PERSISTENT_IDENTIFIER=doi:10.5072/FK2/BCCP9Z - curl -H "X-Dataverse-key: $API_TOKEN" -X PUT "$SERVER_URL/api/datasets/:persistentId/versions/compareSummary?persistentId=$PERSISTENT_IDENTIFIER" + curl -H "X-Dataverse-key: $API_TOKEN" -X GET "$SERVER_URL/api/datasets/:persistentId/versions/compareSummary?persistentId=$PERSISTENT_IDENTIFIER" The fully expanded example above (without environment variables) looks like this: .. code-block:: bash - curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/versions/compareSummary?persistentId=doi:10.5072/FK2/BCCP9Z" + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X GET "https://demo.dataverse.org/api/datasets/:persistentId/versions/compareSummary?persistentId=doi:10.5072/FK2/BCCP9Z" You can control pagination of the results using the following optional query parameters. @@ -2116,7 +2116,7 @@ For example, to get the second page of results, with 2 items per page, you would .. code-block:: bash - curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/versions/compareSummary?persistentId=doi:10.5072/FK2/BCCP9Z&limit=2&offset=2" + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X GET "https://demo.dataverse.org/api/datasets/:persistentId/versions/compareSummary?persistentId=doi:10.5072/FK2/BCCP9Z&limit=2&offset=2" Update Metadata For a Dataset ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 80eae81a498b4f426633dc54cd8ba092664be2b1 Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 13:43:33 +0100 Subject: [PATCH 26/45] Added: release notes for #11855 --- ...ion-summaries-pagination-and-perfomance.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 doc/release-notes/11855-version-summaries-pagination-and-perfomance.md diff --git a/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md b/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md new file mode 100644 index 00000000000..b71ad4826a8 --- /dev/null +++ b/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md @@ -0,0 +1,24 @@ +### Pagination for API Version Summaries + +We've added pagination support to the following API endpoints: + +- File Version Differences: api/files/{id}/versionDifferences + +- Dataset Version Summaries: api/datasets/:persistentId/versions/compareSummary + +You can now use two new query parameters to control the results: + +- **limit**: An integer specifying the maximum number of results to return per page. + +- **offset**: An integer specifying the number of results to skip before starting to return items. This is used to navigate to different pages. + +### Performance enhancements for API Version Summaries + +In addition to adding pagination, we've significantly improved the performance of these endpoints by implementing more efficient database queries. + +These changes address performance bottlenecks that were previously encountered, especially with datasets or files containing a large number of versions. + +### Related issues and PRs + +- https://github.com/IQSS/dataverse/issues/11855 +- https://github.com/IQSS/dataverse/pull/11859 From 5217e68fbfb2478478140503e9f9d3521538017b Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 16:44:28 +0100 Subject: [PATCH 27/45] Refactor: FileVersionDifferenceJsonPrinter with unit tests --- .../FileVersionDifferenceJsonPrinter.java | 328 ++++++++++++------ .../FileVersionDifferenceJsonPrinterTest.java | 229 ++++++++++++ 2 files changed, 456 insertions(+), 101 deletions(-) create mode 100644 src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java index e0e561fc0f8..712b31325f7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java @@ -1,17 +1,22 @@ package edu.harvard.iq.dataverse.util.json; +import edu.harvard.iq.dataverse.DataFile; +import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.FileMetadata; import edu.harvard.iq.dataverse.FileVersionDifference; import edu.harvard.iq.dataverse.util.StringUtil; import jakarta.json.*; -import java.util.*; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; /** * Utility class responsible for producing JSON representations of {@link edu.harvard.iq.dataverse.FileVersionDifference} objects. - * TODO: The logic in this class was originally extracted from {@code FileMetadataVersionHelper}. Further refactoring and testing is required. */ public class FileVersionDifferenceJsonPrinter { @@ -21,125 +26,246 @@ public class FileVersionDifferenceJsonPrinter { private static final String ACTION_REPLACED = "Replaced"; private static final String GROUP_FILE_ACCESS = "File Access"; + /** + * Creates a JSON representation of a FileVersionDifference. + * + * @param fileVersionDifference The object to serialize. + * @return A JsonObjectBuilder containing the serialized data. + */ public static JsonObjectBuilder jsonFileVersionDifference(FileVersionDifference fileVersionDifference) { - JsonObjectBuilder job = jsonObjectBuilder(); - FileMetadata fileMetadata = fileVersionDifference.getNewFileMetadata(); - if (fileMetadata.getDatasetVersion() != null) { - job.add("datasetVersion", fileMetadata.getDatasetVersion().getFriendlyVersionNumber()); - if (fileMetadata.getDatasetVersion().getVersionNumber() != null) { - job - .add("versionNumber", fileMetadata.getDatasetVersion().getVersionNumber()) - .add("versionMinorNumber", fileMetadata.getDatasetVersion().getMinorVersionNumber()); - } + JsonObjectBuilder jsonBuilder = jsonObjectBuilder(); + FileMetadata newFileMetadata = fileVersionDifference.getNewFileMetadata(); + + addDatasetVersionDetails(jsonBuilder, newFileMetadata); + addDataFileDetails(jsonBuilder, newFileMetadata); + addFileDifferenceSummary(jsonBuilder, fileVersionDifference); + + return jsonBuilder; + } - job - .add("isDraft", fileMetadata.getDatasetVersion().isDraft()) - .add("isReleased", fileMetadata.getDatasetVersion().isReleased()) - .add("isDeaccessioned", fileMetadata.getDatasetVersion().isDeaccessioned()) - .add("versionState", fileMetadata.getDatasetVersion().getVersionState().name()) - .add("summary", fileMetadata.getDatasetVersion().getVersionNote()) - .add("contributors", fileMetadata.getContributorNames()) - .add("publishedDate", fileMetadata.getDataFile() != null ? (fileMetadata.getDataFile().getPublicationDate() != null ? fileMetadata.getDataFile().getPublicationDate().toString() : null) : null) - ; + /** + * Adds details from the DatasetVersion to the JSON object. + */ + private static void addDatasetVersionDetails(JsonObjectBuilder jsonBuilder, FileMetadata fileMetadata) { + DatasetVersion datasetVersion = fileMetadata.getDatasetVersion(); + if (datasetVersion == null) { + return; } - if (fileMetadata.getDataFile() != null) { - job.add("datafileId", fileMetadata.getDataFile().getId()); - job.add("persistentId", (fileMetadata.getDataFile().getGlobalId() != null ? fileMetadata.getDataFile().getGlobalId().asString() : "")); + jsonBuilder.add("datasetVersion", datasetVersion.getFriendlyVersionNumber()); + + Long versionNumber = datasetVersion.getVersionNumber(); + if (versionNumber != null) { + jsonBuilder + .add("versionNumber", versionNumber) + .add("versionMinorNumber", datasetVersion.getMinorVersionNumber()); + } + + DatasetVersion.VersionState versionState = datasetVersion.getVersionState(); + if (versionState != null) { + jsonBuilder.add("versionState", datasetVersion.getVersionState().name()); } + + jsonBuilder + .add("isDraft", datasetVersion.isDraft()) + .add("isReleased", datasetVersion.isReleased()) + .add("isDeaccessioned", datasetVersion.isDeaccessioned()) + .add("summary", datasetVersion.getVersionNote()) + .add("contributors", fileMetadata.getContributorNames()); + + if (fileMetadata.getDataFile() != null && fileMetadata.getDataFile().getPublicationDate() != null) { + jsonBuilder.add("publishedDate", fileMetadata.getDataFile().getPublicationDate().toString()); + } + } + + /** + * Adds details from the DataFile to the JSON object. + */ + private static void addDataFileDetails(JsonObjectBuilder jsonBuilder, FileMetadata fileMetadata) { + DataFile dataFile = fileMetadata.getDataFile(); + if (dataFile == null) { + return; + } + jsonBuilder.add("datafileId", dataFile.getId()); + jsonBuilder.add("persistentId", dataFile.getGlobalId() != null ? dataFile.getGlobalId().asString() : ""); + } + + /** + * Adds the file difference summary, which contains the core change details. + */ + private static void addFileDifferenceSummary(JsonObjectBuilder jsonBuilder, FileVersionDifference fileVersionDifference) { + FileMetadata newFileMetadata = fileVersionDifference.getNewFileMetadata(); List groups = fileVersionDifference.getDifferenceSummaryGroups(); - JsonObjectBuilder fileDifferenceSummary = jsonObjectBuilder() - .add("versionNote", fileMetadata.getDatasetVersion().getVersionNote()) - .add("deaccessionedReason", fileMetadata.getDatasetVersion().getDeaccessionNote()) - .add("file", getFileAction(fileVersionDifference.getOriginalFileMetadata(), fileVersionDifference.getNewFileMetadata())); + + JsonObjectBuilder summaryBuilder = jsonObjectBuilder() + .add("versionNote", newFileMetadata.getDatasetVersion().getVersionNote()) + .add("deaccessionedReason", newFileMetadata.getDatasetVersion().getDeaccessionNote()) + .add("file", getFileAction(fileVersionDifference.getOriginalFileMetadata(), newFileMetadata)); if (groups != null && !groups.isEmpty()) { - List sortedGroups = groups.stream() - .sorted(Comparator.comparing(FileVersionDifference.FileDifferenceSummaryGroup::getName)) - .toList(); - String groupName = null; - final JsonArrayBuilder groupsArrayBuilder = Json.createArrayBuilder(); - final JsonObjectBuilder groupsObjectBuilder = jsonObjectBuilder(); - Map itemCounts = new HashMap<>(); - - for (FileVersionDifference.FileDifferenceSummaryGroup group : sortedGroups) { - if (!StringUtil.isEmpty(group.getName())) { - // if the group name changed then add its data to the fileDifferenceSummary and reset list for next group - if (groupName != null && groupName.compareTo(group.getName()) != 0) { - addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); - // Note: groupsArrayBuilder.build() also clears the data within it - itemCounts.clear(); - } - groupName = group.getName(); - - group.getFileDifferenceSummaryItems().forEach(item -> { - JsonObjectBuilder itemObjectBuilder = jsonObjectBuilder(); - if (item.getName().isEmpty()) { - // 'groupName': {'Added'=#, 'Changed'=#, ...} - // accumulate the counts since we can't make a separate array item - itemCounts.merge(ACTION_ADDED, item.getAdded(), Integer::sum); - itemCounts.merge(ACTION_CHANGED, item.getChanged(), Integer::sum); - itemCounts.merge(ACTION_DELETED, item.getDeleted(), Integer::sum); - itemCounts.merge(ACTION_REPLACED, item.getReplaced(), Integer::sum); - } else if (GROUP_FILE_ACCESS.equals(group.getName())) { - // 'groupName': 'getNameValue' - groupsObjectBuilder.add(group.getName(), group.getFileDifferenceSummaryItems().get(0).getName()); - } else { - // 'groupName': [{name='', action=''}, {name='', action=''}] - String action = item.getAdded() > 0 ? ACTION_ADDED : item.getChanged() > 0 ? ACTION_CHANGED : - item.getDeleted() > 0 ? ACTION_DELETED : item.getReplaced() > 0 ? ACTION_REPLACED : ""; - itemObjectBuilder.add("name", item.getName()); - if (!action.isEmpty()) { - itemObjectBuilder.add("action", action); - } - groupsArrayBuilder.add(itemObjectBuilder.build()); - } - }); - } + processDifferenceGroups(summaryBuilder, groups); + } + + JsonObject summaryObject = summaryBuilder.build(); + if (!summaryObject.isEmpty()) { + jsonBuilder.add("fileDifferenceSummary", summaryObject); + } + } + + /** + * Processes and adds the categorized difference groups to the summary. + */ + private static void processDifferenceGroups(JsonObjectBuilder summaryBuilder, List groups) { + List sortedGroups = groups.stream() + .filter(g -> !StringUtil.isEmpty(g.getName())) + .sorted(Comparator.comparing(FileVersionDifference.FileDifferenceSummaryGroup::getName)) + .toList(); + + if (sortedGroups.isEmpty()) { + return; + } + + String currentGroupName = sortedGroups.get(0).getName(); + GroupDataAccumulator accumulator = new GroupDataAccumulator(); + + for (FileVersionDifference.FileDifferenceSummaryGroup group : sortedGroups) { + // When the group name changes, build and add the JSON for the previous group. + if (!Objects.equals(currentGroupName, group.getName())) { + addGroupDataToJson(summaryBuilder, currentGroupName, accumulator); + accumulator.reset(); + currentGroupName = group.getName(); } - // process last group - addJsonGroupObject(fileDifferenceSummary, groupName, groupsObjectBuilder.build(), groupsArrayBuilder.build(), itemCounts); + + // Accumulate data for the current group. + group.getFileDifferenceSummaryItems().forEach(item -> processSummaryItem(group, item, accumulator)); + } + // Add the last processed group. + addGroupDataToJson(summaryBuilder, currentGroupName, accumulator); + } + + /** + * Determines how to process a summary item based on its structure and content. + */ + private static void processSummaryItem(FileVersionDifference.FileDifferenceSummaryGroup group, FileVersionDifference.FileDifferenceSummaryItem item, GroupDataAccumulator accumulator) { + if (item.getName().isEmpty()) { + // Case 1: The item represents simple counts (Added, Changed, etc.) for the group. + accumulator.mergeCounts(item); + } else if (GROUP_FILE_ACCESS.equals(group.getName())) { + // Case 2: Special handling for "File Access", which is a single name/value pair. + accumulator.setNameValue(group.getName(), item.getName()); + } else { + // Case 3: The item is a named entity with an associated action. + accumulator.addListItem(item); + } + } + + /** + * Builds the final JSON structure for a completed group and adds it to the parent builder. + */ + /** + * Builds the final JSON structure for a completed group and adds it to the parent builder. + */ + private static void addGroupDataToJson(JsonObjectBuilder parentBuilder, String groupName, GroupDataAccumulator accumulator) { + if (groupName == null || groupName.isEmpty()) { + return; } - JsonObject fileDifferenceSummaryObject = fileDifferenceSummary.build(); - if (!fileDifferenceSummaryObject.isEmpty()) { - job.add("fileDifferenceSummary", fileDifferenceSummaryObject); + String sanitizedKey = groupName.replaceAll("\\s+", ""); + + if (!accumulator.getItemCounts().isEmpty()) { + parentBuilder.add(sanitizedKey, buildCountsObject(accumulator.getItemCounts())); + } else { + JsonArray listItemsArray = accumulator.getListItems().build(); + if (!listItemsArray.isEmpty()) { + parentBuilder.add(sanitizedKey, listItemsArray); + } else if (accumulator.getNameValue() != null) { + parentBuilder.add(sanitizedKey, accumulator.getNameValue().getValue("/" + groupName)); + } } - return job; } + /** + * Builds a JSON object from the accumulated action counts. + */ + private static JsonObject buildCountsObject(Map itemCounts) { + JsonObjectBuilder countsBuilder = jsonObjectBuilder(); + itemCounts.forEach((action, count) -> { + if (count != 0) { + countsBuilder.add(action, count); + } + }); + return countsBuilder.build(); + } + + /** + * Determines the overall action performed on the file itself (Added, Deleted, Replaced). + */ private static String getFileAction(FileMetadata originalFileMetadata, FileMetadata newFileMetadata) { - if (newFileMetadata.getDataFile() != null && originalFileMetadata == null) { + boolean newFileExists = newFileMetadata.getDataFile() != null; + boolean originalFileExists = originalFileMetadata != null; + + if (newFileExists && !originalFileExists) { return ACTION_ADDED; - } else if (newFileMetadata.getDataFile() == null && originalFileMetadata != null) { + } + if (!newFileExists && originalFileExists) { return ACTION_DELETED; - } else if (originalFileMetadata != null && - newFileMetadata.getDataFile() != null && originalFileMetadata.getDataFile() != null && !originalFileMetadata.getDataFile().equals(newFileMetadata.getDataFile())) { + } + if (originalFileExists && !originalFileMetadata.getDataFile().equals(newFileMetadata.getDataFile())) { return ACTION_REPLACED; - } else { - return null; } + return null; } - private static void addJsonGroupObject(JsonObjectBuilder jsonObjectBuilder, String key, JsonObject jsonObjectValue, JsonArray jsonArrayValue, Map itemCounts) { - if (key != null && !key.isEmpty()) { - String sanitizedKey = key.replaceAll("\\s+", ""); - if (itemCounts.isEmpty()) { - if (jsonArrayValue.isEmpty()) { - // add the object - jsonObjectBuilder.add(sanitizedKey, jsonObjectValue.getValue("/" + key)); - } else { - // add the array - jsonObjectBuilder.add(sanitizedKey, jsonArrayValue); - } - } else { - // add the accumulated totals - JsonObjectBuilder accumulatedTotalsObjectBuilder = jsonObjectBuilder(); - itemCounts.forEach((k, v) -> { - if (v != 0) { - accumulatedTotalsObjectBuilder.add(k, v); - } - }); - jsonObjectBuilder.add(sanitizedKey, accumulatedTotalsObjectBuilder.build()); + /** + * An inner helper class to hold the state of the JSON builders and counters for a single group while iterating. + */ + private static class GroupDataAccumulator { + private JsonObject nameValue; + private JsonArrayBuilder listItems = Json.createArrayBuilder(); + private Map itemCounts = new HashMap<>(); + + void mergeCounts(FileVersionDifference.FileDifferenceSummaryItem item) { + itemCounts.merge(ACTION_ADDED, item.getAdded(), Integer::sum); + itemCounts.merge(ACTION_CHANGED, item.getChanged(), Integer::sum); + itemCounts.merge(ACTION_DELETED, item.getDeleted(), Integer::sum); + itemCounts.merge(ACTION_REPLACED, item.getReplaced(), Integer::sum); + } + + void setNameValue(String groupName, String value) { + this.nameValue = jsonObjectBuilder().add(groupName, value).build(); + } + + void addListItem(FileVersionDifference.FileDifferenceSummaryItem item) { + String action = getActionFromItem(item); + JsonObjectBuilder itemObjectBuilder = jsonObjectBuilder().add("name", item.getName()); + if (!action.isEmpty()) { + itemObjectBuilder.add("action", action); } + listItems.add(itemObjectBuilder.build()); + } + + private String getActionFromItem(FileVersionDifference.FileDifferenceSummaryItem item) { + if (item.getAdded() > 0) return ACTION_ADDED; + if (item.getChanged() > 0) return ACTION_CHANGED; + if (item.getDeleted() > 0) return ACTION_DELETED; + if (item.getReplaced() > 0) return ACTION_REPLACED; + return ""; + } + + void reset() { + nameValue = null; + listItems = Json.createArrayBuilder(); + itemCounts.clear(); + } + + JsonObject getNameValue() { + return nameValue; + } + + JsonArrayBuilder getListItems() { + return listItems; + } + + Map getItemCounts() { + return itemCounts; } } } diff --git a/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java b/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java new file mode 100644 index 00000000000..7eca3928750 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java @@ -0,0 +1,229 @@ +package edu.harvard.iq.dataverse.util.json; + +import edu.harvard.iq.dataverse.DataFile; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.FileMetadata; +import edu.harvard.iq.dataverse.FileVersionDifference; +import jakarta.json.JsonObject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class FileVersionDifferenceJsonPrinterTest { + @Mock + private FileVersionDifference fileVersionDifference; + @Mock + private FileMetadata newFileMetadata; + @Mock + private FileMetadata originalFileMetadata; + @Mock + private DatasetVersion datasetVersion; + @Mock + private DataFile dataFile; + + @BeforeEach + void setUp() { + when(fileVersionDifference.getNewFileMetadata()).thenReturn(newFileMetadata); + when(newFileMetadata.getDatasetVersion()).thenReturn(datasetVersion); + when(datasetVersion.getVersionNote()).thenReturn("Test Version Note"); + } + + @Test + @DisplayName("should correctly serialize basic dataset and file details") + void jsonFileVersionDifference_with_basic_dataset_and_file_details() { + // Arrange + when(newFileMetadata.getDataFile()).thenReturn(dataFile); + when(dataFile.getId()).thenReturn(42L); + when(datasetVersion.getFriendlyVersionNumber()).thenReturn("V1"); + when(datasetVersion.getVersionState()).thenReturn(DatasetVersion.VersionState.RELEASED); + when(fileVersionDifference.getDifferenceSummaryGroups()).thenReturn(Collections.emptyList()); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + assertEquals("V1", result.getString("datasetVersion")); + assertEquals("RELEASED", result.getString("versionState")); + assertEquals(42, result.getInt("datafileId")); + assertNotNull(result.getJsonObject("fileDifferenceSummary")); + assertEquals("Test Version Note", result.getJsonObject("fileDifferenceSummary").getString("versionNote")); + } + + @Test + @DisplayName("should report file action as 'Added' for a new file") + void jsonFileVersionDifference_file_added() { + // Arrange + when(fileVersionDifference.getOriginalFileMetadata()).thenReturn(null); + when(newFileMetadata.getDataFile()).thenReturn(dataFile); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + assertEquals("Added", result.getJsonObject("fileDifferenceSummary").getString("file")); + } + + @Test + @DisplayName("should report file action as 'Deleted' for a removed file") + void jsonFileVersionDifference_file_deleted() { + // Arrange + when(fileVersionDifference.getOriginalFileMetadata()).thenReturn(originalFileMetadata); + when(newFileMetadata.getDataFile()).thenReturn(null); // Key condition for deleted + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + assertEquals("Deleted", result.getJsonObject("fileDifferenceSummary").getString("file")); + } + + @Test + @DisplayName("should report file action as 'Replaced' for a replaced file") + void jsonFileVersionDifference_file_replaced() { + // Arrange + DataFile originalDataFile = new DataFile(); + originalDataFile.setId(100L); + DataFile newDataFile = new DataFile(); + newDataFile.setId(101L); // Different file object + + when(fileVersionDifference.getOriginalFileMetadata()).thenReturn(originalFileMetadata); + when(originalFileMetadata.getDataFile()).thenReturn(originalDataFile); + when(newFileMetadata.getDataFile()).thenReturn(newDataFile); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + assertEquals("Replaced", result.getJsonObject("fileDifferenceSummary").getString("file")); + } + + @Test + @DisplayName("should correctly serialize count-based summary groups") + void jsonFileVersionDifference_as_count_based_summary() { + // Arrange + FileVersionDifference fvd = new FileVersionDifference(newFileMetadata, originalFileMetadata, true); + FileVersionDifference.FileDifferenceSummaryItem item = fvd.new FileDifferenceSummaryItem("", 2, 1, 0, 0, true); + FileVersionDifference.FileDifferenceSummaryGroup group = fvd.new FileDifferenceSummaryGroup("Tags"); + group.getFileDifferenceSummaryItems().add(item); + + when(fileVersionDifference.getDifferenceSummaryGroups()).thenReturn(Collections.singletonList(group)); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + JsonObject summary = result.getJsonObject("fileDifferenceSummary"); + assertNotNull(summary); + JsonObject tags = summary.getJsonObject("Tags"); + assertNotNull(tags); + assertEquals(2, tags.getInt("Added")); + assertEquals(1, tags.getInt("Changed")); + assertFalse(tags.containsKey("Deleted"), "Zero-count fields should not be present"); + } + + @Test + @DisplayName("should correctly serialize the 'File Access' group as a name-value pair") + void jsonFileVersionDifference_as_name_value_for_file_access() { + // Arrange + FileVersionDifference fvd = new FileVersionDifference(newFileMetadata, originalFileMetadata, true); + FileVersionDifference.FileDifferenceSummaryItem item = fvd.new FileDifferenceSummaryItem("Public", 0, 1, 0, 0, false); + FileVersionDifference.FileDifferenceSummaryGroup group = fvd.new FileDifferenceSummaryGroup("File Access"); + group.getFileDifferenceSummaryItems().add(item); + when(fileVersionDifference.getDifferenceSummaryGroups()).thenReturn(Collections.singletonList(group)); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + JsonObject summary = result.getJsonObject("fileDifferenceSummary"); + assertEquals("Public", summary.getString("FileAccess")); + } + + + @Test + @DisplayName("should correctly serialize list-item-based summary groups") + void jsonFileVersionDifference_as_list_item_based_summary() { + // Arrange + FileVersionDifference fvd = new FileVersionDifference(newFileMetadata, originalFileMetadata, true); + + FileVersionDifference.FileDifferenceSummaryItem itemA = fvd.new FileDifferenceSummaryItem("Category A", 1, 0, 0, 0, false); + FileVersionDifference.FileDifferenceSummaryItem itemB = fvd.new FileDifferenceSummaryItem("Category B", 0, 0, 1, 0, false); + + FileVersionDifference.FileDifferenceSummaryGroup group = fvd.new FileDifferenceSummaryGroup("Categories"); + group.getFileDifferenceSummaryItems().addAll(Arrays.asList(itemA, itemB)); + + when(fileVersionDifference.getDifferenceSummaryGroups()).thenReturn(Collections.singletonList(group)); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + JsonObject summary = result.getJsonObject("fileDifferenceSummary"); + assertNotNull(summary); + jakarta.json.JsonArray categories = summary.getJsonArray("Categories"); + assertNotNull(categories); + assertEquals(2, categories.size()); + assertEquals("Category A", categories.getJsonObject(0).getString("name")); + assertEquals("Added", categories.getJsonObject(0).getString("action")); + assertEquals("Category B", categories.getJsonObject(1).getString("name")); + assertEquals("Deleted", categories.getJsonObject(1).getString("action")); + } + + @Test + @DisplayName("should correctly serialize a mix of all summary group types") + void jsonFileVersionDifference_with_multiple_and_mixed_types() { + // Arrange + FileVersionDifference fvd = new FileVersionDifference(newFileMetadata, originalFileMetadata, true); + + List groups = new ArrayList<>(); + + // Group 1: Tags (counts) + FileVersionDifference.FileDifferenceSummaryItem tagsItem = fvd.new FileDifferenceSummaryItem("", 2, 0, 1, 0, true); + FileVersionDifference.FileDifferenceSummaryGroup tagsGroup = fvd.new FileDifferenceSummaryGroup("Tags"); + tagsGroup.getFileDifferenceSummaryItems().add(tagsItem); + groups.add(tagsGroup); + + // Group 2: Categories (list items) + FileVersionDifference.FileDifferenceSummaryItem categoriesItem = fvd.new FileDifferenceSummaryItem("Science", 1, 0, 0, 0, false); + FileVersionDifference.FileDifferenceSummaryGroup categoriesGroup = fvd.new FileDifferenceSummaryGroup("Categories"); + categoriesGroup.getFileDifferenceSummaryItems().add(categoriesItem); + groups.add(categoriesGroup); + + // Group 3: File Access (name/value) + FileVersionDifference.FileDifferenceSummaryItem accessItem = fvd.new FileDifferenceSummaryItem("Restricted", 0, 1, 0, 0, false); + FileVersionDifference.FileDifferenceSummaryGroup accessGroup = fvd.new FileDifferenceSummaryGroup("File Access"); + accessGroup.getFileDifferenceSummaryItems().add(accessItem); + groups.add(accessGroup); + + when(fileVersionDifference.getDifferenceSummaryGroups()).thenReturn(groups); + + // Act + JsonObject result = FileVersionDifferenceJsonPrinter.jsonFileVersionDifference(fileVersionDifference).build(); + + // Assert + JsonObject summary = result.getJsonObject("fileDifferenceSummary"); + + // Check Categories (should be first alphabetically) + assertEquals(1, summary.getJsonArray("Categories").size()); + assertEquals("Science", summary.getJsonArray("Categories").getJsonObject(0).getString("name")); + + // Check File Access + assertEquals("Restricted", summary.getString("FileAccess")); + + // Check Tags + assertEquals(2, summary.getJsonObject("Tags").getInt("Added")); + assertEquals(1, summary.getJsonObject("Tags").getInt("Deleted")); + } +} From 0f54454ff77c0a6c20727dabbacbb55bcdb5439f Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 13 Oct 2025 16:52:17 +0100 Subject: [PATCH 28/45] Fixed: typo in javadoc --- .../dataverse/util/json/FileVersionDifferenceJsonPrinter.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java index 712b31325f7..d9761bd580b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java @@ -158,9 +158,6 @@ private static void processSummaryItem(FileVersionDifference.FileDifferenceSumma } } - /** - * Builds the final JSON structure for a completed group and adds it to the parent builder. - */ /** * Builds the final JSON structure for a completed group and adds it to the parent builder. */ From 0b21a9a21b0aa6bba0795eac7aa5f870c70fb790 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 14 Oct 2025 20:03:33 +0100 Subject: [PATCH 29/45] Fixed: DataFileServiceBean.findFileMetadataHistory behavior --- .../iq/dataverse/DataFileServiceBean.java | 112 +++++++++--------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java index 0014cce282c..271cdcafd11 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -37,11 +38,7 @@ import jakarta.ejb.TransactionAttribute; import jakarta.ejb.TransactionAttributeType; import jakarta.inject.Named; -import jakarta.persistence.EntityManager; -import jakarta.persistence.NoResultException; -import jakarta.persistence.PersistenceContext; -import jakarta.persistence.Query; -import jakarta.persistence.TypedQuery; +import jakarta.persistence.*; import jakarta.persistence.criteria.*; /** @@ -380,18 +377,20 @@ public FileMetadata findFileMetadataByDatasetVersionIdAndDataFileId(Long dataset } /** - * Finds the concise history of a file, returning an entry only for each - * version where the file's metadata was created or changed. + * Finds the complete history of a file's presence across all dataset versions. *

- * This method correctly handles file replacements by searching for all files - * sharing the same {@code rootDataFileId}. + * This method returns a {@link VersionedFileMetadata} entry for every version + * of the specified dataset. If a version does not contain the file, the + * {@code fileMetadata} field in the corresponding DTO will be {@code null}. + * It correctly handles file replacements by searching for all files sharing the + * same {@code rootDataFileId}. * * @param datasetId The ID of the parent dataset. * @param dataFile The DataFile entity to find the history for. * @param canViewUnpublishedVersions A boolean indicating if the user has permission to view non-released versions. * @param limit (Optional) The maximum number of results to return. * @param offset (Optional) The starting point of the result list. - * @return A chronologically sorted, paginated list of the file's version history. + * @return A chronologically sorted, paginated list of the file's version history, including versions where the file is absent. */ public List findFileMetadataHistory(Long datasetId, DataFile dataFile, @@ -402,65 +401,66 @@ public List findFileMetadataHistory(Long datasetId, return Collections.emptyList(); } + // Query 1: Get the paginated list of relevant DatasetVersions CriteriaBuilder cb = em.getCriteriaBuilder(); - CriteriaQuery criteriaQuery = cb.createQuery(FileMetadata.class); - Root fileMetadata = criteriaQuery.from(FileMetadata.class); + CriteriaQuery versionQuery = cb.createQuery(DatasetVersion.class); + Root versionRoot = versionQuery.from(DatasetVersion.class); - // --- Joins --- - // Define relationships for filtering and ordering. - Join version = fileMetadata.join("datasetVersion"); - Join dataset = version.join("dataset"); - Join file = fileMetadata.join("dataFile"); - - // --- Predicates (WHERE clause) --- - // Build a list of conditions for the query. - List predicates = new ArrayList<>(); - - // Filter by the parent dataset. - predicates.add(cb.equal(dataset.get("id"), datasetId)); - - // Filter by the file's lineage, handling both new and replaced files. - if (dataFile.getRootDataFileId() < 0) { - // For a new file, match by its specific ID. - predicates.add(cb.equal(file.get("id"), dataFile.getId())); - } else { - // For a replaced file, match the entire history via its root ID. - predicates.add(cb.equal(file.get("rootDataFileId"), dataFile.getRootDataFileId())); - } - - // Add permission-based filter for version states. + List versionPredicates = new ArrayList<>(); + versionPredicates.add(cb.equal(versionRoot.join("dataset").get("id"), datasetId)); if (!canViewUnpublishedVersions) { - predicates.add(version.get("versionState").in( + versionPredicates.add(versionRoot.get("versionState").in( VersionState.RELEASED, VersionState.DEACCESSIONED)); } - - criteriaQuery.where(predicates.toArray(new Predicate[0])); - - // --- Ordering --- - // Order results from newest to oldest version. - criteriaQuery.orderBy( - cb.desc(version.get("versionNumber")), - cb.desc(version.get("minorVersionNumber")) + versionQuery.where(versionPredicates.toArray(new Predicate[0])); + versionQuery.orderBy( + cb.desc(versionRoot.get("versionNumber")), + cb.desc(versionRoot.get("minorVersionNumber")) ); - // --- Execution & Transformation --- - // Create the query object from the criteria definition. - TypedQuery typedQuery = em.createQuery(criteriaQuery); - - // --- Pagination --- - // Apply pagination if limit and/or offset are provided. + TypedQuery typedVersionQuery = em.createQuery(versionQuery); if (limit != null) { - typedQuery.setMaxResults(limit); + typedVersionQuery.setMaxResults(limit); } if (offset != null) { - typedQuery.setFirstResult(offset); + typedVersionQuery.setFirstResult(offset); } + List datasetVersions = typedVersionQuery.getResultList(); + + if (datasetVersions.isEmpty()) { + return Collections.emptyList(); + } + + // Query 2: Get all FileMetadata for this file's history in this dataset + CriteriaQuery fmQuery = cb.createQuery(FileMetadata.class); + Root fmRoot = fmQuery.from(FileMetadata.class); + + List fmPredicates = new ArrayList<>(); + fmPredicates.add(cb.equal(fmRoot.get("datasetVersion").get("dataset").get("id"), datasetId)); + + // Find the file by its entire lineage + if (dataFile.getRootDataFileId() < 0) { + fmPredicates.add(cb.equal(fmRoot.get("dataFile").get("id"), dataFile.getId())); + } else { + fmPredicates.add(cb.equal(fmRoot.get("dataFile").get("rootDataFileId"), dataFile.getRootDataFileId())); + } + fmQuery.where(fmPredicates.toArray(new Predicate[0])); + + List fileHistory = em.createQuery(fmQuery).getResultList(); - // Execute the query and map the results to the VersionedFileMetadata list. - List results = typedQuery.getResultList(); + // Combine results + Map fmMap = fileHistory.stream() + .collect(Collectors.toMap( + fm -> fm.getDatasetVersion().getId(), + Function.identity() + )); - return results.stream() - .map(metadata -> new VersionedFileMetadata(metadata.getDatasetVersion(), metadata)) + // Create the final list, looking up the FileMetadata for each version + return datasetVersions.stream() + .map(version -> new VersionedFileMetadata( + version, + fmMap.get(version.getId()) // This will be null if no entry exists for that version ID + )) .collect(Collectors.toList()); } From 4557600bfbc1bec2adb4cbd50b644b25a92a0374 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 14 Oct 2025 23:12:13 +0100 Subject: [PATCH 30/45] Fixed: added missing contributor names to file metadata in GetFileVersionDifferencesCommand --- .../impl/GetFileVersionDifferencesCommand.java | 13 ++++++++----- .../impl/GetFileVersionDifferencesCommandTest.java | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index 62bfb24b801..d676b61d67c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -59,12 +59,15 @@ private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) * Transforms a single historical entry into a FileVersionDifference object. */ private FileVersionDifference createDifferenceFromHistory(CommandContext ctxt, VersionedFileMetadata versionedFileMetadata) { - FileMetadata current = versionedFileMetadata.getFileMetadata(); - FileMetadata previous = ctxt.files().getPreviousFileMetadata(current); + FileMetadata fileMetadata = versionedFileMetadata.getFileMetadata(); + FileMetadata previous = ctxt.files().getPreviousFileMetadata(fileMetadata); - return (current != null) - ? new FileVersionDifference(current, previous, false) - : createDifferenceForNonexistentFile(versionedFileMetadata.getDatasetVersion(), previous); + if (fileMetadata != null) { + fileMetadata.setContributorNames(ctxt.datasetVersion().getContributorsNames(fileMetadata.getDatasetVersion())); + return new FileVersionDifference(fileMetadata, previous, false); + } + + return createDifferenceForNonexistentFile(versionedFileMetadata.getDatasetVersion(), previous); } /** diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java index 9b672fd2ca5..ad6ad495eeb 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -39,6 +39,8 @@ class GetFileVersionDifferencesCommandTest { @Mock private DataFileServiceBean fileServiceMock; @Mock + private DatasetVersionServiceBean datasetVersionServiceMock; + @Mock private Dataset datasetMock; @Mock private DatasetVersion datasetVersionMock; @@ -147,13 +149,19 @@ void execute_should_pass_pagination_parameters_correctly() throws CommandExcepti void execute_should_convert_history_to_differences() throws CommandException { // Arrange setupMocksForSuccessfulExecution(); + // This test needs an extra mock because it processes items from the history list + when(contextMock.datasetVersion()).thenReturn(datasetVersionServiceMock); + // Create mock history: 3 versions of a file's metadata FileMetadata fm3 = new FileMetadata(); // Newest fm3.setId(3L); + fm3.setDatasetVersion(datasetVersionMock); FileMetadata fm2 = new FileMetadata(); // Middle fm2.setId(2L); + fm2.setDatasetVersion(datasetVersionMock); FileMetadata fm1 = new FileMetadata(); // Oldest fm1.setId(1L); + fm1.setDatasetVersion(datasetVersionMock); VersionedFileMetadata vfm3 = mock(VersionedFileMetadata.class); VersionedFileMetadata vfm2 = mock(VersionedFileMetadata.class); @@ -171,6 +179,9 @@ void execute_should_convert_history_to_differences() throws CommandException { when(fileServiceMock.getPreviousFileMetadata(fm2)).thenReturn(fm1); when(fileServiceMock.getPreviousFileMetadata(fm1)).thenReturn(null); // The oldest has no predecessor + // Mock the logic to retrieve contributor names + when(datasetVersionServiceMock.getContributorsNames(any(DatasetVersion.class))).thenReturn(Collections.emptyList().toString()); + GetFileVersionDifferencesCommand command = new GetFileVersionDifferencesCommand(requestMock, fileMetadataMock, null, null); // Act @@ -190,6 +201,9 @@ void execute_should_convert_history_to_differences() throws CommandException { // Check the difference for the oldest version (v1 vs null) assertThat(differences.get(2).getNewFileMetadata()).isEqualTo(fm1); assertThat(differences.get(2).getOriginalFileMetadata()).isNull(); + + // Verify setting contributor for each file metadata + verify(datasetVersionServiceMock, times(3)).getContributorsNames(datasetVersionMock); } @Test From ef42d87f5a80659315ae7544998740afc4638e52 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 14 Oct 2025 23:17:36 +0100 Subject: [PATCH 31/45] Added: explanatory comment to GetFileVersionDifferencesCommand --- .../engine/command/impl/GetFileVersionDifferencesCommand.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index d676b61d67c..7f9541e6e73 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -57,6 +57,7 @@ private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) /** * Transforms a single historical entry into a FileVersionDifference object. + * As part of this process, it also enriches the file metadata with contributor names from that version. */ private FileVersionDifference createDifferenceFromHistory(CommandContext ctxt, VersionedFileMetadata versionedFileMetadata) { FileMetadata fileMetadata = versionedFileMetadata.getFileMetadata(); From 664c90b5f9ebb0c01e1feb42761c6045d0d5d222 Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 16 Oct 2025 12:18:01 +0100 Subject: [PATCH 32/45] Fixed: FileVersionDifferenceJsonPrinter to show dataset version release date --- .../util/json/FileVersionDifferenceJsonPrinter.java | 7 ++----- .../util/json/FileVersionDifferenceJsonPrinterTest.java | 2 ++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java index d9761bd580b..44941203fc6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinter.java @@ -70,11 +70,8 @@ private static void addDatasetVersionDetails(JsonObjectBuilder jsonBuilder, File .add("isReleased", datasetVersion.isReleased()) .add("isDeaccessioned", datasetVersion.isDeaccessioned()) .add("summary", datasetVersion.getVersionNote()) - .add("contributors", fileMetadata.getContributorNames()); - - if (fileMetadata.getDataFile() != null && fileMetadata.getDataFile().getPublicationDate() != null) { - jsonBuilder.add("publishedDate", fileMetadata.getDataFile().getPublicationDate().toString()); - } + .add("contributors", fileMetadata.getContributorNames()) + .add("publishedDate", fileMetadata.getDatasetVersion().getPublicationDateAsString()); } /** diff --git a/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java b/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java index 7eca3928750..fff0cebcd3e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/util/json/FileVersionDifferenceJsonPrinterTest.java @@ -48,6 +48,7 @@ void jsonFileVersionDifference_with_basic_dataset_and_file_details() { when(dataFile.getId()).thenReturn(42L); when(datasetVersion.getFriendlyVersionNumber()).thenReturn("V1"); when(datasetVersion.getVersionState()).thenReturn(DatasetVersion.VersionState.RELEASED); + when(datasetVersion.getPublicationDateAsString()).thenReturn("2025-10-16"); when(fileVersionDifference.getDifferenceSummaryGroups()).thenReturn(Collections.emptyList()); // Act @@ -56,6 +57,7 @@ void jsonFileVersionDifference_with_basic_dataset_and_file_details() { // Assert assertEquals("V1", result.getString("datasetVersion")); assertEquals("RELEASED", result.getString("versionState")); + assertEquals("2025-10-16", result.getString("publishedDate")); assertEquals(42, result.getInt("datafileId")); assertNotNull(result.getJsonObject("fileDifferenceSummary")); assertEquals("Test Version Note", result.getJsonObject("fileDifferenceSummary").getString("versionNote")); From 272e50ec17bd5519b7ae83584307226b0b6e8d04 Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 16 Oct 2025 12:19:06 +0100 Subject: [PATCH 33/45] Fixed: GetFileVersionDifferencesCommand to display correct results --- .../GetFileVersionDifferencesCommand.java | 120 ++++++++++++------ .../GetFileVersionDifferencesCommandTest.java | 16 ++- 2 files changed, 96 insertions(+), 40 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index 7f9541e6e73..d822fa9455b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -1,19 +1,24 @@ package edu.harvard.iq.dataverse.engine.command.impl; -import edu.harvard.iq.dataverse.*; +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.FileMetadata; +import edu.harvard.iq.dataverse.FileVersionDifference; +import edu.harvard.iq.dataverse.VersionedFileMetadata; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; +import java.util.ArrayList; +import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; /** - * A command that retrieves the version history for a single file, - * highlighting the differences between each version. + * Retrieves the version history for a single file, highlighting the + * differences between each version. This command is pagination-aware. */ @RequiredPermissions({}) public class GetFileVersionDifferencesCommand extends AbstractPaginatedCommand> { @@ -25,13 +30,80 @@ public GetFileVersionDifferencesCommand(DataverseRequest request, FileMetadata f this.fileMetadata = Objects.requireNonNull(fileMetadata); } + /** + * Executes the command to generate a list of file version differences. + *

+ * This method processes a paginated list of file history. + * It pairs each version with its successor from the list. + * For the last item on the page, it performs a single query to find its true predecessor, handling the pagination case. + * + * @param ctxt The command context. + * @return A list of {@link FileVersionDifference} objects. + */ @Override public List executeCommand(CommandContext ctxt) { List fileHistory = findFileHistory(ctxt); + if (fileHistory.isEmpty()) { + return Collections.emptyList(); + } + + List differences = new ArrayList<>(); + + // Process all but the last item, using the next item in the list as the predecessor. + for (int i = 0; i < fileHistory.size() - 1; i++) { + VersionedFileMetadata current = fileHistory.get(i); + FileMetadata previous = fileHistory.get(i + 1).getFileMetadata(); + differences.add(buildDifference(ctxt, current, previous)); + } + + // Handle the last item on the page separately. Its predecessor may not be in the current + // list, so we must query the database for it. + VersionedFileMetadata lastItemOnPage = fileHistory.get(fileHistory.size() - 1); + FileMetadata predecessor = findPredecessorFor(ctxt, lastItemOnPage); + differences.add(buildDifference(ctxt, lastItemOnPage, predecessor)); + + return differences; + } + + /** + * Constructs a {@link FileVersionDifference} from a version's metadata and its predecessor. + * + * @param ctxt The command context. + * @param currentVersionMetadata The metadata for the current version in history. + * @param previousFileMetadata The metadata for the preceding version (can be null). + * @return A new {@link FileVersionDifference} object. + */ + private FileVersionDifference buildDifference(CommandContext ctxt, VersionedFileMetadata currentVersionMetadata, FileMetadata previousFileMetadata) { + FileMetadata currentFileMetadata = currentVersionMetadata.getFileMetadata(); + + if (currentFileMetadata != null) { + return createDifferenceForExistingFile(ctxt, currentFileMetadata, previousFileMetadata); + } else { + return createDifferenceForMissingFile(currentVersionMetadata.getDatasetVersion(), previousFileMetadata); + } + } - return fileHistory.stream() - .map(versionedFileMetadata -> createDifferenceFromHistory(ctxt, versionedFileMetadata)) - .collect(Collectors.toList()); + /** + * Creates a difference object for a version where the file exists. + * As a side effect, this method also enriches the metadata with contributor names. + */ + private FileVersionDifference createDifferenceForExistingFile(CommandContext ctxt, FileMetadata current, FileMetadata previous) { + current.setContributorNames(ctxt.datasetVersion().getContributorsNames(current.getDatasetVersion())); + return new FileVersionDifference(current, previous, false); + } + + /** + * Creates a placeholder difference object for a version where the file was absent. + */ + private FileVersionDifference createDifferenceForMissingFile(DatasetVersion version, FileMetadata previous) { + FileMetadata placeholder = new FileMetadata(); + placeholder.setDatasetVersion(version); + placeholder.setDataFile(null); // Explicitly null to indicate absence. + + FileVersionDifference difference = new FileVersionDifference(placeholder, previous, true); + placeholder.setFileVersionDifference(difference); + + return difference; } /** @@ -40,12 +112,11 @@ public List executeCommand(CommandContext ctxt) { private List findFileHistory(CommandContext ctxt) { Dataset dataset = fileMetadata.getDatasetVersion().getDataset(); boolean canViewUnpublished = canViewUnpublishedVersions(ctxt, dataset); - return ctxt.files().findFileMetadataHistory(dataset.getId(), fileMetadata.getDataFile(), canViewUnpublished, limit, offset); } /** - * Determines if the user has permission to view non-released versions of the dataset. + * Determines if the user can view non-released versions of the dataset. */ private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) { return ctxt.permissions().hasPermissionsFor( @@ -56,33 +127,10 @@ private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) } /** - * Transforms a single historical entry into a FileVersionDifference object. - * As part of this process, it also enriches the file metadata with contributor names from that version. + * Finds the chronological predecessor for a given {@link VersionedFileMetadata}. */ - private FileVersionDifference createDifferenceFromHistory(CommandContext ctxt, VersionedFileMetadata versionedFileMetadata) { - FileMetadata fileMetadata = versionedFileMetadata.getFileMetadata(); - FileMetadata previous = ctxt.files().getPreviousFileMetadata(fileMetadata); - - if (fileMetadata != null) { - fileMetadata.setContributorNames(ctxt.datasetVersion().getContributorsNames(fileMetadata.getDatasetVersion())); - return new FileVersionDifference(fileMetadata, previous, false); - } - - return createDifferenceForNonexistentFile(versionedFileMetadata.getDatasetVersion(), previous); - } - - /** - * Creates a special FileVersionDifference for versions where the file does not exist. - */ - private FileVersionDifference createDifferenceForNonexistentFile(DatasetVersion version, FileMetadata previous) { - FileMetadata placeholder = new FileMetadata(); - placeholder.setDatasetVersion(version); - // Explicitly null DataFile indicates the file does not exist. - placeholder.setDataFile(null); - - FileVersionDifference difference = new FileVersionDifference(placeholder, previous, true); - placeholder.setFileVersionDifference(difference); - - return difference; + private FileMetadata findPredecessorFor(CommandContext ctxt, VersionedFileMetadata versionedMetadata) { + FileMetadata current = versionedMetadata.getFileMetadata(); + return (current != null) ? ctxt.files().getPreviousFileMetadata(current) : null; } } diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java index ad6ad495eeb..f131f81cdff 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommandTest.java @@ -174,10 +174,18 @@ void execute_should_convert_history_to_differences() throws CommandException { when(fileServiceMock.findFileMetadataHistory(any(), any(), anyBoolean(), any(), any())) .thenReturn(history); - // Mock the logic to find the direct predecessor of each version - when(fileServiceMock.getPreviousFileMetadata(fm3)).thenReturn(fm2); - when(fileServiceMock.getPreviousFileMetadata(fm2)).thenReturn(fm1); - when(fileServiceMock.getPreviousFileMetadata(fm1)).thenReturn(null); // The oldest has no predecessor + when(fileServiceMock.getPreviousFileMetadata(any(FileMetadata.class))).thenAnswer( + invocation -> { + FileMetadata current = invocation.getArgument(0); + if (current != null && current.getId().equals(3L)) { + return fm2; + } + if (current != null && current.getId().equals(2L)) { + return fm1; + } + return null; + } + ); // Mock the logic to retrieve contributor names when(datasetVersionServiceMock.getContributorsNames(any(DatasetVersion.class))).thenReturn(Collections.emptyList().toString()); From 4f9edceab0be9eb5dffe495ec6f4c07e8874cbdf Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 10:45:56 +0100 Subject: [PATCH 34/45] Added: getDatasetVersionCount JPA-Criteria based method to DatasetVersionServiceBean --- .../dataverse/DatasetVersionServiceBean.java | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java index 0a60051ea10..60df1fd3dfd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java @@ -36,6 +36,10 @@ import jakarta.persistence.PersistenceContext; import jakarta.persistence.Query; import jakarta.persistence.TypedQuery; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Predicate; +import jakarta.persistence.criteria.Root; import org.apache.commons.lang3.StringUtils; /** @@ -1284,5 +1288,49 @@ public List getUnarchivedDatasetVersions(){ logger.log(Level.WARNING, "EJBException exception: {0}", e.getMessage()); return null; } - } // end getUnarchivedDatasetVersions -} // end class + } + + /** + * Calculates the total number of versions for a specified dataset. + *

+ * This method provides a flexible way to count dataset versions. It can either + * return a total count of all versions or restrict the count to only those + * that are publicly visible (i.e., {@code RELEASED} or {@code DEACCESSIONED}). + * This is particularly useful for displaying different counts to users with + * different permission levels. + * + * @param datasetId The unique identifier of the dataset for which to count versions. + * Must not be {@code null}. + * @param canViewUnpublishedVersions A boolean flag that controls the scope of the count: + *

+ * @return A {@code Long} representing the total count of matching dataset versions. + * This will be {@code 0L} if the dataset has no versions or does not exist. + */ + public Long getDatasetVersionCount(Long datasetId, boolean canViewUnpublishedVersions) { + CriteriaBuilder cb = em.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(Long.class); + Root versionRoot = cq.from(DatasetVersion.class); + + List predicates = new ArrayList<>(); + + // Add the primary predicate to filter by the dataset's ID. + predicates.add(cb.equal(versionRoot.join("dataset").get("id"), datasetId)); + + // Conditionally add a predicate to filter for public versions only. + if (!canViewUnpublishedVersions) { + predicates.add(versionRoot.get("versionState").in( + VersionState.RELEASED, VersionState.DEACCESSIONED)); + } + + cq.select(cb.count(versionRoot)) + .where(predicates.toArray(new Predicate[0])); + + return em.createQuery(cq).getSingleResult(); + } +} From ea27c82cc786c51e8def31a8c1a502237d040f39 Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 10:50:58 +0100 Subject: [PATCH 35/45] Added: GetDatasetVersionCountCommand --- .../impl/GetDatasetVersionCountCommand.java | 42 +++++++ .../GetDatasetVersionCountCommandTest.java | 118 ++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommand.java create mode 100644 src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommandTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommand.java new file mode 100644 index 00000000000..752f316783e --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommand.java @@ -0,0 +1,42 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.engine.command.AbstractCommand; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; + +import java.util.EnumSet; + +/** + * Retrieves the version count for a single dataset. + * Depending on user permissions, unpublished versions will be counted or not. + */ +@RequiredPermissions({}) +public class GetDatasetVersionCountCommand extends AbstractCommand { + + private final Dataset dataset; + + public GetDatasetVersionCountCommand(DataverseRequest request, Dataset dataset) { + super(request, dataset); + this.dataset = dataset; + } + + @Override + public Long execute(CommandContext ctxt) throws CommandException { + return ctxt.datasetVersion().getDatasetVersionCount(dataset.getId(), canViewUnpublishedVersions(ctxt, dataset)); + } + + /** + * Determines if the user can view non-released versions of the dataset. + */ + private boolean canViewUnpublishedVersions(CommandContext ctxt, Dataset dataset) { + return ctxt.permissions().hasPermissionsFor( + getRequest().getUser(), + dataset, + EnumSet.of(Permission.ViewUnpublishedDataset) + ); + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommandTest.java new file mode 100644 index 00000000000..a02ffa4b50f --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionCountCommandTest.java @@ -0,0 +1,118 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersionServiceBean; +import edu.harvard.iq.dataverse.PermissionServiceBean; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.EnumSet; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class GetDatasetVersionCountCommandTest { + + @Mock + private CommandContext commandContextMock; + @Mock + private DataverseRequest dataverseRequestMock; + @Mock + private DatasetVersionServiceBean datasetVersionServiceMock; + @Mock + private PermissionServiceBean permissionServiceMock; + @Mock + private Dataset datasetMock; + @Mock + private AuthenticatedUser userMock; + + private static final Long TEST_DATASET_ID = 42L; + + @BeforeEach + void setUp() { + when(commandContextMock.datasetVersion()).thenReturn(datasetVersionServiceMock); + when(commandContextMock.permissions()).thenReturn(permissionServiceMock); + when(datasetMock.getId()).thenReturn(TEST_DATASET_ID); + when(dataverseRequestMock.getUser()).thenReturn(userMock); + } + + @Test + @DisplayName("Should count ALL versions when user has ViewUnpublishedDataset permission") + void execute_whenUserCanViewUnpublished_countsAllVersions() throws CommandException { + // Arrange + // Simulate that the user has the required permission + when(permissionServiceMock.hasPermissionsFor( + eq(userMock), + eq(datasetMock), + eq(EnumSet.of(Permission.ViewUnpublishedDataset)) + )).thenReturn(true); + + // Define the expected count when all versions are included + Long expectedTotalCount = 10L; + when(datasetVersionServiceMock.getDatasetVersionCount(TEST_DATASET_ID, true)).thenReturn(expectedTotalCount); + + GetDatasetVersionCountCommand command = new GetDatasetVersionCountCommand(dataverseRequestMock, datasetMock); + + // Act + Long actualCount = command.execute(commandContextMock); + + // Assert + assertEquals(expectedTotalCount, actualCount, "The count should include all versions (published and unpublished)."); + + // Verify that the permission check was performed + verify(permissionServiceMock).hasPermissionsFor( + eq(userMock), + eq(datasetMock), + eq(EnumSet.of(Permission.ViewUnpublishedDataset)) + ); + + // Verify the service was called with 'true' to indicate unpublished versions should be counted + verify(datasetVersionServiceMock).getDatasetVersionCount(TEST_DATASET_ID, true); + } + + @Test + @DisplayName("Should count ONLY published versions when user lacks ViewUnpublishedDataset permission") + void execute_whenUserCannotViewUnpublished_countsOnlyPublishedVersions() throws CommandException { + // Arrange + // Simulate that the user does NOT have the required permission + when(permissionServiceMock.hasPermissionsFor( + eq(userMock), + eq(datasetMock), + eq(EnumSet.of(Permission.ViewUnpublishedDataset)) + )).thenReturn(false); + + // Define the expected count when only published versions are included + Long expectedPublishedCount = 5L; + when(datasetVersionServiceMock.getDatasetVersionCount(TEST_DATASET_ID, false)).thenReturn(expectedPublishedCount); + + GetDatasetVersionCountCommand command = new GetDatasetVersionCountCommand(dataverseRequestMock, datasetMock); + + // Act + Long actualCount = command.execute(commandContextMock); + + // Assert + assertEquals(expectedPublishedCount, actualCount, "The count should only include published versions."); + + // Verify that the permission check was performed + verify(permissionServiceMock).hasPermissionsFor( + eq(userMock), + eq(datasetMock), + eq(EnumSet.of(Permission.ViewUnpublishedDataset)) + ); + + // Verify the service was called with 'false' to indicate only published versions should be counted + verify(datasetVersionServiceMock).getDatasetVersionCount(TEST_DATASET_ID, false); + } +} From 5cd3811dd6be75cdeeffd6731566c3fda6dae1a9 Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 10:53:38 +0100 Subject: [PATCH 36/45] Added: totalCount to response in dataset version differences API --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 7 ++++++- .../edu/harvard/iq/dataverse/util/json/JsonPrinter.java | 8 ++++---- .../java/edu/harvard/iq/dataverse/api/DatasetsIT.java | 9 +++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index e418d7e79b7..3fd490c38bc 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -16,6 +16,7 @@ import edu.harvard.iq.dataverse.batch.jobs.importer.ImportMode; import edu.harvard.iq.dataverse.dataaccess.*; import edu.harvard.iq.dataverse.datacapturemodule.DataCaptureModuleUtil; +import edu.harvard.iq.dataverse.datasetversionsummaries.DatasetVersionSummary; import software.amazon.awssdk.services.s3.model.CompletedPart; import edu.harvard.iq.dataverse.datacapturemodule.ScriptRequestResponse; import edu.harvard.iq.dataverse.dataset.*; @@ -3143,7 +3144,11 @@ public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, @QueryParam("offset") Integer offset) { return response(req -> { try { - return ok(jsonDatasetVersionSummaries(execCommand(new GetDatasetVersionSummariesCommand(req, findDatasetOrDie(id), limit, offset)))); + Dataset dataset = findDatasetOrDie(id); + List versionSummaries = execCommand(new GetDatasetVersionSummariesCommand(req, dataset, limit, offset)); + JsonArrayBuilder versionSummariesArrayBuilder = jsonDatasetVersionSummaries(versionSummaries); + long datasetVersionTotalCount = execCommand(new GetDatasetVersionCountCommand(req, dataset)); + return ok(versionSummariesArrayBuilder, datasetVersionTotalCount); } catch (WrappedResponse wr) { return wr.getResponse(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index 0e34ed975e3..4cc392ac976 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -1720,14 +1720,14 @@ public static JsonArrayBuilder json(List notifications, Authen return notificationsArray; } - public static JsonArray jsonDatasetVersionSummaries(List summaries) { + public static JsonArrayBuilder jsonDatasetVersionSummaries(List summaries) { JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); summaries.stream() .filter(Objects::nonNull) .map(JsonPrinter::json) .forEach(arrayBuilder::add); - return arrayBuilder.build(); + return arrayBuilder; } private static JsonObjectBuilder json(DatasetVersionSummary summary) { @@ -1762,13 +1762,13 @@ private static JsonObjectBuilder json(DatasetVersionSummary summary) { return jsonObjectBuilder; } - public static JsonArray jsonFileVersionSummaries(List differences) { + public static JsonArrayBuilder jsonFileVersionSummaries(List differences) { JsonArrayBuilder arrayBuilder = Json.createArrayBuilder(); differences.stream() .filter(Objects::nonNull) .map(diff -> jsonFileVersionDifference(diff).build()) .forEach(arrayBuilder::add); - return arrayBuilder.build(); + return arrayBuilder; } } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index 31f4669cd42..c648326e598 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -6534,6 +6534,8 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti compareResponse.prettyPrint(); compareResponse.then().assertThat() + .body("totalCount", is(2)) + .body("data.size()", is(2)) .body("data[1].versionNumber", equalTo("1.0")) .body("data[1].summary", equalTo("firstPublished")) .body("data[0].versionNumber", equalTo("DRAFT")) @@ -6556,6 +6558,8 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti Response compareResponse2 = UtilIT.summaryDatasetVersionDifferences(datasetPersistentId, apiTokenNoPriv); compareResponse2.prettyPrint(); compareResponse2.then().assertThat() + .body("totalCount", is(1)) + .body("data.size()", is(1)) .body("data[0].versionNumber", CoreMatchers.equalTo("1.0")) .body("data[0].summary", CoreMatchers.equalTo("firstPublished")) .statusCode(OK.getStatusCode()); @@ -6567,6 +6571,8 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti compareResponse.prettyPrint(); compareResponse.then().assertThat() + .body("totalCount", is(2)) + .body("data.size()", is(2)) .body("data[1].versionNumber", equalTo("1.0")) .body("data[1].summary.deaccessioned.reason", equalTo("Test deaccession reason.")) .body("data[0].versionNumber", equalTo("DRAFT")) @@ -6591,6 +6597,7 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti compareAllResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.size()", is(3)) + .body("totalCount", is(3)) .body("data[0].versionNumber", equalTo("DRAFT")) .body("data[1].versionNumber", equalTo("2.0")) .body("data[2].versionNumber", equalTo("1.0")); @@ -6600,6 +6607,7 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti compareLimitResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.size()", is(1)) + .body("totalCount", is(3)) .body("data[0].versionNumber", equalTo("DRAFT")); // Test pagination: Use limit=1 and offset=1. Should skip the first result and return the second (2.0). @@ -6607,6 +6615,7 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti compareOffsetResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.size()", is(1)) + .body("totalCount", is(3)) .body("data[0].versionNumber", equalTo("2.0")); // Test invalid pagination: limit=-1 (should return a bad request error) From c0e100897408297b6d5baa276ff00f0b6d1bd083 Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 11:02:03 +0100 Subject: [PATCH 37/45] Added: total count to file version differences API --- .../edu/harvard/iq/dataverse/api/Files.java | 5 ++++- .../edu/harvard/iq/dataverse/api/FilesIT.java | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 0b5e5b02b85..ea3035333b9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -1183,7 +1183,10 @@ public Response getFileVersionsList(@Context ContainerRequestContext crc, if (fm == null) { return notFound(BundleUtil.getStringFromBundle("files.api.fileNotFound")); } - return ok(jsonFileVersionSummaries(execCommand(new GetFileVersionDifferencesCommand(req, fm, limit, offset)))); + List versionDifferences = execCommand(new GetFileVersionDifferencesCommand(req, fm, limit, offset)); + JsonArrayBuilder versionDifferencesArrayBuilder = jsonFileVersionSummaries(versionDifferences); + long datasetVersionTotalCount = execCommand(new GetDatasetVersionCountCommand(req, fm.getDatasetVersion().getDataset())); + return ok(versionDifferencesArrayBuilder, datasetVersionTotalCount); } catch (WrappedResponse ex) { return ex.getResponse(); } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index ec24fb1124c..285f2a87223 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1472,6 +1472,8 @@ public void getFileVersionDifferences() { getFileDataResponse.prettyPrint(); getFileDataResponse.then().assertThat() .body("status", equalTo("OK")) + .body("totalCount", is(1)) + .body("data.size()", is(1)) .body("data[0].datasetVersion", equalTo("DRAFT")) .body("data[0].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); @@ -1492,6 +1494,8 @@ public void getFileVersionDifferences() { getFileDataResponse.prettyPrint(); getFileDataResponse.then().assertThat() .body("status", equalTo("OK")) + .body("totalCount", is(1)) + .body("data.size()", is(1)) .body("data[0].datasetVersion", equalTo("1.0")) .body("data[0].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); @@ -1506,6 +1510,8 @@ public void getFileVersionDifferences() { getFileDataResponse.prettyPrint(); getFileDataResponse.then().assertThat() .body("status", equalTo("OK")) + .body("totalCount", is(1)) + .body("data.size()", is(1)) .body("data[0].datasetVersion", equalTo("1.0")) .body("data[0].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); @@ -1520,7 +1526,10 @@ public void getFileVersionDifferences() { getFileDataResponse.prettyPrint(); getFileDataResponse.then().assertThat() .body("status", equalTo("OK")) + .body("totalCount", is(2)) + .body("data.size()", is(2)) .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[0].fileDifferenceSummary.file", equalTo(null)) .body("data[1].datasetVersion", equalTo("1.0")) .body("data[1].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); @@ -1531,6 +1540,7 @@ public void getFileVersionDifferences() { paginatedResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("status", equalTo("OK")) + .body("totalCount", is(2)) .body("data.size()", is(1)) .body("data[0].datasetVersion", equalTo("DRAFT")); @@ -1540,6 +1550,7 @@ public void getFileVersionDifferences() { paginatedResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("status", equalTo("OK")) + .body("totalCount", is(2)) .body("data.size()", is(1)) .body("data[0].datasetVersion", equalTo("1.0")); @@ -1549,6 +1560,7 @@ public void getFileVersionDifferences() { paginatedResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("status", equalTo("OK")) + .body("totalCount", is(2)) .body("data.size()", is(0)); // Test pagination: limit=2, offset=0 (should get both items) @@ -1557,6 +1569,7 @@ public void getFileVersionDifferences() { paginatedResponse.then().assertThat() .statusCode(OK.getStatusCode()) .body("status", equalTo("OK")) + .body("totalCount", is(2)) .body("data.size()", is(2)) .body("data[0].datasetVersion", equalTo("DRAFT")) .body("data[1].datasetVersion", equalTo("1.0")); @@ -1588,6 +1601,8 @@ public void getFileVersionDifferences() { getFileDataResponse.prettyPrint(); getFileDataResponse.then().assertThat() .body("status", equalTo("OK")) + .body("totalCount", is(2)) + .body("data.size()", is(2)) .body("data[0].datasetVersion", equalTo("DRAFT")) .body("data[0].fileDifferenceSummary.file", equalTo("Replaced")) .body("data[0].datafileId", equalTo(Integer.parseInt(replacedDataFileId))) @@ -1614,6 +1629,8 @@ public void getFileVersionDifferences() { getFileDataResponse.prettyPrint(); getFileDataResponse.then().assertThat() .body("status", equalTo("OK")) + .body("totalCount", is(2)) + .body("data.size()", is(2)) .body("data[1].datasetVersion", equalTo("1.0")) .body("data[1].fileDifferenceSummary.deaccessionedReason", equalTo("Test reason")) .body("data[1].fileDifferenceSummary.file", equalTo("Added")) From 9bb5fca4b696df2b8146368b1399f5c910604644 Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 11:09:07 +0100 Subject: [PATCH 38/45] Added: release note tweaks --- ...version-summaries-pagination-and-perfomance.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md b/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md index b71ad4826a8..ca867ba9697 100644 --- a/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md +++ b/doc/release-notes/11855-version-summaries-pagination-and-perfomance.md @@ -10,15 +10,24 @@ You can now use two new query parameters to control the results: - **limit**: An integer specifying the maximum number of results to return per page. -- **offset**: An integer specifying the number of results to skip before starting to return items. This is used to navigate to different pages. +- **offset**: An integer specifying the number of results to skip before starting to return items. This is used to + navigate to different pages. ### Performance enhancements for API Version Summaries -In addition to adding pagination, we've significantly improved the performance of these endpoints by implementing more efficient database queries. +In addition to adding pagination, we've significantly improved the performance of these endpoints by implementing more +efficient database queries. -These changes address performance bottlenecks that were previously encountered, especially with datasets or files containing a large number of versions. +These changes address performance bottlenecks that were previously encountered, especially with datasets or files +containing a large number of versions. + +### Fixes for File Version Summaries API + +The implementation for file version summaries was unreliable, leading to exceptions and functional inconsistencies, as +documented in issue #11561. This functionality has been reviewed and fixed to ensure correctness and stability. ### Related issues and PRs - https://github.com/IQSS/dataverse/issues/11855 - https://github.com/IQSS/dataverse/pull/11859 +- https://github.com/IQSS/dataverse/issues/11561 \ No newline at end of file From d264183b84b07d3df75991126626a47a38290ccc Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 13:00:08 +0100 Subject: [PATCH 39/45] Added: extended IT cases for getFileVersionDifferences --- .../edu/harvard/iq/dataverse/api/FilesIT.java | 72 ++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 285f2a87223..5ebe0475adb 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1461,7 +1461,8 @@ public void getFileVersionDifferences() { // Create dataverse and dataset. Upload 1 file String dataverseAlias = createDataverseGetAlias(superUserApiToken); UtilIT.publishDataverseViaNativeApi(dataverseAlias, superUserApiToken); - Integer datasetId = createDatasetGetId(dataverseAlias, superUserApiToken); + Response createDatasetResponse = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, superUserApiToken); + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDatasetResponse); String pathToFile = "scripts/search/data/binary/trees.png"; Response addResponse = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile, superUserApiToken); addResponse.prettyPrint(); @@ -1635,6 +1636,75 @@ public void getFileVersionDifferences() { .body("data[1].fileDifferenceSummary.deaccessionedReason", equalTo("Test reason")) .body("data[1].fileDifferenceSummary.file", equalTo("Added")) .statusCode(OK.getStatusCode()); + + // Test when the file was not present on previous versions + + pathToFile = "src/test/resources/images/coffeeshop.png"; + addResponse = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile, superUserApiToken); + addResponse.then().assertThat().statusCode(OK.getStatusCode()); + dataFileId = addResponse.getBody().jsonPath().getString("data.files[0].dataFile.id"); + + getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); + getFileDataResponse.then().assertThat() + .body("status", equalTo("OK")) + .body("totalCount", is(2)) + .body("data.size()", is(2)) + .body("data[0].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[0].versionState", equalTo("DRAFT")) + .body("data[0].fileDifferenceSummary.file", equalTo("Added")) + .body("data[0].isDraft", equalTo(true)) + .body("data[0].isDeaccessioned", equalTo(false)) + .body("data[0].publishedDate", equalTo("")) + // Since the file was not present on the previous version, datafileId should be null + .body("data[1].datafileId", equalTo(null)) + .body("data[1].datasetVersion", equalTo("1.0")) + .body("data[1].versionState", equalTo("DEACCESSIONED")) + .body("data[1].fileDifferenceSummary.deaccessionedReason", equalTo("Test reason")) + // No differences to report at this stage since we are requesting differences for a newly added file + .body("data[1].fileDifferenceSummary.file", equalTo(null)) + .body("data[1].isDraft", equalTo(false)) + .body("data[1].isDeaccessioned", equalTo(true)) + .body("data[1].publishedDate", not(equalTo(""))); + + // Test when no changes were made to the file in a new version + + // First, publish the current DRAFT version + UtilIT.publishDatasetViaNativeApi(datasetId, "major", superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + + // Second, generate a new version by updating the dataset title, and publish it + String newTitle = "I am changing the title"; + String datasetPersistentId = UtilIT.getDatasetPersistentIdFromResponse(createDatasetResponse); + UtilIT.updateDatasetTitleViaSword(datasetPersistentId, newTitle, superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + UtilIT.publishDatasetViaNativeApi(datasetId, "major", superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + + getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); + getFileDataResponse.prettyPrint(); + getFileDataResponse.then().assertThat() + .body("status", equalTo("OK")) + .body("totalCount", is(3)) + .body("data.size()", is(3)) + .body("data[0].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[0].datasetVersion", equalTo("3.0")) + .body("data[0].versionState", equalTo("RELEASED")) + .body("data[0].fileDifferenceSummary", equalTo(null)) + .body("data[0].isDraft", equalTo(false)) + .body("data[0].isDeaccessioned", equalTo(false)) + .body("data[0].publishedDate", not(equalTo(""))) + // Since the file was already present in the previous version, datafileId should be present + .body("data[1].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[1].datasetVersion", equalTo("2.0")) + .body("data[1].versionState", equalTo("RELEASED")) + .body("data[1].fileDifferenceSummary.file", equalTo("Added")) + .body("data[1].isDraft", equalTo(false)) + .body("data[1].isDeaccessioned", equalTo(false)) + .body("data[1].publishedDate", not(equalTo(""))) + .body("data[2].fileDifferenceSummary.file", equalTo(null)) + .body("data[2].isDraft", equalTo(false)) + // Since the file was not present in this version, datafileId should be null + .body("data[2].datafileId", equalTo(null)) + .body("data[2].isDeaccessioned", equalTo(true)) + .body("data[2].publishedDate", not(equalTo(""))); } @Test From 175f8229f72630955bdf3f31a6e886ab581b99b9 Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 17 Oct 2025 13:01:18 +0100 Subject: [PATCH 40/45] Added: minor tweaks to native API docs about totalCount response field in version differences endpoints --- doc/sphinx-guides/source/api/native-api.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 3999d34c54e..ada3ae5a8af 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2156,6 +2156,8 @@ You can control pagination of the results using the following optional query par * ``limit``: The maximum number of version differences to return. * ``offset``: The number of version differences to skip from the beginning of the list. Used for retrieving subsequent pages of results. +To aid in pagination the JSON response also includes the total number of rows (totalCount) available. + For example, to get the second page of results, with 2 items per page, you would use ``limit=2`` and ``offset=2`` (skipping the first two results). .. code-block:: bash @@ -4340,6 +4342,8 @@ You can control pagination of the results using the following optional query par * ``limit``: The maximum number of version differences to return. * ``offset``: The number of version differences to skip from the beginning of the list. Used for retrieving subsequent pages of results. +To aid in pagination the JSON response also includes the total number of rows (totalCount) available. + For example, to get the second page of results, with 2 items per page, you would use ``limit=2`` and ``offset=2`` (skipping the first two results). .. code-block:: bash From b26a14d5182e2dfe5e77c1e092bacb786f65c27f Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 22 Oct 2025 16:29:45 +0100 Subject: [PATCH 41/45] Refactor: GetDatasetVersionSummariesCommand to include explanatory method doc and removed unused exception throws --- .../impl/GetDatasetVersionSummariesCommand.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index 802c4189e45..0cc0e20b8a5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -7,7 +7,6 @@ import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; -import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import java.util.EnumSet; import java.util.List; @@ -26,8 +25,19 @@ public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset datas this.dataset = dataset; } + /** + * Executes the command to retrieve a paginated list of dataset version summaries. + *

+ * This method first checks if the user has permission to view unpublished + * versions of the dataset. It then fetches the appropriate {@link DatasetVersion}s, + * respecting pagination parameters (limit and offset), and converts each + * version into a {@link DatasetVersionSummary}. + * + * @param ctxt The command context. + * @return A list of {@link DatasetVersionSummary} objects. + */ @Override - public List executeCommand(CommandContext ctxt) throws CommandException { + public List executeCommand(CommandContext ctxt) { boolean canViewUnpublished = ctxt.permissions().hasPermissionsFor( getRequest().getUser(), dataset, From 71edb4731e8e42e601bdc2308664ecdec466ca8e Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 22 Oct 2025 16:46:59 +0100 Subject: [PATCH 42/45] Fixed: contributor names retrieved in dataset version summaries --- .../impl/GetDatasetVersionSummariesCommand.java | 9 +++++++-- .../edu/harvard/iq/dataverse/api/DatasetsIT.java | 2 ++ .../impl/GetDatasetVersionSummariesCommandTest.java | 12 ++++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java index 0cc0e20b8a5..ab716658eff 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommand.java @@ -30,8 +30,9 @@ public GetDatasetVersionSummariesCommand(DataverseRequest request, Dataset datas *

* This method first checks if the user has permission to view unpublished * versions of the dataset. It then fetches the appropriate {@link DatasetVersion}s, - * respecting pagination parameters (limit and offset), and converts each - * version into a {@link DatasetVersionSummary}. + * respecting pagination parameters (limit and offset). Each version is then + * enriched with its contributor names before being converted into a + * {@link DatasetVersionSummary}. * * @param ctxt The command context. * @return A list of {@link DatasetVersionSummary} objects. @@ -52,6 +53,10 @@ public List executeCommand(CommandContext ctxt) { true ); + for (DatasetVersion version : versions) { + version.setContributorNames(ctxt.datasetVersion().getContributorsNames(version)); + } + return versions.stream() .map(DatasetVersionSummary::from) .flatMap(Optional::stream) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index c648326e598..62e8dfdc59e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -6537,8 +6537,10 @@ public void testSummaryDatasetVersionsDifferencesAPI() throws InterruptedExcepti .body("totalCount", is(2)) .body("data.size()", is(2)) .body("data[1].versionNumber", equalTo("1.0")) + .body("data[1].contributors", notNullValue()) .body("data[1].summary", equalTo("firstPublished")) .body("data[0].versionNumber", equalTo("DRAFT")) + .body("data[0].contributors", notNullValue()) .body("data[0].summary.'Citation Metadata'.Author.added", equalTo(2)) .body("data[0].summary.'Citation Metadata'.Subject.added", equalTo(2)) .body("data[0].summary.'Additional Citation Metadata'.changed", equalTo(0)) diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java index e47deb86c8f..b86188f440e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/command/impl/GetDatasetVersionSummariesCommandTest.java @@ -144,13 +144,17 @@ void execute_should_pass_pagination_parameters_correctly() throws CommandExcepti } @Test - @DisplayName("execute should correctly convert DatasetVersion list to DatasetVersionSummary list") - void execute_should_convert_versions_to_summaries() throws CommandException { + @DisplayName("execute should enrich contributors and convert versions to summaries") + void execute_should_enrich_contributors_and_convert_to_summaries() throws CommandException { // Arrange setupMocksForSuccessfulExecution(); when(versionServiceMock.findVersions(anyLong(), any(), any(), anyBoolean(), anyBoolean())) .thenReturn(List.of(versionMock)); + // Arrange: Mock contributor names retrieval + String expectedContributors = "Contributor"; + when(versionServiceMock.getContributorsNames(versionMock)).thenReturn(expectedContributors); + // Arrange: Prepare a dummy summary object DatasetVersionSummary dummySummary = new DatasetVersionSummary(1L, "V1", null, null, null, null); @@ -165,6 +169,10 @@ void execute_should_convert_versions_to_summaries() throws CommandException { List result = command.execute(contextMock); // Assert + verify(versionServiceMock).getContributorsNames(versionMock); + verify(versionMock).setContributorNames(expectedContributors); + + // Verify final conversion assertThat(result) .isNotNull() .hasSize(1) From 2515f786e19551488bd9c243e4804a1b7011d72b Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 22 Oct 2025 19:00:39 +0100 Subject: [PATCH 43/45] Fixed: correctly sending fileDifferenceSummary.FileAccess in file version summaries --- .../GetFileVersionDifferencesCommand.java | 2 +- .../edu/harvard/iq/dataverse/api/FilesIT.java | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java index d822fa9455b..459ab1095e0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetFileVersionDifferencesCommand.java @@ -89,7 +89,7 @@ private FileVersionDifference buildDifference(CommandContext ctxt, VersionedFile */ private FileVersionDifference createDifferenceForExistingFile(CommandContext ctxt, FileMetadata current, FileMetadata previous) { current.setContributorNames(ctxt.datasetVersion().getContributorsNames(current.getDatasetVersion())); - return new FileVersionDifference(current, previous, false); + return new FileVersionDifference(current, previous, true); } /** diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 5ebe0475adb..32f6e2a6381 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1705,6 +1705,41 @@ public void getFileVersionDifferences() { .body("data[2].datafileId", equalTo(null)) .body("data[2].isDeaccessioned", equalTo(true)) .body("data[2].publishedDate", not(equalTo(""))); + + // Test FileAccess restricted + + UtilIT.allowAccessRequests(datasetPersistentId, true, superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + UtilIT.restrictFile(dataFileId, true, superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); + getFileDataResponse.then().assertThat() + .body("status", equalTo("OK")) + .body("totalCount", is(4)) + .body("data.size()", is(4)) + .body("data[0].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[0].versionState", equalTo("DRAFT")) + .body("data[0].fileDifferenceSummary.FileAccess", equalTo("Restricted")) + .body("data[0].isDraft", equalTo(true)) + .body("data[0].isDeaccessioned", equalTo(false)) + .body("data[0].publishedDate", equalTo("")); + + // Test FileAccess unrestricted + + UtilIT.publishDatasetViaNativeApi(datasetId, "major", superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + + UtilIT.restrictFile(dataFileId, false, superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); + getFileDataResponse.then().assertThat() + .body("status", equalTo("OK")) + .body("totalCount", is(5)) + .body("data.size()", is(5)) + .body("data[0].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[0].versionState", equalTo("DRAFT")) + .body("data[0].fileDifferenceSummary.FileAccess", equalTo("Unrestricted")) + .body("data[0].isDraft", equalTo(true)) + .body("data[0].isDeaccessioned", equalTo(false)) + .body("data[0].publishedDate", equalTo("")); } @Test From b37941f1fe69ed4129d4ae11d2a2e75414b7528e Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 22 Oct 2025 19:25:11 +0100 Subject: [PATCH 44/45] Added: IT test cases for FileTags and FileMetadata updates in file version summaries --- .../edu/harvard/iq/dataverse/api/FilesIT.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 32f6e2a6381..175d93b57a6 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1709,6 +1709,7 @@ public void getFileVersionDifferences() { // Test FileAccess restricted UtilIT.allowAccessRequests(datasetPersistentId, true, superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); + UtilIT.restrictFile(dataFileId, true, superUserApiToken).then().assertThat().statusCode(OK.getStatusCode()); getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); getFileDataResponse.then().assertThat() @@ -1740,6 +1741,48 @@ public void getFileVersionDifferences() { .body("data[0].isDraft", equalTo(true)) .body("data[0].isDeaccessioned", equalTo(false)) .body("data[0].publishedDate", equalTo("")); + + // Test FileMetadata update + + JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder() + .add("label", "new_name.png"); + UtilIT.updateFileMetadata(dataFileId, updateFileMetadata.build().toString(), superUserApiToken).then().statusCode(OK.getStatusCode()); + + getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); + getFileDataResponse.then().assertThat() + .body("status", equalTo("OK")) + .body("totalCount", is(5)) + .body("data.size()", is(5)) + .body("data[0].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[0].versionState", equalTo("DRAFT")) + .body("data[0].fileDifferenceSummary.FileAccess", equalTo("Unrestricted")) + .body("data[0].fileDifferenceSummary.FileMetadata[0].name", equalTo("File Name")) + .body("data[0].fileDifferenceSummary.FileMetadata[0].action", equalTo("Changed")) + .body("data[0].isDraft", equalTo(true)) + .body("data[0].isDeaccessioned", equalTo(false)) + .body("data[0].publishedDate", equalTo("")); + + // Test FileTags update + + Response setFileCategoriesResponse = UtilIT.setFileCategories(dataFileId, superUserApiToken, List.of("Category")); + setFileCategoriesResponse.then().assertThat().statusCode(OK.getStatusCode()); + + getFileDataResponse = UtilIT.getFileVersionDifferences(dataFileId, superUserApiToken); + getFileDataResponse.then().assertThat() + .body("status", equalTo("OK")) + .body("totalCount", is(5)) + .body("data.size()", is(5)) + .body("data[0].datafileId", equalTo(Integer.parseInt(dataFileId))) + .body("data[0].datasetVersion", equalTo("DRAFT")) + .body("data[0].versionState", equalTo("DRAFT")) + .body("data[0].fileDifferenceSummary.FileAccess", equalTo("Unrestricted")) + .body("data[0].fileDifferenceSummary.FileMetadata[0].name", equalTo("File Name")) + .body("data[0].fileDifferenceSummary.FileMetadata[0].action", equalTo("Changed")) + .body("data[0].fileDifferenceSummary.FileTags.Added", equalTo(1)) + .body("data[0].isDraft", equalTo(true)) + .body("data[0].isDeaccessioned", equalTo(false)) + .body("data[0].publishedDate", equalTo("")); } @Test From 06f35070518d730962c87e745b4b4224bcab0aad Mon Sep 17 00:00:00 2001 From: GPortas Date: Thu, 23 Oct 2025 09:32:29 +0100 Subject: [PATCH 45/45] Fixed: reverted method from public to private --- .../edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java index 368bc1b0628..4d408a72c8c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileMetadataVersionsHelper.java @@ -93,7 +93,7 @@ private FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, FileMeta } //TODO: this could use some refactoring to cut down on the number of for loops! - public FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, DatasetVersion currentversion) { + private FileMetadata getPreviousFileMetadata(FileMetadata fileMetadata, DatasetVersion currentversion) { List allfiles = allRelatedFiles(fileMetadata); boolean foundCurrent = false; DatasetVersion priorVersion = null;