Add overall count to collection results

scm-core/src/main/java/sonia/scm/Manager.java

@@ -33,8 +33,6 @@
 
 package sonia.scm;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import sonia.scm.util.Util;
 
 import java.io.IOException;
@@ -130,6 +128,8 @@ public interface Manager<T extends ModelObject, E extends Exception>
    * Returns objects from the store divided into pages with the given page
    * size for the given page number (zero based) and sorted by the given
    * {@link java.util.Comparator}.
+   * <p>This default implementation reads all items, first, so you might want to adapt this
+   * whenever reading is expensive!</p>
    *
    * @param comparator to sort the returned objects
    * @param pageNumber the number of the page to be returned (zero based)
@@ -144,9 +144,11 @@ public interface Manager<T extends ModelObject, E extends Exception>
     checkArgument(pageSize > 0, "pageSize must be at least 1");
     checkArgument(pageNumber >= 0, "pageNumber must be non-negative");
 
-    Collection<T> entities = getAll(comparator, pageNumber * pageSize, pageSize + 1);
+    Collection<T> allEntities = getAll(comparator);
-    boolean hasMore = entities.size() > pageSize;
+
-    return new PageResult<>(Util.createSubCollection(entities, 0, pageSize), hasMore);
+    Collection<T> pagedEntities = Util.createSubCollection(allEntities, pageNumber * pageSize, pageSize);
+
+    return new PageResult<>(pagedEntities, allEntities.size());
   }
 
 }

scm-core/src/main/java/sonia/scm/PageResult.java

