RFR: 8268642: Improve property system to facilitate correct usage
Michael Strauß
mstrauss at openjdk.java.net
Wed Nov 3 15:04:30 UTC 2021
Based on previous discussions, this PR attempts to improve the JavaFX property system by enforcing correct API usage in several cases that are outlined below. It also streamlines the API by deprecating untyped APIs in favor of typed APIs that better express intent.
### 1. Behavioral changes for regular bindings
var target = new SimpleObjectProperty(bean, "target");
var source = new SimpleObjectProperty(bean, "source");
target.bind(source);
target.bindBidirectional(source);
_Before:_ `RuntimeException` --> "bean.target: A bound value cannot be set."
_After:_ `IllegalStateException` --> "bean.target: Bidirectional binding cannot target a bound property."
var target = new SimpleObjectProperty(bean, "target");
var source = new SimpleObjectProperty(bean, "source");
var other = new SimpleObjectProperty(bean, "other");
source.bind(other);
target.bindBidirectional(source);
_Before:_ no error
_After:_ `IllegalArgumentException` --> "bean.source: Bidirectional binding cannot target a bound property."
var target = new SimpleObjectProperty(bean, "target");
var source = new SimpleObjectProperty(bean, "source");
target.bindBidirectional(source);
target.bind(source);
_Before:_ no error
_After:_ `IllegalStateException` --> "bean.target: Cannot bind a property that is targeted by a bidirectional binding."
### 2. Behavioral changes for content bindings
var target = new SimpleListProperty(bean, "target");
var source = new SimpleListProperty(bean, "source");
target.bindContent(source);
target.bindContentBidirectional(source);
_Before:_ no error
_After:_ `IllegalStateException` --> "bean.target: Bidirectional content binding cannot target a bound collection."
var target = new SimpleListProperty(bean, "target");
var source = new SimpleListProperty(bean, "source");
var other = new SimpleListProperty();
source.bindContent(other);
target.bindContentBidirectional(source);
_Before:_ no error
_After:_ `IllegalArgumentException` --> "bean.source: Bidirectional content binding cannot target a bound collection."
var target = new SimpleListProperty(bean, "target");
var source = new SimpleListProperty(bean, "source");
target.bindContentBidirectional(source);
target.bindContent(source);
_Before:_ no error
_After:_ `IllegalStateException` --> "bean.target: Cannot bind a collection that is targeted by a bidirectional content binding."
var target = new SimpleListProperty(bean, "target");
var source = new SimpleListProperty(bean, "source");
target.bindContent(source);
target.set(FXCollections.observableArrayList());
_Before:_ no error
_After:_ `IllegalStateException` --> "bean.target: Cannot set the value of a content-bound property."
var target = new SimpleListProperty(bean, "target");
var source = new SimpleListProperty(bean, "source");
target.bindContent(source);
target.add("foo");
_Before_: no error, but binding is broken because the lists are in an inconsistent state
_After:_ `RuntimeExeption` via `Thread.UncaughtExceptionHandler` --> "Illegal list modification: Content binding was removed because the lists are out-of-sync."
### 3. Align un-binding of unidirectional content bindings with regular unidirectional bindings
The API of unidirectional content bindings is aligned with regular unidirectional bindings because the semantics are similar. Like `unbind()`, `unbindContent(Object)` should not need an argument because there can only be a single content binding. For this reason, `unbindContent(Object)` will be deprecated in favor of `unbindContent()`:
void bindContent(ObservableList<E> source);
+ void unbindContent();
+ boolean isContentBound();
+ @Deprecated(since = "18", forRemoval = true)
void unbindContent(Object object);
### 4. Replace untyped binding APIs with typed APIs
The following untyped APIs will be marked for removal in favor of typed replacements:
In `javafx.beans.binding.Bindings`:
@Deprecated(since = "18", forRemoval = true)
void unbindBidirectional(Object property1, Object property2)
@Deprecated(since = "18", forRemoval = true)
void unbindContentBidirectional(Object obj1, Object obj2)
@Deprecated(since = "18", forRemoval = true)
void unbindContent(Object obj1, Object obj2)
In `ReadOnlyListProperty`, `ReadOnlySetProperty`, `ReadOnlyMapProperty`:
@Deprecated(since = "18", forRemoval = true)
void unbindContentBidirectional(Object object)
~~5. Support un-binding bidirectional conversion bindings with typed API
At this moment, `StringProperty` is the only property implementation that offers bidirectional conversion bindings, where the `StringProperty` is bound to a property of another type:~~
<U> void bindBidirectional(Property<U> other, StringConverter<U> converter)
~~The inherited `Property.unbindBidirectional(Property<String>)` API cannot be used to unbind a conversion binding, because it only accepts arguments of type `Property<String>`.
`StringProperty` solves this issue by adding an untyped overload: `unbindBidirectional(Object)`.~~
~~I propose to relax the definition of `Property.unbindBidirectional`:~~
- void unbindBidirectional(Property<T> other);
+ void unbindBidirectional(Property<?> other);
~~This allows property implementations to use the same API to un-bind regular bidirectional bindings, as well as converting bidirectional bindings. The `Property` typing is retained with a minimal impact on existing code (most _usages_ of this API will continue to compile, while classes that override `unbindBidirectional` will need to be changed to match the new API).~~
~~As a side-effect, this allows the option of easily adding conversion bindings for other property implementations as well.~~
-------------
Commit messages:
- centralized Property.toString
- test error message in content binding tests
- fix whitespace issues
- fix things that broke
- Clean up content binding API
- Rework exception messages
- changes
- Javadoc changes
- replace Object with Property
- fix merge
- ... and 15 more: https://git.openjdk.java.net/jfx/compare/55faac49...244b76f2
Changes: https://git.openjdk.java.net/jfx/pull/590/files
Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=590&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8268642
Stats: 4696 lines in 95 files changed: 3102 ins; 822 del; 772 mod
Patch: https://git.openjdk.java.net/jfx/pull/590.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/590/head:pull/590
PR: https://git.openjdk.java.net/jfx/pull/590
More information about the openjfx-dev
mailing list