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.

Sign up for GitHub

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

Jump to bottom

[DS-4010] Removed the escaping on the query parameter for the discove… #2206

Merged
merged 11 commits into from Oct 31, 2018
Merged

[DS-4010] Removed the escaping on the query parameter for the discove… #2206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b3abd77
[DS-4010] Removed the escaping on the query parameter for the discove…
Raf-atmire Sep 17, 2018
a73b980
[DS-4010] removed the comments that don't represent the code anymore
Raf-atmire Sep 17, 2018
26a5531
[DS-4010] Fixed the tests
Raf-atmire Sep 18, 2018
aa4f3bb
[Task 55655] set the status to Bad Request if the query entered wasn'…
Raf-atmire Sep 19, 2018
b192552
[DS-4010] added a catch for the search error and throw illegalargueme…
Raf-atmire Sep 21, 2018
ea42765
Revert "[DS-4010] added a catch for the search error and throw illega…
Raf-atmire Sep 21, 2018
df5816d
Revert "Revert "[DS-4010] added a catch for the search error and thro…
Raf-atmire Sep 21, 2018
47dc948
[DS-4010] added IT test for the discoverQuery with a query parameter …
Raf-atmire Sep 25, 2018
06b4e5d
[DS-4010] added IT tests for the query functionality on the discover …
Raf-atmire Oct 1, 2018
7e1ca49
[DS-4010] fixed test case
Raf-atmire Oct 1, 2018
9e99c91
Added information into the exception message
Raf-atmire Oct 31, 2018
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
3 changes: 2 additions & 1 deletion dspace-spring-rest/src/main/java/org/dspace/app/rest/DiscoveryRestController.java
View file
Open in desktop
Expand Up @@ -142,7 +142,8 @@ public SearchResultsResource getSearchObjects(@RequestParam(name = "query", requ
}

//Get the Search results in JSON format
SearchResultsRest searchResultsRest = discoveryRestRepository
SearchResultsRest searchResultsRest = null;
searchResultsRest = discoveryRestRepository
.getSearchObjects(query, dsoType, dsoScope, configurationName, searchFilters, page);

//Convert the Search JSON results to paginated HAL resources
Expand Down
7 changes: 7 additions & 0 deletions ...-rest/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@
*/
@ControllerAdvice
public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionHandler {

@Autowired
private RestAuthenticationService restAuthenticationService;

Expand All @@ -51,6 +52,12 @@ protected void handleAuthorizeException(HttpServletRequest request, HttpServletR
}
}

@ExceptionHandler(IllegalArgumentException.class)
protected void handleIllegalArgumentException(HttpServletRequest request, HttpServletResponse response,
Exception ex) throws IOException {
sendErrorResponse(request, response, ex, ex.getMessage(), HttpServletResponse.SC_BAD_REQUEST);
}

@ExceptionHandler(SQLException.class)
protected void handleSQLException(HttpServletRequest request, HttpServletResponse response, Exception ex)
throws IOException {
Expand Down
5 changes: 4 additions & 1 deletion dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java
View file
Open in desktop
Expand Up @@ -9,6 +9,8 @@

import java.util.List;

import javax.ws.rs.BadRequestException;

import org.apache.log4j.Logger;
import org.dspace.app.rest.converter.DiscoverConfigurationConverter;
import org.dspace.app.rest.converter.DiscoverFacetConfigurationConverter;
Expand Down Expand Up @@ -91,7 +93,7 @@ public SearchConfigurationRest getSearchConfiguration(final String dsoScope, fin
public SearchResultsRest getSearchObjects(final String query, final String dsoType, final String dsoScope,
final String configurationName,
final List<SearchFilter> searchFilters, final Pageable page)
throws InvalidRequestException {
throws InvalidRequestException, BadRequestException {
Context context = obtainContext();

DSpaceObject scopeObject = scopeResolver.resolveScope(context, dsoScope);
Expand All @@ -108,6 +110,7 @@ public SearchResultsRest getSearchObjects(final String query, final String dsoTy

} catch (SearchServiceException e) {
log.error("Error while searching with Discovery", e);
throw new IllegalArgumentException("Error while searching with Discovery: " + e.getMessage());
}

return discoverResultConverter
Expand Down
4 changes: 1 addition & 3 deletions dspace-spring-rest/src/main/java/org/dspace/app/rest/utils/DiscoverQueryBuilder.java
View file
Open in desktop
Expand Up @@ -181,9 +181,7 @@ private DiscoverQuery buildCommonDiscoverQuery(Context context, DiscoveryConfigu

//Set search query
if (StringUtils.isNotBlank(query)) {
//Note that these quotes are needed incase we try to query OR for example.
//If the quotes aren't present, it'll crash.
queryArgs.setQuery("\"" + searchService.escapeQueryChars(query) + "\"");
queryArgs.setQuery(query);
}

//Limit results to DSO type
Expand Down
278 changes: 278 additions & 0 deletions dspace-spring-rest/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java
View file
Open in desktop
Expand Up @@ -2678,4 +2678,282 @@ public void discoverSearchObjectsWithQueryOperatorNotAuthority() throws Exceptio
;

}

@Test
public void discoverSearchObjectsTestWithDateIssuedQueryTest() throws Exception {
//We turn off the authorization system in order to create the structure as defined below
context.turnOffAuthorisationSystem();

//** GIVEN **
//1. A community-collection structure with one parent community with sub-community and two collections.
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity)
.withName("Sub Community")
.build();
Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build();
Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build();

//2. Three public items that are readable by Anonymous with different subjects
Item publicItem1 = ItemBuilder.createItem(context, col1)
.withTitle("Test")
.withIssueDate("2010-10-17")
.withAuthor("Smith, Donald")
.withSubject("ExtraEntry")
.build();

Item publicItem2 = ItemBuilder.createItem(context, col2)
.withTitle("Test 2")
.withIssueDate("1990-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane")
.withSubject("TestingForMore").withSubject("ExtraEntry")
.build();

Item publicItem3 = ItemBuilder.createItem(context, col2)
.withTitle("Public item 2")
.withIssueDate("2010-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane").withAuthor("test,test")
.withAuthor("test2, test2").withAuthor("Maybe, Maybe")
.withSubject("AnotherTest").withSubject("TestingForMore")
.withSubject("ExtraEntry")
.build();
String bitstreamContent = "ThisIsSomeDummyText";
//Add a bitstream to an item
try (InputStream is = IOUtils.toInputStream(bitstreamContent, CharEncoding.UTF_8)) {
Bitstream bitstream = BitstreamBuilder.
createBitstream(context, publicItem1, is)
.withName("Bitstream")
.withMimeType("text/plain")
.build();
}

//Run the filter media to make the text in the bitstream searchable through the query
runDSpaceScript("filter-media", "-f", "-i", publicItem1.getHandle());

//** WHEN **
getClient().perform(get("/api/discover/search/objects")
.param("query", "dc.date.issued:2010-02-13"))

//** THEN **
//The status has to be 200 OK
.andExpect(status().isOk())
//The type has to be 'discover'
.andExpect(jsonPath("$.type", is("discover")))
//The page object needs to look like this
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 1)
)))
//This is the only item that should be returned with the query given
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.contains(
SearchResultMatcher.matchOnItemName("item", "items", "Public item 2")
)))