@@ -5,16 +5,16 @@ import java.util.Collections;
 
 /**
  * This represents the result of a page request. Contains the results for
- * the page and a flag whether there are more pages or not.
+ * the page and the overall count of all elements.
  */
 public class PageResult<T extends ModelObject> {
 
   private final Collection<T> entities;
-  private final boolean hasMore;
+  private final int overallCount;
 
-  public PageResult(Collection<T> entities, boolean hasMore) {
+  public PageResult(Collection<T> entities, int overallCount) {
     this.entities = entities;
-    this.hasMore = hasMore;
+    this.overallCount = overallCount;
   }
 
   /**
@@ -25,9 +25,9 @@ public class PageResult<T extends ModelObject> {
   }
 
   /**
-   * If this is <code>true</code>, there are more pages (that is, more entities).
+   * The overall count of all available elements.
    */
-  public boolean hasMore() {
+  public int getOverallCount() {
-    return hasMore;
+    return overallCount;
   }
 }

scm-core/src/test/java/sonia/scm/ManagerTest.java

@@ -4,15 +4,17 @@ import org.junit.Test;
 import org.mockito.Mock;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Comparator;
+import java.util.stream.IntStream;
 
-import static org.junit.Assert.*;
+import static java.util.stream.Collectors.toList;
+import static org.junit.Assert.assertEquals;
 
 public class ManagerTest {
 
+  private int givenItemCount = 0;
+
   private Manager manager = new Manager() {
     @Override
     public void refresh(ModelObject object) throws IOException {
@@ -26,12 +28,12 @@ public class ManagerTest {
 
     @Override
     public Collection getAll() {
-      return null;
+      return IntStream.range(0, givenItemCount).boxed().collect(toList());
     }
 
     @Override
     public Collection getAll(Comparator comparator) {
-      return null;
+      return getAll();
     }
 
     @Override
@@ -41,13 +43,7 @@ public class ManagerTest {
 
     @Override
     public Collection getAll(Comparator comparator, int start, int limit) {
-      if (start == 0 && (limit == 3) || (limit == 5)) {
+      return null;
-        return Arrays.asList(1, 2, 3);
-      } else if (start == 0 && limit == 6) {
-        return Collections.emptyList();
-      } else {
-        return Arrays.asList(3);
-      }
     }
 
     @Override
@@ -85,28 +81,37 @@ public class ManagerTest {
 
   @Test
   public void getsNoPage() {
+    givenItemCount = 0;
     PageResult singlePage = manager.getPage(comparator, 0, 5);
-    assertFalse(singlePage.hasMore());
     assertEquals(0, singlePage.getEntities().size());
+    assertEquals(givenItemCount, singlePage.getOverallCount());
   }
 
   @Test
-  public void getsSinglePage() {
+  public void getsSinglePageWithoutEnoughItems() {
-
+    givenItemCount = 3;
     PageResult singlePage = manager.getPage(comparator, 0, 4);
-    assertFalse(singlePage.hasMore());
     assertEquals(3, singlePage.getEntities().size() );
+    assertEquals(givenItemCount, singlePage.getOverallCount());
+  }
+
+  @Test
+  public void getsSinglePageWithExactCountOfItems() {
+    givenItemCount = 3;
+    PageResult singlePage = manager.getPage(comparator, 0, 3);
+    assertEquals(3, singlePage.getEntities().size() );
+    assertEquals(givenItemCount, singlePage.getOverallCount());
   }
 
   @Test
   public void getsTwoPages() {
-
+    givenItemCount = 3;
     PageResult page1 = manager.getPage(comparator, 0, 2);
-    assertTrue(page1.hasMore());
     assertEquals(2, page1.getEntities().size());
+    assertEquals(givenItemCount, page1.getOverallCount());
 
     PageResult page2 = manager.getPage(comparator, 1, 2);
-    assertFalse(page2.hasMore());
     assertEquals(1, page2.getEntities().size());
+    assertEquals(givenItemCount, page2.getOverallCount());
   }
 }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapper.java

@@ -11,13 +11,13 @@ import sonia.scm.group.GroupPermissions;
 import javax.inject.Inject;
 import java.util.EnumSet;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import static com.damnhandy.uri.template.UriTemplate.fromTemplate;
 import static de.otto.edison.hal.Embedded.embeddedBuilder;
 import static de.otto.edison.hal.Link.link;
 import static de.otto.edison.hal.Links.linkingTo;
 import static de.otto.edison.hal.paging.NumberedPaging.zeroBasedNumberedPaging;
+import static java.util.stream.Collectors.toList;
 import static sonia.scm.api.v2.resources.ResourceLinks.groupCollection;
 
 public class GroupCollectionToDtoMapper {
@@ -32,8 +32,8 @@ public class GroupCollectionToDtoMapper {
   }
 
   public GroupCollectionDto map(int pageNumber, int pageSize, PageResult<Group> pageResult) {
-    NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.hasMore());
+    NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.getOverallCount());
-    List<GroupDto> dtos = pageResult.getEntities().stream().map(user -> groupToDtoMapper.map(user)).collect(Collectors.toList());
+    List<GroupDto> dtos = pageResult.getEntities().stream().map(groupToDtoMapper::map).collect(toList());
 
     GroupCollectionDto groupCollectionDto = new GroupCollectionDto(
       createLinks(paging),

scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapper.java

@@ -12,13 +12,13 @@ import javax.inject.Inject;
 import javax.ws.rs.core.UriInfo;
 import java.util.EnumSet;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import static com.damnhandy.uri.template.UriTemplate.fromTemplate;
 import static de.otto.edison.hal.Embedded.embeddedBuilder;
 import static de.otto.edison.hal.Link.link;
 import static de.otto.edison.hal.Links.linkingTo;
 import static de.otto.edison.hal.paging.NumberedPaging.zeroBasedNumberedPaging;
+import static java.util.stream.Collectors.toList;
 import static sonia.scm.api.v2.resources.ResourceLinks.userCollection;
 
 public class UserCollectionToDtoMapper {
@@ -37,8 +37,8 @@ public class UserCollectionToDtoMapper {
   }
 
   public UserCollectionDto map(int pageNumber, int pageSize, PageResult<User> pageResult) {
-    NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.hasMore());
+    NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.getOverallCount());
-    List<UserDto> dtos = pageResult.getEntities().stream().map(userToDtoMapper::map).collect(Collectors.toList());
+    List<UserDto> dtos = pageResult.getEntities().stream().map(userToDtoMapper::map).collect(toList());
 
     UserCollectionDto userCollectionDto = new UserCollectionDto(
       createLinks(uriInfoStore.get(), paging),

scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapperTest.java

@@ -54,35 +54,37 @@ public class GroupCollectionToDtoMapperTest {
 
   @Test
   public void shouldSetPageNumber() {
-    PageResult<Group> pageResult = mockPageResult(true, "nobodies");
+    PageResult<Group> pageResult = mockPageResult("nobodies");
     GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult);
     assertEquals(1, groupCollectionDto.getPage());
   }
 
   @Test
   public void shouldHaveSelfLink() {
-    PageResult<Group> pageResult = mockPageResult(true, "nobodies");
+    PageResult<Group> pageResult = mockPageResult("nobodies");
     GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult);
     assertTrue(groupCollectionDto.getLinks().getLinkBy("self").get().getHref().startsWith(expectedBaseUri.toString()));
   }
 
   @Test
   public void shouldCreateNextPageLink_whenHasMore() {
-    PageResult<Group> pageResult = mockPageResult(true, "nobodies");
+    PageResult<Group> intermediate = mockPageResult("nobodies");
+    PageResult<Group> pageResult = new PageResult<>(intermediate.getEntities(), 2);
+
     GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult);
     assertTrue(groupCollectionDto.getLinks().getLinkBy("next").get().getHref().contains("page=2"));
   }
 
   @Test
   public void shouldNotCreateNextPageLink_whenNoMore() {
-    PageResult<Group> pageResult = mockPageResult(false, "nobodies");
+    PageResult<Group> pageResult = mockPageResult("nobodies");
     GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult);
     assertFalse(groupCollectionDto.getLinks().stream().anyMatch(link -> link.getHref().contains("page=2")));
   }
 
   @Test
   public void shouldHaveCreateLink_whenHasPermission() {
-    PageResult<Group> pageResult = mockPageResult(false, "nobodies");
+    PageResult<Group> pageResult = mockPageResult("nobodies");
     when(subject.isPermitted("group:create")).thenReturn(true);
 
     GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult);
@@ -92,7 +94,7 @@ public class GroupCollectionToDtoMapperTest {
 
   @Test
   public void shouldNotHaveCreateLink_whenHasNoPermission() {
-    PageResult<Group> pageResult = mockPageResult(false, "nobodies");
+    PageResult<Group> pageResult = mockPageResult("nobodies");
     when(subject.isPermitted("group:create")).thenReturn(false);
 
     GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult);
@@ -102,7 +104,7 @@ public class GroupCollectionToDtoMapperTest {
 
   @Test
   public void shouldMapGroups() {
-    PageResult<Group> pageResult = mockPageResult(false, "nobodies", "bosses");
+    PageResult<Group> pageResult = mockPageResult("nobodies", "bosses");
     GroupCollectionDto groupCollectionDto = mapper.map(1, 2, pageResult);
     List<HalRepresentation> groups = groupCollectionDto.getEmbedded().getItemsBy("groups");
     assertEquals(2, groups.size());
@@ -110,9 +112,9 @@ public class GroupCollectionToDtoMapperTest {
     assertEquals("bosses", ((GroupDto) groups.get(1)).getName());
   }
 
-  private PageResult<Group> mockPageResult(boolean hasMore, String... groupNames) {
+  private PageResult<Group> mockPageResult(String... groupNames) {
     Collection<Group> groups = Arrays.stream(groupNames).map(this::mockGroupWithDto).collect(toList());
-    return new PageResult<>(groups, hasMore);
+    return new PageResult<>(groups, groups.size());
   }
 
   private Group mockGroupWithDto(String groupName) {

scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapperTest.java

@@ -62,35 +62,37 @@ public class UserCollectionToDtoMapperTest {
 
   @Test
   public void shouldSetPageNumber() {
-    PageResult<User> pageResult = mockPageResult(true, "Hannes");
+    PageResult<User> pageResult = mockPageResult("Hannes");
     UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult);
     assertEquals(1, userCollectionDto.getPage());
   }
 
   @Test
   public void shouldHaveSelfLink() {
-    PageResult<User> pageResult = mockPageResult(true, "Hannes");
+    PageResult<User> pageResult = mockPageResult("Hannes");
     UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult);
     assertTrue(userCollectionDto.getLinks().getLinkBy("self").get().getHref().startsWith(expectedBaseUri.toString()));
   }
 
   @Test
   public void shouldCreateNextPageLink_whenHasMore() {
-    PageResult<User> pageResult = mockPageResult(true, "Hannes");
+    PageResult<User> intermediate = mockPageResult("Hannes");
+    PageResult<User> pageResult = new PageResult<>(intermediate.getEntities(), 2);
+
     UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult);
     assertTrue(userCollectionDto.getLinks().getLinkBy("next").get().getHref().contains("page=2"));
   }
 
   @Test
   public void shouldNotCreateNextPageLink_whenNoMore() {
-    PageResult<User> pageResult = mockPageResult(false, "Hannes");
+    PageResult<User> pageResult = mockPageResult("Hannes");
     UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult);
     assertFalse(userCollectionDto.getLinks().stream().anyMatch(link -> link.getHref().contains("page=2")));
   }
 
   @Test
   public void shouldHaveCreateLink_whenHasPermission() {
-    PageResult<User> pageResult = mockPageResult(false, "Hannes");
+    PageResult<User> pageResult = mockPageResult("Hannes");
     when(subject.isPermitted("user:create")).thenReturn(true);
 
     UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult);
@@ -100,7 +102,7 @@ public class UserCollectionToDtoMapperTest {
 
   @Test
   public void shouldNotHaveCreateLink_whenHasNoPermission() {
-    PageResult<User> pageResult = mockPageResult(false, "Hannes");
+    PageResult<User> pageResult = mockPageResult("Hannes");
     when(subject.isPermitted("user:create")).thenReturn(false);
 
     UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult);
@@ -110,7 +112,7 @@ public class UserCollectionToDtoMapperTest {
 
   @Test
   public void shouldMapUsers() {
-    PageResult<User> pageResult = mockPageResult(false, "Hannes", "Wurst");
+    PageResult<User> pageResult = mockPageResult("Hannes", "Wurst");
     UserCollectionDto userCollectionDto = mapper.map(1, 2, pageResult);
     List<HalRepresentation> users = userCollectionDto.getEmbedded().getItemsBy("users");
     assertEquals(2, users.size());
@@ -118,9 +120,9 @@ public class UserCollectionToDtoMapperTest {
     assertEquals("Wurst", ((UserDto) users.get(1)).getName());
   }
 
-  private PageResult<User> mockPageResult(boolean hasMore, String... userNames) {
+  private PageResult<User> mockPageResult(String... userNames) {
     Collection<User> users = Arrays.stream(userNames).map(this::mockUserWithDto).collect(toList());
-    return new PageResult<>(users, hasMore);
+    return new PageResult<>(users, users.size());
   }
 
   private User mockUserWithDto(String userName) {

scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java

@@ -26,8 +26,8 @@ import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.util.Collections;
 
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -70,7 +70,7 @@ public class UserRootResourceTest {
   public void prepareEnvironment() throws IOException, UserException {
     initMocks(this);
     User dummyUser = createDummyUser();
-    when(userManager.getPage(any(), eq(0), eq(10))).thenReturn(new PageResult<>(Collections.singletonList(dummyUser), true));
+    when(userManager.getPage(any(), eq(0), eq(10))).thenReturn(new PageResult<>(singletonList(dummyUser), 1));
     when(userManager.get("Neo")).thenReturn(dummyUser);
     doNothing().when(userManager).create(userCaptor.capture());