Merge branch 'main' into CST-4121-MakeDiscoveryMoreResilientToStaleObjects

This commit is contained in:
Mykhaylo
2021-05-19 11:33:05 +02:00
15 changed files with 186 additions and 43 deletions

View File

@@ -12,6 +12,7 @@ import java.util.List;
import org.dspace.content.Item;
import org.dspace.core.Context;
import org.dspace.discovery.configuration.DiscoveryConfiguration;
import org.dspace.discovery.configuration.DiscoveryMoreLikeThisConfiguration;
import org.dspace.discovery.configuration.DiscoverySearchFilterFacet;
@@ -62,11 +63,14 @@ public interface SearchService {
* @param field the field of the filter query
* @param operator equals/notequals/notcontains/authority/notauthority
* @param value the filter query value
* @param config (nullable) the discovery configuration (if not null, field's corresponding facet.type checked to
* be standard so suffix is not added for equals operator)
* @return a filter query
* @throws SQLException if database error
* An exception that provides information on a database access error or other errors.
*/
DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value) throws SQLException;
DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value,
DiscoveryConfiguration config) throws SQLException;
List<Item> getRelatedItems(Context context, Item item,
DiscoveryMoreLikeThisConfiguration moreLikeThisConfiguration);

View File

@@ -60,6 +60,7 @@ import org.dspace.core.Context;
import org.dspace.core.Email;
import org.dspace.core.I18nUtil;
import org.dspace.core.LogManager;
import org.dspace.discovery.configuration.DiscoveryConfiguration;
import org.dspace.discovery.configuration.DiscoveryConfigurationParameters;
import org.dspace.discovery.configuration.DiscoveryMoreLikeThisConfiguration;
import org.dspace.discovery.configuration.DiscoverySearchFilterFacet;
@@ -1079,9 +1080,9 @@ public class SolrServiceImpl implements SearchService, IndexingService {
return new ArrayList<>(0);
}
}
@Override
public DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value)
public DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value,
DiscoveryConfiguration config)
throws SQLException {
DiscoverFilterQuery result = new DiscoverFilterQuery();
@@ -1091,7 +1092,14 @@ public class SolrServiceImpl implements SearchService, IndexingService {
if (operator.endsWith("equals")) {
filterQuery.append("_keyword");
final boolean isStandardField
= Optional.ofNullable(config)
.flatMap(c -> Optional.ofNullable(c.getSidebarFacet(field)))
.map(facet -> facet.getType().equals(DiscoveryConfigurationParameters.TYPE_STANDARD))
.orElse(false);
if (!isStandardField) {
filterQuery.append("_keyword");
}
} else if (operator.endsWith("authority")) {
filterQuery.append("_authority");
}

View File

@@ -11,18 +11,17 @@ import static org.springframework.web.servlet.DispatcherServlet.EXCEPTION_ATTRIB
import java.io.IOException;
import java.sql.SQLException;
import java.util.Objects;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.dspace.app.rest.security.RestAuthenticationService;
import org.dspace.app.rest.utils.ContextUtil;
import org.dspace.authorize.AuthorizeException;
import org.dspace.core.Context;
import org.springframework.beans.TypeMismatchException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.data.repository.support.QueryMethodParameterConversionException;
import org.springframework.http.HttpHeaders;
@@ -59,13 +58,11 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH
*/
private static final Set<Integer> LOG_AS_ERROR = Set.of(422);
@Autowired
private RestAuthenticationService restAuthenticationService;
@ExceptionHandler({AuthorizeException.class, RESTAuthorizationException.class, AccessDeniedException.class})
protected void handleAuthorizeException(HttpServletRequest request, HttpServletResponse response, Exception ex)
throws IOException {
if (restAuthenticationService.hasAuthenticationData(request)) {
Context context = ContextUtil.obtainContext(request);
if (Objects.nonNull(context.getCurrentUser())) {
sendErrorResponse(request, response, ex, "Access is denied", HttpServletResponse.SC_FORBIDDEN);
} else {
sendErrorResponse(request, response, ex, "Authentication is required", HttpServletResponse.SC_UNAUTHORIZED);

View File

@@ -190,22 +190,22 @@ public class MetadataFieldRestRepository extends DSpaceRestRepository<MetadataFi
"forming schema.element.qualifier metadata field name");
}
filterQueries.add(searchService.toFilterQuery(context, MetadataFieldIndexFactoryImpl.FIELD_NAME_VARIATIONS,
OPERATOR_EQUALS, query).getFilterQuery() + "*");
OPERATOR_EQUALS, query, null).getFilterQuery() + "*");
}
if (StringUtils.isNotBlank(schemaName)) {
filterQueries.add(
searchService.toFilterQuery(context, MetadataFieldIndexFactoryImpl.SCHEMA_FIELD_NAME, OPERATOR_EQUALS,
schemaName).getFilterQuery());
schemaName, null).getFilterQuery());
}
if (StringUtils.isNotBlank(elementName)) {
filterQueries.add(
searchService.toFilterQuery(context, MetadataFieldIndexFactoryImpl.ELEMENT_FIELD_NAME, OPERATOR_EQUALS,
elementName).getFilterQuery());
elementName, null).getFilterQuery());
}
if (StringUtils.isNotBlank(qualifierName)) {
filterQueries.add(searchService
.toFilterQuery(context, MetadataFieldIndexFactoryImpl.QUALIFIER_FIELD_NAME, OPERATOR_EQUALS,
qualifierName).getFilterQuery());
qualifierName, null).getFilterQuery());
}
DiscoverQuery discoverQuery = new DiscoverQuery();

