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 all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion doc/sphinx-guides/source/api/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ This API changelog is experimental and we would love feedback on its usefulness.
:local:
:depth: 1

v6.2
----

- **/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.

v6.1
----

Expand All @@ -15,4 +21,4 @@ v6.1
v6.0
----

- **/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 @@ -99,7 +99,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 @@ -108,10 +107,8 @@
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 edu.harvard.iq.dataverse.globus.GlobusUtil;

import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
Expand All @@ -130,7 +127,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 @@ -154,7 +150,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 @@ -271,12 +266,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 @@ -473,14 +466,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 @@ -491,21 +485,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 @@ -2772,11 +2772,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 @@ -2791,12 +2806,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 @@ -4503,7 +4518,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 @@ -4514,7 +4529,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;
}

}