diff --git a/scm-core/src/main/java/sonia/scm/Manager.java b/scm-core/src/main/java/sonia/scm/Manager.java index bfebb01b9c..4067213ebd 100644 --- a/scm-core/src/main/java/sonia/scm/Manager.java +++ b/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 * 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}. + *

This default implementation reads all items, first, so you might want to adapt this + * whenever reading is expensive!

* * @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 checkArgument(pageSize > 0, "pageSize must be at least 1"); checkArgument(pageNumber >= 0, "pageNumber must be non-negative"); - Collection entities = getAll(comparator, pageNumber * pageSize, pageSize + 1); - boolean hasMore = entities.size() > pageSize; - return new PageResult<>(Util.createSubCollection(entities, 0, pageSize), hasMore); + Collection allEntities = getAll(comparator); + + Collection pagedEntities = Util.createSubCollection(allEntities, pageNumber * pageSize, pageSize); + + return new PageResult<>(pagedEntities, allEntities.size()); } } diff --git a/scm-core/src/main/java/sonia/scm/PageResult.java b/scm-core/src/main/java/sonia/scm/PageResult.java index 47ef08c4fc..b4000107b3 100644 --- a/scm-core/src/main/java/sonia/scm/PageResult.java +++ b/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 { private final Collection entities; - private final boolean hasMore; + private final int overallCount; - public PageResult(Collection entities, boolean hasMore) { + public PageResult(Collection entities, int overallCount) { this.entities = entities; - this.hasMore = hasMore; + this.overallCount = overallCount; } /** @@ -25,9 +25,9 @@ public class PageResult { } /** - * If this is true, there are more pages (that is, more entities). + * The overall count of all available elements. */ - public boolean hasMore() { - return hasMore; + public int getOverallCount() { + return overallCount; } } diff --git a/scm-core/src/test/java/sonia/scm/ManagerTest.java b/scm-core/src/test/java/sonia/scm/ManagerTest.java index f54ce6d11c..68f64b7212 100644 --- a/scm-core/src/test/java/sonia/scm/ManagerTest.java +++ b/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 Arrays.asList(1, 2, 3); - } else if (start == 0 && limit == 6) { - return Collections.emptyList(); - } else { - return Arrays.asList(3); - } + return null; } @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()); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapper.java index a6ecba1ab8..915573e5b0 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapper.java +++ b/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 pageResult) { - NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.hasMore()); - List dtos = pageResult.getEntities().stream().map(user -> groupToDtoMapper.map(user)).collect(Collectors.toList()); + NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.getOverallCount()); + List dtos = pageResult.getEntities().stream().map(groupToDtoMapper::map).collect(toList()); GroupCollectionDto groupCollectionDto = new GroupCollectionDto( createLinks(paging), diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapper.java index e3edff91cd..fd0d66660b 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapper.java +++ b/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 pageResult) { - NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.hasMore()); - List dtos = pageResult.getEntities().stream().map(userToDtoMapper::map).collect(Collectors.toList()); + NumberedPaging paging = zeroBasedNumberedPaging(pageNumber, pageSize, pageResult.getOverallCount()); + List dtos = pageResult.getEntities().stream().map(userToDtoMapper::map).collect(toList()); UserCollectionDto userCollectionDto = new UserCollectionDto( createLinks(uriInfoStore.get(), paging), diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapperTest.java index 302affb299..6abb8f5049 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupCollectionToDtoMapperTest.java @@ -54,35 +54,37 @@ public class GroupCollectionToDtoMapperTest { @Test public void shouldSetPageNumber() { - PageResult pageResult = mockPageResult(true, "nobodies"); + PageResult pageResult = mockPageResult("nobodies"); GroupCollectionDto groupCollectionDto = mapper.map(1, 1, pageResult); assertEquals(1, groupCollectionDto.getPage()); } @Test public void shouldHaveSelfLink() { - PageResult pageResult = mockPageResult(true, "nobodies"); + PageResult 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 pageResult = mockPageResult(true, "nobodies"); + PageResult intermediate = mockPageResult("nobodies"); + PageResult 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 pageResult = mockPageResult(false, "nobodies"); + PageResult 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 pageResult = mockPageResult(false, "nobodies"); + PageResult 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 pageResult = mockPageResult(false, "nobodies"); + PageResult 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 pageResult = mockPageResult(false, "nobodies", "bosses"); + PageResult pageResult = mockPageResult("nobodies", "bosses"); GroupCollectionDto groupCollectionDto = mapper.map(1, 2, pageResult); List 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 mockPageResult(boolean hasMore, String... groupNames) { + private PageResult mockPageResult(String... groupNames) { Collection 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) { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapperTest.java index 48d7359eac..3af96071cc 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserCollectionToDtoMapperTest.java @@ -62,35 +62,37 @@ public class UserCollectionToDtoMapperTest { @Test public void shouldSetPageNumber() { - PageResult pageResult = mockPageResult(true, "Hannes"); + PageResult pageResult = mockPageResult("Hannes"); UserCollectionDto userCollectionDto = mapper.map(1, 1, pageResult); assertEquals(1, userCollectionDto.getPage()); } @Test public void shouldHaveSelfLink() { - PageResult pageResult = mockPageResult(true, "Hannes"); + PageResult 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 pageResult = mockPageResult(true, "Hannes"); + PageResult intermediate = mockPageResult("Hannes"); + PageResult 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 pageResult = mockPageResult(false, "Hannes"); + PageResult 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 pageResult = mockPageResult(false, "Hannes"); + PageResult 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 pageResult = mockPageResult(false, "Hannes"); + PageResult 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 pageResult = mockPageResult(false, "Hannes", "Wurst"); + PageResult pageResult = mockPageResult("Hannes", "Wurst"); UserCollectionDto userCollectionDto = mapper.map(1, 2, pageResult); List 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 mockPageResult(boolean hasMore, String... userNames) { + private PageResult mockPageResult(String... userNames) { Collection 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) { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java index 93f0499f14..cdbde92739 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java +++ b/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());