Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return deaccessioned version of dataset in more cases #10191

Merged
merged 18 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions doc/sphinx-guides/source/api/changelog.rst
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
API Changelog
=============
API Changelog (Breaking Changes)
================================

This API changelog is experimental and we would love feedback on its usefulness. Its primary purpose is to inform API developers of any breaking changes. (We try not ship any backward incompatible changes, but it happens.) To see a list of new APIs and backward-compatible changes to existing API, please see each version's release notes at https://github.com/IQSS/dataverse/releases

.. contents:: |toctitle|
:local:
:depth: 1

v6.1
v6.2
----

New
~~~
- **/api/dataverses/{id}/datasetSchema**: See :ref:`get-dataset-json-schema`.
- **/api/dataverses/{id}/validateDatasetJson**: See :ref:`validate-dataset-json`.
- **/api/datasets/{id}/versions/{versionId}**: The includeFiles parameter has been renamed to excludeFiles. The default behavior remains the same, which is to include files. However, when excludeFiles is set to true, the files will be excluded. A bug that caused the API to only return a deaccessioned dataset if the user had edit privileges has been fixed.
- **/api/datasets/{id}/versions**: The includeFiles parameter has been renamed to excludeFiles. The default behavior remains the same, which is to include files. However, when excludeFiles is set to true, the files will be excluded.


New
~~~
- **/api/admin/clearThumbnailFailureFlag**: See :ref:`thumbnail_reset`.
- **/api/admin/downloadTmpFile**: See :ref:`download-file-from-tmp`.
v6.1
----

Changes
~~~~~~~
- **/api/datasets/{id}/versions/{versionId}/citation**: This endpoint now accepts a new boolean optional query parameter "includeDeaccessioned", which, if enabled, causes the endpoint to consider deaccessioned versions when searching for versions to obtain the citation. See :ref:`get-citation`.
- The metadata field "Alternative Title" now supports multiple values so you must pass an array rather than a string when populating that field via API. See https://github.com/IQSS/dataverse/pull/9440

v6.0
----

Changes
~~~~~~~
- **/api/access/datafile**: When a null or invalid API token is provided to download a public (non-restricted) file with this API call, it will result on a ``401`` error response. Previously, the download was allowed (``200`` response). Please note that we noticed this change sometime between 5.9 and 6.0. If you can help us pinpoint the exact version (or commit!), please get in touch. See :doc:`dataaccess`.
- **/api/access/datafile**: When a null or invalid API token is provided to download a public (non-restricted) file with this API call, it will result on a ``401`` error response. Previously, the download was allowed (``200`` response). Please note that we noticed this change sometime between 5.9 and 6.0. If you can help us pinpoint the exact version (or commit!), please get in touch. See :doc:`dataaccess`.
55 changes: 35 additions & 20 deletions src/main/java/edu/harvard/iq/dataverse/api/Datasets.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@
import edu.harvard.iq.dataverse.util.json.JsonUtil;
import edu.harvard.iq.dataverse.util.SignpostingResources;
import edu.harvard.iq.dataverse.search.IndexServiceBean;

import static edu.harvard.iq.dataverse.api.ApiConstants.*;
import static edu.harvard.iq.dataverse.util.json.JsonPrinter.*;
import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder;
Expand All @@ -106,9 +105,7 @@
import edu.harvard.iq.dataverse.workflow.WorkflowContext;
import edu.harvard.iq.dataverse.workflow.WorkflowServiceBean;
import edu.harvard.iq.dataverse.workflow.WorkflowContext.TriggerType;

import edu.harvard.iq.dataverse.globus.GlobusServiceBean;

import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
Expand All @@ -127,7 +124,6 @@
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import jakarta.ejb.EJB;
import jakarta.ejb.EJBException;
import jakarta.inject.Inject;
Expand All @@ -151,7 +147,6 @@
import jakarta.ws.rs.core.*;
import jakarta.ws.rs.core.Response.Status;
import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST;