//There always needs to be a self link available
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
;


//** WHEN **
getClient().perform(get("/api/discover/search/objects")
.param("query", "dc.date.issued:2013-02-13"))

//** THEN **
//The status has to be 200 OK
.andExpect(status().isOk())
//The type has to be 'discover'
.andExpect(jsonPath("$.type", is("discover")))
//The page object needs to look like this
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 0, 0)
)))

//There always needs to be a self link available
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
;

}

@Test
public void discoverSearchObjectsTestWithLuceneSyntaxQueryTest() throws Exception {
//We turn off the authorization system in order to create the structure as defined below
context.turnOffAuthorisationSystem();

//** GIVEN **
//1. A community-collection structure with one parent community with sub-community and two collections.
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity)
.withName("Sub Community")
.build();
Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build();
Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build();

//2. Three public items that are readable by Anonymous with different subjects
Item publicItem1 = ItemBuilder.createItem(context, col1)
.withTitle("Test")
.withIssueDate("2010-10-17")
.withAuthor("Smith, Donald")
.withSubject("ExtraEntry")
.build();

Item publicItem2 = ItemBuilder.createItem(context, col2)
.withTitle("TestItem2")
.withIssueDate("1990-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane")
.withSubject("TestingForMore").withSubject("ExtraEntry")
.build();

Item publicItem3 = ItemBuilder.createItem(context, col2)
.withTitle("azeazeazeazeazeaze")
.withIssueDate("2010-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane").withAuthor("test,test")
.withAuthor("test2, test2").withAuthor("Maybe, Maybe")
.withSubject("AnotherTest").withSubject("TestingForMore")
.withSubject("ExtraEntry")
.build();
//** WHEN **
getClient().perform(get("/api/discover/search/objects")
.param("query", "((dc.date.issued:2010 OR dc.date.issued:1990-02-13)" +
" AND (dc.title:Test OR dc.title:TestItem2))"))

