From e46cc83122fc2c46f3cfb868757bcbd6a420e721 Mon Sep 17 00:00:00 2001 From: nicolaiparlog Date: Tue, 28 Apr 2015 21:39:52 +0200 Subject: [PATCH 1/3] Enable creation of nestings on a broken observable hierarchy As pointed out by #9, creating a 'DeepNesting' on an outer observable which contains null does not work as intended. Its 'innerObservableProperty' will contain null instead of an empty 'Oprional'. The same faulty behavior was found when one of the nested observables contains null. The problem was that the nesting's initialization could not distinguish between uninitialized values (which were represented as null) and null values contained in those observable. This lead to the initialization following an incorrect (and ultimately incomplete) computational path. The fix changes the way the value array is created. Instead of containing null for uninitialized values, it contains arbitrary instances which are not null an equal no instances which appears "in the wild". --- .../org/codefx/libfx/nesting/DeepNesting.java | 27 +++++++++-- .../nesting/AbstractDeepNestingTest.java | 48 +++++++++++++++++++ ...tractDeepNestingTestForDefaultNesting.java | 11 +++++ .../libfx/nesting/AbstractNestingTest.java | 10 ++++ 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/codefx/libfx/nesting/DeepNesting.java b/src/main/java/org/codefx/libfx/nesting/DeepNesting.java index 453e93b..c0d6178 100644 --- a/src/main/java/org/codefx/libfx/nesting/DeepNesting.java +++ b/src/main/java/org/codefx/libfx/nesting/DeepNesting.java @@ -74,6 +74,9 @@ final class DeepNesting implements Nesting { /** * The values currently held by the observables. + *

