[
https://issues.apache.org/jira/browse/SLING-13235?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Stefan Seifert reassigned SLING-13235:
--------------------------------------
Assignee: Stefan Seifert
thanks for the contribution! - PR looks good, i will apply it
> 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
> Assignee: Stefan Seifert
> Priority: Major
>
> 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)
