Support deletion of saved searches #10198
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ### Saved search deletion | ||
|
|
||
| Saved search can now be removed using API `/api/admin/savedsearches/$id`. See PR #10198. | ||
| This is reflected in the [Saved Search Native API section](https://guides.dataverse.org/en/latest/api/native-api.html#saved-search) of the Guide. | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5745,8 +5745,7 @@ The ``$identifier`` should start with an ``@`` if it's a user. Groups start with | |
| Saved Search | ||
| ~~~~~~~~~~~~ | ||
|
|
||
|
There was a problem hiding this comment. There's a conversation going on in #10628 about how these docs used to explain to people that they might have to wait up to 24 hours for their saved search to show results. Should we add that back as part of this pull request? There was a problem hiding this comment. Updated with The update of all saved search is run by a timer once a week (See :ref: |
||
| The Saved Search, Linked Dataverses, and Linked Datasets features are only accessible to superusers except for Linking a dataset. The following API endpoints were added to help people with access to the “admin” API make use of these features in their current form, keep in mind that they are partially experimental. | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| List all saved searches. :: | ||
|
|
||
| GET http://$SERVER/api/admin/savedsearches/list | ||
|
|
@@ -5755,6 +5754,10 @@ List a saved search by database id. :: | |
|
|
||
| GET http://$SERVER/api/admin/savedsearches/$id | ||
|
|
||
| Delete a saved search by database id. The ``unlink=true`` query parameter unlink links (Linked dataset or Dataverse collection) related to the deleted saved search. This parameter should be well considered as you cannot know if the saved search created the links or if someone else did via other API. Also, it may be followed ``/makelinks/all`` depending on the need if other saved searches could recreate some deleted links or by reindexing some Dataverse or Dataset. :: | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| DELETE http://$SERVER/api/admin/savedsearches/$id?unlink=true | ||
|
|
||
| Execute a saved search by database id and make links to Dataverse collections and datasets that are found. The JSON response indicates which Dataverse collections and datasets were newly linked versus already linked. The ``debug=true`` query parameter adds to the JSON response extra information about the saved search being executed (which you could also get by listing the saved search). :: | ||
|
|
||
| PUT http://$SERVER/api/admin/savedsearches/makelinks/$id?debug=true | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package edu.harvard.iq.dataverse.api; | ||
|
|
||
| import edu.harvard.iq.dataverse.Dataverse; | ||
| import edu.harvard.iq.dataverse.api.auth.AuthRequired; | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; | ||
| import edu.harvard.iq.dataverse.engine.command.DataverseRequest; | ||
| import edu.harvard.iq.dataverse.engine.command.exception.CommandException; | ||
|
|
@@ -24,6 +25,8 @@ | |
| import jakarta.ws.rs.Path; | ||
| import jakarta.ws.rs.PathParam; | ||
| import jakarta.ws.rs.QueryParam; | ||
| import jakarta.ws.rs.container.ContainerRequestContext; | ||
| import jakarta.ws.rs.core.Context; | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| import jakarta.ws.rs.core.Response; | ||
| import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; | ||
| import static jakarta.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; | ||
|
|
@@ -46,22 +49,46 @@ public Response meta() { | |
| } | ||
|
|
||
| @GET | ||
| @AuthRequired | ||
| @Path("list") | ||
| public Response list(@Context ContainerRequestContext crc) { | ||
|
|
||
| AuthenticatedUser superuser = null; | ||
| try { | ||
| superuser = getRequestAuthenticatedUserOrDie(crc); | ||
| } catch (WrappedResponse wr) { | ||
| return wr.getResponse(); | ||
| } | ||
| if (superuser == null || !superuser.isSuperuser()) { | ||
| return error(Response.Status.UNAUTHORIZED, "Superusers only."); | ||
|
There was a problem hiding this comment. Do we need this backward-incompatible change? I'm only going to leave this comment once but it applies to all endpoints that formerly were not superuser-only. @luddaniel I know we talked about this a bit at https://dataverse.zulipchat.com/#narrow/stream/379673-dev/topic/admin.20API.20and.20superuser.20authorization/near/443079852 but my understanding was that we were talking theoretically about the future. I didn't realize you were thinking about making backward-incompatible changes now, in the name of consistency. We should probably pause and get rough consensus from more of the core team and decide on the timing for introducing these changes, if at all. |
||
| } | ||
|
|
||
| JsonArrayBuilder savedSearchesBuilder = Json.createArrayBuilder(); | ||
| List<SavedSearch> savedSearches = savedSearchSvc.findAll(); | ||
| for (SavedSearch savedSearch : savedSearches) { | ||
| JsonObjectBuilder thisSavedSearch = toJson(savedSearch); | ||
| savedSearchesBuilder.add(thisSavedSearch); | ||
| } | ||
| JsonObjectBuilder response = Json.createObjectBuilder(); | ||
| response.add("savedSearches", savedSearchesBuilder); | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return ok(response); | ||
| } | ||
|
|
||
| @GET | ||
| @AuthRequired | ||
| @Path("{id}") | ||
| public Response show(@Context ContainerRequestContext crc, @PathParam("id") long id) { | ||
|
|
||
| AuthenticatedUser superuser = null; | ||
| try { | ||
| superuser = getRequestAuthenticatedUserOrDie(crc); | ||
| } catch (WrappedResponse wr) { | ||
| return wr.getResponse(); | ||
| } | ||
| if (superuser == null || !superuser.isSuperuser()) { | ||
| return error(Response.Status.UNAUTHORIZED, "Superusers only."); | ||
| } | ||
|
|
||
| SavedSearch savedSearch = savedSearchSvc.find(id); | ||
| if (savedSearch != null) { | ||
| JsonObjectBuilder response = toJson(savedSearch); | ||
|
|
@@ -89,7 +116,18 @@ private JsonObjectBuilder toJson(SavedSearch savedSearch) { | |
| } | ||
|
|
||
| @POST | ||
| @AuthRequired | ||
| public Response add(@Context ContainerRequestContext crc, JsonObject body) { | ||
|
|
||
| AuthenticatedUser superuser = null; | ||
| try { | ||
| superuser = getRequestAuthenticatedUserOrDie(crc); | ||
| } catch (WrappedResponse wr) { | ||
| return wr.getResponse(); | ||
| } | ||
| if (superuser == null || !superuser.isSuperuser()) { | ||
| return error(Response.Status.UNAUTHORIZED, "Superusers only."); | ||
| } | ||
|
|
||
| if (body == null) { | ||
| return error(BAD_REQUEST, "JSON is expected."); | ||
|
|
@@ -159,7 +197,7 @@ public Response add(JsonObject body) { | |
|
|
||
| try { | ||
| SavedSearch persistedSavedSearch = savedSearchSvc.add(toPersist); | ||
| return ok("Added: " + persistedSavedSearch, Json.createObjectBuilder().add("id", persistedSavedSearch.getId())); | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } catch (EJBException ex) { | ||
| StringBuilder errors = new StringBuilder(); | ||
| Throwable throwable = ex.getCause(); | ||
|
|
@@ -172,17 +210,31 @@ public Response add(JsonObject body) { | |
| } | ||
|
|
||
| @DELETE | ||
| @AuthRequired | ||
| @Path("{id}") | ||
| public Response delete(@Context ContainerRequestContext crc, @PathParam("id") long doomedId, @QueryParam("unlink") boolean unlink) { | ||
|
|
||
| AuthenticatedUser superuser = null; | ||
| try { | ||
| superuser = getRequestAuthenticatedUserOrDie(crc); | ||
| } catch (WrappedResponse wr) { | ||
| return wr.getResponse(); | ||
| } | ||
| if (superuser == null || !superuser.isSuperuser()) { | ||
| return error(Response.Status.UNAUTHORIZED, "Superusers only."); | ||
| } | ||
|
|
||
| SavedSearch doomed = savedSearchSvc.find(doomedId); | ||
| if (doomed == null) { | ||
| return error(NOT_FOUND, "Could not find saved search id " + doomedId); | ||
| } | ||
| boolean wasDeleted; | ||
| try { | ||
| wasDeleted = savedSearchSvc.delete(doomedId, unlink); | ||
|
There was a problem hiding this comment. Do we want to put this in a command? There's a "CreateSavedSearchCommand" There was a problem hiding this comment. Hey @sekmiller, if my understanding is correct I note that It could be prematured to create a There was a problem hiding this comment. One reason that I suggested the command was because commands also check for permissions. I didn't notice that this is a superuser only feature - so that permission check wouldn't be available in the command. |
||
| } catch (Exception e) { | ||
| return error(INTERNAL_SERVER_ERROR, "Problem while trying to unlink links of saved search id " + doomedId); | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (wasDeleted) { | ||
| return ok(Json.createObjectBuilder().add("Deleted", doomedId)); | ||
| } else { | ||
|
|
@@ -191,8 +243,20 @@ public Response delete(@PathParam("id") long doomedId) { | |
| } | ||
|
|
||
| @PUT | ||
| @AuthRequired | ||
| @Path("makelinks/all") | ||
| public Response makeLinksForAllSavedSearches(@Context ContainerRequestContext crc, @QueryParam("debug") boolean debug) { | ||
|
|
||
| AuthenticatedUser superuser = null; | ||
| try { | ||
| superuser = getRequestAuthenticatedUserOrDie(crc); | ||
| } catch (WrappedResponse wr) { | ||
| return wr.getResponse(); | ||
| } | ||
| if (superuser == null || !superuser.isSuperuser()) { | ||
| return error(Response.Status.UNAUTHORIZED, "Superusers only."); | ||
| } | ||
|
|
||
| JsonObjectBuilder makeLinksResponse; | ||
| try { | ||
| makeLinksResponse = savedSearchSvc.makeLinksForAllSavedSearches(debug); | ||
|
|
@@ -205,8 +269,20 @@ public Response makeLinksForAllSavedSearches(@QueryParam("debug") boolean debug) | |
| } | ||
|
|
||
| @PUT | ||
| @AuthRequired | ||
| @Path("makelinks/{id}") | ||
| public Response makeLinksForSingleSavedSearch(@Context ContainerRequestContext crc, @PathParam("id") long savedSearchIdToLookUp, @QueryParam("debug") boolean debug) { | ||
|
|
||
| AuthenticatedUser superuser = null; | ||
| try { | ||
| superuser = getRequestAuthenticatedUserOrDie(crc); | ||
| } catch (WrappedResponse wr) { | ||
| return wr.getResponse(); | ||
| } | ||
| if (superuser == null || !superuser.isSuperuser()) { | ||
| return error(Response.Status.UNAUTHORIZED, "Superusers only."); | ||
| } | ||
|
|
||
| SavedSearch savedSearchToMakeLinksFor = savedSearchSvc.find(savedSearchIdToLookUp); | ||
| if (savedSearchToMakeLinksFor == null) { | ||
| return error(BAD_REQUEST, "Count not find saved search id " + savedSearchIdToLookUp); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to leave comments below about backward incompatible changes. I'm pretty sure my preference is to NOT introduce any backward-incompatible changes but if we do the should be listed in
doc/sphinx-guides/source/api/changelog.rstand the release notes should give an overview of backward-incompatible changes and tell people to check that changelog.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I removed every authentication requirement for
/api/admin/savedsearches