View File

@@ -393,7 +393,8 @@ public class DiscoverQueryBuilder implements InitializingBean {
DiscoverFilterQuery filterQuery = searchService.toFilterQuery(context,
filter.getIndexFieldName(),
searchFilter.getOperator(),
searchFilter.getValue());
searchFilter.getValue(),
discoveryConfiguration);
if (filterQuery != null) {
filterQueries.add(filterQuery.getFilterQuery());

View File

@@ -216,7 +216,9 @@
}
},
success : successHandler,
error : function() {
error : function(xhr) {
// Check for an update to the CSRF Token & save to a MyHalBrowserCsrfToken cookie (if found)
checkForUpdatedCSRFTokenInResponse(xhr);
toastr.error('The credentials you entered are invalid. Please try again.', 'Login Failed');
}
});

View File

@@ -1193,7 +1193,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
String loginToken = getAuthToken(eperson.getEmail(), password);
getClient().perform(get("/api/core/bitstreams/" + bitstream.getID()
+ "/content?authentication-token=" + loginToken))
.andExpect(status().isForbidden());
.andExpect(status().isUnauthorized());
}
@Test
@@ -1204,7 +1204,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
Thread.sleep(1);
getClient().perform(get("/api/core/bitstreams/" + bitstream.getID()
+ "/content?authentication-token=" + shortLivedToken))
.andExpect(status().isForbidden());
.andExpect(status().isUnauthorized());
}
@Test

View File

@@ -5238,6 +5238,137 @@ public class DiscoveryRestControllerIT extends AbstractControllerIntegrationTest
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")));
}
@Test
public void discoverSearchObjectsTestForAdministrativeViewWithFiltersEquals() throws Exception {
context.turnOffAuthorisationSystem();
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();
ItemBuilder.createItem(context, col1)
.withTitle("Public Test Item")
.withIssueDate("2010-10-17")
.withAuthor("Smith, Donald")
.withSubject("ExtraEntry")
.build();
ItemBuilder.createItem(context, col2)
.withTitle("Withdrawn Test Item")
.withIssueDate("1990-02-13")
.withAuthor("Smith, Maria")
.withAuthor("Doe, Jane")
.withSubject("ExtraEntry")
.withdrawn()
.build();
ItemBuilder.createItem(context, col2)
.withTitle("Private Test Item")
.withIssueDate("2010-02-13")
.withAuthor("Smith, Maria")
.withAuthor("Doe, Jane")
.withSubject("AnotherTest")
.withSubject("ExtraEntry")
.makeUnDiscoverable()
.build();
context.restoreAuthSystemState();
String adminToken = getAuthToken(admin.getEmail(), password);
getClient(adminToken)
.perform(get("/api/discover/search/objects")
.param("configuration", "administrativeView")
.param("query", "Test")
.param("f.withdrawn", "true,equals")
)
.andExpect(status().isOk())
.andExpect(jsonPath("$.type", is("discover")))
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 1)
)))
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects",
Matchers.contains(
SearchResultMatcher.matchOnItemName("item", "items", "Withdrawn Test Item")
)
))
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")));
getClient(adminToken)
.perform(get("/api/discover/search/objects")
.param("configuration", "administrativeView")
.param("query", "Test")
.param("f.withdrawn", "false,equals")
)
.andExpect(status().isOk())
.andExpect(jsonPath("$.type", is("discover")))
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 2)
)))
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects",
Matchers.containsInAnyOrder(
SearchResultMatcher.matchOnItemName("item", "items", "Public Test Item"),
SearchResultMatcher.matchOnItemName("item", "items", "Private Test Item")
)
))
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")));
getClient(adminToken)
.perform(get("/api/discover/search/objects")
.param("configuration", "administrativeView")
.param("query", "Test")
.param("f.discoverable", "true,equals")
)
.andExpect(status().isOk())
.andExpect(jsonPath("$.type", is("discover")))
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 2)
)))
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects",
Matchers.containsInAnyOrder(
SearchResultMatcher.matchOnItemName("item", "items", "Public Test Item"),
SearchResultMatcher.matchOnItemName("item", "items", "Withdrawn Test Item")
)
))
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")));
getClient(adminToken)
.perform(get("/api/discover/search/objects")
.param("configuration", "administrativeView")
.param("query", "Test")
.param("f.discoverable", "false,equals")
)
.andExpect(status().isOk())
.andExpect(jsonPath("$.type", is("discover")))
.andExpect(jsonPath("$._embedded.searchResult.page", is(
PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 1)
)))
.andExpect(jsonPath("$._embedded.searchResult._embedded.objects",
Matchers.contains(
SearchResultMatcher.matchOnItemName("item", "items", "Private Test Item")
)
))
.andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects")));
}
@Test
public void discoverSearchPoolTaskObjectsTest() throws Exception {
context.turnOffAuthorisationSystem();

View File

@@ -179,7 +179,7 @@ public class StatisticsRestRepositoryIT extends AbstractControllerIntegrationTes
getClient("unvalidToken").perform(
get("/api/statistics/usagereports/" + itemNotVisitedWithBitstreams.getID() + "_" + TOTAL_VISITS_REPORT_ID))
// ** THEN **
.andExpect(status().isForbidden());
.andExpect(status().isUnauthorized());
}
@Test
@@ -829,7 +829,7 @@ public class StatisticsRestRepositoryIT extends AbstractControllerIntegrationTes
.perform(get("/api/statistics/usagereports/search/object?uri=http://localhost:8080/server/api/core" +
"/items/" + itemNotVisitedWithBitstreams.getID()))
// ** THEN **
.andExpect(status().isForbidden());
.andExpect(status().isUnauthorized());
}
@Test

