This is an automated email from the ASF dual-hosted git repository.
sseifert pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-impl.git
The following commit(s) were added to refs/heads/master by this push:
new 6cbc8a4 SLING-12472 Sling Models Impl: Implementation Picker service
ranking is inversed (#60)
6cbc8a4 is described below
commit 6cbc8a4425930ae62586e84ff442bcec79d6e552
Author: Stefan Seifert <[email protected]>
AuthorDate: Tue Nov 12 12:17:15 2024 +0100
SLING-12472 Sling Models Impl: Implementation Picker service ranking is
inversed (#60)
---
.../sling/models/impl/AdapterImplementations.java | 6 +-
.../sling/models/impl/ModelAdapterFactory.java | 10 +-
.../models/impl/AdapterImplementationsTest.java | 4 +-
.../sling/models/impl/ImplementsExtendsTest.java | 2 +-
...apterFactory_ImplementationPickerOrderTest.java | 114 ++++++++++++++++++++
.../ModelAdapterFactory_InjectorOrderTest.java | 117 +++++++++++++++++++++
.../sling/models/impl/MultipleInjectorTest.java | 2 +-
7 files changed, 246 insertions(+), 9 deletions(-)
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 c454bae..a992d69 100644
--- a/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
+++ b/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
@@ -212,7 +212,11 @@ final class AdapterImplementations {
implementationsArray[i] = implementationWrappersArray[i].getType();
}
- for (ImplementationPicker picker : sortedImplementationPickers) {
+ // find first-matching implementation (look in service ranking REVERSE
order)
+ ImplementationPicker[] localPickers =
+ sortedImplementationPickers.toArray(new
ImplementationPicker[sortedImplementationPickers.size()]);
+ for (int pickerIndex = localPickers.length - 1; pickerIndex >= 0;
pickerIndex--) {
+ ImplementationPicker picker = localPickers[pickerIndex];
Class<?> implementation = picker.pick(adapterType,
implementationsArray, adaptable);
if (implementation != null) {
for (int i = 0; i < implementationWrappersArray.length; i++) {
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 69a039b..cbcf41a 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
@@ -572,10 +572,12 @@ public class ModelAdapterFactory implements
AdapterFactory, Runnable, ModelFacto
if (StringUtils.isEmpty(source)) {
source = null;
}
- // find the right injector
- final List<Injector> localInjectors = this.injectors;
+ // find the right injector (look in service ranking REVERSE order)
+ final Injector[] localInjectors = this.injectors.toArray(new
Injector[this.injectors.size()]);
boolean foundSource = false;
- for (final Injector injector : localInjectors) {
+ for (int injectorIndex = localInjectors.length - 1; injectorIndex
>= 0; injectorIndex--) {
+ final Injector injector = localInjectors[injectorIndex];
+
// if a source is given only use injectors with this name.
if (source != null && !source.equals(injector.getName())) {
continue;
@@ -1231,7 +1233,7 @@ public class ModelAdapterFactory implements
AdapterFactory, Runnable, ModelFacto
synchronized (this) {
final Map<Comparable<?>, StaticInjectAnnotationProcessorFactory>
factoryMap =
new
TreeMap<>(this.staticInjectAnnotationProcessorFactories);
- factoryMap.remove((Comparable<?>) props);
+ factoryMap.remove(props);
this.staticInjectAnnotationProcessorFactories = factoryMap;
this.adapterImplementations.setStaticInjectAnnotationProcessorFactories(
staticInjectAnnotationProcessorFactories.values());
diff --git
a/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java
b/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java
index 7d068d0..0fd8b67 100644
--- a/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java
+++ b/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java
@@ -136,9 +136,9 @@ public class AdapterImplementationsTest {
SAMPLE_ADAPTER,
SAMPLE_ADAPTABLE,
Arrays.asList(
- new NoneImplementationPicker(),
+ new FirstImplementationPicker(),
new LastImplementationPicker(),
- new FirstImplementationPicker()))
+ new NoneImplementationPicker()))
.getType());
}
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 1b80283..9d09897 100644
--- a/src/test/java/org/apache/sling/models/impl/ImplementsExtendsTest.java
+++ b/src/test/java/org/apache/sling/models/impl/ImplementsExtendsTest.java
@@ -248,7 +248,7 @@ public class ImplementsExtendsTest {
@Test
public void testImplementsInterfaceModelWithPickLastImplementationPicker()
{
factory.implementationPickers =
- Arrays.asList(new
AdapterImplementationsTest.LastImplementationPicker(),
firstImplementationPicker);
+ Arrays.asList(firstImplementationPicker, new
AdapterImplementationsTest.LastImplementationPicker());
Resource res = getMockResourceWithProps();
SampleServiceInterface model = factory.getAdapter(res,
SampleServiceInterface.class);
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
new file mode 100644
index 0000000..3818f37
--- /dev/null
+++
b/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_ImplementationPickerOrderTest.java
@@ -0,0 +1,114 @@
+/*
+ * 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.function.IntSupplier;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.adapter.AdapterManager;
+import org.apache.sling.models.annotations.Model;
+import org.apache.sling.models.spi.ImplementationPicker;
+import org.apache.sling.models.testutil.ModelAdapterFactoryUtil;
+import org.apache.sling.scripting.api.BindingsValuesProvidersByContext;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertEquals;
+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).
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ModelAdapterFactory_ImplementationPickerOrderTest {
+
+ @Rule
+ public final OsgiContext context = new OsgiContext();
+
+ @Mock
+ private AdapterManager adapterManager;
+
+ @Mock
+ private BindingsValuesProvidersByContext bindingsValuesProvidersByContext;
+
+ @Mock
+ private SlingHttpServletRequest request;
+
+ private ModelAdapterFactory factory;
+
+ @Before
+ public void setUp() {
+ context.registerService(BindingsValuesProvidersByContext.class,
bindingsValuesProvidersByContext);
+ context.registerService(AdapterManager.class, adapterManager);
+ factory =
context.registerInjectActivateService(ModelAdapterFactory.class);
+
+ ModelAdapterFactoryUtil.addModelsForPackage(context.bundleContext(),
Model1.class, Model2.class);
+ }
+
+ @Test
+ public void testFirstImplementationPicker() {
+ context.registerService(
+ ImplementationPicker.class, new FirstImplementationPicker(),
SERVICE_RANKING, Integer.MAX_VALUE);
+
+ IntSupplier result = factory.createModel(request, IntSupplier.class);
+ assertEquals(1, result.getAsInt());
+ }
+
+ @Test
+ public void testMultipleImplementationPickers() {
+ // LastImplementationPicker has higher priority
+ context.registerService(
+ ImplementationPicker.class, new FirstImplementationPicker(),
SERVICE_RANKING, Integer.MAX_VALUE);
+ context.registerService(ImplementationPicker.class, new
LastImplementationPicker(), SERVICE_RANKING, 100);
+
+ IntSupplier result = factory.createModel(request, IntSupplier.class);
+ assertEquals(2, result.getAsInt());
+ }
+
+ static final class LastImplementationPicker implements
ImplementationPicker {
+ @Override
+ public Class<?> pick(
+ @NotNull Class<?> adapterType, Class<?>[]
implementationsTypes, @NotNull Object adaptable) {
+ return implementationsTypes[implementationsTypes.length - 1];
+ }
+ }
+
+ @Model(adaptables = SlingHttpServletRequest.class, adapters =
IntSupplier.class)
+ static final class Model1 implements IntSupplier {
+ @Override
+ public int getAsInt() {
+ return 1;
+ }
+ }
+
+ @Model(adaptables = SlingHttpServletRequest.class, adapters =
IntSupplier.class)
+ static final class Model2 implements IntSupplier {
+ @Override
+ public int getAsInt() {
+ return 2;
+ }
+ }
+}
diff --git
a/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_InjectorOrderTest.java
b/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_InjectorOrderTest.java
new file mode 100644
index 0000000..63cf71c
--- /dev/null
+++
b/src/test/java/org/apache/sling/models/impl/ModelAdapterFactory_InjectorOrderTest.java
@@ -0,0 +1,117 @@
+/*
+ * 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 javax.inject.Inject;
+
+import java.util.Map;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.adapter.AdapterManager;
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.api.wrappers.ValueMapDecorator;
+import org.apache.sling.models.annotations.Model;
+import org.apache.sling.models.impl.injectors.RequestAttributeInjector;
+import org.apache.sling.models.impl.injectors.ValueMapInjector;
+import org.apache.sling.models.spi.Injector;
+import org.apache.sling.models.testutil.ModelAdapterFactoryUtil;
+import org.apache.sling.scripting.api.BindingsValuesProvidersByContext;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.when;
+import static org.osgi.framework.Constants.SERVICE_RANKING;
+
+/**
+ * Tests in which order the injectors are handled depending on service ranking.
+ * For historic/backwards compatibility reasons, higher ranking value means
lower priority (inverse to DS behavior).
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ModelAdapterFactory_InjectorOrderTest {
+
+ @Rule
+ public final OsgiContext context = new OsgiContext();
+
+ @Mock
+ private AdapterManager adapterManager;
+
+ @Mock
+ private BindingsValuesProvidersByContext bindingsValuesProvidersByContext;
+
+ @Mock
+ private SlingHttpServletRequest request;
+
+ @Mock
+ private Resource resource;
+
+ private ModelAdapterFactory factory;
+
+ @SuppressWarnings("null")
+ @Before
+ public void setUp() {
+ context.registerService(BindingsValuesProvidersByContext.class,
bindingsValuesProvidersByContext);
+ context.registerService(AdapterManager.class, adapterManager);
+ factory =
context.registerInjectActivateService(ModelAdapterFactory.class);
+
+ ModelAdapterFactoryUtil.addModelsForPackage(context.bundleContext(),
TestModel.class);
+
+ when(request.getResource()).thenReturn(resource);
+ when(resource.adaptTo(ValueMap.class)).thenReturn(new
ValueMapDecorator(Map.of("prop1", 1)));
+ when(request.getAttribute("prop1")).thenReturn(2);
+ }
+
+ @Test
+ public void testSingleInjector_ValueMap() {
+ context.registerService(Injector.class, new ValueMapInjector(),
SERVICE_RANKING, 2000);
+
+ TestModel model = factory.createModel(request, TestModel.class);
+ assertEquals((Integer) 1, model.getProp1());
+ }
+
+ @Test
+ public void testSingleInjector_RequestAttribute() {
+ context.registerService(Injector.class, new
RequestAttributeInjector(), SERVICE_RANKING, 4000);
+
+ TestModel model = factory.createModel(request, TestModel.class);
+ assertEquals((Integer) 2, model.getProp1());
+ }
+
+ @Test
+ public void testMultipleInjectors() {
+ // ValueMapInjector has higher priority
+ context.registerService(Injector.class, new
RequestAttributeInjector(), SERVICE_RANKING, 4000);
+ context.registerService(Injector.class, new ValueMapInjector(),
SERVICE_RANKING, 2000);
+
+ TestModel model = factory.createModel(request, TestModel.class);
+ assertEquals((Integer) 1, model.getProp1());
+ }
+
+ @Model(adaptables = SlingHttpServletRequest.class)
+ private interface TestModel {
+ @Inject
+ Integer getProp1();
+ }
+}
diff --git
a/src/test/java/org/apache/sling/models/impl/MultipleInjectorTest.java
b/src/test/java/org/apache/sling/models/impl/MultipleInjectorTest.java
index e7bc4ac..2b49009 100644
--- a/src/test/java/org/apache/sling/models/impl/MultipleInjectorTest.java
+++ b/src/test/java/org/apache/sling/models/impl/MultipleInjectorTest.java
@@ -64,7 +64,7 @@ public class MultipleInjectorTest {
factory = AdapterFactoryTest.createModelAdapterFactory();
// binding injector should be asked first as it has a lower service
ranking!
- factory.injectors = Arrays.asList(bindingsInjector,
attributesInjector);
+ factory.injectors = Arrays.asList(attributesInjector,
bindingsInjector);
factory.bindStaticInjectAnnotationProcessorFactory(bindingsInjector,
new ServicePropertiesMap(1, 1));
when(request.getAttribute(SlingBindings.class.getName())).thenReturn(bindings);
