This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 7154d07eda17db46a74ba6bf1b65e07a3e6c768e Author: James Bognar <[email protected]> AuthorDate: Tue May 12 13:02:16 2026 -0400 refactor: harden commons-incompatible code paths on BeanPropertyMeta/BeanMap (TODO-5 Step 7) Co-authored-by: Cursor <[email protected]> --- .../src/main/java/org/apache/juneau/BeanMap.java | 14 ++++- .../java/org/apache/juneau/BeanPropertyMeta.java | 58 +++++++++++++++++- .../apache/juneau/commons/bean/BeanMeta_Test.java | 71 ++++++++++++++++++++++ todo/TODO-5-bean-runtime-types-to-commons.md | 29 ++++++++- 4 files changed, 165 insertions(+), 7 deletions(-) diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanMap.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanMap.java index 7da3909d2b..2024ecf378 100644 --- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanMap.java +++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanMap.java @@ -407,6 +407,13 @@ public class BeanMap<T> extends AbstractMap<String,Object> implements Delegate<T * Triggers bean creation if bean has read-only properties set through a constructor defined by the * {@link BeanCtor @BeanCtor} annotation. * + * <p> + * The post-creation Optional<X> initialization step (which seeds null {@link Optional} properties with + * {@link ClassMeta#getOptionalDefault()}) is skipped for properties built via the bean-modeling-only path + * ({@link BeanMeta#of(Class, BeanConfigContext)}) because the per-property {@link ClassMeta} is unavailable; + * those properties are left untouched and any {@link Optional}-typed field stays at its constructor-assigned + * value. + * * @return The inner bean object. */ public T getBean() { @@ -424,17 +431,18 @@ public class BeanMap<T> extends AbstractMap<String,Object> implements Delegate<T arrayPropertyCache = null; } - // Initialize any null Optional<X> fields. + // Initialize any null Optional<X> fields. Skip properties whose ClassMeta is unavailable + // (bean-modeling-only path — Optional handling is a marshalling concern that requires type metadata). meta.getProperties().forEach((k,v) -> { var cm = v.getClassMeta(); - if (cm.isOptional() && v.get(this, k) == null) + if (nn(cm) && cm.isOptional() && v.get(this, k) == null) v.set(this, k, cm.getOptionalDefault()); }); // Do the same for hidden fields. meta.getHiddenProperties().forEach((k, v) -> { var cm = v.getClassMeta(); - if (cm.isOptional() && v.get(this, k) == null) + if (nn(cm) && cm.isOptional() && v.get(this, k) == null) v.set(this, k, cm.getOptionalDefault()); }); diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanPropertyMeta.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanPropertyMeta.java index 22cf098fd6..735d2ce234 100644 --- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanPropertyMeta.java +++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/BeanPropertyMeta.java @@ -632,18 +632,32 @@ public class BeanPropertyMeta implements Comparable<BeanPropertyMeta> { * Note that adding values to an array property is inefficient for large arrays since it must copy the array into a * larger array on each operation. * + * <p> + * <b>Marshalling-only path.</b> Requires a property built via the marshalling-side construction path + * (i.e. with a non-null {@link #getClassMeta() rawTypeMeta} and a non-null backing + * {@link MarshallingSession} on the supplied {@link BeanMap}). When the owning {@link BeanMeta} was built via + * {@link BeanMeta#of(Class, BeanConfigContext)}, this method throws + * {@link UnsupportedOperationException} because adding into a Collection/array property requires + * type-aware element conversion (the marshalling session's {@code convertToType}) that is not available in the + * bean-modeling-only path. + * * @param m The bean of the field being set. * @param pName * The property name if this is a dyna property (i.e. <js>"*"</js>). * <br>Otherwise can be <jk>null</jk>. * @param value The value to add to the field. * @throws BeanRuntimeException If field is not a collection or array. + * @throws UnsupportedOperationException If this property was built via the bean-modeling-only path + * ({@link BeanMeta#of(Class, BeanConfigContext)}). */ @SuppressWarnings({ "java:S3776" // Cognitive complexity acceptable for property add operation with various collection types }) public void add(BeanMap<?> m, String pName, Object value) throws BeanRuntimeException { + if (rawTypeMeta == null) + throw unsupportedOp("Property ''{0}'' was built via the bean-modeling-only path; Collection/array add operations require a marshalling context.", name); + // Read-only beans get their properties stored in a cache. if (m.bean == null) { if (! m.propertyCache.containsKey(name)) @@ -720,6 +734,15 @@ public class BeanPropertyMeta implements Comparable<BeanPropertyMeta> { /** * Adds a value to a {@link Map} or bean property. * + * <p> + * <b>Marshalling-only path.</b> Requires a property built via the marshalling-side construction path + * (i.e. with a non-null {@link #getClassMeta() rawTypeMeta} and a non-null backing + * {@link MarshallingSession} on the supplied {@link BeanMap}). When the owning {@link BeanMeta} was built via + * {@link BeanMeta#of(Class, BeanConfigContext)}, this method throws + * {@link UnsupportedOperationException} because adding into a Map/bean property requires type-aware + * value conversion (the marshalling session's {@code convertToType} / {@code toBeanMap}) that is not available + * in the bean-modeling-only path. + * * @param m The bean of the field being set. * @param pName * The property name if this is a dyna property (i.e. <js>"*"</js>). @@ -727,12 +750,17 @@ public class BeanPropertyMeta implements Comparable<BeanPropertyMeta> { * @param key The key to add to the field. * @param value The value to add to the field. * @throws BeanRuntimeException If field is not a map or array. + * @throws UnsupportedOperationException If this property was built via the bean-modeling-only path + * ({@link BeanMeta#of(Class, BeanConfigContext)}). */ @SuppressWarnings({ "java:S3776" // Cognitive complexity acceptable for property add operation with map key handling }) public void add(BeanMap<?> m, String pName, String key, Object value) throws BeanRuntimeException { + if (rawTypeMeta == null) + throw unsupportedOp("Property ''{0}'' was built via the bean-modeling-only path; Map/bean add operations require a marshalling context.", name); + // Read-only beans get their properties stored in a cache. if (m.bean == null) { if (! m.propertyCache.containsKey(name)) @@ -1328,7 +1356,21 @@ public class BeanPropertyMeta implements Comparable<BeanPropertyMeta> { return r(properties()); } + /** + * Applies the {@link MarshalledProp#properties() @MarshalledProp(properties)} child-property filter to a value. + * + * <p> + * <b>Marshalling-only.</b> The signature itself takes a {@link ClassMeta} and {@link MarshallingSession}, + * which are marshalling-side types; this helper is only reachable from {@link #swapAndFilterProperty}, + * which short-circuits when the owning {@link BeanMeta} was built via + * {@link BeanMeta#of(Class, BeanConfigContext)} (no {@link MarshallingContext} on the property, + * therefore no {@code rawTypeMeta}). + * + * @throws UnsupportedOperationException If invoked on a property built via the bean-modeling-only path. + */ private Object applyChildPropertiesFilter(MarshallingSession session, ClassMeta cm, Object o) { + if (bc == null) + throw unsupportedOp("Property ''{0}'' was built via the bean-modeling-only path; child-properties filtering requires a marshalling context.", name); if (o == null) return null; if (cm.isBean()) @@ -1394,7 +1436,10 @@ public class BeanPropertyMeta implements Comparable<BeanPropertyMeta> { o = readTransform.apply(session, o); if (o == null) return null; - if (nn(properties)) { + // rawTypeMeta is null on the bean-modeling-only path — child-properties filtering is a marshalling + // concern (it builds DelegateList / FilteredKeyMap / nested BeanMetaFiltered instances). Skip it and + // return the raw value. + if (nn(properties) && nn(rawTypeMeta)) { if (rawTypeMeta.isArray()) { var a = (Object[])o; var l1 = new DelegateList(rawTypeMeta); @@ -1482,13 +1527,24 @@ public class BeanPropertyMeta implements Comparable<BeanPropertyMeta> { * <p> * Works on both <c>Object</c> and primitive arrays. * + * <p> + * <b>Marshalling-only path.</b> Requires a property built via the marshalling-side construction path + * (i.e. with a non-null {@link #getClassMeta() rawTypeMeta}). When the owning {@link BeanMeta} was built via + * {@link BeanMeta#of(Class, BeanConfigContext)}, this method throws + * {@link UnsupportedOperationException} because the array element type is only known via {@code rawTypeMeta}, + * which is not populated in the bean-modeling-only path. + * * @param bean The bean of the field. * @param l The collection to use to set the array field. * @throws IllegalArgumentException Thrown by method invocation. * @throws IllegalAccessException Thrown by method invocation. * @throws InvocationTargetException Thrown by method invocation. + * @throws UnsupportedOperationException If this property was built via the bean-modeling-only path + * ({@link BeanMeta#of(Class, BeanConfigContext)}). */ protected void setArray(Object bean, List l) throws IllegalArgumentException, IllegalAccessException, InvocationTargetException { + if (rawTypeMeta == null) + throw unsupportedOp("Property ''{0}'' was built via the bean-modeling-only path; setArray requires a marshalling context.", name); var array = toArray(l, this.rawTypeMeta.getElementType().inner()); invokeSetter(bean, name, array); } diff --git a/juneau-utest/src/test/java/org/apache/juneau/commons/bean/BeanMeta_Test.java b/juneau-utest/src/test/java/org/apache/juneau/commons/bean/BeanMeta_Test.java index f20e24ecd1..5101ada344 100644 --- a/juneau-utest/src/test/java/org/apache/juneau/commons/bean/BeanMeta_Test.java +++ b/juneau-utest/src/test/java/org/apache/juneau/commons/bean/BeanMeta_Test.java @@ -18,6 +18,8 @@ package org.apache.juneau.commons.bean; import static org.junit.jupiter.api.Assertions.*; +import java.util.*; + import org.apache.juneau.*; import org.junit.jupiter.api.*; @@ -171,4 +173,73 @@ class BeanMeta_Test extends TestBase { var bm = BeanMeta.of(D_FluentPojo.class, cfg); assertTrue(bm.getProperties().containsKey("name")); } + + //==================================================================================================== + // Marshalling-only paths on a commons-built BeanPropertyMeta (Step 7 hardening) + //==================================================================================================== + + public static class E_CollectionPojo { + private List<String> tags = new ArrayList<>(); + private Map<String,String> attrs = new LinkedHashMap<>(); + public List<String> getTags() { return tags; } + public void setTags(List<String> value) { tags = value; } + public Map<String,String> getAttrs() { return attrs; } + public void setAttrs(Map<String,String> value) { attrs = value; } + } + + @Test + void e01_add_collection_throwsOnCommonsBuiltProperty() { + var bm = BeanMeta.of(E_CollectionPojo.class); + var p = new E_CollectionPojo(); + var map = BeanMap.of(p, bm); + var pTags = bm.getPropertyMeta("tags"); + var ex = assertThrows(UnsupportedOperationException.class, () -> pTags.add(map, "tags", "x")); + assertTrue(ex.getMessage().contains("bean-modeling-only"), () -> "Got: " + ex.getMessage()); + assertTrue(ex.getMessage().contains("tags"), () -> "Got: " + ex.getMessage()); + } + + @Test + void e02_add_map_throwsOnCommonsBuiltProperty() { + var bm = BeanMeta.of(E_CollectionPojo.class); + var p = new E_CollectionPojo(); + var map = BeanMap.of(p, bm); + var pAttrs = bm.getPropertyMeta("attrs"); + var ex = assertThrows(UnsupportedOperationException.class, () -> pAttrs.add(map, "attrs", "k", "v")); + assertTrue(ex.getMessage().contains("bean-modeling-only"), () -> "Got: " + ex.getMessage()); + assertTrue(ex.getMessage().contains("attrs"), () -> "Got: " + ex.getMessage()); + } + + //==================================================================================================== + // BeanMap.getBean() on a commons-built BeanMap (Step 7 hardening) + //==================================================================================================== + + public static class F_OptionalPojo { + private Optional<String> maybe; + public Optional<String> getMaybe() { return maybe; } + public void setMaybe(Optional<String> value) { maybe = value; } + } + + @Test + void f01_getBean_skipsOptionalInitOnCommonsBuiltProperty() { + // Optional initialization (cm.isOptional() — seeds null Optional<X> properties with cm.getOptionalDefault()) + // requires a per-property ClassMeta which is unavailable on the bean-modeling-only path. getBean() must + // still return the wrapped bean rather than NPE. + var bm = BeanMeta.of(F_OptionalPojo.class); + var p = new F_OptionalPojo(); + var map = BeanMap.of(p, bm); + assertSame(p, map.getBean()); + // Optional field stays at its constructor-assigned value (null) because the marshalling-side + // Optional-default seeding is skipped for commons-built properties. + assertNull(p.getMaybe()); + } + + @Test + void f02_getBean_returnsBeanForSimplePojo() { + var bm = BeanMeta.of(A_Pojo.class); + var p = new A_Pojo(); + p.setX("hi"); + var map = BeanMap.of(p, bm); + assertSame(p, map.getBean()); + assertEquals("hi", p.getX()); + } } diff --git a/todo/TODO-5-bean-runtime-types-to-commons.md b/todo/TODO-5-bean-runtime-types-to-commons.md index 51f0589d83..4b8b3eb699 100644 --- a/todo/TODO-5-bean-runtime-types-to-commons.md +++ b/todo/TODO-5-bean-runtime-types-to-commons.md @@ -4,7 +4,25 @@ This is the remaining work from **Phase 5 of the bean-layer split**. Phase 5a (t --- -## Status (as of Phase 5f checkpoint) +## Status (as of Phase 5g checkpoint) + +**Step 7 complete (no-op + hardening).** Per-format extension survey confirmed: `ExtendedBeanMeta` (composes `BeanMeta<?>`) lives in `juneau-marshall`; `XmlBeanMeta` and `RdfBeanMeta` extend `ExtendedBeanMeta`; no `HtmlBeanMeta` exists. All three are marshalling-side types (built only from `XmlSerializer`/`XmlParser`/`RdfSerializer`/`RdfParser`, always over a marshalling-built `BeanMeta` where `classMeta` is non-null). They stay in `juneau-marshall` unchanged. + +**Pre-Step-8 hardening landed.** The unguarded `BeanPropertyMeta` and `BeanMap` paths flagged in the Step-6 risk notes are now defensive against commons-built properties/maps: +- `BeanPropertyMeta.add(BeanMap, String, Object)` and `add(BeanMap, String, String, Object)` — strategy (b): throw `UnsupportedOperationException` with a clear message ("Property '...' was built via the bean-modeling-only path; ... add operations require a marshalling context.") when `rawTypeMeta == null`. Documented in Javadoc. +- `BeanPropertyMeta.setArray(Object, List)` — strategy (b): same UOE pattern. Documented. +- `BeanPropertyMeta.applyChildPropertiesFilter(MarshallingSession, ClassMeta, Object)` — strategy (b): UOE when `bc == null`. Caller `swapAndFilterProperty` was also hardened: the `if (nn(properties))` guard now also checks `nn(rawTypeMeta)` so we never attempt the marshalling-side child-properties branching on a commons-built property. The defensive UOE inside `applyChildPropertiesFilter` itself is therefore unreachable in practice — kept as a belt-and-braces. +- `BeanPropertyMeta.setPropertyValue` (the back half of `set`) — already gated by the existing early-return `if (rawTypeMeta == null) { invokeSetter(...); return old; }` block in `set`. No change needed; documented in the Step-7 narrative. +- `BeanMap.getBean()` — strategy (c): the `Optional<X>` initialization loops (over `meta.getProperties()` and `meta.getHiddenProperties()`) now null-check `v.getClassMeta()` before reading `cm.isOptional()` / `cm.getOptionalDefault()`. Properties built via the bean-modeling-only path are skipped — their `Optional<X>` field is left at its constructor-assigned value. Documented in `BeanMap.getBean()` Javadoc. +- `BeanMap.getBean(boolean create)` — the constructor-args path (only triggered when `bean == null && create && ne(meta.getConstructorArgs())`) is unreachable in practice on the commons-side path because `BeanMap.of(T, BeanMeta<T>)` always supplies a non-null bean. Left as-is (would NPE on `session.convertToType` if a user built a commons `BeanMap` with a null bean for a `@BeanCtor` class — a pathological combination). + +Test coverage: `BeanMeta_Test` grew from 10 → 14 scenarios. New tests: +- `e01_add_collection_throwsOnCommonsBuiltProperty` — verifies `add(BeanMap, String, Object)` on a commons-built Collection property raises `UnsupportedOperationException` with the expected message. +- `e02_add_map_throwsOnCommonsBuiltProperty` — same for the `add(BeanMap, String, String, Object)` Map overload. +- `f01_getBean_skipsOptionalInitOnCommonsBuiltProperty` — verifies `BeanMap.getBean()` on a commons-built bean with an `Optional<X>` property no longer NPEs and returns the wrapped bean (Optional field stays at its constructor-assigned value). +- `f02_getBean_returnsBeanForSimplePojo` — sanity check that `BeanMap.getBean()` round-trips on a simple commons-built bean. + +Full test suite green (`scripts/test.py --full`). **Step 6 complete.** `BeanMeta` and `BeanPropertyMeta` can now be constructed without a `MarshallingContext`. Public surface added: - `BeanMeta.of(Class<T>, BeanConfigContext)` and `BeanMeta.of(Class<T>)` static factories. @@ -52,12 +70,17 @@ Known limitations of the commons-side path (acceptable for Step 6, scoped for la - [x] **Step 4** — Removed `MarshallingSession` from the `BeanMap` constructor signature (Option c — transitional setter). `BeanMap` now exposes `setMarshallingSession(MarshallingSession)` that the marshalling layer wires in immediately after construction. The `session` field defaults to null on direct construction; only `BeanMap.getBean()` (for read-only beans with constructor args), `BeanPropertyMeta.add`/`set`, and child-properties-filter operations need it set, and they all go throug [...] - [x] **Step 5** — Removed `BeanRegistry` field from `BeanPropertyMeta`. Picked **Option B** — side-map keyed by `BeanPropertyMeta` lives on `BeanMeta` (`Map<BeanPropertyMeta,BeanRegistry> propertyBeanRegistries`). Per-property registries are constructed by `BeanMeta` after `v.build()` from the builder's package-private `dictionaryClasses` field (populated during `Builder.validate(...)`). `Builder.beanRegistry(...)` public setter, the `beanRegistry` builder field, and the `parentBeanRegi [...] - [x] **Step 6** — `BeanMeta.of(Class<T>, BeanConfigContext)` factory + `protected BeanMeta(Class<T>, BeanConfigContext)` constructor wired up. `BeanMeta` now carries a non-null `BeanConfigContext config` facade for all settings reads; the `marshallingContext` and `classMeta` fields are documented-nullable and stay null on the commons-side path. `BeanPropertyMeta.Builder.bc` and `BeanPropertyMeta.bc` similarly nullable; new mirrored `config` field on both. `Builder.validate(...)` accepts [...] -- [ ] **Step 7** — Re-check whether `ExtendedBeanMeta` and per-format extensions (`XmlBeanMeta`, `RdfBeanMeta`, `HtmlBeanMeta`) need to follow `BeanMeta` to `commons.bean`. Default expectation: they stay in `juneau-marshall`. +- [x] **Step 7** — Per-format extension survey + pre-Step-8 hardening. (a) Survey result: `ExtendedBeanMeta` (composes `BeanMeta<?>`), `XmlBeanMeta` (extends `ExtendedBeanMeta`), `RdfBeanMeta` (extends `ExtendedBeanMeta`); no `HtmlBeanMeta` exists. All marshalling-side, all stay in `juneau-marshall`. (b) Hardened `BeanPropertyMeta.add(BeanMap,String,Object)` / `add(BeanMap,String,String,Object)` / `setArray` / `applyChildPropertiesFilter` (all throw `UnsupportedOperationException` with a [...] - [ ] **Step 8** — `git mv` the eight runtime types into `juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/bean/`. Verify `juneau-commons` still compiles standalone (`cd juneau-core/juneau-commons && mvn clean compile`). - [ ] **Step 9** — Reference sweep: 80–120 unique files (mostly inside `juneau-marshall`). Update imports, Javadoc `{@link …}` references, package-info docs. - [ ] **Step 10** — Update `juneau-docs` release notes / migration guide (`docs/pages/release-notes/9.5.0.md`, `## Package Moves` section) with the bean-runtime relocations. -The "incomplete-but-documented over broken-build" rule from Phase 5a still applies. When picking up the next slice of this work, **Step 7 is the recommended next checkpoint** — Step 6 wired the commons-side construction path through `BeanMeta.of(Class<T>, BeanConfigContext)`, made `marshallingContext`/`classMeta` nullable on `BeanMeta`, and made `bc`/`rawTypeMeta` nullable on `BeanPropertyMeta`. The marshalling-side construction path remains behaviorally identical: `BeanMetaFiltered` and [...] +The "incomplete-but-documented over broken-build" rule from Phase 5a still applies. When picking up the next slice of this work, **Step 8 is the recommended next checkpoint** — the physical `git mv` of the eight runtime types into `juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/bean/`. Pre-flight is now clean: per-format `BeanMeta` extensions (`ExtendedBeanMeta`/`XmlBeanMeta`/`RdfBeanMeta`) are confirmed marshalling-side and stay put (Step 7); the previously unguarded [...] + +**Remaining risks/surprises for Step 8:** +- The eight files reference `MarshallingContext`/`MarshallingSession`/`ClassMeta`/`BeanRegistry`/`ObjectSwap` in many places that survived Step 6/7 (the `installSwapAwareTransforms` install path, the `BeanRegistry` side-map on `BeanMeta`, the `MarshallingSession` field on `BeanMap`, the `ClassMeta classMeta` field on `BeanMeta`, etc.). Physically moving the files into `commons.bean` will require either: (a) leaving these fields in place but moving the types, accepting that `juneau-common [...] +- `BeanPropertyMeta.applyChildPropertiesFilter` directly takes a `ClassMeta` parameter — the signature itself encodes a marshalling-side type. If it stays on `BeanPropertyMeta` after the move, `commons.bean` will need a forward reference to `ClassMeta` (which is in `juneau-marshall`). Likely needs to migrate to a side helper in the marshalling layer. +- `BeanMap.getBean(boolean create)`'s constructor-args path uses `session.convertToType(rawVal, cm)` — this entire block is `@BeanCtor` parser-side behavior. Should migrate to a marshalling-side helper after the move (Step 9 or later). ---
