joerghoh commented on code in PR #119:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/119#discussion_r1687993213


##########
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java:
##########
@@ -1393,6 +1437,27 @@ public Resource next() throws NoSuchElementException {
             next = null;
             return result;
         }
+
+        private void doPageStats() {
+            largestPage = Math.max(largestPage, count - 1);
+            log.debug("read {} query (page {}); {} entries, last key was: {}, 
largest page so far: {}", subject, page - 1,
+                    count, lastKey, largestPage);
+        }
+
+        public @NotNull String getStatistics() {
+            return String.format(" (max. page size: %d, number of pages: %d)", 
largestPage, page);
+        }
+
+        public @NotNull String getWarning() {
+            int warnAt = pageSize * 10;
+            if (largestKeyCount > warnAt) {
+                return String.format(
+                        "Largest number of aliases with the same 'first' 
selector exceeds expectations (value '%s' appears %d times)",
+                        largestKeyValue, largestKeyCount, warnAt);

Review Comment:
   I don't find a format specifier for ``warnAt``



##########
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java:
##########
@@ -1393,6 +1437,27 @@ public Resource next() throws NoSuchElementException {
             next = null;
             return result;
         }
+
+        private void doPageStats() {

Review Comment:
   The method name is a bit unspecific, maybe this?
   ```suggestion
           private void updatePageStats() {
   ```



##########
src/test/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIteratorTest.java:
##########
@@ -106,23 +105,39 @@ public void testSimpleWrongOrder() {
         }
     }
 
-    @Ignore("SLING-12384: resources with empty keys lost")
     @Test
     public void testPagedWithEmpty() {
         String[] expected = new String[] { "", "a", "b", "c", "d" };
         Collection<Resource> expectedResources = toResourceList(expected);
         Collection<Resource> expectedFilteredResources = filter("", 
expectedResources);
         when(resourceResolver.findResources(eq("testPagedWithEmpty ''"), 
eq("JCR-SQL2")))
                 .thenReturn(expectedFilteredResources.iterator());
-        Iterator<Resource> it = mapEntries.new PagedQueryIterator("alias", 
PROPNAME, resourceResolver, "testPagedWithEmpty '%s'",
+        PagedQueryIterator it = mapEntries.new PagedQueryIterator("alias", 
PROPNAME, resourceResolver, "testPagedWithEmpty '%s'",
                 2000);
-        for (String key : expected) {
-            assertEquals(key, getFirstValueOf(it.next(), PROPNAME));
-        }
-        assertFalse(it.hasNext());
+        checkResult(it, expected);
+        assertEquals("", it.getWarning());
+    }
+
+    @Test
+    public void testPagedLargePage() {
+        String[] expected = new String[] { "a", "a", "a", "a", "a", "a", "a", 
"a", "a", "a", "a", "a", "a", "a", "a", "a", "a", "a",

Review Comment:
   ```suggestion
           String[] expected = new String[140];
           Arrays.fill(expected,"a");
   ```
   (I haven't counted the instances of 'a', but I assume it's 140 because that 
number is mentioned below.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to