On Sat, 5 Sep 2026 14:36:38 GMT, Michael Strauß <[email protected]> wrote:
>> Implementation of [enhanced property >> metadata](https://gist.github.com/mstr2/2fec0303fc440b8eaeb126befc76eb5c). >> >> ### New API >> This PR includes the following API additions: >> >> 1. `ReadOnlyProperty.getDeclaringClass()` and its default implementation. >> 2. The `javafx.beans.property.AttachedProperty` interface. >> 3. New constructors for all `Simple<*>Property` and `ReadOnly<*>Wrapper` >> classes, accepting the declaring class of the property. >> >> The declaring class is stored in a new field in the `Simple<*>Property` >> classes. If a legacy constructor is used that doesn't specify the declaring >> class, the `ReadOnlyProperty.getDeclaringClass()` default implementation is >> called the first time the `Simple<*>Property.getDeclaringClass()` method is >> called, and its result is stored for future retrieval. >> >> ### Testing >> For testing, this PR also includes the >> `test.util.property.PropertyMetadataVerifier` tool. It systematically tests >> all public and protected properties of a class, and ensures conformance to >> the following rules: >> * `ReadOnlyProperty.getBean()` returns the object instance of the enclosing >> class, or the target object instance if the property is an attached property. >> * `ReadOnlyProperty.getName()` returns the name of the property, which must >> correspond to the name of the property getter (excluding the word >> "Property"). >> * `ReadOnlyProperty.getDeclaringClass()` returns the enclosing class of the >> property getter. >> * The declaring class of a `Simple<*>Property` or `ReadOnly<*>Wrapper` must >> be specified in the constructor, not resolved at runtime. >> * `getBean()`, `getName()`, and `getDeclaringClass()` must not be overridden >> in subclasses of `Simple<*>Property` or `ReadOnly<*>Wrapper`. >> * An instance property does not implement `AttachedProperty`. >> * An instance property has a parameterless property getter. >> * An attached property implements `AttachedProperty`. >> * An attached property has a static single-argument property getter that >> accepts the target object. >> * `AttachedProperty.getTargetClass()` returns the class of the single >> parameter of the static property getter. >> * A property getter does not return an instance of `ReadOnly<*>Wrapper`, it >> returns the result of calling `ReadOnly<*>Wrapper.getReadOnlyProperty()`. >> >> Many properties in existing JavaFX classes violate the >> `PropertyMetadataVerifier` rules in some way or shape. This PR won't address >> these issues, this will be done in a future cleanup PR. >> >> --------- >> - [x] I confirm... > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > cache negative declaringClass lookup I think this looks good, some minor findings modules/javafx.base/src/main/java/javafx/beans/property/ReadOnlyProperty.java line 110: > 108: } while (beanClass != null); > 109: > 110: return null; Is it perhaps an idea here to use a cache for this? Many properties over the lifetime of an FX application will share the same bean class and name; something like this: private static final ClassValue<ConcurrentHashMap<String, Class<?>>> CACHE = new ClassValue<>() { @Override protected ConcurrentHashMap<String, Class<?>> computeValue(Class<?> beanClass) { return new ConcurrentHashMap<>(); } }; static Class<?> lookup(Class<?> beanClass, String propertyName) { Class<?> result = CACHE.get(beanClass) .computeIfAbsent(propertyName, name -> { Class<?> found = findDeclaringClass(beanClass, name); return found == null ? NOT_FOUND : found; }); return result != NOT_FOUND ? result : null; } The `ClassValue` is specifically intended for this kind of use. modules/javafx.base/src/main/java/javafx/beans/property/SimpleObjectProperty.java line 132: > 130: public SimpleObjectProperty(Object bean, Class<?> declaringClass, > String name) { > 131: this(bean, name); > 132: this.declaringClass = declaringClass; One could pass in `declaringClass` here as `null` - Should we allow this? - If so, what does it mean? - `null` means look-up just in time - `null` means don't look this up, it is intended to not be specified modules/javafx.base/src/test/java/test/util/property/PropertyMetadataVerifier.java line 319: > 317: String displayName = declaringClass.getName() + "." + > propertyName; > 318: > 319: return method.getParameterCount() == 1 nit (test code): This code assumes that a parameter count of 1 means means it is the static accessor, but an instance one could also have a parameter: ie: public Property<String> findKeyedProperty(String key) { ... } would be found; later it is called with `method.invoke(null, bean)` ------------- PR Review: https://git.openjdk.org/jfx/pull/2015#pullrequestreview-5124961963 PR Review Comment: https://git.openjdk.org/jfx/pull/2015#discussion_r3943572595 PR Review Comment: https://git.openjdk.org/jfx/pull/2015#discussion_r3943597726 PR Review Comment: https://git.openjdk.org/jfx/pull/2015#discussion_r3943612790
