This is an automated email from the ASF dual-hosted git repository.
stefanseifert 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 1eae10d SLING-13235 Record-based Sling Model with static fields fails
to instantiate (#87)
1eae10d is described below
commit 1eae10d1d78f9a733acd95b90fc401282045c392
Author: Herman Ciechanowiec <[email protected]>
AuthorDate: Fri Jun 26 18:04:18 2026 +0200
SLING-13235 Record-based Sling Model with static fields fails to
instantiate (#87)
Detect the canonical record constructor exactly via Java 17 record APIs
(Class.isRecord() / Class.getRecordComponents()) instead of the pre-Java-17
field-counting heuristic, which miscounted static fields and prevented
records with static fields from being instantiated as Sling Models.
Drop the obsolete reflective record machinery from ReflectionUtil and add
record-based test fixtures and RecordModelTest.
---
.../apache/sling/models/impl/ReflectionUtil.java | 38 -----
.../apache/sling/models/impl/model/ModelClass.java | 2 +-
.../models/impl/model/ModelClassConstructor.java | 13 +-
.../apache/sling/models/impl/RecordModelTest.java | 85 ++++++++++++
.../sling/models/impl/ReflectionUtilTest.java | 153 ---------------------
.../classes/constructorinjection/RecordModel.java | 29 ++++
.../RecordModelWithExtraConstructor.java | 34 +++++
.../RecordModelWithStaticFields.java | 39 ++++++
8 files changed, 197 insertions(+), 196 deletions(-)
diff --git a/src/main/java/org/apache/sling/models/impl/ReflectionUtil.java
b/src/main/java/org/apache/sling/models/impl/ReflectionUtil.java
index 7cd6b0a..5cfa000 100644
--- a/src/main/java/org/apache/sling/models/impl/ReflectionUtil.java
+++ b/src/main/java/org/apache/sling/models/impl/ReflectionUtil.java
@@ -23,7 +23,6 @@ import javax.inject.Inject;
import java.lang.annotation.Annotation;
import java.lang.reflect.*;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.List;
import org.apache.commons.lang3.ClassUtils;
@@ -34,18 +33,6 @@ import
org.apache.sling.models.spi.injectorspecific.InjectAnnotation;
*/
public final class ReflectionUtil {
- static Class<?> recordType;
-
- static {
- try {
- recordType = Class.forName("java.lang.Record");
- } catch (ClassNotFoundException e) {
- // this happens when running with Java11, which is supported, but
- // of course does not have support for Record types
- recordType = null;
- }
- }
-
private ReflectionUtil() {
// static methods only
}
@@ -133,29 +120,4 @@ public final class ReflectionUtil {
return type;
}
}
-
- public static boolean isRecord(Class<?> checkedType) {
- if (recordType == null) {
- return false;
- }
- return recordType.isAssignableFrom(checkedType);
- }
-
- /**
- * Checks if the number of parameters in the specified constructor matches
the number of fields in the class
- * that declares the constructor. Can be useful for detection of canonical
constructors in records.
- * Synthetic fields are ignored.
- *
- * @param constructor the constructor to check
- * @return {@code true} if the number of constructor parameters equals the
number of fields in the declaring class,
- * {@code false} otherwise
- */
- public static boolean areBalancedCtorParamsAndFields(Constructor<?>
constructor) {
- int numOfCtorParams = constructor.getParameterCount();
- Class<?> declaringClass = constructor.getDeclaringClass();
- Field[] fields = declaringClass.getDeclaredFields();
- long numOfFields =
- Arrays.stream(fields).filter(field ->
!field.isSynthetic()).count();
- return numOfCtorParams == numOfFields;
- }
}
diff --git a/src/main/java/org/apache/sling/models/impl/model/ModelClass.java
b/src/main/java/org/apache/sling/models/impl/model/ModelClass.java
index 9ee2b81..a49ec5f 100644
--- a/src/main/java/org/apache/sling/models/impl/model/ModelClass.java
+++ b/src/main/java/org/apache/sling/models/impl/model/ModelClass.java
@@ -84,7 +84,7 @@ public class ModelClass<ModelType> {
Class<?> type,
StaticInjectAnnotationProcessorFactory[] processorFactories,
DefaultInjectionStrategy defaultInjectionStrategy) {
- if (type.isInterface() || ReflectionUtil.isRecord(type)) {
+ if (type.isInterface() || type.isRecord()) {
return new InjectableField[0];
}
List<Field> injectableFields =
ReflectionUtil.collectInjectableFields(type);
diff --git
a/src/main/java/org/apache/sling/models/impl/model/ModelClassConstructor.java
b/src/main/java/org/apache/sling/models/impl/model/ModelClassConstructor.java
index 35fed65..be41e06 100644
---
a/src/main/java/org/apache/sling/models/impl/model/ModelClassConstructor.java
+++
b/src/main/java/org/apache/sling/models/impl/model/ModelClassConstructor.java
@@ -23,10 +23,11 @@ import javax.inject.Inject;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Parameter;
+import java.lang.reflect.RecordComponent;
+import java.util.Arrays;
import java.util.stream.IntStream;
import org.apache.sling.models.annotations.DefaultInjectionStrategy;
-import org.apache.sling.models.impl.ReflectionUtil;
import
org.apache.sling.models.spi.injectorspecific.StaticInjectAnnotationProcessorFactory;
public class ModelClassConstructor<M> {
@@ -86,8 +87,12 @@ public class ModelClassConstructor<M> {
public boolean isCanonicalRecordConstructor() {
Class<M> declaringClass = constructor.getDeclaringClass();
- boolean areBalancedCtorParamsAndFields =
ReflectionUtil.areBalancedCtorParamsAndFields(constructor);
- boolean isRecordDeclaringClass =
ReflectionUtil.isRecord(declaringClass);
- return areBalancedCtorParamsAndFields && isRecordDeclaringClass;
+ if (!declaringClass.isRecord()) {
+ return false;
+ }
+ Class<?>[] componentTypes =
Arrays.stream(declaringClass.getRecordComponents())
+ .map(RecordComponent::getType)
+ .toArray(Class<?>[]::new);
+ return Arrays.equals(componentTypes, constructor.getParameterTypes());
}
}
diff --git a/src/test/java/org/apache/sling/models/impl/RecordModelTest.java
b/src/test/java/org/apache/sling/models/impl/RecordModelTest.java
new file mode 100644
index 0000000..b690ffc
--- /dev/null
+++ b/src/test/java/org/apache/sling/models/impl/RecordModelTest.java
@@ -0,0 +1,85 @@
+/*
+ * 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.Arrays;
+
+import org.apache.sling.api.SlingJakartaHttpServletRequest;
+import org.apache.sling.models.impl.injectors.RequestAttributeInjector;
+import org.apache.sling.models.impl.injectors.SelfInjector;
+import
org.apache.sling.models.testmodels.classes.constructorinjection.RecordModel;
+import
org.apache.sling.models.testmodels.classes.constructorinjection.RecordModelWithExtraConstructor;
+import
org.apache.sling.models.testmodels.classes.constructorinjection.RecordModelWithStaticFields;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.Mockito.lenient;
+
+@ExtendWith(MockitoExtension.class)
+class RecordModelTest {
+
+ private ModelAdapterFactory factory;
+
+ @Mock
+ private SlingJakartaHttpServletRequest request;
+
+ private static final int INT_VALUE = 42;
+
+ private static final String STRING_VALUE = "myValue";
+
+ @BeforeEach
+ void setup() {
+
lenient().when(request.getAttribute("attribute")).thenReturn(INT_VALUE);
+
lenient().when(request.getAttribute("attribute2")).thenReturn(STRING_VALUE);
+
+ factory = AdapterFactoryTest.createModelAdapterFactory();
+ factory.injectors = Arrays.asList(new RequestAttributeInjector(), new
SelfInjector());
+ factory.adapterImplementations.addClassesAsAdapterAndImplementation(
+ RecordModel.class, RecordModelWithStaticFields.class,
RecordModelWithExtraConstructor.class);
+ }
+
+ @Test
+ void testRecordModel() {
+ RecordModel model = factory.getAdapter(request, RecordModel.class);
+ assertNotNull(model);
+ assertEquals(INT_VALUE, model.attribute());
+ assertEquals(STRING_VALUE, model.attribute2());
+ }
+
+ @Test
+ void testRecordModelWithStaticFields() {
+ RecordModelWithStaticFields model = factory.getAdapter(request,
RecordModelWithStaticFields.class);
+ assertNotNull(model);
+ assertEquals(INT_VALUE, model.attribute());
+ assertEquals(STRING_VALUE, model.attribute2());
+ }
+
+ @Test
+ void testRecordModelWithExtraConstructorUsesCanonicalConstructor() {
+ RecordModelWithExtraConstructor model = factory.getAdapter(request,
RecordModelWithExtraConstructor.class);
+ assertNotNull(model);
+ assertEquals(INT_VALUE, model.attribute());
+ assertEquals(STRING_VALUE, model.attribute2());
+ }
+}
diff --git a/src/test/java/org/apache/sling/models/impl/ReflectionUtilTest.java
b/src/test/java/org/apache/sling/models/impl/ReflectionUtilTest.java
deleted file mode 100644
index 1dccb0f..0000000
--- a/src/test/java/org/apache/sling/models/impl/ReflectionUtilTest.java
+++ /dev/null
@@ -1,153 +0,0 @@
-/*
- * 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.lang.reflect.Constructor;
-
-import org.junit.jupiter.api.Test;
-
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-@SuppressWarnings({
- "PackageVisibleInnerClass",
- "FieldCanBeLocal",
- "unused",
- "java:S1172",
- "InstanceVariableMayNotBeInitialized",
- "PublicConstructorInNonPublicClass"
-})
-class ReflectionUtilTest {
-
- static class TestClassOne {
- private final int field1;
- private final String field2;
-
- public TestClassOne(int field1, String field2) {
- this.field1 = field1;
- this.field2 = field2;
- }
- }
-
- static class TestClassTwo {
- private final int field1;
- private final String field2;
- private boolean field3;
-
- public TestClassTwo(int field1, String field2) {
- this.field1 = field1;
- this.field2 = field2;
- }
- }
-
- static class TestClassThree {
- private final int field1;
-
- public TestClassThree(int field1, String field2) {
- this.field1 = field1;
- }
- }
-
- static class TestClassFour {
-
- public TestClassFour(int field1, String field2, boolean field3) {}
- }
-
- static class TestClassFive {
- private final int field1;
-
- public TestClassFive(int field1) {
- this.field1 = field1;
- }
- }
-
- static class TestClassSix {
- private final int field1;
-
- public TestClassSix() {
- this.field1 = 0;
- }
- }
-
- static class TestClassSeven {
- private final String field1;
- private final String field2;
-
- public TestClassSeven(String field1, String field2) {
- this.field1 = field1;
- this.field2 = field2;
- }
- }
-
- static class TestClassEight {
- private final String field1;
-
- public TestClassEight(String field1, String field2) {
- this.field1 = field1;
- }
- }
-
- @Test
- void testBalancedConstructor() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassOne.class.getConstructor(int.class, String.class);
- assertTrue(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testMoreFieldsThanParams() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassTwo.class.getConstructor(int.class, String.class);
-
assertFalse(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testMoreParamsThanFields() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassThree.class.getConstructor(int.class, String.class);
-
assertFalse(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testNoFields() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassFour.class.getConstructor(int.class, String.class, boolean.class);
-
assertFalse(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testOneFieldOneParam() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassFive.class.getConstructor(int.class);
- assertTrue(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testNoParams() throws NoSuchMethodException {
- Constructor<?> constructor = TestClassSix.class.getConstructor();
-
assertFalse(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testBalancedRepeatableTypes() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassSeven.class.getConstructor(String.class, String.class);
- assertTrue(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-
- @Test
- void testUnbalancedRepeatableTypes() throws NoSuchMethodException {
- Constructor<?> constructor =
TestClassEight.class.getConstructor(String.class, String.class);
-
assertFalse(ReflectionUtil.areBalancedCtorParamsAndFields(constructor));
- }
-}
diff --git
a/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModel.java
b/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModel.java
new file mode 100644
index 0000000..b38825c
--- /dev/null
+++
b/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModel.java
@@ -0,0 +1,29 @@
+/*
+ * 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.testmodels.classes.constructorinjection;
+
+import javax.inject.Named;
+
+import org.apache.sling.api.SlingJakartaHttpServletRequest;
+import org.apache.sling.models.annotations.Model;
+
+@Model(adaptables = SlingJakartaHttpServletRequest.class)
+public record RecordModel(
+ @Named("attribute") int attribute,
+ @Named("attribute2") String attribute2) {}
diff --git
a/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModelWithExtraConstructor.java
b/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModelWithExtraConstructor.java
new file mode 100644
index 0000000..ae3cc57
--- /dev/null
+++
b/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModelWithExtraConstructor.java
@@ -0,0 +1,34 @@
+/*
+ * 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.testmodels.classes.constructorinjection;
+
+import javax.inject.Named;
+
+import org.apache.sling.api.SlingJakartaHttpServletRequest;
+import org.apache.sling.models.annotations.Model;
+
+@Model(adaptables = SlingJakartaHttpServletRequest.class)
+public record RecordModelWithExtraConstructor(
+ @Named("attribute") int attribute,
+ @Named("attribute2") String attribute2) {
+
+ public RecordModelWithExtraConstructor(@Named("attribute") int attribute) {
+ this(attribute, "fromExtraConstructor");
+ }
+}
diff --git
a/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModelWithStaticFields.java
b/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModelWithStaticFields.java
new file mode 100644
index 0000000..0b5f059
--- /dev/null
+++
b/src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/RecordModelWithStaticFields.java
@@ -0,0 +1,39 @@
+/*
+ * 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.testmodels.classes.constructorinjection;
+
+import javax.inject.Named;
+
+import org.apache.sling.api.SlingJakartaHttpServletRequest;
+import org.apache.sling.models.annotations.Model;
+
+@Model(adaptables = SlingJakartaHttpServletRequest.class)
+public record RecordModelWithStaticFields(
+ @Named("attribute") int attribute,
+ @Named("attribute2") String attribute2) {
+
+ public static final String STATIC_CONSTANT = "constant";
+
+ @SuppressWarnings("unused")
+ private static int staticCounter;
+
+ public static String staticConstant() {
+ return STATIC_CONSTANT;
+ }
+}