View File

@@ -49,8 +49,8 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio
String token = "nonValidToken";
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test
@@ -112,8 +112,8 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio
WorkflowActionConfig existentWorkflow = xmlWorkflowFactory.getActionByName(nameActionWithOptions);
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT + "/" + nameActionWithOptions))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test

View File

@@ -121,8 +121,8 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr
String token = "NonValidToken";
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test
@@ -192,8 +192,8 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr
String workflowName = defaultWorkflow.getID();
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + workflowName))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test
@@ -402,8 +402,8 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + defaultWorkflow.getID()
+ "/collections"))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test
@@ -441,8 +441,8 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + defaultWorkflow.getID()
+ "/steps"))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test

View File

@@ -47,8 +47,8 @@ public class WorkflowStepRestRepositoryIT extends AbstractControllerIntegrationT
String token = "NonValidToken";
//When we call this facets endpoint
getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT))
//We expect a 403 Forbidden status
.andExpect(status().isForbidden());
//We expect a 401 Unauthorized status
.andExpect(status().isUnauthorized());
}
@Test

View File

@@ -113,7 +113,8 @@ public class DiscoverQueryBuilderTest {
any(), any(DiscoverQuery.class)))
.then(invocation -> new FacetYearRange((DiscoverySearchFilterFacet) invocation.getArguments()[2]));
when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class)))
when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class),
any(DiscoveryConfiguration.class)))
.then(invocation -> new DiscoverFilterQuery((String) invocation.getArguments()[1],
invocation.getArguments()[1] + ":\"" + invocation.getArguments()[3] + "\"",
(String) invocation.getArguments()[3]));
@@ -291,7 +292,8 @@ public class DiscoverQueryBuilderTest {
@Test(expected = DSpaceBadRequestException.class)
public void testInvalidSearchFilter2() throws Exception {
when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class)))
when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class),
any(DiscoveryConfiguration.class)))
.thenThrow(SQLException.class);
queryBuilder

View File

@@ -426,10 +426,9 @@ public final class DSpaceServiceManager implements ServiceManagerSystem {
service = (T) applicationContext.getBean(name, type);
} catch (BeansException e) {
// no luck, try the fall back option
log.info(
log.warn(
"Unable to locate bean by name or id={}."
+ " Will try to look up bean by type next."
+ " BeansException: {}", name, e.getMessage());
+ " Will try to look up bean by type next.", name, e);
service = null;
}
} else {
@@ -438,9 +437,8 @@ public final class DSpaceServiceManager implements ServiceManagerSystem {
service = (T) applicationContext.getBean(type.getName(), type);
} catch (BeansException e) {
// no luck, try the fall back option
log.info("Unable to locate bean by name or id={}."
+ " Will try to look up bean by type next."
+ " BeansException: {}", type.getName(), e.getMessage());
log.warn("Unable to locate bean by name or id={}."
+ " Will try to look up bean by type next.", type.getName(), e);
service = null;
}
}

View File

@@ -82,7 +82,7 @@
<Logger name='org.dspace.services'
level='ERROR'/>
<Logger name='org.dspace.servicemanager'
level='ERROR'/>
level='WARN'/>
<Logger name='org.dspace.providers'
level='ERROR'/>
<Logger name='org.dspace.utils'