You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2026/06/28 14:30:05 UTC
(sling-org-apache-sling-models-impl) 01/01: SLING-12217 Cache model implementation class lookups per ResourceResolver
This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch SLING-12217-cache-implementation-lookups
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-impl.git
commit db2a88e99d098d77fad5b0648d5ee6fa3dee1d9d
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Jun 28 16:28:19 2026 +0200
SLING-12217 Cache model implementation class lookups per ResourceResolver
- ModelAdapterFactory: cache the resolved ModelClass (or a negative-result
sentinel) in the backing ResourceResolver.getPropertyMap(), as a bounded LRU
map to avoid unbounded growth
- AdapterImplementations: add requiresImplementationPickerLookup() to limit
caching to the picker-based path.
- ModelAdapterFactoryConfiguration: add cache_implementation_lookups (default
true) and implementation_lookup_cache_size (default 100).
- add test cases and run relevant existing tests with and without this feature
enabled
---
pom.xml | 5 +
.../sling/models/impl/AdapterImplementations.java | 22 ++
.../sling/models/impl/ModelAdapterFactory.java | 126 ++++++++++-
.../impl/ModelAdapterFactoryConfiguration.java | 17 ++
.../sling/models/impl/AdapterFactoryTest.java | 10 +
.../org/apache/sling/models/impl/CachingTest.java | 12 +-
.../impl/ImplementationLookupCachingTest.java | 245 +++++++++++++++++++++
.../sling/models/impl/ImplementsExtendsTest.java | 12 +-
...apterFactory_ImplementationPickerOrderTest.java | 15 +-
9 files changed, 460 insertions(+), 4 deletions(-)
diff --git a/pom.xml b/pom.xml
index bd2ed9c..66a7c50 100644
--- a/pom.xml
+++ b/pom.xml
@@ -175,6 +175,11 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.junit.platform</groupId>
<artifactId>junit-platform-engine</artifactId>
diff --git a/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java b/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
index 956cabc..367d0e2 100644
--- a/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
+++ b/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
@@ -227,6 +227,25 @@ final class AdapterImplementations {
return null;
}
+ /**
+ * Checks whether resolving the implementation for the given adapter type requires consulting the
+ * {@link ImplementationPicker}s. This is only the case when the adapter type is not directly registered as its own
+ * model (the cheap, deterministic fast path in {@link #lookup}) but has one or more implementations registered with
+ * a different adapter class. Only that path can cause repository access (resource-type based picking) and is
+ * therefore worth caching, see SLING-12217.
+ *
+ * @param adapterType the adapter type to check
+ * @return {@code true} if a picker has to be consulted to resolve the implementation
+ */
+ public boolean requiresImplementationPickerLookup(Class<?> adapterType) {
+ String key = adapterType.getName();
+ if (modelClasses.containsKey(key)) {
+ return false;
+ }
+ ConcurrentNavigableMap<String, ModelClass<?>> implementations = adapterImplementations.get(key);
+ return implementations != null && !implementations.isEmpty();
+ }
+
/**
* @param adapterType the type to check
* @return {@code true} in case the given type is a model (may be with a different adapter class)
@@ -319,6 +338,9 @@ final class AdapterImplementations {
return null;
}
ResourceResolver resolver = resource.getResourceResolver();
+ if (resolver == null) {
+ return null;
+ }
final String originalResourceType = resource.getResourceType();
Class<?> modelClass = getClassFromResourceTypeMap(originalResourceType, map, resolver);
if (modelClass != null) {
diff --git a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
index 5b7fb8e..6252608 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
@@ -36,6 +36,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Hashtable;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -54,6 +55,7 @@ import org.apache.sling.api.adapter.Adaptable;
import org.apache.sling.api.adapter.AdapterFactory;
import org.apache.sling.api.adapter.AdapterManager;
import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.models.annotations.Model;
import org.apache.sling.models.annotations.ValidationStrategy;
import org.apache.sling.models.annotations.ViaProviderType;
@@ -129,6 +131,30 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
private static final String REQUEST_CACHE_ATTRIBUTE = ModelAdapterFactory.class.getName() + ".AdapterCache";
+ // Implementation lookup cache:
+ // Resolving the implementation class for an adapter type is purely a function of the requested adapter type, the
+ // type of the adaptable and the resource type of the (possibly wrapped) resource. Only the picker-based path walks
+ // the resource super type hierarchy (repository access), so caching is limited to that path (see
+ // AdapterImplementations#requiresImplementationPickerLookup) and is scoped per ResourceResolver via its property
+ // map. This removes redundant lookups for the canCreateFromAdaptable/createModel pair and across components sharing
+ // a resource type, while leaving the cheap fast path untouched. The cache is a bounded LRU map (see
+ // implementationLookupCacheSize) so it cannot grow unbounded for long-lived resolvers or large numbers of distinct
+ // resource types. The cache key is "adaptableClass|adapterType|resourceType"; the concrete adaptable type is
+ // included so custom adaptables (e.g. request wrappers) do not share entries with unrelated ones.
+
+ /**
+ * Key under which the (bounded) implementation lookup cache is stored in the backing
+ * {@link ResourceResolver#getPropertyMap()} (see {@link #getImplementationTypeForAdapterType(Class, Object)}).
+ */
+ private static final String IMPLEMENTATION_LOOKUP_CACHE_KEY =
+ ModelAdapterFactory.class.getName() + ".ImplementationLookup";
+
+ /**
+ * Sentinel stored in the implementation lookup cache to memoize a negative result (no model implementation found
+ * for the given adaptable and resource type), so that repeated probing does not trigger the lookup again.
+ */
+ private static final Object NO_MODEL_IMPLEMENTATION = new Object();
+
private final Logger log = LoggerFactory.getLogger(ModelAdapterFactory.class);
private ReferenceQueue<Object> queue;
@@ -212,6 +238,12 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
private Map<Object, Map<Class<?>, SoftReference<Object>>> adapterCache;
+ /** Whether to cache model implementation class lookups per ResourceResolver, see SLING-12217. */
+ private boolean cacheImplementationLookups;
+
+ /** Maximum number of entries in the per-ResourceResolver implementation lookup cache, see SLING-12217. */
+ private int implementationLookupCacheSize;
+
private SlingModelsScriptEngineFactory scriptEngineFactory;
@Override
@@ -324,22 +356,111 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
* href="http://sling.apache.org/documentation/bundles/models.html#specifying-an-alternate-adapter-class-since-sling-models-110">Specifying
* an Alternate Adapter Class</a>
*/
+ @SuppressWarnings("unchecked")
private <ModelType> ModelClass<ModelType> getImplementationTypeForAdapterType(
Class<ModelType> requestedType, Object adaptable) {
+ // only the (repository-accessing) picker-based path is cached
+ final boolean cacheable =
+ cacheImplementationLookups && adapterImplementations.requiresImplementationPickerLookup(requestedType);
+ final Map<String, Object> cache = cacheable ? getImplementationLookupCache(adaptable) : null;
+ final String cacheKey = cache != null ? getImplementationLookupCacheKey(requestedType, adaptable) : null;
+ if (cache != null && cacheKey != null) {
+ final Object cached = cache.get(cacheKey);
+ if (cached == NO_MODEL_IMPLEMENTATION) {
+ throw newNoImplementationException(requestedType, adaptable);
+ } else if (cached != null) {
+ return (ModelClass<ModelType>) cached;
+ }
+ }
+
// lookup ModelClass wrapper for implementation type
// additionally check if a different implementation class was registered for this adapter type
// the adapter implementation is initially filled by the ModelPackageBundleList
ModelClass<ModelType> modelClass =
this.adapterImplementations.lookup(requestedType, adaptable, this.implementationPickers);
+
+ if (cache != null && cacheKey != null) {
+ cache.put(cacheKey, modelClass != null ? modelClass : NO_MODEL_IMPLEMENTATION);
+ }
+
if (modelClass != null) {
log.debug("Using implementation type {} for requested adapter type {}", modelClass, requestedType);
return modelClass;
}
// throw exception here
- throw new ModelClassException("Could not yet find an adapter factory for the model " + requestedType
+ throw newNoImplementationException(requestedType, adaptable);
+ }
+
+ private ModelClassException newNoImplementationException(Class<?> requestedType, Object adaptable) {
+ return new ModelClassException("Could not yet find an adapter factory for the model " + requestedType
+ " from adaptable " + adaptable.getClass());
}
+ /**
+ * Returns the (resolver scoped) cache for implementation lookups, lazily creating it in the
+ * {@link ResourceResolver#getPropertyMap() property map} of the resolver backing the given adaptable. Returns
+ * {@code null} if no resource (and hence no resolver) can be derived from the adaptable.
+ *
+ * <p>The cache is a bounded LRU map: even for long-lived (e.g. service) resolvers or pathological numbers of
+ * distinct resource types, it never holds more than {@link #implementationLookupCacheSize} entries, evicting the
+ * least recently used ones.
+ */
+ @SuppressWarnings("unchecked")
+ private @Nullable Map<String, Object> getImplementationLookupCache(Object adaptable) {
+ final Resource resource = getResourceFromAdaptable(adaptable);
+ if (resource == null) {
+ return null;
+ }
+ final ResourceResolver resolver = resource.getResourceResolver();
+ if (resolver == null) {
+ return null;
+ }
+ final Map<String, Object> propertyMap = resolver.getPropertyMap();
+ Map<String, Object> cache = (Map<String, Object>) propertyMap.get(IMPLEMENTATION_LOOKUP_CACHE_KEY);
+ if (cache == null) {
+ cache = createBoundedLookupCache(implementationLookupCacheSize);
+ propertyMap.put(IMPLEMENTATION_LOOKUP_CACHE_KEY, cache);
+ }
+ return cache;
+ }
+
+ private static Map<String, Object> createBoundedLookupCache(final int maxSize) {
+ return Collections.synchronizedMap(new LinkedHashMap<String, Object>(16, 0.75f, true) {
+ @Override
+ protected boolean removeEldestEntry(Map.Entry<String, Object> eldest) {
+ return size() > maxSize;
+ }
+ });
+ }
+
+ /**
+ * Builds the cache key for an implementation lookup, or {@code null} if no resource type is available, in which case
+ * the lookup must not be cached.
+ */
+ private @Nullable String getImplementationLookupCacheKey(Class<?> requestedType, Object adaptable) {
+ final Resource resource = getResourceFromAdaptable(adaptable);
+ if (resource == null) {
+ return null;
+ }
+ final String resourceType = resource.getResourceType();
+ if (resourceType == null) {
+ return null;
+ }
+ return adaptable.getClass().getName() + '|' + requestedType.getName() + '|' + resourceType;
+ }
+
+ @SuppressWarnings("deprecation")
+ private @Nullable Resource getResourceFromAdaptable(Object adaptable) {
+ if (adaptable instanceof Resource resource) {
+ return resource;
+ } else if (adaptable instanceof SlingJakartaHttpServletRequest jakartaRequest) {
+ return jakartaRequest.getResource();
+ } else if (adaptable instanceof SlingHttpServletRequest javaxRequest) {
+ return javaxRequest.getResource();
+ }
+ return null;
+ }
+
@SuppressWarnings("unchecked")
private Map<Class<?>, SoftReference<Object>> getOrCreateCache(final Object adaptable) {
Map<Class<?>, SoftReference<Object>> adaptableCache;
@@ -1186,6 +1307,9 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
this.adapterCache =
Collections.synchronizedMap(new WeakHashMap<Object, Map<Class<?>, SoftReference<Object>>>());
+ this.cacheImplementationLookups = configuration.cache_implementation_lookups();
+ this.implementationLookupCacheSize = Math.max(1, configuration.implementation_lookup_cache_size());
+
BundleContext bundleContext = ctx.getBundleContext();
this.queue = new ReferenceQueue<>();
this.disposalCallbacks = new ConcurrentHashMap<>();
diff --git a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactoryConfiguration.java b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactoryConfiguration.java
index 86b25e1..e2c9f59 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactoryConfiguration.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactoryConfiguration.java
@@ -30,4 +30,21 @@ public @interface ModelAdapterFactoryConfiguration {
name = "Cleanup Job Period",
description = "Period in seconds at which OSGi service references from ThreadLocals will be cleaned up.")
long cleanup_job_period() default 30l;
+
+ @AttributeDefinition(
+ name = "Cache Implementation Lookups",
+ description = "Cache the resolution of a model implementation class per adaptable type and resource type "
+ + "within a ResourceResolver. This avoids redundant repository lookups when the same model is "
+ + "resolved repeatedly (e.g. once for canCreateFromAdaptable and once for createModel, or across "
+ + "many components sharing a resource type). Disable this if a custom ImplementationPicker selects "
+ + "an implementation based on more than the resource type (e.g. selectors or request attributes).")
+ boolean cache_implementation_lookups() default true;
+
+ @AttributeDefinition(
+ name = "Implementation Lookup Cache Size",
+ description = "Maximum number of entries in the per-ResourceResolver implementation lookup cache (see "
+ + "'Cache Implementation Lookups'). Once exceeded, the least recently used entries are evicted, so "
+ + "that the cache cannot grow unbounded for long-lived resolvers or large numbers of distinct "
+ + "resource types. Must be greater than 0.")
+ int implementation_lookup_cache_size() default 100;
}
diff --git a/src/test/java/org/apache/sling/models/impl/AdapterFactoryTest.java b/src/test/java/org/apache/sling/models/impl/AdapterFactoryTest.java
index 003dd5f..1da4e48 100644
--- a/src/test/java/org/apache/sling/models/impl/AdapterFactoryTest.java
+++ b/src/test/java/org/apache/sling/models/impl/AdapterFactoryTest.java
@@ -80,13 +80,23 @@ public class AdapterFactoryTest {
return createModelAdapterFactory(bundleContext);
}
+ public static ModelAdapterFactory createModelAdapterFactory(boolean cacheImplementationLookups) {
+ return createModelAdapterFactory(Mockito.mock(BundleContext.class), cacheImplementationLookups);
+ }
+
public static ModelAdapterFactory createModelAdapterFactory(BundleContext bundleContext) {
+ return createModelAdapterFactory(bundleContext, true);
+ }
+
+ public static ModelAdapterFactory createModelAdapterFactory(
+ BundleContext bundleContext, boolean cacheImplementationLookups) {
ComponentContext componentCtx = Mockito.mock(ComponentContext.class);
when(componentCtx.getBundleContext()).thenReturn(bundleContext);
ModelAdapterFactory factory = new ModelAdapterFactory();
Converter c = Converters.standardConverter();
Map<String, String> map = new HashMap<>();
+ map.put("cache.implementation.lookups", Boolean.toString(cacheImplementationLookups));
ModelAdapterFactoryConfiguration config = c.convert(map).to(ModelAdapterFactoryConfiguration.class);
factory.activate(componentCtx, config);
factory.injectAnnotationProcessorFactories = Collections.emptyList();
diff --git a/src/test/java/org/apache/sling/models/impl/CachingTest.java b/src/test/java/org/apache/sling/models/impl/CachingTest.java
index b9217f3..8c46a60 100644
--- a/src/test/java/org/apache/sling/models/impl/CachingTest.java
+++ b/src/test/java/org/apache/sling/models/impl/CachingTest.java
@@ -39,6 +39,9 @@ import org.apache.sling.servlethelpers.MockSlingJakartaHttpServletRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.mockito.Spy;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -50,9 +53,16 @@ import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+// run with the implementation-lookup cache (SLING-12217) both enabled and disabled - the model instance caching
+// behavior asserted here must be identical regardless of that optimization
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
@ExtendWith(MockitoExtension.class)
class CachingTest {
+ @Parameter
+ private boolean cacheImplementationLookups;
+
@Spy
private MockSlingJakartaHttpServletRequest request = new MockSlingJakartaHttpServletRequest(null);
@@ -65,7 +75,7 @@ class CachingTest {
@BeforeEach
void setup() {
- factory = AdapterFactoryTest.createModelAdapterFactory();
+ factory = AdapterFactoryTest.createModelAdapterFactory(cacheImplementationLookups);
factory.injectors = Arrays.asList(new RequestAttributeInjector(), new ValueMapInjector());
factory.adapterImplementations.addClassesAsAdapterAndImplementation(
CachedModel.class,
diff --git a/src/test/java/org/apache/sling/models/impl/ImplementationLookupCachingTest.java b/src/test/java/org/apache/sling/models/impl/ImplementationLookupCachingTest.java
new file mode 100644
index 0000000..519c215
--- /dev/null
+++ b/src/test/java/org/apache/sling/models/impl/ImplementationLookupCachingTest.java
@@ -0,0 +1,245 @@
+/*
+ * 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.models.impl;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.models.annotations.Model;
+import org.apache.sling.models.spi.ImplementationPicker;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.osgi.framework.BundleContext;
+import org.osgi.service.component.ComponentContext;
+import org.osgi.util.converter.Converters;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for the per-{@link ResourceResolver} caching of model implementation class lookups (SLING-12217).
+ *
+ * <p>The lookup of the implementation class is the operation that - for resource-type-based adaptation - walks the
+ * resource super type hierarchy and therefore causes repository access. By counting how often the
+ * {@link ImplementationPicker} is consulted, these tests verify that the lookup is performed at most once per
+ * {@code (adaptable type, requested type, resource type)} within a single resolver.
+ */
+@ExtendWith(MockitoExtension.class)
+class ImplementationLookupCachingTest {
+
+ /** The single resolver shared by all resources, providing the (resolver-scoped) cache via its property map. */
+ private ResourceResolver resolver;
+
+ private final Map<String, Object> propertyMap = new HashMap<>();
+
+ /** Counts how often the implementation lookup actually consults the picker. */
+ private final AtomicInteger pickCount = new AtomicInteger();
+
+ private final ImplementationPicker countingPicker = new ImplementationPicker() {
+ @Override
+ public Class<?> pick(
+ @NotNull Class<?> adapterType, Class<?> @NotNull [] implementationsTypes, @NotNull Object adaptable) {
+ pickCount.incrementAndGet();
+ if (adapterType == TestAdapter.class) {
+ return TestModel.class;
+ }
+ // UnmatchedAdapter has registered implementations but the picker selects none -> negative result
+ return null;
+ }
+ };
+
+ @BeforeEach
+ void setUp() {
+ resolver = mock(ResourceResolver.class);
+ lenient().when(resolver.getPropertyMap()).thenReturn(propertyMap);
+ }
+
+ private ModelAdapterFactory createFactory(boolean cacheLookups) {
+ return createFactory(cacheLookups, 100);
+ }
+
+ private ModelAdapterFactory createFactory(boolean cacheLookups, int cacheSize) {
+ ComponentContext componentCtx = mock(ComponentContext.class);
+ BundleContext bundleContext = mock(BundleContext.class);
+ when(componentCtx.getBundleContext()).thenReturn(bundleContext);
+
+ ModelAdapterFactory factory = new ModelAdapterFactory();
+ Map<String, Object> config = new HashMap<>();
+ config.put("cache.implementation.lookups", cacheLookups);
+ config.put("implementation.lookup.cache.size", cacheSize);
+ factory.activate(
+ componentCtx,
+ Converters.standardConverter().convert(config).to(ModelAdapterFactoryConfiguration.class));
+ factory.injectAnnotationProcessorFactories = Collections.emptyList();
+ factory.injectAnnotationProcessorFactories2 = Collections.emptyList();
+ factory.injectors = Collections.emptyList();
+ factory.implementationPickers = Collections.singletonList(countingPicker);
+ // register with an explicit adapter type so that the picker (and hence the lookup) is consulted
+ factory.adapterImplementations.addAll(TestModel.class, TestAdapter.class);
+ factory.adapterImplementations.addAll(UnmatchedModel.class, UnmatchedAdapter.class);
+ return factory;
+ }
+
+ /** Size of the nested (bounded) lookup cache stored in the resolver property map, or 0 if none was created yet. */
+ @SuppressWarnings("unchecked")
+ private int nestedCacheSize() {
+ if (propertyMap.isEmpty()) {
+ return 0;
+ }
+ return ((Map<String, Object>) propertyMap.values().iterator().next()).size();
+ }
+
+ private Resource resourceOfType(String resourceType) {
+ Resource resource = mock(Resource.class);
+ lenient().when(resource.getResourceType()).thenReturn(resourceType);
+ lenient().when(resource.getResourceResolver()).thenReturn(resolver);
+ return resource;
+ }
+
+ @Test
+ void lookupIsCachedAcrossComponentsWithSameResourceType() {
+ ModelAdapterFactory factory = createFactory(true);
+
+ TestAdapter first = factory.getAdapter(resourceOfType("my/type"), TestAdapter.class);
+ TestAdapter second = factory.getAdapter(resourceOfType("my/type"), TestAdapter.class);
+
+ assertNotNull(first);
+ assertNotNull(second);
+ assertEquals(1, pickCount.get(), "the implementation lookup must run only once for a given resource type");
+ }
+
+ @Test
+ void canCreateAndCreateShareTheCachedLookup() {
+ ModelAdapterFactory factory = createFactory(true);
+ Resource resource = resourceOfType("my/type");
+
+ // this is the JavaUseProvider pattern: guard with canCreateFromAdaptable, then createModel
+ assertTrue(factory.canCreateFromAdaptable(resource, TestAdapter.class));
+ TestAdapter model = factory.createModel(resource, TestAdapter.class);
+
+ assertNotNull(model);
+ assertEquals(1, pickCount.get(), "canCreateFromAdaptable and createModel must not look up the type twice");
+ }
+
+ @Test
+ void differentResourceTypesAreNotSharedAndResolveCorrectly() {
+ ModelAdapterFactory factory = createFactory(true);
+
+ TestAdapter a = factory.getAdapter(resourceOfType("type/a"), TestAdapter.class);
+ TestAdapter b = factory.getAdapter(resourceOfType("type/b"), TestAdapter.class);
+
+ assertNotNull(a);
+ assertNotNull(b);
+ assertEquals(2, pickCount.get(), "different resource types must each trigger their own lookup");
+ }
+
+ @Test
+ void negativeResultIsCached() {
+ ModelAdapterFactory factory = createFactory(true);
+
+ // UnmatchedAdapter has registered implementations but the picker returns none -> no model
+ assertFalse(factory.canCreateFromAdaptable(resourceOfType("my/type"), UnmatchedAdapter.class));
+ assertFalse(factory.canCreateFromAdaptable(resourceOfType("my/type"), UnmatchedAdapter.class));
+
+ assertEquals(1, pickCount.get(), "a negative lookup result must be memoized as well");
+ }
+
+ @Test
+ void freshResolverDoesNotShareTheCache() {
+ ModelAdapterFactory factory = createFactory(true);
+
+ factory.getAdapter(resourceOfType("my/type"), TestAdapter.class);
+
+ // a second resolver has its own property map -> the cache does not carry over
+ ResourceResolver otherResolver = mock(ResourceResolver.class);
+ when(otherResolver.getPropertyMap()).thenReturn(new HashMap<>());
+ Resource otherResource = mock(Resource.class);
+ when(otherResource.getResourceType()).thenReturn("my/type");
+ when(otherResource.getResourceResolver()).thenReturn(otherResolver);
+
+ factory.getAdapter(otherResource, TestAdapter.class);
+
+ assertEquals(2, pickCount.get(), "a different resolver must not reuse the cached lookup");
+ }
+
+ @Test
+ void leastRecentlyUsedEntryIsEvictedBeyondTheCap() {
+ ModelAdapterFactory factory = createFactory(true, 2);
+
+ factory.getAdapter(resourceOfType("type/a"), TestAdapter.class); // miss -> cache [a]
+ factory.getAdapter(resourceOfType("type/b"), TestAdapter.class); // miss -> cache [a, b]
+ factory.getAdapter(resourceOfType("type/c"), TestAdapter.class); // miss -> evicts a -> cache [b, c]
+ assertEquals(3, pickCount.get());
+
+ // type/a was evicted, so it must be looked up again
+ factory.getAdapter(resourceOfType("type/a"), TestAdapter.class); // miss -> evicts b -> cache [c, a]
+ assertEquals(4, pickCount.get(), "evicted entries must be recomputed");
+
+ // type/c is still cached and was made most-recently-used above, so no new lookup
+ factory.getAdapter(resourceOfType("type/c"), TestAdapter.class);
+ assertEquals(4, pickCount.get(), "a still-cached entry must not be looked up again");
+
+ assertEquals(2, nestedCacheSize(), "the cache must never exceed its configured size");
+ }
+
+ @Test
+ void longLivedResolverCacheStaysBounded() {
+ ModelAdapterFactory factory = createFactory(true, 3);
+
+ for (int i = 0; i < 50; i++) {
+ factory.getAdapter(resourceOfType("type/" + i), TestAdapter.class);
+ }
+
+ assertEquals(50, pickCount.get(), "every distinct resource type triggers a lookup once");
+ assertEquals(3, nestedCacheSize(), "the cache must stay bounded regardless of resolver lifetime");
+ }
+
+ @Test
+ void cachingCanBeDisabled() {
+ ModelAdapterFactory factory = createFactory(false);
+
+ factory.getAdapter(resourceOfType("my/type"), TestAdapter.class);
+ factory.getAdapter(resourceOfType("my/type"), TestAdapter.class);
+
+ assertEquals(2, pickCount.get(), "with caching disabled the lookup runs on every adaptation");
+ assertTrue(propertyMap.isEmpty(), "no cache entries must be written when caching is disabled");
+ }
+
+ public interface TestAdapter {}
+
+ @Model(adaptables = Resource.class, adapters = TestAdapter.class)
+ public static class TestModel implements TestAdapter {}
+
+ public interface UnmatchedAdapter {}
+
+ @Model(adaptables = Resource.class, adapters = UnmatchedAdapter.class)
+ public static class UnmatchedModel implements UnmatchedAdapter {}
+}
diff --git a/src/test/java/org/apache/sling/models/impl/ImplementsExtendsTest.java b/src/test/java/org/apache/sling/models/impl/ImplementsExtendsTest.java
index fe70e0a..6d6110b 100644
--- a/src/test/java/org/apache/sling/models/impl/ImplementsExtendsTest.java
+++ b/src/test/java/org/apache/sling/models/impl/ImplementsExtendsTest.java
@@ -47,6 +47,9 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -67,9 +70,16 @@ import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
+// run with the implementation-lookup cache (SLING-12217) both enabled and disabled - resolving the alternate
+// implementation class via the picker is exactly the path that gets cached, so its result must not change
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
@ExtendWith(MockitoExtension.class)
class ImplementsExtendsTest {
+ @Parameter
+ private boolean cacheImplementationLookups;
+
@Mock
private BundleContext bundleContext;
@@ -109,7 +119,7 @@ class ImplementsExtendsTest {
}
});
- factory = AdapterFactoryTest.createModelAdapterFactory(bundleContext);
+ factory = AdapterFactoryTest.createModelAdapterFactory(bundleContext, cacheImplementationLookups);
factory.injectors = Collections.singletonList(new ValueMapInjector());
factory.implementationPickers = Collections.singletonList(firstImplementationPicker);
diff --git a/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_ImplementationPickerOrderTest.java b/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_ImplementationPickerOrderTest.java
index 991aca1..1200934 100644
--- a/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_ImplementationPickerOrderTest.java
+++ b/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_ImplementationPickerOrderTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.sling.models.impl;
+import java.util.Map;
import java.util.function.IntSupplier;
import org.apache.sling.api.SlingJakartaHttpServletRequest;
@@ -32,6 +33,9 @@ import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -41,10 +45,18 @@ import static org.osgi.framework.Constants.SERVICE_RANKING;
/**
* Tests in which order the implementation pickers are handled depending on service ranking.
* For historic/backwards compatibility reasons, higher ranking value means lower priority (inverse to DS behavior).
+ *
+ * <p>Run with the implementation-lookup cache (SLING-12217) both enabled and disabled, as consulting the pickers is
+ * exactly the operation that gets cached - the picked implementation must be the same in both cases.
*/
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
@ExtendWith({OsgiContextExtension.class, MockitoExtension.class})
class ModelAdapterFactory_ImplementationPickerOrderTest {
+ @Parameter
+ boolean cacheImplementationLookups;
+
final OsgiContext context = new OsgiContext();
@Mock
@@ -62,7 +74,8 @@ class ModelAdapterFactory_ImplementationPickerOrderTest {
void setUp() {
context.registerService(BindingsValuesProvidersByContext.class, bindingsValuesProvidersByContext);
context.registerService(AdapterManager.class, adapterManager);
- factory = context.registerInjectActivateService(ModelAdapterFactory.class);
+ factory = context.registerInjectActivateService(
+ ModelAdapterFactory.class, Map.of("cache.implementation.lookups", cacheImplementationLookups));
ModelAdapterFactoryUtil.addModelsForPackage(context.bundleContext(), Model1.class, Model2.class);
}