From 0c6ddbfb9300d0d4fc09d18c7e41d4db55030277 Mon Sep 17 00:00:00 2001
From: Phillipus <p.beauvoir@dadabeatnik.com>
Date: Tue, 10 Sep 2024 21:16:48 +0100
Subject: [PATCH] Fix FormatPainter not referencing original model object

- We need to reference the source object and its parent model in case we are copying images

- When the source model is closed reset the FormatPainterInfo

- See https://github.com/archimatetool/archi/issues/1075
---
 .../diagram/tools/FormatPainterInfo.java      | 29 ++++++++++---------
 .../diagram/tools/FormatPainterInfoTests.java |  4 +--
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterInfo.java b/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterInfo.java
index dcc477718..02695bef8 100644
--- a/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterInfo.java
+++ b/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterInfo.java
@@ -7,15 +7,18 @@
 
 import java.beans.PropertyChangeListener;
 import java.beans.PropertyChangeSupport;
+import java.util.Objects;
 
 import org.eclipse.swt.graphics.Cursor;
 import org.eclipse.swt.graphics.ImageData;
 import org.eclipse.swt.graphics.PaletteData;
 import org.eclipse.swt.graphics.RGB;
 
+import com.archimatetool.editor.model.IEditorModelManager;
 import com.archimatetool.editor.ui.ColorFactory;
 import com.archimatetool.editor.ui.IArchiImages;
 import com.archimatetool.editor.ui.ImageFactory;
+import com.archimatetool.model.IArchimateModel;
 import com.archimatetool.model.IDiagramModelComponent;
 import com.archimatetool.model.IDiagramModelConnection;
 import com.archimatetool.model.IDiagramModelObject;
@@ -45,7 +48,17 @@ public class FormatPainterInfo {
     private Cursor coloredCursor, defaultCursor;
     private PropertyChangeSupport listeners = new PropertyChangeSupport(this);
     
-    private FormatPainterInfo() {}
+    private FormatPainterInfo() {
+        // Listen to a model being closed in case it's the model that we are referencing. If it is, reset.
+        IEditorModelManager.INSTANCE.addPropertyChangeListener(evt -> {
+            if(evt.getPropertyName() == IEditorModelManager.PROPERTY_MODEL_REMOVED) {
+                IArchimateModel model = (IArchimateModel)evt.getNewValue();
+                if(sourceComponent != null && Objects.equals(model, sourceComponent.getArchimateModel())) {
+                    reset();
+                }
+            }
+        });
+    }
     
     /**
      * Reset source component to null and notify listeners.
@@ -99,18 +112,8 @@ RGB getCursorColor() {
      * Set the source component from which we will copy the formatting
      */
     private void setSourceComponent(IDiagramModelComponent component) {
-        if(component != null) {
-            // Make a snapshot copy of the source component so we don't reference the original object.
-            // Before this change we used to hold a reference to the original object
-            // but if the model was closed we still held a reference to it and it couldn't be garbage collected
-            // until the user cleared the FormatPainter which they might forget to do.
-            // Another side effect of that old way was if the user changed an attribute of the source component
-            // the FormatPainter would now hold that new value which might not be what the user expects.
-            sourceComponent = (IDiagramModelComponent)component.getCopy();
-        }
-        else {
-            sourceComponent = null;
-        }
+        // Reference the original object and its parent model in case we need to copy its images
+        sourceComponent = component;
 
         cursorColor = null;
         
diff --git a/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterInfoTests.java b/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterInfoTests.java
index ddd38eb68..3bf2e12fb 100644
--- a/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterInfoTests.java
+++ b/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterInfoTests.java
@@ -7,7 +7,6 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
@@ -57,8 +56,7 @@ public void testUpdateWithSourceComponent() throws Exception {
         info.updateWithSourceComponent(sourceComponent);
         
         assertNotNull(info.getSourceComponent());
-        // FormatPaintInfo will make a copy of the source component
-        assertNotEquals(sourceComponent, info.getSourceComponent());
+        assertEquals(sourceComponent, info.getSourceComponent());
         
         // Colored cursor
         assertSame(info.getCursor(), TestUtils.getPrivateField(info, "coloredCursor"));