This is an automated email from the ASF dual-hosted git repository.
reschke pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
The following commit(s) were added to refs/heads/master by this push:
new c695cf6f SLING-13287: paging sanity checks - log type when not String
(#226)
c695cf6f is described below
commit c695cf6fd81a7ffcc6c0756e905b12d3814b1cd9
Author: Julian Reschke <[email protected]>
AuthorDate: Mon Aug 10 12:59:37 2026 +0200
SLING-13287: paging sanity checks - log type when not String (#226)
---
pom.xml | 5 +-
.../impl/mapping/PagedQueryIterator.java | 21 ++++-
.../impl/mapping/PagedQueryIteratorTest.java | 45 +++++++++-
.../resourceresolver/impl/mapping/TestLogger.java | 95 ++++++++++++++++++++++
4 files changed, 159 insertions(+), 7 deletions(-)
diff --git a/pom.xml b/pom.xml
index cde5ff73..27756543 100644
--- a/pom.xml
+++ b/pom.xml
@@ -169,8 +169,9 @@
<scope>test</scope>
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
+ <version>1.6.1</version>
<scope>test</scope>
</dependency>
<dependency>
diff --git
a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIterator.java
b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIterator.java
index 2081daa9..d98adb4c 100644
---
a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIterator.java
+++
b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIterator.java
@@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ValueMap;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -82,29 +83,43 @@ public class PagedQueryIterator implements
Iterator<Resource> {
page += 1;
}
+ private static String getDiagInformationWhenNotString(ValueMap valueMap,
String propertyName) {
+ Object value = valueMap.get(propertyName);
+ if (value == null || value instanceof String[] || value instanceof
String) {
+ // all good
+ return "";
+ } else {
+ return " (type: '" + value.getClass() + "')";
+ }
+ }
+
private Resource getNext() throws NoSuchElementException {
Resource resource = it.next();
count += 1;
- final String[] values = resource.getValueMap().get(propertyName,
defaultValue);
+
+ final ValueMap valueMap = resource.getValueMap();
+ final String[] values = valueMap.get(propertyName, defaultValue);
if (values.length > 0) {
String value = values[0];
if (value.compareTo(lastKey) < 0) {
log.warn(
- "unexpected query result in page {}, property name
'{}', got '{}', despite querying for > '{}'"
+ "unexpected query result in page {}, property name
'{}', got '{}'{}, despite querying for > '{}'"
+ " (the async index may not yet reflect the
current property value)",
(page - 1),
propertyName,
value,
+ getDiagInformationWhenNotString(valueMap,
propertyName),
lastKey);
}
if (lastValue != null && value.compareTo(lastValue) < 0) {
log.warn(
- "unexpected query result in page {}, property name
'{}', got '{}', last value was '{}'"
+ "unexpected query result in page {}, property name
'{}', got '{}'{}, last value was '{}'"
+ " (the async index may not yet reflect the
current property value)",
(page - 1),
propertyName,
value,
+ getDiagInformationWhenNotString(valueMap,
propertyName),
lastValue);
}
diff --git
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIteratorTest.java
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIteratorTest.java
index 24ac0777..33a013b8 100644
---
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIteratorTest.java
+++
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PagedQueryIteratorTest.java
@@ -22,7 +22,9 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Date;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.function.Predicate;
import java.util.stream.Collectors;
@@ -31,12 +33,15 @@ import org.apache.sling.api.resource.QuerySyntaxException;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ValueMap;
import org.apache.sling.api.resource.path.Path;
+import org.apache.sling.api.wrappers.ValueMapDecorator;
+import org.apache.sling.api.wrappers.impl.ObjectConverter;
import org.junit.Before;
import org.junit.Test;
import org.mockito.MockitoAnnotations;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
@@ -98,14 +103,50 @@ public class PagedQueryIteratorTest extends
AbstractMappingMapEntriesTest {
checkResult(it, expected);
}
+ @Test
+ public void testSimpleWrongType() {
+ try (TestLogger logger = TestLogger.create(PagedQueryIterator.class)
+ .contains("unexpected")
+ .start()) {
+
+ String[] expected = new String[] {"a", "b", "c"};
+ Collection<Resource> expectedResources = toResourceList(expected);
+
+ Date oneMore = new Date(0);
+ ValueMap properties = new ValueMapDecorator(Map.of(PROPNAME, new
Date[] {oneMore}));
+ Resource r = mock(Resource.class);
+ when(r.getValueMap()).thenReturn(properties);
+
+ expectedResources.add(r);
+
+ when(resourceResolver.findResources(eq("testSimpleWrongType"),
eq("JCR-SQL2")))
+ .thenReturn(expectedResources.iterator());
+ PagedQueryIterator it =
+ new PagedQueryIterator("alias", PROPNAME,
resourceResolver, "testSimpleWrongType", 2000);
+
+ String[] expWithOneMore = Arrays.copyOf(expected, expected.length
+ 1);
+ // implementation detail: this assumes the way Sling converts
Dates to Strings
+ expWithOneMore[expected.length] = ObjectConverter.convert(oneMore,
String.class);
+
+ checkResult(it, expWithOneMore);
+
+ // implementation detail: assumes format of log message
+ List<String> logEntries = logger.stopAndGetLogs();
+ assertTrue(
+ "Log should contain 'class [Ljava.util.Date;', but got: "
+ logEntries,
+ logEntries.toString().contains("class [Ljava.util.Date;"));
+ }
+ }
+
@Test
public void testSimpleWrongResultAfterKey() {
// SLING-13284: out-of-order results across page boundaries should not
abort iteration
String[] expected = new String[] {"x", "x", "a", "a"};
Collection<Resource> expectedResources = toResourceList(expected);
- when(resourceResolver.findResources("testSimpleWrongOrder",
"JCR-SQL2"))
+ when(resourceResolver.findResources("testSimpleWrongResultAfterKey",
"JCR-SQL2"))
.thenReturn(expectedResources.iterator());
- PagedQueryIterator it = new PagedQueryIterator("alias", PROPNAME,
resourceResolver, "testSimpleWrongOrder", 1);
+ PagedQueryIterator it =
+ new PagedQueryIterator("alias", PROPNAME, resourceResolver,
"testSimpleWrongResultAfterKey", 1);
int count = 0;
while (it.hasNext()) {
it.next();
diff --git
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/TestLogger.java
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/TestLogger.java
new file mode 100755
index 00000000..df2a9dca
--- /dev/null
+++
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/TestLogger.java
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.resourceresolver.impl.mapping;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.Appender;
+import ch.qos.logback.core.AppenderBase;
+import org.slf4j.LoggerFactory;
+
+// inspired by Oak's LogCustomizer
+public class TestLogger implements AutoCloseable {
+ private final Appender<ILoggingEvent> customLogger;
+
+ private final Logger delegate;
+ private String matchContainsMessage;
+ private final List<String> logs = Collections.synchronizedList(new
ArrayList<>());
+
+ private TestLogger(Class<?> clazz) {
+ this.delegate = getLogger(clazz);
+
+ this.customLogger = new AppenderBase<>() {
+ @Override
+ protected void append(ILoggingEvent e) {
+ String message = e.getFormattedMessage();
+ if (matchContainsMessage == null ||
message.contains(matchContainsMessage)) {
+ logs.add(message);
+ }
+ }
+ };
+
+ this.customLogger.setContext((LoggerContext)
LoggerFactory.getILoggerFactory());
+ }
+
+ public static TestLogger create(Class<?> clazz) {
+ return new TestLogger(clazz);
+ }
+
+ public TestLogger start() {
+ if (delegate == null) {
+ throw new IllegalStateException();
+ }
+ this.delegate.addAppender(this.customLogger);
+ this.customLogger.start();
+ return this;
+ }
+
+ public TestLogger contains(String matchContainsMessage) {
+ this.matchContainsMessage = matchContainsMessage;
+ return this;
+ }
+
+ public List<String> stopAndGetLogs() {
+ if (this.delegate == null) {
+ throw new IllegalStateException();
+ }
+ delegate.detachAppender(customLogger);
+ customLogger.stop();
+ return logs;
+ }
+
+ public void close() {
+ if (this.delegate == null) {
+ throw new IllegalStateException();
+ }
+ delegate.detachAppender(customLogger);
+ customLogger.stop();
+ logs.clear();
+ }
+
+ private static Logger getLogger(Class<?> clazz) {
+ return ((LoggerContext)
LoggerFactory.getILoggerFactory()).getLogger(clazz);
+ }
+}
