From e53bc8af2835b9a8dfe0df83b5efe97b4fade58c Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Wed, 19 Feb 2020 14:00:13 +0100 Subject: [PATCH] 68781: Update groups and epersons GETS based on REST contract --- .../org/dspace/app/rest/model/GroupRest.java | 5 ++ .../repository/EPersonRestRepository.java | 39 ++------ .../GroupEPersonLinkRepository.java | 55 ++++++++++++ .../rest/repository/GroupRestRepository.java | 29 +++++- .../app/rest/EPersonRestRepositoryIT.java | 90 ++++++------------- .../app/rest/GroupRestRepositoryIT.java | 90 +++++++++++++++++++ .../dspace/app/rest/matcher/GroupMatcher.java | 7 +- 7 files changed, 219 insertions(+), 96 deletions(-) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupEPersonLinkRepository.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/GroupRest.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/GroupRest.java index 693c73a0ca..5eab3a148a 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/GroupRest.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/GroupRest.java @@ -21,6 +21,10 @@ import org.dspace.app.rest.RestResourceController; @LinkRest( name = GroupRest.GROUPS, method = "getGroups" + ), + @LinkRest( + name = GroupRest.EPERSONS, + method = "getMembers" ) }) public class GroupRest extends DSpaceObjectRest { @@ -28,6 +32,7 @@ public class GroupRest extends DSpaceObjectRest { public static final String CATEGORY = RestAddressableModel.EPERSON; public static final String GROUPS = "groups"; + public static final String EPERSONS = "epersons"; private String name; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java index 8ee88d50c2..50c30a64b6 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java @@ -116,52 +116,31 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository findByName(@Parameter(value = "q", required = true) String q, + @PreAuthorize("hasAuthority('ADMIN')") + @SearchRestMethod(name = "byMetadata") + public Page findByMetadata(@Parameter(value = "query", required = true) String query, Pageable pageable) { + try { Context context = obtainContext(); - long total = es.searchResultCount(context, q); - List epersons = es.search(context, q, Math.toIntExact(pageable.getOffset()), - Math.toIntExact(pageable.getOffset() + pageable.getPageSize())); + long total = es.searchResultCount(context, query); + List epersons = es.search(context, query, Math.toIntExact(pageable.getOffset()), + Math.toIntExact(pageable.getOffset() + pageable.getPageSize())); return converter.toRestPage(epersons, pageable, total, utils.obtainProjection()); } catch (SQLException e) { throw new RuntimeException(e.getMessage(), e); } } - /** - * Find the eperson with the provided email address if any. The search is delegated to the - * {@link EPersonService#findByEmail(Context, String)} method - * - * @param email - * is the *required* email address - * @return the EPersonRest instance, if any, matching the user query - */ - @SearchRestMethod(name = "byEmail") - public EPersonRest findByEmail(@Parameter(value = "email", required = true) String email) { - EPerson eperson = null; - try { - Context context = obtainContext(); - eperson = es.findByEmail(context, email); - } catch (SQLException e) { - throw new RuntimeException(e.getMessage(), e); - } - if (eperson == null) { - return null; - } - return converter.toRest(eperson, utils.obtainProjection()); - } - @Override @PreAuthorize("hasPermission(#uuid, 'EPERSON', #patch)") protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, UUID uuid, diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupEPersonLinkRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupEPersonLinkRepository.java new file mode 100644 index 0000000000..9c2026d0b2 --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupEPersonLinkRepository.java @@ -0,0 +1,55 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.rest.repository; + +import java.sql.SQLException; +import java.util.UUID; +import javax.annotation.Nullable; +import javax.servlet.http.HttpServletRequest; + +import org.dspace.app.rest.model.GroupRest; +import org.dspace.app.rest.projection.Projection; +import org.dspace.core.Context; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.dspace.eperson.service.GroupService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; +import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.stereotype.Component; + +/** + * Link repository for "groups" subresource of an individual group. + */ +@Component(GroupRest.CATEGORY + "." + GroupRest.NAME + "." + GroupRest.EPERSONS) +public class GroupEPersonLinkRepository extends AbstractDSpaceRestRepository + implements LinkRestRepository { + + @Autowired + GroupService groupService; + + @PreAuthorize("hasPermission(#groupId, 'GROUP', 'READ')") + public Page getMembers(@Nullable HttpServletRequest request, + UUID groupId, + @Nullable Pageable optionalPageable, + Projection projection) { + try { + Context context = obtainContext(); + Group group = groupService.find(context, groupId); + if (group == null) { + throw new ResourceNotFoundException("No such group: " + groupId); + } + Page ePersons = utils.getPage(group.getMembers(), optionalPageable); + return converter.toRestPage(ePersons, projection); + } catch (SQLException e) { + throw new RuntimeException(e); + } + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java index 9909a3f011..0a57494780 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java @@ -14,6 +14,8 @@ import java.util.UUID; import javax.servlet.http.HttpServletRequest; import com.fasterxml.jackson.databind.ObjectMapper; +import org.dspace.app.rest.Parameter; +import org.dspace.app.rest.SearchRestMethod; import org.dspace.app.rest.converter.MetadataConverter; import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException; import org.dspace.app.rest.exception.UnprocessableEntityException; @@ -98,7 +100,7 @@ public class GroupRestRepository extends DSpaceObjectRestRepository groups = gs.findAll(context, null, pageable.getPageSize(), - Math.toIntExact(pageable.getOffset())); + Math.toIntExact(pageable.getOffset())); return converter.toRestPage(groups, pageable, total, utils.obtainProjection()); } catch (SQLException e) { throw new RuntimeException(e.getMessage(), e); @@ -112,6 +114,31 @@ public class GroupRestRepository extends DSpaceObjectRestRepository findByMetadata(@Parameter(value = "query", required = true) String query, + Pageable pageable) { + + try { + Context context = obtainContext(); + long total = gs.searchResultCount(context, query); + List groups = gs.search(context, query, Math.toIntExact(pageable.getOffset()), + Math.toIntExact(pageable.getOffset() + pageable.getPageSize())); + return converter.toRestPage(groups, pageable, total, utils.obtainProjection()); + } catch (SQLException e) { + throw new RuntimeException(e.getMessage(), e); + } + } + @Override public Class getDomainClass() { return GroupRest.class; diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java index 072aa647b0..ae908d8962 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java @@ -296,62 +296,11 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { getClient(authToken).perform(get("/api/eperson/epersons/search")) .andExpect(status().isOk()) .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$._links.byEmail", Matchers.notNullValue())) - .andExpect(jsonPath("$._links.byName", Matchers.notNullValue())); + .andExpect(jsonPath("$._links.byMetadata", Matchers.notNullValue())); } @Test - public void findByEmail() throws Exception { - context.turnOffAuthorisationSystem(); - - EPerson ePerson = EPersonBuilder.createEPerson(context) - .withNameInMetadata("John", "Doe") - .withEmail("Johndoe@fake-email.com") - .build(); - - // create a second eperson to put the previous one in a no special position (is not the first as we have default - // epersons is not the latest created) - EPerson ePerson2 = EPersonBuilder.createEPerson(context) - .withNameInMetadata("Jane", "Smith") - .withEmail("janesmith@fake-email.com") - .build(); - - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/eperson/epersons/search/byEmail") - .param("email", ePerson.getEmail())) - .andExpect(status().isOk()) - .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$", is( - EPersonMatcher.matchEPersonEntry(ePerson) - ))); - - // it must be case-insensitive - getClient(authToken).perform(get("/api/eperson/epersons/search/byEmail") - .param("email", ePerson.getEmail().toUpperCase())) - .andExpect(status().isOk()) - .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$", is( - EPersonMatcher.matchEPersonEntry(ePerson) - ))); - } - - @Test - public void findByEmailUndefined() throws Exception { - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/eperson/epersons/search/byEmail") - .param("email", "undefined@undefined.com")) - .andExpect(status().isNoContent()); - } - - @Test - public void findByEmailUnprocessable() throws Exception { - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/eperson/epersons/search/byEmail")) - .andExpect(status().isBadRequest()); - } - - @Test - public void findByName() throws Exception { + public void findByMetadata() throws Exception { context.turnOffAuthorisationSystem(); EPerson ePerson = EPersonBuilder.createEPerson(context) .withNameInMetadata("John", "Doe") @@ -379,8 +328,8 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { .build(); String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/eperson/epersons/search/byName") - .param("q", ePerson.getLastName())) + getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", ePerson.getLastName())) .andExpect(status().isOk()) .andExpect(content().contentType(contentType)) .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( @@ -392,8 +341,8 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(jsonPath("$.page.totalElements", is(4))); // it must be case insensitive - getClient(authToken).perform(get("/api/eperson/epersons/search/byName") - .param("q", ePerson.getLastName().toLowerCase())) + getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", ePerson.getLastName().toLowerCase())) .andExpect(status().isOk()) .andExpect(content().contentType(contentType)) .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( @@ -406,20 +355,35 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { } @Test - public void findByNameUndefined() throws Exception { + public void findByMetadataUnauthorized() throws Exception { + getClient().perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Doe, John")) + .andExpect(status().isUnauthorized()); + } + + @Test + public void findByMetadataForbidden() throws Exception { + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Doe, John")) + .andExpect(status().isForbidden()); + } + + @Test + public void findByMetadataUndefined() throws Exception { String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/eperson/epersons/search/byName") - .param("q", "Doe, John")) + getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Doe, John")) .andExpect(status().isOk()) .andExpect(content().contentType(contentType)) .andExpect(jsonPath("$.page.totalElements", is(0))); } @Test - public void findByNameUnprocessable() throws Exception { + public void findByMetadataUnprocessable() throws Exception { String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/eperson/epersons/search/byName")) - .andExpect(status().isBadRequest()); + getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")) + .andExpect(status().isUnprocessableEntity()); } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java index efccd50582..0dac1b846f 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java @@ -257,6 +257,96 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { ; } + @Test + public void searchMethodsExist() throws Exception { + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(get("/api/eperson/epersons")) + .andExpect(jsonPath("$._links.search.href", Matchers.notNullValue())); + + getClient(authToken).perform(get("/api/eperson/epersons/search")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._links.byMetadata", Matchers.notNullValue())); + } + + @Test + public void findByMetadata() throws Exception { + context.turnOffAuthorisationSystem(); + + Group group1 = GroupBuilder.createGroup(context) + .withName("Test group") + .build(); + + Group group2 = GroupBuilder.createGroup(context) + .withName("Test group 2") + .build(); + + Group group3 = GroupBuilder.createGroup(context) + .withName("Test group 3") + .build(); + + Group group4 = GroupBuilder.createGroup(context) + .withName("Test other group") + .build(); + + + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups", Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName()) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + // it must be case insensitive + getClient(authToken).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", String.valueOf(group1.getID()))) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups", Matchers.contains( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()) + ))) + .andExpect(jsonPath("$.page.totalElements", is(1))); + } + + @Test + public void findByMetadataUnauthorized() throws Exception { + String authToken = getAuthToken(admin.getEmail(), password); + getClient().perform(get("/api/eperson/groups/search/byMetadata") + .param("query", "Administrator")) + .andExpect(status().isUnauthorized()); + } + + @Test + public void findByMetadataForbidden() throws Exception { + String authToken = getAuthToken(eperson.getEmail(), password); + getClient(authToken).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", "Administrator")) + .andExpect(status().isForbidden()); + } + + @Test + public void findByMetadataUndefined() throws Exception { + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", "Non-existing Group")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$.page.totalElements", is(0))); + } + + @Test + public void findByMetadataUnprocessable() throws Exception { + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(get("/api/eperson/groups/search/byMetadata")) + .andExpect(status().isUnprocessableEntity()); + } + + @Test public void patchGroupMetadataAuthorized() throws Exception { runPatchMetadataTests(admin, 200); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/matcher/GroupMatcher.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/matcher/GroupMatcher.java index a841c2ea8d..c8522b2748 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/matcher/GroupMatcher.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/matcher/GroupMatcher.java @@ -43,7 +43,8 @@ public class GroupMatcher { */ public static Matcher matchFullEmbeds() { return matchEmbeds( - "groups[]" + "groups[]", + "epersons[]" ); } @@ -53,6 +54,7 @@ public class GroupMatcher { public static Matcher matchLinks(UUID uuid) { return HalMatcher.matchLinks(REST_SERVER_URL + "eperson/groups/" + uuid, "groups", + "epersons", "self" ); } @@ -63,7 +65,8 @@ public class GroupMatcher { hasJsonPath("$.name", is(name)), hasJsonPath("$.type", is("group")), hasJsonPath("$._links.self.href", containsString("/api/eperson/groups/" + uuid.toString())), - hasJsonPath("$._links.groups.href", endsWith(uuid.toString() + "/groups")) + hasJsonPath("$._links.groups.href", endsWith(uuid.toString() + "/groups")), + hasJsonPath("$._links.epersons.href", endsWith(uuid.toString() + "/epersons")) ); } }