Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8273138: BidirectionalBinding fails to observe changes of invalid properties #5

Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -37,6 +37,14 @@
import java.text.ParseException;
import java.util.Objects;

/**
* @implNote Bidirectional bindings are implemented with InvalidationListeners, which are fired once
* when the changed property was valid, but elided for any future changes until the property
* is again validated by calling its getValue()/get() method.
* Since bidirectional bindings require that we observe all property changes, independent
* of whether the property was validated by user code, we manually validate both properties
* by calling their getter method in all relevant places.
*/
public abstract class BidirectionalBinding implements InvalidationListener, WeakListener {

private static void checkParameters(Object property1, Object property2) {
@@ -62,6 +70,7 @@ private static void checkParameters(Object property1, Object property2) {
new BidirectionalBooleanBinding((BooleanProperty) property1, (BooleanProperty) property2)
: new TypedGenericBidirectionalBinding<T>(property1, property2);
property1.setValue(property2.getValue());
property1.getValue();
property1.addListener(binding);
property2.addListener(binding);
return binding;
@@ -72,6 +81,7 @@ public static Object bind(Property<String> stringProperty, Property<?> otherProp
Objects.requireNonNull(format, "Format cannot be null");
final var binding = new StringFormatBidirectionalBinding(stringProperty, otherProperty, format);
stringProperty.setValue(format.format(otherProperty.getValue()));
stringProperty.getValue();
stringProperty.addListener(binding);
otherProperty.addListener(binding);
return binding;
@@ -82,6 +92,7 @@ public static Object bind(Property<String> stringProperty, Property<?> otherProp
Objects.requireNonNull(converter, "Converter cannot be null");
final var binding = new StringConverterBidirectionalBinding<>(stringProperty, otherProperty, converter);
stringProperty.setValue(converter.toString(otherProperty.getValue()));
stringProperty.getValue();
stringProperty.addListener(binding);
otherProperty.addListener(binding);
return binding;
@@ -143,6 +154,7 @@ public static BidirectionalBinding bindNumber(DoubleProperty property1, Property
final BidirectionalBinding binding = new TypedNumberBidirectionalBinding<>(property2, property1);

property1.setValue(property2.getValue());
property1.getValue();
property1.addListener(binding);
property2.addListener(binding);
return binding;
@@ -154,6 +166,7 @@ public static BidirectionalBinding bindNumber(DoubleProperty property1, Property
final BidirectionalBinding binding = new TypedNumberBidirectionalBinding<>(property1, property2);

property1.setValue((T)property2.getValue());
property1.getValue();
property1.addListener(binding);
property2.addListener(binding);
return binding;
@@ -250,18 +263,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
boolean newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
boolean newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -323,18 +340,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
double newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
double newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -396,18 +417,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
float newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
float newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -469,18 +494,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
int newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
int newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -542,18 +571,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
long newValue = property1.get();
property2.set(newValue);
property2.get();
oldValue = newValue;
} else {
long newValue = property2.get();
property1.set(newValue);
property1.get();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.set(oldValue);
property1.get();
} else {
property2.set(oldValue);
property2.get();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -615,18 +648,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
T newValue = property1.getValue();
property2.setValue(newValue);
property2.getValue();
oldValue = newValue;
} else {
T newValue = property2.getValue();
property1.setValue(newValue);
property1.getValue();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.setValue(oldValue);
property1.getValue();
} else {
property2.setValue(oldValue);
property2.getValue();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -688,18 +725,22 @@ public void invalidated(Observable sourceProperty) {
if (property1 == sourceProperty) {
T newValue = property1.getValue();
property2.setValue(newValue);
property2.getValue();
oldValue = newValue;
} else {
T newValue = (T)property2.getValue();
property1.setValue(newValue);
property1.getValue();
oldValue = newValue;
}
} catch (RuntimeException e) {
try {
if (property1 == sourceProperty) {
property1.setValue((T)oldValue);
property1.getValue();
} else {
property2.setValue(oldValue);
property2.getValue();
}
} catch (Exception e2) {
e2.addSuppressed(e);
@@ -789,16 +830,20 @@ public void invalidated(Observable observable) {
if (property1 == observable) {
try {
property2.setValue(fromString(property1.getValue()));
property2.getValue();
} catch (Exception e) {
Logging.getLogger().warning("Exception while parsing String in bidirectional binding", e);
property2.setValue(null);
property2.getValue();
}
} else {
try {
property1.setValue(toString(property2.getValue()));
property1.getValue();
} catch (Exception e) {
Logging.getLogger().warning("Exception while converting Object to String in bidirectional binding", e);
property1.setValue("");
property1.getValue();
}
}
} finally {
@@ -301,6 +301,15 @@ public void testDoubleBrokenBind() {
assertEquals(v[1], op2.getValue());
}

@Test
public void testSetValueWithoutIntermediateValidation() {
BidirectionalBinding.bind(op1, op2);
op1.setValue(v[0]);
op2.setValue(v[1]);
assertEquals(v[1], op1.getValue());
assertEquals(v[1], op2.getValue());
}

@Parameterized.Parameters
public static Collection<Object[]> parameters() {
final Boolean[] booleanData = new Boolean[] {true, false, true, false};