//** THEN **
//The status has to be 200 OK
.andExpect(status().isOk())
//The type has to be 'discover'
.andExpect(jsonPath("$.type", is("discover")))
//The page object needs to look like this
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 2)
)))
//This is the only item that should be returned with the query given
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.containsInAnyOrder(
SearchResultMatcher.matchOnItemName("item", "items", "Test"),
SearchResultMatcher.matchOnItemName("item", "items", "TestItem2")
)))
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.not(Matchers.contains(
SearchResultMatcher.matchOnItemName("item", "items", "azeazeazeazeazeaze")
))))

//There always needs to be a self link available
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
;

}

@Test
public void discoverSearchObjectsTestWithEscapedLuceneCharactersTest() throws Exception {
//We turn off the authorization system in order to create the structure as defined below
context.turnOffAuthorisationSystem();

//** GIVEN **
//1. A community-collection structure with one parent community with sub-community and two collections.
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity)
.withName("Sub Community")
.build();
Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build();
Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build();

//2. Three public items that are readable by Anonymous with different subjects
Item publicItem1 = ItemBuilder.createItem(context, col1)
.withTitle("Faithful Infidel: Exploring Conformity (2nd edition)")
.withIssueDate("2010-10-17")
.withAuthor("Smith, Donald")
.withSubject("ExtraEntry")
.build();

Item publicItem2 = ItemBuilder.createItem(context, col2)
.withTitle("Test")
.withIssueDate("1990-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane")
.withSubject("TestingForMore").withSubject("ExtraEntry")
.build();

Item publicItem3 = ItemBuilder.createItem(context, col2)
.withTitle("NotAProperTestTitle")
.withIssueDate("2010-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane").withAuthor("test,test")
.withAuthor("test2, test2").withAuthor("Maybe, Maybe")
.withSubject("AnotherTest").withSubject("TestingForMore")
.withSubject("ExtraEntry")
.build();
getClient().perform(get("/api/discover/search/objects")
.param("query", "\"Faithful Infidel: Exploring Conformity (2nd edition)\""))

//** THEN **
//The status has to be 200 OK
.andExpect(status().isOk())
//The type has to be 'discover'
.andExpect(jsonPath("$.type", is("discover")))
//The page object needs to look like this
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 1)
)))
//This is the only item that should be returned with the query given
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.contains(
SearchResultMatcher.matchOnItemName
("item", "items", "Faithful Infidel: Exploring Conformity (2nd edition)")
)))
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects",
Matchers.not(Matchers.containsInAnyOrder(
SearchResultMatcher.matchOnItemName("item", "items", "Test"),
SearchResultMatcher.matchOnItemName("item", "items", "NotAProperTestTitle")
))))

//There always needs to be a self link available
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")))
;

}
@Test
public void discoverSearchObjectsTestWithUnEscapedLuceneCharactersTest() throws Exception {
//We turn off the authorization system in order to create the structure as defined below
context.turnOffAuthorisationSystem();

//** GIVEN **
//1. A community-collection structure with one parent community with sub-community and two collections.
parentCommunity = CommunityBuilder.createCommunity(context)
.withName("Parent Community")
.build();
Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity)
.withName("Sub Community")
.build();
Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build();
Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build();

//2. Three public items that are readable by Anonymous with different subjects
Item publicItem1 = ItemBuilder.createItem(context, col1)
.withTitle("Faithful Infidel: Exploring Conformity (2nd edition)")
.withIssueDate("2010-10-17")
.withAuthor("Smith, Donald")
.withSubject("ExtraEntry")
.build();

Item publicItem2 = ItemBuilder.createItem(context, col2)
.withTitle("Test")
.withIssueDate("1990-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane")
.withSubject("TestingForMore").withSubject("ExtraEntry")
.build();

Item publicItem3 = ItemBuilder.createItem(context, col2)
.withTitle("NotAProperTestTitle")
.withIssueDate("2010-02-13")
.withAuthor("Smith, Maria").withAuthor("Doe, Jane").withAuthor("test,test")
.withAuthor("test2, test2").withAuthor("Maybe, Maybe")
.withSubject("AnotherTest").withSubject("TestingForMore")
.withSubject("ExtraEntry")
.build();
//** WHEN **
getClient().perform(get("/api/discover/search/objects")
.param("query", "OR"))

.andExpect(status().isBadRequest())
;

}
}