fix: prevented _EVENT_BY_DELEGATINGNODE marker from stopping actual w…

propertly.core/pom.xml

@@ -11,7 +11,7 @@
   </parent>
 
   <artifactId>propertly.core</artifactId>
-  <version>1.0.12</version>
+  <version>1.0.13</version>
 
   <dependencies>
     <dependency>

propertly.core/src/main/java/de/adito/propertly/core/api/DelegatingNode.java

@@ -10,8 +10,8 @@
 
 /**
  * @author PaL
- *         Date: 09.02.13
- *         Time: 18:17
+ * Date: 09.02.13
+ * Time: 18:17
  */
 public class DelegatingNode extends AbstractNode
 {
@@ -71,8 +71,8 @@ else if (value == null)
         if (myChild == null)
           children.add(createChild(delegateChild));
 
-        // If there is a child, but it is invalid -> remove from children list and recreate
-        else if(!myChild.isValid())
+          // If there is a child, but it is invalid -> remove from children list and recreate
+        else if (!myChild.isValid())
         {
           int idx = children.indexOf(delegateDescription);
           children.remove(myChild);
@@ -114,8 +114,8 @@ public Object setValue(@Nullable Object pValue, @NotNull Set<Object> pAttributes
     List<Runnable> onFinish = new ArrayList<>();
     fireValueWillBeChange(oldValue, pValue, onFinish::add, pAttributes);
 
-    executeWriteOnDelegate(pDelegate -> {
-      pDelegate.setValue(pValue, pAttributes);
+    executeWriteOnDelegate(pAttributes, pAttr -> pDelegate -> {
+      pDelegate.setValue(pValue, pAttr);
       clearListeners();
     });
 
@@ -174,7 +174,7 @@ public INode addProperty(@Nullable Integer pIndex, @NotNull IPropertyDescription
     INode node = findNode(pPropertyDescription.getName());
     if (node != null)
       throw new IllegalStateException("name already exists: " + pPropertyDescription);
-    executeWriteOnDelegate(pDelegate -> pDelegate.addProperty(pIndex, pPropertyDescription, pAttributes));
+    executeWriteOnDelegate(pAttributes, pAttr -> pDelegate -> pDelegate.addProperty(pIndex, pPropertyDescription, pAttr));
     DelegatingNode child = createChild(executeReadOnDelegate(pDelegate -> pDelegate.findNode(pPropertyDescription)));
     if (children == null)
       children = new NodeChildren();
@@ -199,7 +199,7 @@ public boolean removeProperty(@NotNull IPropertyDescription pPropertyDescription
       List<Runnable> onFinish = new ArrayList<>();
       fireNodeWillBeRemoved(description, onFinish::add, pAttributes);
       assert children != null;
-      executeWriteOnDelegate(pDelegate -> pDelegate.removeProperty(pPropertyDescription, pAttributes));
+      executeWriteOnDelegate(pAttributes, pAttr -> pDelegate -> pDelegate.removeProperty(pPropertyDescription, pAttr));
       children.remove(childNode);
       HierarchyHelper.getNode(property).remove();
       fireNodeRemoved(description, pAttributes);
@@ -221,7 +221,7 @@ public void removeProperty(int pIndex, @NotNull Set<Object> pAttributes)
     IPropertyDescription description = property.getDescription();
     List<Runnable> onFinish = new ArrayList<>();
     fireNodeWillBeRemoved(description, onFinish::add, pAttributes);
-    executeWriteOnDelegate(pDelegate -> pDelegate.removeProperty(pIndex, pAttributes));
+    executeWriteOnDelegate(pAttributes, pAttr -> pDelegate -> pDelegate.removeProperty(pIndex, pAttr));
     children.remove(pIndex);
     HierarchyHelper.getNode(property).remove();
     fireNodeRemoved(description, pAttributes);
@@ -243,7 +243,14 @@ public void reorder(@NotNull Comparator pComparator, @NotNull Set<Object> pAttri
       List<Runnable> onFinish = new ArrayList<>();
       firePropertyOrderWillBeChanged(onFinish::add, pAttributes);
       children.reorder(pComparator);
-      executeWriteOnDelegate(pDelegate -> pDelegate.reorder(Comparator.<IProperty>comparingInt(p -> children.indexOf(p.getDescription())), pAttributes));
+      executeWriteOnDelegate(
+          pAttributes,
+          pAttr -> pDelegate ->
+              pDelegate.reorder(
+                  Comparator.<IProperty>comparingInt(p -> children.indexOf(p.getDescription())),
+                  pAttr
+              )
+      );
       firePropertyOrderChanged(pAttributes);
       onFinish.forEach(Runnable::run);
     }
@@ -259,7 +266,7 @@ public void rename(@NotNull String pName, @NotNull Set<Object> pAttributes) thro
 
     try
     {
-      executeWriteOnDelegate(pDelegate -> pDelegate.rename(pName, pAttributes));
+      executeWriteOnDelegate(pAttributes, pAttr -> pDelegate -> pDelegate.rename(pName, pAttr));
 
       String oldName = property.getName();
       DelegatingNode parent = (DelegatingNode) getParent();
@@ -287,27 +294,38 @@ public boolean isValid()
   public void remove()
   {
     assert delegate != null;
-    executeWriteOnDelegate(INode::remove);
+    executeWriteOnDelegate(new HashSet<>(), pAttr -> INode::remove);
     if (isValid() && children != null)
     {
       for (INode child : children)
         child.remove();
     }
-    executeWriteOnDelegate(INode::remove);
+    executeWriteOnDelegate(new HashSet<>(), pAttr -> INode::remove);
     children = null;
     pitProvider = null;
     delegate = null;
     super.remove();
   }
 
-  protected void executeWriteOnDelegate(@NotNull Consumer<INode> pOnDelegate)
+  /**
+   * Executes the given write operation on the delegate.
+   *
+   * @param pAttributes the attributes to pass to the delegate
+   * @param pOnDelegate the method to execute on the delegate,
+   *                    It is important to use the function parameter for the attributes,
+   *                    even though Set given to this method could also be used directly.
+   *                    This is because the function might add its own attributes to the set in different implementations.
+   *                    For example, UpdateableDelegatingNode adds the '_EVENT_BY_DELEGATINGNODE' flag to the set.
+   *                    Why that is necessary is better explained in that override.
+   */
+  protected void executeWriteOnDelegate(@NotNull Set<Object> pAttributes, @NotNull Function<Set<Object>, Consumer<INode>> pOnDelegate)
   {
-    pOnDelegate.accept(delegate);
+    pOnDelegate.apply(pAttributes).accept(delegate);
   }
 
   protected <T> T executeReadOnDelegate(@NotNull Function<INode, T> pOnDelegate)
   {
-    if(delegate == null)
+    if (delegate == null)
       return null;
 
     return pOnDelegate.apply(delegate);

propertly.core/src/main/java/de/adito/propertly/core/api/UpdateableDelegatingNode.java

@@ -1,13 +1,12 @@
 package de.adito.propertly.core.api;
 
 import de.adito.propertly.core.common.PropertyPitEventAdapter;
-import de.adito.propertly.core.common.exception.PropertlyRenameException;
 import de.adito.propertly.core.common.path.PropertyPath;
 import de.adito.propertly.core.spi.*;
 import org.jetbrains.annotations.*;
 
 import java.util.*;
-import java.util.function.Consumer;
+import java.util.function.*;
 
 /**
  * @author w.glanzer, 16.06.2021
@@ -37,51 +36,9 @@ public Object setValue(@Nullable Object pValue, @NotNull Set<Object> pAttributes
     if ((!hasCreatedCopyOfValue() && delegateValue instanceof IPropertyPitProvider) || (hasCreatedCopyOfValue() && delegateValue == null))
       alignToDelegate();
 
-    pAttributes = new HashSet<>(pAttributes);
-    pAttributes.add(_EVENT_BY_DELEGATINGNODE);
     return super.setValue(pValue, pAttributes);
   }
 
-  @Override
-  public INode addProperty(@Nullable Integer pIndex, @NotNull IPropertyDescription pPropertyDescription, @NotNull Set<Object> pAttributes)
-  {
-    pAttributes = new HashSet<>(pAttributes);
-    pAttributes.add(_EVENT_BY_DELEGATINGNODE);
-    return super.addProperty(pIndex, pPropertyDescription, pAttributes);
-  }
-
-  @Override
-  public boolean removeProperty(@NotNull IPropertyDescription pPropertyDescription, @NotNull Set<Object> pAttributes)
-  {
-    pAttributes = new HashSet<>(pAttributes);
-    pAttributes.add(_EVENT_BY_DELEGATINGNODE);
-    return super.removeProperty(pPropertyDescription, pAttributes);
-  }
-
-  @Override
-  public void removeProperty(int pIndex, @NotNull Set<Object> pAttributes)
-  {
-    pAttributes = new HashSet<>(pAttributes);
-    pAttributes.add(_EVENT_BY_DELEGATINGNODE);
-    super.removeProperty(pIndex, pAttributes);
-  }
-
-  @Override
-  public void reorder(@NotNull Comparator pComparator, @NotNull Set<Object> pAttributes)
-  {
-    pAttributes = new HashSet<>(pAttributes);
-    pAttributes.add(_EVENT_BY_DELEGATINGNODE);
-    super.reorder(pComparator, pAttributes);
-  }
-
-  @Override
-  public void rename(@NotNull String pName, @NotNull Set<Object> pAttributes) throws PropertlyRenameException
-  {
-    pAttributes = new HashSet<>(pAttributes);
-    pAttributes.add(_EVENT_BY_DELEGATINGNODE);
-    super.rename(pName, pAttributes);
-  }
-
   @Override
   public void remove()
   {
@@ -92,7 +49,14 @@ public void remove()
     });
 
     delegateListener = null;
-    super.remove();
+    // If the delegate was already removed (e.g. because a parent node reacted to the
+    // delegate being removed and triggered this node's removal in turn), suppress
+    // the write-through inside DelegatingNode.remove() to avoid calling remove() on
+    // an already-invalidated delegate whose internal state (delegate field) is null.
+    if (executeReadOnDelegate(INode::isValid) != Boolean.TRUE)
+      _runWithoutWriteThrough(super::remove);
+    else
+      super.remove();
     writeOnDelegate = null;
   }
 
@@ -120,10 +84,54 @@ protected void alignToDelegate()
   }
 
   @Override
-  protected void executeWriteOnDelegate(@NotNull Consumer<INode> pOnDelegate)
+  protected void executeWriteOnDelegate(@NotNull Set<Object> pAttributes, @NotNull Function<Set<Object>, Consumer<INode>> pOnDelegate)
   {
     if (writeOnDelegate == null || writeOnDelegate.get() != Boolean.FALSE)
-      super.executeWriteOnDelegate(pOnDelegate);
+    {
+      /*
+      This flag is set here because otherwise it would block too much.
+      Before this flag was set here, it was set in the overrides of the Operational Methods (addProperty, removeProperty, reorder and rename) before the super variant was called.
+      e.g. for the removeProperty:
+      ---------------------------
+        removeProperty(pPropertyDescription, pAttributes)
+        {
+          pAttributes = new HashSet<>(pAttributes);
+          pAttributes.add(_EVENT_BY_DELEGATINGNODE);
+          return super.removeProperty(pPropertyDescription, pAttributes);
+        }
+
+      This causes the flag to be set before the super method runs.
+      Crucially, this means all the listeners will be fired with the flag already in place therefore,
+      no Listener will update any DelegatingNode that has us as its delegate.
+
+      for example, the (simplified) super method that was called before:
+      ----------------------
+         public boolean removeProperty(IPropertyDescription pPropertyDescription,Set<Object> pAttributes)
+         {
+            fireNodeWillBeRemoved(description, onFinish::add, pAttributes);
+            executeWriteOnDelegate(pAttributes, pAttr -> pDelegate -> pDelegate.removeProperty(pPropertyDescription, pAttr));
+            fireNodeRemoved(description, pAttributes);
+         }
+      this is a very simplivied version of the removeProperty of our super class (DelegatingNode)
+
+      Now we can see that if the attribute is set before super is called (as was the case before),
+      both fireNodeWillBeRemoved and fireNodeRemoved will fire with the attribute already in place.
+      This means all the listeners waiting for this exact event will
+      immediately return without doing anything because the flag told them to do so.
+
+      In the above example for the super method you can already see that the new 'executeWriteOnDelegate' variant,
+      that now gets the attribute as a separate Parameter and then gives them to the lambda method containing the actual operation.
+
+      This lets us set the '_EVENT_BY_DELEGATINGNODE' flag here instead of before the super, meaning only the delegates know of the flag,
+      and there it blocks their listeners from writing back up to us.
+
+      At the same time, our Listeners will never hear of the flag and can still notify any DelegatingNodes, for whom we are their delegate.
+      */
+      Set<Object> attributes = new HashSet<>(pAttributes);
+      attributes.add(_EVENT_BY_DELEGATINGNODE);
+
+      super.executeWriteOnDelegate(attributes, pOnDelegate);
+    }
   }
 
   /**