Skip to content

Commit

Permalink
Saved Search: better fix than 6f8ee3a #2718
Browse files Browse the repository at this point in the history 
- Remove `@Context HttpServletRequest httpRequest` from
  SavedSearchServiceBean to avoid stale state.
- Pass a DataverseRequest object to the "make links" method rather than
  having the method construct the DataverseRequest object.
- Set the HttpServletRequest in the DataverseRequest object to null for
  better security.
  • Loading branch information
@pdurbin
pdurbin committed Nov 24, 2015
1 parent 2598873 commit c0e60b6
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 35 deletions.
4 changes: 3 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/DataversePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
import edu.harvard.iq.dataverse.authorization.users.User;
import edu.harvard.iq.dataverse.engine.command.Command;
import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
import edu.harvard.iq.dataverse.engine.command.impl.CreateDataverseCommand;
import edu.harvard.iq.dataverse.engine.command.impl.CreateSavedSearchCommand;
Expand Down Expand Up @@ -762,7 +763,8 @@ public String saveLinkedDataverse() {
try {
// create links (does indexing) right now (might be expensive)
boolean debug = false;
savedSearchService.makeLinksForSingleSavedSearch(savedSearchOfChildren, debug);
DataverseRequest dataverseRequest = new DataverseRequest(savedSearchCreator, SavedSearchServiceBean.getHttpServletRequest());
savedSearchService.makeLinksForSingleSavedSearch(dataverseRequest, savedSearchOfChildren, debug);
//JsfHelper.addSuccessMessage(dataverse.getDisplayName() + " has been successfully linked to " + linkingDataverse.getDisplayName());
List<String> arguments = new ArrayList();
arguments.add(dataverse.getDisplayName());
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/api/SavedSearches.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

import edu.harvard.iq.dataverse.Dataverse;
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;
import edu.harvard.iq.dataverse.search.SearchException;
import edu.harvard.iq.dataverse.search.savedsearch.SavedSearch;
import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchFilterQuery;
import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean;
import java.util.ArrayList;
import java.util.List;
import java.util.logging.Logger;
Expand Down Expand Up @@ -210,7 +212,8 @@ public Response makeLinksForSingleSavedSearch(@PathParam("id") long savedSearchI
return errorResponse(BAD_REQUEST, "Count not find saved search id " + savedSearchIdToLookUp);
}
try {
JsonObjectBuilder response = savedSearchSvc.makeLinksForSingleSavedSearch(savedSearchToMakeLinksFor, debug);
DataverseRequest dataverseRequest = new DataverseRequest(savedSearchToMakeLinksFor.getCreator(), SavedSearchServiceBean.getHttpServletRequest());
JsonObjectBuilder response = savedSearchSvc.makeLinksForSingleSavedSearch(dataverseRequest, savedSearchToMakeLinksFor, debug);
return okResponse(response);
} catch (CommandException ex) {
return errorResponse(BAD_REQUEST, ex.getLocalizedMessage());
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateSavedSearchCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public SavedSearch execute(CommandContext ctxt) throws CommandException {
SavedSearch persistedSavedSearch = ctxt.savedSearches().save(savedSearchToCreate);
if (persistedSavedSearch != null) {
try {
JsonObjectBuilder result = ctxt.savedSearches().makeLinksForSingleSavedSearch(persistedSavedSearch, true);
DataverseRequest dataverseRequest = new DataverseRequest(savedSearchToCreate.getCreator(), SavedSearchServiceBean.getHttpServletRequest());
JsonObjectBuilder result = ctxt.savedSearches().makeLinksForSingleSavedSearch(dataverseRequest, persistedSavedSearch, true);
logger.log(Level.INFO, "result from attempt to make links from saved search: {0}", result.build().toString());
} catch (SearchException ex) {
logger.info(ex.getLocalizedMessage());
Expand Down
62 changes: 30 additions & 32 deletions src/main/java/edu/harvard/iq/dataverse/search/savedsearch/SavedSearchServiceBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.logging.Logger;
import javax.ejb.EJB;
import javax.ejb.Stateless;
import javax.faces.context.FacesContext;
import javax.inject.Named;
import javax.json.Json;
import javax.json.JsonArrayBuilder;
Expand All @@ -34,7 +33,6 @@
import javax.persistence.PersistenceContext;
import javax.persistence.TypedQuery;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.Context;

@Stateless
@Named
Expand All @@ -52,8 +50,6 @@ public class SavedSearchServiceBean {
DataverseLinkingServiceBean dataverseLinkingService;
@EJB
EjbDataverseEngine commandEngine;
@Context
HttpServletRequest httpReq;

private final String resultString = "result";

Expand Down Expand Up @@ -120,14 +116,30 @@ public JsonObjectBuilder makeLinksForAllSavedSearches(boolean debugFlag) throws
List<SavedSearch> allSavedSearches = findAll();
JsonArrayBuilder savedSearchArrayBuilder = Json.createArrayBuilder();
for (SavedSearch savedSearch : allSavedSearches) {
JsonObjectBuilder perSavedSearchResponse = makeLinksForSingleSavedSearch(savedSearch, debugFlag);
DataverseRequest dataverseRequest = new DataverseRequest(savedSearch.getCreator(), getHttpServletRequest());
JsonObjectBuilder perSavedSearchResponse = makeLinksForSingleSavedSearch(dataverseRequest, savedSearch, debugFlag);
savedSearchArrayBuilder.add(perSavedSearchResponse);
}
response.add("hits by saved search", savedSearchArrayBuilder);
return response;
}

public JsonObjectBuilder makeLinksForSingleSavedSearch(SavedSearch savedSearch, boolean debugFlag) throws SearchException, CommandException {
/**
* The "Saved Search" and highly related "Linked Dataverses and Linked
* Datasets" features can be thought of as periodic execution of the
* LinkDataverseCommand and LinkDatasetCommand. As of this writing that
* periodic execution can be triggered via a cron job but we'd like to put
* it on an EJB timer as part of
* https://github.com/IQSS/dataverse/issues/2543 .
*
* The commands are executed by the creator of the SavedSearch. What happens
* if the users loses the permission that the command requires? Should the
* commands continue to be executed periodically as some "system" user?
*
* @return Debug information in the form of a JSON object, which is much
* more structured that a simple String.
*/
public JsonObjectBuilder makeLinksForSingleSavedSearch(DataverseRequest dvReq, SavedSearch savedSearch, boolean debugFlag) throws SearchException, CommandException {
JsonObjectBuilder response = Json.createObjectBuilder();
JsonArrayBuilder savedSearchArrayBuilder = Json.createArrayBuilder();
JsonArrayBuilder infoPerHit = Json.createArrayBuilder();
Expand All @@ -144,8 +156,6 @@ public JsonObjectBuilder makeLinksForSingleSavedSearch(SavedSearch savedSearch,
infoPerHit.add(hitInfo);
break;
}
mutateHttpRequestSoJsfCanMakeLinks();
DataverseRequest dvReq = new DataverseRequest(savedSearch.getCreator(), httpReq);
if (dvObjectThatDefinitionPointWillLinkTo.isInstanceofDataverse()) {
Dataverse dataverseToLinkTo = (Dataverse) dvObjectThatDefinitionPointWillLinkTo;
if (wouldResultInLinkingToItself(savedSearch.getDefinitionPoint(), dataverseToLinkTo)) {
Expand Down Expand Up @@ -188,30 +198,6 @@ public JsonObjectBuilder makeLinksForSingleSavedSearch(SavedSearch savedSearch,
return response;
}

/**
* This method fixes the "java.lang.IllegalStateException: Not inside a
* request scope" issue reported at
* https://github.com/IQSS/dataverse/issues/2718 where it was impossible to
* use the "Link Dataset" feature from JSF.
*/
private void mutateHttpRequestSoJsfCanMakeLinks() {
FacesContext facesContext = FacesContext.getCurrentInstance();
if (facesContext != null) {
httpReq = (HttpServletRequest) facesContext.getExternalContext().getRequest();
} else {
/**
* No-op. When the "makeLinksForSingleSavedSearch" method is called
* from the API such as from `curl -X PUT
* http://localhost:8080/api/admin/savedsearches/makelinks/all` is
* is expected that facesContext will be null and it is not
* necessary to mutate the httpReq. The links are able to be made
* (in the datasetlinkingdataverse table, for example) and we don't
* get "java.lang.IllegalStateException: Not inside a request
* scope". It works fine.
*/
}
}

private SolrQueryResponse findHits(SavedSearch savedSearch) throws SearchException {
String sortField = SearchFields.RELEVANCE;
String sortOrder = SortBy.DESCENDING;
Expand Down Expand Up @@ -303,4 +289,16 @@ private boolean datasetAncestorAlreadyLinked(Dataverse definitionPoint, Dataset
return false;
}

public static HttpServletRequest getHttpServletRequest() {
/**
* This HttpServletRequest object is purposefully set to null. "There's
* another issue here, though - the IP address. The request is sent from
* a cron job - I assume localhost? - and it's source IP address is
* different from the one the user may have, and quite possibly more
* privileged. It maybe safest to pass in a null http request at this
* stage." -- michbarsinai
*/
return null;
}

}

0 comments on commit c0e60b6

Please sign in to comment.