diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java index 7599e569eb..b8a3dfc45d 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java @@ -55,6 +55,11 @@ import javax.ws.rs.core.Request; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.UriInfo; +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.Date; @@ -76,17 +81,15 @@ public abstract class AbstractManagerResource manager; + private final Class type; - /** - * Constructs ... - * - * - * @param manager - */ - public AbstractManagerResource(Manager manager) - { + protected int cacheMaxAge = 0; + protected boolean disableCache = false; + + public AbstractManagerResource(Manager manager, Class type) { this.manager = manager; + this.type = type; } //~--- methods -------------------------------------------------------------- @@ -526,45 +529,25 @@ public abstract class AbstractManagerResource createComparator(String sortby, boolean desc) + private Comparator createComparator(String sortBy, boolean desc) { + checkSortByField(sortBy); Comparator comparator; if (desc) { - comparator = new BeanReverseComparator(sortby); + comparator = new BeanReverseComparator(sortBy); } else { - comparator = new BeanComparator(sortby); + comparator = new BeanComparator(sortBy); } return comparator; } - /** - * Method description - * - * - * - * @param sortby - * @param desc - * @param start - * @param limit - * - * @return - */ - private Collection fetchItems(String sortby, boolean desc, int start, + private Collection fetchItems(String sortBy, boolean desc, int start, int limit) { AssertUtil.assertPositive(start); @@ -573,18 +556,18 @@ public abstract class AbstractManagerResource 0) { - if (Util.isEmpty(sortby)) + if (Util.isEmpty(sortBy)) { // replace with something useful - sortby = "id"; + sortBy = "id"; } - items = manager.getAll(createComparator(sortby, desc), start, limit); + items = manager.getAll(createComparator(sortBy, desc), start, limit); } - else if (Util.isNotEmpty(sortby)) + else if (Util.isNotEmpty(sortBy)) { - items = manager.getAll(createComparator(sortby, desc)); + items = manager.getAll(createComparator(sortBy, desc)); } else { @@ -594,17 +577,32 @@ public abstract class AbstractManagerResource fetchPage(String sortby, boolean desc, int pageNumber, + // We have to handle IntrospectionException here, because it's a checked exception + // It shouldn't occur really - so creating a new unchecked exception would be over-engineered here + @SuppressWarnings("squid:S00112") + private void checkSortByField(String sortBy) { + try { + BeanInfo info = Introspector.getBeanInfo(type); + PropertyDescriptor[] pds = info.getPropertyDescriptors(); + if (Arrays.stream(pds).noneMatch(p -> p.getName().equals(sortBy))) { + throw new IllegalArgumentException("sortBy"); + } + } catch (IntrospectionException e) { + throw new RuntimeException("error introspecting model type " + type.getName(), e); + } + } + + protected PageResult fetchPage(String sortBy, boolean desc, int pageNumber, int pageSize) { AssertUtil.assertPositive(pageNumber); AssertUtil.assertPositive(pageSize); - if (Util.isEmpty(sortby)) { + if (Util.isEmpty(sortBy)) { // replace with something useful - sortby = "id"; + sortBy = "id"; } - return manager.getPage(createComparator(sortby, desc), pageNumber, pageSize); + return manager.getPage(createComparator(sortBy, desc), pageNumber, pageSize); } //~--- get methods ---------------------------------------------------------- @@ -676,16 +674,4 @@ public abstract class AbstractManagerResource manager; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/GroupResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/GroupResource.java index 364a1b200e..b9701f7e38 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/GroupResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/GroupResource.java @@ -41,18 +41,12 @@ import com.webcohesion.enunciate.metadata.rs.ResponseCode; import com.webcohesion.enunciate.metadata.rs.ResponseHeader; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; - import org.apache.shiro.SecurityUtils; - import sonia.scm.group.Group; import sonia.scm.group.GroupException; import sonia.scm.group.GroupManager; import sonia.scm.security.Role; -//~--- JDK imports ------------------------------------------------------------ - -import java.util.Collection; - import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.DefaultValue; @@ -69,6 +63,9 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Request; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; +import java.util.Collection; + +//~--- JDK imports ------------------------------------------------------------ /** * RESTful Web Service Resource to manage groups and their members. @@ -97,7 +94,7 @@ public class GroupResource @Inject public GroupResource(GroupManager groupManager) { - super(groupManager); + super(groupManager, Group.class); } //~--- methods -------------------------------------------------------------- diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java index 18b9c7f037..bf974b2fa4 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java @@ -131,7 +131,7 @@ public class RepositoryResource extends AbstractManagerResource @Inject public UserResource(UserManager userManager, PasswordService passwordService) { - super(userManager); + super(userManager, User.class); this.passwordService = passwordService; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionResource.java index 2d777c7487..912d890fe8 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionResource.java @@ -1,13 +1,23 @@ package sonia.scm.api.v2.resources; -import com.webcohesion.enunciate.metadata.rs.*; +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.ResponseHeader; +import com.webcohesion.enunciate.metadata.rs.ResponseHeaders; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; +import com.webcohesion.enunciate.metadata.rs.TypeHint; import sonia.scm.group.Group; import sonia.scm.group.GroupException; import sonia.scm.group.GroupManager; import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.*; +import javax.ws.rs.Consumes; +import javax.ws.rs.DefaultValue; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.Response; import java.io.IOException; @@ -26,16 +36,17 @@ public class GroupCollectionResource { this.dtoToGroupMapper = dtoToGroupMapper; this.groupCollectionToDtoMapper = groupCollectionToDtoMapper; this.resourceLinks = resourceLinks; - this.adapter = new ResourceManagerAdapter<>(manager); + this.adapter = new ResourceManagerAdapter<>(manager, Group.class); } /** * Returns all groups for a given page number with a given page size (default page size is {@value DEFAULT_PAGE_SIZE}). - * + * * Note: This method requires "group" privilege. - * @param page the number of the requested page + * + * @param page the number of the requested page * @param pageSize the page size (default page size is {@value DEFAULT_PAGE_SIZE}) - * @param sortBy sort parameter + * @param sortBy sort parameter (if empty - undefined sorting) * @param desc sort direction desc or aesc */ @GET @@ -44,6 +55,7 @@ public class GroupCollectionResource { @TypeHint(GroupDto[].class) @StatusCodes({ @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 400, condition = "\"sortBy\" field unknown"), @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), @ResponseCode(code = 403, condition = "not authorized, the current user does not have the \"group\" privilege"), @ResponseCode(code = 500, condition = "internal server error") diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupResource.java index 3c1f2aeb64..62bdaad38f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupResource.java @@ -29,7 +29,7 @@ public class GroupResource { GroupDtoToGroupMapper groupDtoToGroupMapper) { this.groupToGroupDtoMapper = groupToGroupDtoMapper; this.dtoToGroupMapper = groupDtoToGroupMapper; - this.adapter = new ResourceManagerAdapter<>(manager); + this.adapter = new ResourceManagerAdapter<>(manager, Group.class); } /** diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java index c2aab1a058..bc35942900 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceManagerAdapter.java @@ -30,8 +30,8 @@ class ResourceManagerAdapter extends AbstractManagerResource { - ResourceManagerAdapter(Manager manager) { - super(manager); + ResourceManagerAdapter(Manager manager, Class type) { + super(manager, type); } /** diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java index 4b88ea48bf..c269cd9f90 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java @@ -1,13 +1,23 @@ package sonia.scm.api.v2.resources; -import com.webcohesion.enunciate.metadata.rs.*; +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.ResponseHeader; +import com.webcohesion.enunciate.metadata.rs.ResponseHeaders; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; +import com.webcohesion.enunciate.metadata.rs.TypeHint; import sonia.scm.user.User; import sonia.scm.user.UserException; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.*; +import javax.ws.rs.Consumes; +import javax.ws.rs.DefaultValue; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.Response; import java.io.IOException; @@ -25,7 +35,7 @@ public class UserCollectionResource { UserCollectionToDtoMapper userCollectionToDtoMapper, ResourceLinks resourceLinks) { this.dtoToUserMapper = dtoToUserMapper; this.userCollectionToDtoMapper = userCollectionToDtoMapper; - this.adapter = new ResourceManagerAdapter<>(manager); + this.adapter = new ResourceManagerAdapter<>(manager, User.class); this.resourceLinks = resourceLinks; } @@ -33,9 +43,10 @@ public class UserCollectionResource { * Returns all users for a given page number with a given page size (default page size is {@value DEFAULT_PAGE_SIZE}). * * Note: This method requires "user" privilege. - * @param page the number of the requested page + * + * @param page the number of the requested page * @param pageSize the page size (default page size is {@value DEFAULT_PAGE_SIZE}) - * @param sortBy sort parameter + * @param sortBy sort parameter (if empty - undefined sorting) * @param desc sort direction desc or asc */ @GET @@ -44,6 +55,7 @@ public class UserCollectionResource { @TypeHint(UserDto[].class) @StatusCodes({ @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 400, condition = "\"sortBy\" field unknown"), @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), @ResponseCode(code = 403, condition = "not authorized, the current user does not have the \"user\" privilege"), @ResponseCode(code = 500, condition = "internal server error") diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java index bf747bab4d..7805ba3440 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java @@ -29,7 +29,7 @@ public class UserResource { public UserResource(UserDtoToUserMapper dtoToUserMapper, UserToUserDtoMapper userToDtoMapper, UserManager manager) { this.dtoToUserMapper = dtoToUserMapper; this.userToDtoMapper = userToDtoMapper; - this.adapter = new ResourceManagerAdapter<>(manager); + this.adapter = new ResourceManagerAdapter<>(manager, User.class); } /** diff --git a/scm-webapp/src/test/java/sonia/scm/api/rest/resources/AbstractManagerResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/rest/resources/AbstractManagerResourceTest.java new file mode 100644 index 0000000000..e8421bc417 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/rest/resources/AbstractManagerResourceTest.java @@ -0,0 +1,122 @@ +package sonia.scm.api.rest.resources; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import sonia.scm.Manager; +import sonia.scm.ModelObject; + +import javax.ws.rs.core.GenericEntity; +import javax.ws.rs.core.Request; +import java.util.Collection; +import java.util.Comparator; + +import static java.util.Collections.emptyList; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class AbstractManagerResourceTest { + + @Mock + private Manager manager; + @Mock + private Request request; + @Captor + private ArgumentCaptor> comparatorCaptor; + + private AbstractManagerResource abstractManagerResource; + + @Before + public void captureComparator() { + when(manager.getAll(comparatorCaptor.capture(), eq(0), eq(1))).thenReturn(emptyList()); + abstractManagerResource = new SimpleManagerResource(); + } + + @Test + public void shouldAcceptDefaultSortByParameter() { + abstractManagerResource.getAll(request, 0, 1, null, true); + + Comparator comparator = comparatorCaptor.getValue(); + assertTrue(comparator.compare(new Simple("1", null), new Simple("2", null)) > 0); + } + + @Test + public void shouldAcceptValidSortByParameter() { + abstractManagerResource.getAll(request, 0, 1, "data", true); + + Comparator comparator = comparatorCaptor.getValue(); + assertTrue(comparator.compare(new Simple("", "1"), new Simple("", "2")) > 0); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldFailForIllegalSortByParameter() { + abstractManagerResource.getAll(request, 0, 1, "x", true); + } + + + private class SimpleManagerResource extends AbstractManagerResource { + + { + disableCache = true; + } + + private SimpleManagerResource() { + super(AbstractManagerResourceTest.this.manager, Simple.class); + } + + @Override + protected GenericEntity> createGenericEntity(Collection items) { + return null; + } + + @Override + protected String getId(Simple item) { + return null; + } + + @Override + protected String getPathPart() { + return null; + } + } + + public static class Simple implements ModelObject { + + private String id; + private String data; + + Simple(String id, String data) { + this.id = id; + this.data = data; + } + + public String getData() { + return data; + } + + @Override + public String getId() { + return id; + } + + @Override + public Long getLastModified() { + return null; + } + + @Override + public String getType() { + return null; + } + @Override + public boolean isValid() { + return false; + } + } +}