+ * Before the initialization these will be non-null "uninitialized" objects. This is done to distinguish them from + * null values which could be held by the observables. */ private final Object[] values; @@ -120,7 +123,7 @@ public DeepNesting(ObservableValue outerObservable, List nestingSte maxLevel = nestingSteps.size(); this.observables = createObservables(outerObservable, maxLevel); - this.values = new Object[maxLevel]; + this.values = createUnitializedValues(); this.nestingSteps = nestingSteps.toArray(new NestingStep[maxLevel]); this.changeListeners = createChangeListeners(maxLevel); this.inner = new SimpleObjectProperty<>(this, "inner"); @@ -128,6 +131,17 @@ public DeepNesting(ObservableValue outerObservable, List nestingSte initializeNesting(); } + /** + * @return an array of uninitialized values (i.e. non-null values which do not equal any other instances occurring + * "in the wild") + */ + private Object[] createUnitializedValues() { + Object[] values = new Object[maxLevel]; + for (int i = 0; i < maxLevel; i++) + values[i] = new Unitialized(); + return values; + } + /** * Creates an initialized array of observables. Its first item is the specified outer observable (its other items * are null). @@ -234,8 +248,8 @@ public void initialize() { * identical and nothing more needs to be updated. *

* Note that the loop will not stop on null observables and null values. Instead it continues and replaces all - * stored observables and values with null. This is the desired behavior as the hierarchy is in now an incomplete - * state and the old observables and values are obsolete and have to be replaced. + * stored observables and values with null. This is the desired behavior as the hierarchy is now in an incomplete + * state where the old observables and values are obsolete and have to be replaced. */ private class NestingUpdater { @@ -394,6 +408,13 @@ private void updateInnerObservable() { } + /** + * Represents an uninitialized entry in the {@link #values} array. + */ + private static class Unitialized { + // no body needed + } + //#end PRIVATE CLASSES } diff --git a/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTest.java b/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTest.java index dc8f154..a1fecf9 100644 --- a/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTest.java +++ b/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTest.java @@ -1,6 +1,8 @@ package org.codefx.libfx.nesting; import static org.codefx.libfx.nesting.testhelper.NestingAccess.getNestingObservable; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import javafx.beans.Observable; @@ -22,6 +24,37 @@ public abstract class AbstractDeepNestingTest + * This ensures that a "broken" hierarchy is correctly initialized. + */ + @Test + public void testCreatingWhenOuterObservableHasValueNull() { + outerObservable = createNewNestingHierarchyWhereOuterObservableHasNullValue(); + nesting = createNewNestingFromOuterObservable(outerObservable); + + assertNotNull(nesting.innerObservableProperty().getValue()); + assertFalse(nesting.innerObservableProperty().getValue().isPresent()); + } + + /** + * Tests whether creating a {@link DeepNesting} on a hierarchy where on of the nested observables contains null + * works correctly. + *

+ * This ensures that a "broken" hierarchy is correctly initialized. + */ + @Test + public void testCreatingWhenNestedObservableHasValueNull() { + outerObservable = createNewNestingHierarchyWhereNestedObservableHasNullValue(); + nesting = createNewNestingFromOuterObservable(outerObservable); + + assertNotNull(nesting.innerObservableProperty().getValue()); + assertFalse(nesting.innerObservableProperty().getValue().isPresent()); + } + // nested value /** @@ -90,6 +123,21 @@ public void testWhenSettingOuterValueToNull() { // #region ABSTRACT METHODS + /** + * Creates a new outer observable with a null value. The returned instances must be new for each call. + * + * @return an {@link ObservableValue} containing null + */ + protected abstract OO createNewNestingHierarchyWhereOuterObservableHasNullValue(); + + /** + * Creates a new nesting hierarchy where one of the nested observables contains null and returns the outer + * observable. All returned instances must be new for each call. + * + * @return an {@link ObservableValue} containing the outer value of a nesting hierarchy + */ + protected abstract OO createNewNestingHierarchyWhereNestedObservableHasNullValue(); + /** * Sets a new value of the specified kind on the specified level of the nesting hierarchy contained in the specified * outer observable. diff --git a/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTestForDefaultNesting.java b/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTestForDefaultNesting.java index 38d02c5..40b30cf 100644 --- a/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTestForDefaultNesting.java +++ b/src/test/java/org/codefx/libfx/nesting/AbstractDeepNestingTestForDefaultNesting.java @@ -28,6 +28,17 @@ protected Property createNewNestingHierarchy() { return new SimpleObjectProperty<>(outer); } + @Override + protected Property createNewNestingHierarchyWhereOuterObservableHasNullValue() { + return new SimpleObjectProperty<>(null); + } + + @Override + protected Property createNewNestingHierarchyWhereNestedObservableHasNullValue() { + OuterValue outer = OuterValue.createWithNull(); + return new SimpleObjectProperty<>(outer); + } + @Override protected void setNewValue(Property outerObservable, Level level, Value kindOfNewValue) { diff --git a/src/test/java/org/codefx/libfx/nesting/AbstractNestingTest.java b/src/test/java/org/codefx/libfx/nesting/AbstractNestingTest.java index 137aa87..764f52b 100644 --- a/src/test/java/org/codefx/libfx/nesting/AbstractNestingTest.java +++ b/src/test/java/org/codefx/libfx/nesting/AbstractNestingTest.java @@ -44,6 +44,16 @@ public void setUp() { // #region TESTS + // construction + + /** + * Tests whether creating a nesting with on a null outer observable throws an exception. + */ + @Test(expected = NullPointerException.class) + public void testExceptionWhenNullObservable() { + nesting = createNewNestingFromOuterObservable(null); + } + /** * Tests whether the {@link #nesting}'s {@link Nesting#innerObservableProperty() innerObservable} property contains * the correct observable, which is the {@link #outerObservable}'s inner observable. From 9d366128f95b0954a71d3ae9d357aa2d3617bc90 Mon Sep 17 00:00:00 2001 From: nicolaiparlog Date: Tue, 28 Apr 2015 22:21:18 +0200 Subject: [PATCH 2/3] Bump release number to 0.2.1 for this hotfix --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b3e6594..e1a54a4 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.codefx.libfx LibFX - 0.2.0 + 0.2.1 jar From 09ac907c53c283b9a619610d5731d68a64b3d7aa Mon Sep 17 00:00:00 2001 From: nicolaiparlog Date: Fri, 1 May 2015 19:12:59 +0200 Subject: [PATCH 3/3] Update README to new version number --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 93302d6..b6bc3dc 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ License details can be found in the *LICENSE* file in the project's root folder. Releases are published [on GitHub](https://github.com/CodeFX-org/LibFX/releases). The release notes also contain a link to the artifact in Maven Central and its coordinates. -The current version is [0.2.0](http://search.maven.org/#artifactdetails|org.codefx.libfx|LibFX|0.2.0|jar): +The current version is [0.2.1](http://search.maven.org/#artifactdetails|org.codefx.libfx|LibFX|0.2.1|jar): **Maven**: @@ -37,14 +37,14 @@ The current version is [0.2.0](http://search.maven.org/#artifactdetails|org.code org.codefx.libfx LibFX - 0.2.0 + 0.2.1 ``` **Gradle**: ``` - compile 'org.codefx.libfx:LibFX:0.2.0' + compile 'org.codefx.libfx:LibFX:0.2.1' ``` ## Development