From e97fb7c247fab32a29b6f7ecf124ebbc9fbcc8c1 Mon Sep 17 00:00:00 2001 From: Phillipus Date: Tue, 30 Jul 2024 14:51:00 +0100 Subject: [PATCH] Refactor FormatPainter code and don't reference the original diagram object - Don't reference the original diagram object in case the user closes its containing model. Instead make a snapshot copy of it. It might be that the original object is in a large model consuming a lot of memory. If the user closes that model it could not be garbage collected as we would hold a reference to it. - This has the side effect that we now make a snapshot of the object when the FormatPainter copies the object. Thereafter, any pasted attributes come from the snapshot, whereas before if the user changed an attribute of the original object that would be pasted. - When pasting the fill color and the source or target fill is null ("default"), compare on actual colors. This avoids the problem of copying a null fill color and pasting an actual fill color on an object with the same default fill color. - We're not going to sub-class FormatPainterToolEntry so remove that support. This was added in 2011 and, 13 years later, we haven't sub-classed it. :-) - Get rid of PaintFormat inner class - Modernise code --- .../diagram/tools/FormatPainterInfo.java | 136 ++++++++++-------- .../diagram/tools/FormatPainterTool.java | 58 ++++---- .../diagram/tools/FormatPainterToolEntry.java | 44 ++---- .../diagram/tools/FormatPainterInfoTests.java | 55 +++---- .../diagram/tools/FormatPainterToolTests.java | 43 +++--- 5 files changed, 169 insertions(+), 167 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 6827bb643..d88e35c10 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 @@ -23,85 +23,49 @@ /** - * FormatPainter Singleton state of format and cursor + * FormatPainter Singleton state of format and cursor. + * This is a a global class that stores the state of the FormatPainterInfo and the cursor to be used + * by each instance of FormatPainterToolEntry and FormatPainterTool. * * @author Phillip Beauvoir */ public class FormatPainterInfo { - public static FormatPainterInfo INSTANCE = new FormatPainterInfo(); - /** - * Paint format information containing cursor color and source component + * Global static instance */ - public static class PaintFormat { - private IDiagramModelComponent component; - private RGB cursorColor; - - public PaintFormat(IDiagramModelComponent component) { - this.component = component; - - // Default - cursorColor = new RGB(255, 255, 255); - - if(component instanceof IDiagramModelConnection) { - // Line color - String colorValue = ((IDiagramModelConnection)component).getLineColor(); - cursorColor = ColorFactory.convertStringToRGB(colorValue); - if(cursorColor == null) { - cursorColor = ColorFactory.getDefaultLineColor(component).getRGB(); - } - } - else if(component instanceof IDiagramModelObject) { - // Fill color - String colorValue = ((IDiagramModelObject)component).getFillColor(); - cursorColor = ColorFactory.convertStringToRGB(colorValue); - if(cursorColor == null) { - cursorColor = ColorFactory.getDefaultFillColor(component).getRGB(); - } - } - } - - public IDiagramModelComponent getSourceComponent() { - return component; - } - - public RGB getCursorColor() { - return cursorColor; - } - } - - FormatPainterInfo() {} + public static FormatPainterInfo INSTANCE = new FormatPainterInfo(); - private PaintFormat pf; + private IDiagramModelComponent sourceComponent; + private RGB cursorColor; private Cursor coloredCursor, defaultCursor; private PropertyChangeSupport listeners = new PropertyChangeSupport(this); - PaintFormat getPaintFormat() { - return pf; - } + // package protected constructor for unit tests + FormatPainterInfo() {} - void updatePaintFormat(IDiagramModelComponent component) { - if(component != null) { - pf = new PaintFormat(component); - } - else { - pf = null; - } - updateColoredCursor(); + /** + * Reset source component to null and notify listeners. + */ + public void reset() { + setSourceComponent(null); fireUpdated(); } /** - * Reset all copied information + * Set the source component that we will copy formatting from and update the cursor. */ - public void reset() { - pf = null; + void updateWithSourceComponent(IDiagramModelComponent component) { + setSourceComponent(component); + updateColoredCursor(); fireUpdated(); } + /** + * Get the cursor to use for the tool. + */ Cursor getCursor() { - return pf == null ? getDefaultCursor() : coloredCursor; + return hasSourceComponent() ? coloredCursor : getDefaultCursor(); } private Cursor getDefaultCursor() { @@ -113,8 +77,56 @@ private Cursor getDefaultCursor() { return defaultCursor; } - boolean isFat() { - return pf != null; + /** + * @return true if we have a source component that we will copy the format from. + */ + boolean hasSourceComponent() { + return getSourceComponent() != null; + } + + IDiagramModelComponent getSourceComponent() { + return sourceComponent; + } + + RGB getCursorColor() { + return cursorColor; + } + + /** + * 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; + } + + cursorColor = null; + + if(sourceComponent instanceof IDiagramModelConnection dmc) { + // Line color + String colorValue = dmc.getLineColor(); + cursorColor = ColorFactory.convertStringToRGB(colorValue); + if(cursorColor == null) { + cursorColor = ColorFactory.getDefaultLineColor(sourceComponent).getRGB(); + } + } + else if(sourceComponent instanceof IDiagramModelObject dmo) { + // Fill color + String colorValue = dmo.getFillColor(); + cursorColor = ColorFactory.convertStringToRGB(colorValue); + if(cursorColor == null) { + cursorColor = ColorFactory.getDefaultFillColor(sourceComponent).getRGB(); + } + } } /** @@ -127,10 +139,10 @@ private void updateColoredCursor() { ImageData cursorImageData = IArchiImages.ImageFactory.getImage(IArchiImages.CURSOR_FORMAT_PAINTER).getImageData(ImageFactory.getCursorDeviceZoom()); - if(pf.getCursorColor() != null) { + if(getCursorColor() != null) { PaletteData pData = cursorImageData.palette; int whitePixel = pData.getPixel(new RGB(255, 255, 255)); - int fillColor = pData.getPixel(pf.getCursorColor()); + int fillColor = pData.getPixel(getCursorColor()); for(int i = 0; i < cursorImageData.width; i++) { for(int j = 0; j < cursorImageData.height; j++) { diff --git a/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterTool.java b/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterTool.java index 6f24fae42..f7fa7ecb9 100644 --- a/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterTool.java +++ b/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterTool.java @@ -6,6 +6,7 @@ package com.archimatetool.editor.diagram.tools; import java.io.IOException; +import java.util.Objects; import org.eclipse.draw2d.geometry.Point; import org.eclipse.gef.EditPart; @@ -26,7 +27,6 @@ import com.archimatetool.editor.diagram.commands.LineWidthCommand; import com.archimatetool.editor.diagram.commands.TextAlignmentCommand; import com.archimatetool.editor.diagram.commands.TextPositionCommand; -import com.archimatetool.editor.diagram.tools.FormatPainterInfo.PaintFormat; import com.archimatetool.editor.model.IArchiveManager; import com.archimatetool.editor.model.commands.EObjectFeatureCommand; import com.archimatetool.editor.model.commands.FeatureCommand; @@ -73,12 +73,12 @@ protected boolean handleButtonUp(int button) { if(editpart != null && editpart.getModel() != null) { Object object = editpart.getModel(); if(isPaintableObject(object)) { - PaintFormat pf = FormatPainterInfo.INSTANCE.getPaintFormat(); - if(pf == null) { - FormatPainterInfo.INSTANCE.updatePaintFormat((IDiagramModelComponent)object); + IDiagramModelComponent sourceComponent = FormatPainterInfo.INSTANCE.getSourceComponent(); + if(sourceComponent == null) { + FormatPainterInfo.INSTANCE.updateWithSourceComponent((IDiagramModelComponent)object); } else if(!isObjectLocked(object)) { - Command cmd = createCommand(pf, (IDiagramModelComponent)object); + Command cmd = createCommand(sourceComponent, (IDiagramModelComponent)object); if(cmd.canExecute()) { executeCommand(cmd); } @@ -104,18 +104,18 @@ protected boolean handleDoubleClick(int button) { FormatPainterInfo.INSTANCE.reset(); return true; } - else return handleButtonUp(1); } + return false; } - protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent targetComponent) { + protected CompoundCommand createCommand(IDiagramModelComponent sourceComponent, IDiagramModelComponent targetComponent) { CompoundCommand result = new CompoundCommand(Messages.FormatPainterTool_0); IObjectUIProvider provider = ObjectUIFactory.INSTANCE.getProvider(targetComponent); // IFontAttribute - if(pf.getSourceComponent() instanceof IFontAttribute source && targetComponent instanceof IFontAttribute target) { + if(sourceComponent instanceof IFontAttribute source && targetComponent instanceof IFontAttribute target) { Command cmd = new FontStyleCommand(target, source.getFont()); if(cmd.canExecute()) { result.add(cmd); @@ -128,7 +128,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // ILineObject - if(pf.getSourceComponent() instanceof ILineObject source && targetComponent instanceof ILineObject target && provider != null) { + if(sourceComponent instanceof ILineObject source && targetComponent instanceof ILineObject target && provider != null) { // Line color if(provider.shouldExposeFeature(IArchimatePackage.Literals.LINE_OBJECT__LINE_COLOR.getName())) { Command cmd = new LineColorCommand(target, source.getLineColor()); @@ -147,7 +147,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // IBorderObject - if(pf.getSourceComponent() instanceof IBorderObject source && targetComponent instanceof IBorderObject target) { + if(sourceComponent instanceof IBorderObject source && targetComponent instanceof IBorderObject target) { Command cmd = new BorderColorCommand(target, source.getBorderColor()); if(cmd.canExecute()) { result.add(cmd); @@ -155,7 +155,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // IBorderType - if(pf.getSourceComponent() instanceof IBorderType source && targetComponent instanceof IBorderType target && source.eClass() == target.eClass()) { + if(sourceComponent instanceof IBorderType source && targetComponent instanceof IBorderType target && source.eClass() == target.eClass()) { Command cmd = new EObjectFeatureCommand("", target, IArchimatePackage.Literals.BORDER_TYPE__BORDER_TYPE, source.getBorderType()); //$NON-NLS-1$ if(cmd.canExecute()) { result.add(cmd); @@ -163,7 +163,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // ITextPosition - if(pf.getSourceComponent() instanceof ITextPosition source && targetComponent instanceof ITextPosition target) { + if(sourceComponent instanceof ITextPosition source && targetComponent instanceof ITextPosition target) { Command cmd = new TextPositionCommand(target, source.getTextPosition()); if(cmd.canExecute()) { result.add(cmd); @@ -171,7 +171,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // ITextAlignment - if(pf.getSourceComponent() instanceof ITextAlignment source && targetComponent instanceof ITextAlignment target) { + if(sourceComponent instanceof ITextAlignment source && targetComponent instanceof ITextAlignment target) { Command cmd = new TextAlignmentCommand(target, source.getTextAlignment()); if(cmd.canExecute()) { result.add(cmd); @@ -179,16 +179,25 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // IDiagramModelObject - if(pf.getSourceComponent() instanceof IDiagramModelObject source && targetComponent instanceof IDiagramModelObject target) { - // Source fill colour is null which is "default" - String fillColorString = source.getFillColor(); - if(fillColorString == null) { - fillColorString = ColorFactory.convertColorToString(ColorFactory.getDefaultFillColor(source)); + if(sourceComponent instanceof IDiagramModelObject source && targetComponent instanceof IDiagramModelObject target) { + Command cmd; + + String sourcefillColor = source.getFillColor(); + if(sourcefillColor == null) { // Source fill colour is null which is "default" + sourcefillColor = ColorFactory.convertColorToString(ColorFactory.getDefaultFillColor(source)); } - Command cmd = new FillColorCommand(target, fillColorString); - if(cmd.canExecute()) { - result.add(cmd); + String targetfillColor = target.getFillColor(); + if(targetfillColor == null) { // target fill colour is null which is "default" + targetfillColor = ColorFactory.convertColorToString(ColorFactory.getDefaultFillColor(target)); + } + + // Compare actual fill colours rather than null fill colors + if(!Objects.equals(sourcefillColor, targetfillColor)) { + cmd = new FillColorCommand(target, sourcefillColor); + if(cmd.canExecute()) { + result.add(cmd); + } } // Alpha fill opacity @@ -236,7 +245,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // Archimate objects - if(pf.getSourceComponent() instanceof IDiagramModelArchimateObject source && targetComponent instanceof IDiagramModelArchimateObject target) { + if(sourceComponent instanceof IDiagramModelArchimateObject source && targetComponent instanceof IDiagramModelArchimateObject target) { // Image Source Command cmd = new FeatureCommand("", target, IDiagramModelArchimateObject.FEATURE_IMAGE_SOURCE, //$NON-NLS-1$ source.getImageSource(), IDiagramModelArchimateObject.FEATURE_IMAGE_SOURCE_DEFAULT); @@ -246,7 +255,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // IDiagramModelConnection - if(pf.getSourceComponent() instanceof IDiagramModelConnection source && targetComponent instanceof IDiagramModelConnection target) { + if(sourceComponent instanceof IDiagramModelConnection source && targetComponent instanceof IDiagramModelConnection target) { // Connection text position Command cmd = new ConnectionTextPositionCommand(target, source.getTextPosition()); if(cmd.canExecute()) { @@ -263,7 +272,7 @@ protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent t } // IIconic - if(pf.getSourceComponent() instanceof IIconic source && targetComponent instanceof IIconic target) { + if(sourceComponent instanceof IIconic source && targetComponent instanceof IIconic target) { // If we have an image path and the source and target models are different, copy the image bytes String imagePath = source.getImagePath(); if(imagePath != null && source.getArchimateModel() != target.getArchimateModel()) { @@ -317,5 +326,4 @@ protected boolean isPaintableObject(Object object) { protected String getCommandName() { return "FormatPaint"; //$NON-NLS-1$ } - } \ No newline at end of file diff --git a/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterToolEntry.java b/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterToolEntry.java index 6d9bd8ab8..d6c3f3af3 100644 --- a/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterToolEntry.java +++ b/com.archimatetool.editor/src/com/archimatetool/editor/diagram/tools/FormatPainterToolEntry.java @@ -9,13 +9,9 @@ import java.beans.PropertyChangeListener; import org.eclipse.gef.Tool; -import org.eclipse.gef.commands.Command; -import org.eclipse.gef.commands.CompoundCommand; import org.eclipse.gef.palette.ToolEntry; -import com.archimatetool.editor.diagram.tools.FormatPainterInfo.PaintFormat; import com.archimatetool.editor.ui.IArchiImages; -import com.archimatetool.model.IDiagramModelComponent; import com.archimatetool.model.IDiagramModelConnection; @@ -27,7 +23,7 @@ */ public class FormatPainterToolEntry extends ToolEntry implements PropertyChangeListener { - protected FormatPainterTool tool; + private FormatPainterTool tool; public FormatPainterToolEntry() { super("", "", null, null); //$NON-NLS-1$ //$NON-NLS-2$ @@ -39,36 +35,13 @@ public FormatPainterToolEntry() { @Override public Tool createTool() { - tool = new FormatPainterTool() { - @Override - protected CompoundCommand createCommand(PaintFormat pf, IDiagramModelComponent targetComponent) { - CompoundCommand result = super.createCommand(pf, targetComponent); - - // Add any additional commands from Sub-classes - Command extraCommand = FormatPainterToolEntry.this.getCommand(pf.getSourceComponent(), targetComponent); - if(extraCommand != null && extraCommand.canExecute()) { - result.add(extraCommand); - } - - return result; - } - }; - tool.setProperties(getToolProperties()); + tool = (FormatPainterTool)super.createTool(); return tool; } - /** - * Sub-classes can add additional commands to the Main Command that will be executed when the format paint is applied - * @param sourceComponent - * @param targetComponent - */ - public Command getCommand(IDiagramModelComponent sourceComponent, IDiagramModelComponent targetComponent) { - return null; - } - @Override public void propertyChange(PropertyChangeEvent evt) { - if(tool != null) { + if(tool != null) { // tool might not have been created yet tool.setDefaultCursor(FormatPainterInfo.INSTANCE.getCursor()); } @@ -76,13 +49,13 @@ public void propertyChange(PropertyChangeEvent evt) { setIcons(); } - protected void setLabels() { - if(FormatPainterInfo.INSTANCE.isFat()) { + private void setLabels() { + if(FormatPainterInfo.INSTANCE.hasSourceComponent()) { setLabel(Messages.FormatPainterToolEntry_0); String description1 = Messages.FormatPainterToolEntry_1; String description2 = Messages.FormatPainterToolEntry_2; - String description = (FormatPainterInfo.INSTANCE.getPaintFormat().getSourceComponent() instanceof IDiagramModelConnection) ? + String description = (FormatPainterInfo.INSTANCE.getSourceComponent() instanceof IDiagramModelConnection) ? description1 : description2; description += "\n" + Messages.FormatPainterToolEntry_3; //$NON-NLS-1$ @@ -94,8 +67,8 @@ protected void setLabels() { } } - protected void setIcons() { - if(FormatPainterInfo.INSTANCE.isFat()) { + private void setIcons() { + if(FormatPainterInfo.INSTANCE.hasSourceComponent()) { setLargeIcon(IArchiImages.ImageFactory.getImageDescriptor(IArchiImages.ICON_FORMAT_PAINTER)); setSmallIcon(IArchiImages.ImageFactory.getImageDescriptor(IArchiImages.ICON_FORMAT_PAINTER)); } @@ -107,5 +80,6 @@ protected void setIcons() { public void dispose() { FormatPainterInfo.INSTANCE.removePropertyChangeListener(this); + tool = 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 ebcc7012e..7db64a6ed 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,8 +7,10 @@ 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; import static org.junit.jupiter.api.Assertions.assertTrue; import org.eclipse.swt.graphics.RGB; @@ -16,11 +18,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import com.archimatetool.editor.diagram.tools.FormatPainterInfo.PaintFormat; import com.archimatetool.model.IArchimateFactory; import com.archimatetool.model.IDiagramModelArchimateObject; import com.archimatetool.testingtools.ArchimateTestModel; +import com.archimatetool.tests.TestUtils; +@SuppressWarnings("nls") public class FormatPainterInfoTests { private FormatPainterInfo info; @@ -40,49 +43,53 @@ public void runAfterEachTest() { // --------------------------------------------------------------------------------------------- @Test - public void testGetPaintFormat() { - // Should be null initially - PaintFormat pf = info.getPaintFormat(); - assertNull(pf); + public void testInitialValues() throws Exception { + assertNull(info.getSourceComponent()); + assertNull(info.getCursorColor()); + assertSame(info.getCursor(), TestUtils.getPrivateField(info, "defaultCursor")); } @Test - public void testUpdatePaintFormat() { + public void testUpdateWithSourceComponent() throws Exception { // Set FormatPainterInfo to Source component IDiagramModelArchimateObject sourceComponent = ArchimateTestModel.createDiagramModelArchimateObject(IArchimateFactory.eINSTANCE.createBusinessActor()); - info.updatePaintFormat(sourceComponent); - PaintFormat pf = info.getPaintFormat(); - assertNotNull(pf); - assertEquals(sourceComponent, pf.getSourceComponent()); + info.updateWithSourceComponent(sourceComponent); - // Check cursor - assertEquals(new RGB(255, 255, 181), pf.getCursorColor()); + assertNotNull(info.getSourceComponent()); + // FormatPaintInfo will make a copy of the source component + assertNotEquals(sourceComponent, info.getSourceComponent()); + + // Colored cursor + assertSame(info.getCursor(), TestUtils.getPrivateField(info, "coloredCursor")); + + // Cursor color + assertEquals(new RGB(255, 255, 181), info.getCursorColor()); } @Test - public void testIsFat() { - assertFalse(info.isFat()); + public void testHasSourceComponent() { + assertFalse(info.hasSourceComponent()); IDiagramModelArchimateObject sourceComponent = ArchimateTestModel.createDiagramModelArchimateObject(IArchimateFactory.eINSTANCE.createBusinessActor()); - info.updatePaintFormat(sourceComponent); - assertTrue(info.isFat()); + info.updateWithSourceComponent(sourceComponent); + + assertTrue(info.hasSourceComponent()); } @Test - public void testReset() { - PaintFormat pf = info.getPaintFormat(); - assertNull(pf); + public void testReset() throws Exception { + assertNull(info.getSourceComponent()); IDiagramModelArchimateObject sourceComponent = ArchimateTestModel.createDiagramModelArchimateObject(IArchimateFactory.eINSTANCE.createBusinessActor()); - info.updatePaintFormat(sourceComponent); - pf = info.getPaintFormat(); - assertNotNull(pf); + info.updateWithSourceComponent(sourceComponent); + assertNotNull(info.getSourceComponent()); info.reset(); - pf = info.getPaintFormat(); - assertNull(pf); + assertNull(info.getSourceComponent()); + assertNull(info.getCursorColor()); + assertSame(info.getCursor(), TestUtils.getPrivateField(info, "defaultCursor")); } } \ No newline at end of file diff --git a/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterToolTests.java b/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterToolTests.java index 3501240dc..42d73d21d 100644 --- a/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterToolTests.java +++ b/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/diagram/tools/FormatPainterToolTests.java @@ -9,19 +9,15 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import org.eclipse.gef.commands.Command; import org.eclipse.gef.commands.CompoundCommand; import org.junit.jupiter.api.Test; -import com.archimatetool.editor.diagram.commands.FillColorCommand; -import com.archimatetool.editor.diagram.tools.FormatPainterInfo.PaintFormat; import com.archimatetool.model.IArchimateFactory; import com.archimatetool.model.IDiagramModelArchimateConnection; import com.archimatetool.model.IDiagramModelArchimateObject; import com.archimatetool.model.IDiagramModelObject; import com.archimatetool.model.ITextPosition; import com.archimatetool.testingtools.ArchimateTestModel; -import com.archimatetool.tests.TestUtils; @SuppressWarnings("nls") public class FormatPainterToolTests { @@ -29,7 +25,7 @@ public class FormatPainterToolTests { // --------------------------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------------------------- - + @Test public void testCreateCommandForDiagramModelArchimateObject() throws Exception { // Source component @@ -41,23 +37,23 @@ public void testCreateCommandForDiagramModelArchimateObject() throws Exception { ArchimateTestModel.createDiagramModelArchimateObject(IArchimateFactory.eINSTANCE.createBusinessActor()); // Set FormatPainterInfo to Source component - FormatPainterInfo.INSTANCE.updatePaintFormat(sourceComponent); - PaintFormat pf = FormatPainterInfo.INSTANCE.getPaintFormat(); + FormatPainterInfo.INSTANCE.updateWithSourceComponent(sourceComponent); // Execute command FormatPainterTool tool = new FormatPainterTool(); - CompoundCommand compoundCmd = tool.createCommand(pf, targetComponent); + CompoundCommand compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent); - // Source and Target have same properties except for fill color so only one command - assertEquals(1, compoundCmd.getCommands().size()); + // Should be no commands + assertEquals(0, compoundCmd.getCommands().size()); - // Fill Color should be set even if fill colour source is null (default) - Command cmd = (Command)compoundCmd.getCommands().get(0); - assertTrue(cmd instanceof FillColorCommand); - Object newValue = TestUtils.getPrivateField(cmd, "fNewValue"); - assertEquals("#ffffb5", newValue); + // Set the source fill color to its default actual value. + // The fill color command will not be added because the target's fill color (null) is effectively the same. + sourceComponent.setFillColor("#ffffb5"); + compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent); + assertEquals(0, compoundCmd.getCommands().size()); - // Now change some properties on the source component + // Now change some attributes on the source component + sourceComponent.setFillColor("#eeeeee"); sourceComponent.setFont("Consolas"); sourceComponent.setFontColor("#eeeeee"); sourceComponent.setLineColor("#eeeeee"); @@ -70,7 +66,10 @@ public void testCreateCommandForDiagramModelArchimateObject() throws Exception { sourceComponent.setTextPosition(ITextPosition.TEXT_POSITION_BOTTOM); sourceComponent.setDeriveElementLineColor(false); - compoundCmd = tool.createCommand(pf, targetComponent); + // But we have to reset the FormatPainterInfo with the source component because it makes a copy of it + FormatPainterInfo.INSTANCE.updateWithSourceComponent(sourceComponent); + + compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent); assertEquals(12, compoundCmd.getCommands().size()); } @@ -85,12 +84,11 @@ public void testCreateCommandForDiagramModelArchimateConnection() throws Excepti ArchimateTestModel.createDiagramModelArchimateConnection(IArchimateFactory.eINSTANCE.createAccessRelationship()); // Set FormatPainterInfo to Source component - FormatPainterInfo.INSTANCE.updatePaintFormat(sourceComponent); - PaintFormat pf = FormatPainterInfo.INSTANCE.getPaintFormat(); + FormatPainterInfo.INSTANCE.updateWithSourceComponent(sourceComponent); // Execute command FormatPainterTool tool = new FormatPainterTool(); - CompoundCommand compoundCmd = tool.createCommand(pf, targetComponent); + CompoundCommand compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent); // Should be no commands assertEquals(0, compoundCmd.getCommands().size()); @@ -102,7 +100,10 @@ public void testCreateCommandForDiagramModelArchimateConnection() throws Excepti sourceComponent.setLineWidth(3); sourceComponent.setTextPosition(3); - compoundCmd = tool.createCommand(pf, targetComponent); + // But we have to reset the FormatPainterInfo with the source component because it makes a copy of it + FormatPainterInfo.INSTANCE.updateWithSourceComponent(sourceComponent); + + compoundCmd = tool.createCommand(FormatPainterInfo.INSTANCE.getSourceComponent(), targetComponent); assertEquals(5, compoundCmd.getCommands().size()); }