Herman Ciechanowiec created SLING-13235:
-------------------------------------------
Summary: Record-based Sling Model with static fields fails to
instantiate
Key: SLING-13235
URL: https://issues.apache.org/jira/browse/SLING-13235
Project: Sling
Issue Type: Bug
Components: Sling Models
Affects Versions: Models Implementation 2.0.2
Reporter: Herman Ciechanowiec
h3. Problem
A Java record annotated with {{@Model}} fails to be instantiated if the record
declares one or more {{static}} fields. Adaptation returns {{null}} (resp.
{{ModelFactory}} fails with "Unable to find a useable constructor"), even
though the record is a perfectly valid Sling Model:
{code:java}
@Model(adaptables = SlingJakartaHttpServletRequest.class)
public record MyModel(@Named("attribute") int attribute) {
private static final String SOME_CONSTANT = "constant"; // <- breaks model
instantiation
}
{code}
Without the static field, the same model works as expected.
h3. Root Cause
Record support (SLING-12359) was implemented while the codebase still targeted
Java 11, which has no record API. Therefore the canonical record constructor
was detected _heuristically_:
{{ReflectionUtil.areBalancedCtorParamsAndFields()}} compared the constructor's
parameter count against the number of non-synthetic declared fields of the
class.
That heuristic filtered out synthetic fields but *not static fields*. Since
static fields are not record components and thus not part of the canonical
constructor, any static field caused a count mismatch:
* {{MyModel}} above has 2 non-synthetic declared fields ({{attribute}},
{{SOME_CONSTANT}}) but a 1-parameter canonical constructor,
* {{ModelClassConstructor.isCanonicalRecordConstructor()}} returned {{false}},
* {{ModelAdapterFactory.getBestMatchingConstructor()}} skipped the canonical
constructor, and — records having no default constructor — no usable
constructor remained.
The heuristic could also misfire the other way: a _non-canonical_ constructor
whose parameter count happened to equal the field count was wrongly classified
as canonical.
h3. Proposed Fix
The bundle meanwhile requires Java 17, so the pre-Java-17 workarounds are
obsolete and should be replaced with the native record reflection API:
* {{ModelClassConstructor.isCanonicalRecordConstructor()}} should identify the
canonical constructor exactly per the JLS definition: the declaring class is a
record ({{Class.isRecord()}}) and the constructor's parameter types equal the
record component types ({{Class.getRecordComponents()}}), in order. This is
immune to static fields and uniquely identifies the canonical constructor.
* Remove from {{ReflectionUtil}}: the reflective
{{Class.forName("java.lang.Record")}} lookup (SLING-12483),
{{isRecord(Class)}}, and the {{areBalancedCtorParamsAndFields(Constructor)}}
heuristic.
* {{ModelClass}} should use the native {{Class.isRecord()}} to exclude records
from field injection.
h3. Tests
* Add the first record-based test fixtures and a {{RecordModelTest}} covering:
** a plain record model (baseline),
** a record model with static fields and a static method (regression test for
this issue),
** a record model with an additional non-canonical constructor (verifies the
canonical constructor is still selected).
* Remove {{ReflectionUtilTest}}, which exclusively tests the heuristic to be
deleted, using non-record classes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