import org.apache.commons.lang3.StringUtils;
import org.glassfish.jersey.media.multipart.FormDataBodyPart;
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
Expand Down Expand Up @@ -268,12 +263,10 @@ public Response getDataset(@Context ContainerRequestContext crc, @PathParam("id"
}, getRequestUser(crc));
}

// TODO:
// This API call should, ideally, call findUserOrDie() and the GetDatasetCommand
// to obtain the dataset that we are trying to export - which would handle
// Auth in the process... For now, Auth isn't necessary - since export ONLY
// WORKS on published datasets, which are open to the world. -- L.A. 4.5

@GET
@Path("/export")
@Produces({"application/xml", "application/json", "application/html", "application/ld+json" })
Expand Down Expand Up @@ -470,14 +463,15 @@ public Response useDefaultCitationDate(@Context ContainerRequestContext crc, @Pa
@GET
@AuthRequired
@Path("{id}/versions")
public Response listVersions(@Context ContainerRequestContext crc, @PathParam("id") String id, @QueryParam("includeFiles") Boolean includeFiles, @QueryParam("limit") Integer limit, @QueryParam("offset") Integer offset) {
public Response listVersions(@Context ContainerRequestContext crc, @PathParam("id") String id, @QueryParam("excludeFiles") Boolean excludeFiles, @QueryParam("limit") Integer limit, @QueryParam("offset") Integer offset) {

return response( req -> {
Dataset dataset = findDatasetOrDie(id);
Boolean deepLookup = excludeFiles == null ? true : !excludeFiles;

return ok( execCommand( new ListVersionsCommand(req, dataset, offset, limit, (includeFiles == null ? true : includeFiles)) )
return ok( execCommand( new ListVersionsCommand(req, dataset, offset, limit, deepLookup) )
.stream()
.map( d -> json(d, includeFiles == null ? true : includeFiles) )
.map( d -> json(d, deepLookup) )
.collect(toJsonArray()));
}, getRequestUser(crc));
}
Expand All @@ -488,21 +482,27 @@ public Response listVersions(@Context ContainerRequestContext crc, @PathParam("i
public Response getVersion(@Context ContainerRequestContext crc,
@PathParam("id") String datasetId,
@PathParam("versionId") String versionId,
@QueryParam("includeFiles") Boolean includeFiles,
@QueryParam("excludeFiles") Boolean excludeFiles,
@QueryParam("includeDeaccessioned") boolean includeDeaccessioned,
@Context UriInfo uriInfo,
@Context HttpHeaders headers) {
return response( req -> {
DatasetVersion dsv = getDatasetVersionOrDie(req, versionId, findDatasetOrDie(datasetId), uriInfo, headers, includeDeaccessioned);


//If excludeFiles is null the default is to provide the files and because of this we need to check permissions.
boolean checkPerms = excludeFiles == null ? true : !excludeFiles;

Dataset dst = findDatasetOrDie(datasetId);
DatasetVersion dsv = getDatasetVersionOrDie(req, versionId, dst, uriInfo, headers, includeDeaccessioned, checkPerms);

if (dsv == null || dsv.getId() == null) {
return notFound("Dataset version not found");
}

if (includeFiles == null ? true : includeFiles) {
if (excludeFiles == null ? true : !excludeFiles) {
dsv = datasetversionService.findDeep(dsv.getId());
}
return ok(json(dsv, includeFiles == null ? true : includeFiles));
return ok(json(dsv, excludeFiles == null ? true : !excludeFiles));
}, getRequestUser(crc));
}

Expand Down Expand Up @@ -2769,11 +2769,26 @@ public static <T> T handleVersion(String versionId, DsVersionHandler<T> hdl)
}
}

/*
* includeDeaccessioned default to false and checkPermsWhenDeaccessioned to false. Use it only when you are sure that the you don't need to work with
* a deaccessioned dataset.
*/
private DatasetVersion getDatasetVersionOrDie(final DataverseRequest req, String versionNumber, final Dataset ds, UriInfo uriInfo, HttpHeaders headers) throws WrappedResponse {
return getDatasetVersionOrDie(req, versionNumber, ds, uriInfo, headers, false);
//The checkPerms was added to check the permissions ONLY when the dataset is deaccessioned.
return getDatasetVersionOrDie(req, versionNumber, ds, uriInfo, headers, false, false);
jp-tosca marked this conversation as resolved.
Show resolved Hide resolved
}

/*
* checkPermsWhenDeaccessioned default to true. Be aware that the version will be only be obtainable if the user has edit permissions.
*/
private DatasetVersion getDatasetVersionOrDie(final DataverseRequest req, String versionNumber, final Dataset ds, UriInfo uriInfo, HttpHeaders headers, boolean includeDeaccessioned) throws WrappedResponse{
return getDatasetVersionOrDie(req, versionNumber, ds, uriInfo, headers, includeDeaccessioned, true);
}

private DatasetVersion getDatasetVersionOrDie(final DataverseRequest req, String versionNumber, final Dataset ds, UriInfo uriInfo, HttpHeaders headers, boolean includeDeaccessioned) throws WrappedResponse {
/*
* Will allow to define when the permissions should be checked when a deaccesioned dataset is requested. If the user doesn't have edit permissions will result in an error.
*/
private DatasetVersion getDatasetVersionOrDie(final DataverseRequest req, String versionNumber, final Dataset ds, UriInfo uriInfo, HttpHeaders headers, boolean includeDeaccessioned, boolean checkPermsWhenDeaccessioned) throws WrappedResponse {
DatasetVersion dsv = execCommand(handleVersion(versionNumber, new DsVersionHandler<Command<DatasetVersion>>() {

@Override
Expand All @@ -2788,12 +2803,12 @@ public Command<DatasetVersion> handleDraft() {

@Override
public Command<DatasetVersion> handleSpecific(long major, long minor) {
return new GetSpecificPublishedDatasetVersionCommand(req, ds, major, minor, includeDeaccessioned);
return new GetSpecificPublishedDatasetVersionCommand(req, ds, major, minor, includeDeaccessioned, checkPermsWhenDeaccessioned);
}

@Override
public Command<DatasetVersion> handleLatestPublished() {
return new GetLatestPublishedDatasetVersionCommand(req, ds, includeDeaccessioned);
return new GetLatestPublishedDatasetVersionCommand(req, ds, includeDeaccessioned, checkPermsWhenDeaccessioned);
}
}));
if (dsv == null || dsv.getId() == null) {
Expand Down Expand Up @@ -4015,7 +4030,7 @@ public Response getDatasetVersionCitation(@Context ContainerRequestContext crc,
@Context UriInfo uriInfo,
@Context HttpHeaders headers) {
return response(req -> ok(
getDatasetVersionOrDie(req, versionId, findDatasetOrDie(datasetId), uriInfo, headers, includeDeaccessioned).getCitation(true, false)), getRequestUser(crc));
getDatasetVersionOrDie(req, versionId, findDatasetOrDie(datasetId), uriInfo, headers, includeDeaccessioned, false).getCitation(true, false)), getRequestUser(crc));
}

@POST
Expand All @@ -4026,7 +4041,7 @@ public Response deaccessionDataset(@Context ContainerRequestContext crc, @PathPa
return badRequest(BundleUtil.getStringFromBundle("datasets.api.deaccessionDataset.invalid.version.identifier.error", List.of(DS_VERSION_LATEST_PUBLISHED)));
}
return response(req -> {
DatasetVersion datasetVersion = getDatasetVersionOrDie(req, versionId, findDatasetOrDie(datasetId), uriInfo, headers, false);
DatasetVersion datasetVersion = getDatasetVersionOrDie(req, versionId, findDatasetOrDie(datasetId), uriInfo, headers);
try {
JsonObject jsonObject = JsonUtil.getJsonObject(jsonBody);
datasetVersion.setVersionNote(jsonObject.getString("deaccessionReason"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException {
if (ds.getLatestVersion().isDraft() && ctxt.permissions().requestOn(getRequest(), ds).has(Permission.ViewUnpublishedDataset)) {
return ctxt.engine().submit(new GetDraftDatasetVersionCommand(getRequest(), ds));
}
return ctxt.engine().submit(new GetLatestPublishedDatasetVersionCommand(getRequest(), ds, includeDeaccessioned));
return ctxt.engine().submit(new GetLatestPublishedDatasetVersionCommand(getRequest(), ds, includeDeaccessioned, true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,30 @@
public class GetLatestPublishedDatasetVersionCommand extends AbstractCommand<DatasetVersion> {
private final Dataset ds;
private final boolean includeDeaccessioned;
private boolean checkPerms;

public GetLatestPublishedDatasetVersionCommand(DataverseRequest aRequest, Dataset anAffectedDataset) {
this(aRequest, anAffectedDataset, false);
this(aRequest, anAffectedDataset, false, false);
}

public GetLatestPublishedDatasetVersionCommand(DataverseRequest aRequest, Dataset anAffectedDataset, boolean includeDeaccessioned) {
public GetLatestPublishedDatasetVersionCommand(DataverseRequest aRequest, Dataset anAffectedDataset, boolean includeDeaccessioned, boolean checkPerms) {
super(aRequest, anAffectedDataset);
ds = anAffectedDataset;
this.includeDeaccessioned = includeDeaccessioned;
this.checkPerms = checkPerms;
}

@Override
public DatasetVersion execute(CommandContext ctxt) throws CommandException {

for (DatasetVersion dsv : ds.getVersions()) {
if (dsv.isReleased() || (includeDeaccessioned && dsv.isDeaccessioned() && ctxt.permissions().requestOn(getRequest(), ds).has(Permission.EditDataset))) {
if (dsv.isReleased() || (includeDeaccessioned && dsv.isDeaccessioned())) {

if(dsv.isDeaccessioned() && checkPerms){
if(!ctxt.permissions().requestOn(getRequest(), ds).has(Permission.EditDataset)){
return null;
}
}
return dsv;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,42 @@ public class GetSpecificPublishedDatasetVersionCommand extends AbstractCommand<D
private final long majorVersion;
private final long minorVersion;
private boolean includeDeaccessioned;
private boolean checkPerms;

public GetSpecificPublishedDatasetVersionCommand(DataverseRequest aRequest, Dataset anAffectedDataset, long majorVersionNum, long minorVersionNum) {
this(aRequest, anAffectedDataset, majorVersionNum, minorVersionNum, false);
this(aRequest, anAffectedDataset, majorVersionNum, minorVersionNum, false, false);
}

public GetSpecificPublishedDatasetVersionCommand(DataverseRequest aRequest, Dataset anAffectedDataset, long majorVersionNum, long minorVersionNum, boolean includeDeaccessioned) {


public GetSpecificPublishedDatasetVersionCommand(DataverseRequest aRequest, Dataset anAffectedDataset, long majorVersionNum, long minorVersionNum, boolean includeDeaccessioned, boolean checkPerms) {
super(aRequest, anAffectedDataset);
ds = anAffectedDataset;
majorVersion = majorVersionNum;
minorVersion = minorVersionNum;
this.includeDeaccessioned = includeDeaccessioned;
this.checkPerms = checkPerms;
}


@Override
public DatasetVersion execute(CommandContext ctxt) throws CommandException {

for (DatasetVersion dsv : ds.getVersions()) {
if (dsv.isReleased() || (includeDeaccessioned && dsv.isDeaccessioned() && ctxt.permissions().requestOn(getRequest(), ds).has(Permission.EditDataset))) {
if (dsv.isReleased() || (includeDeaccessioned && dsv.isDeaccessioned())) {

if(dsv.isDeaccessioned() && checkPerms){
if(!ctxt.permissions().requestOn(getRequest(), ds).has(Permission.EditDataset)){
return null;
}
}

if (dsv.getVersionNumber().equals(majorVersion) && dsv.getMinorVersionNumber().equals(minorVersion)) {
return dsv;
}
}
}
return null;
}

}
Loading