From e76e9840c45033880075fb2639b556bbb3a23c5f Mon Sep 17 00:00:00 2001 From: Phillipus Date: Thu, 21 Nov 2024 17:07:46 +0000 Subject: [PATCH] Refactor and tidy DeleteCommandHandler - Add constructor that doesn't take a TreeViewer. This means that DeleteCommandHandlerTests doesn't need to create a mock object --- .../tree/commands/DeleteCommandHandler.java | 126 +++++++++--------- .../commands/DeleteCommandHandlerTests.java | 22 ++- 2 files changed, 73 insertions(+), 75 deletions(-) diff --git a/com.archimatetool.editor/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandler.java b/com.archimatetool.editor/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandler.java index 234705b34..0557e4cd5 100644 --- a/com.archimatetool.editor/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandler.java +++ b/com.archimatetool.editor/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandler.java @@ -41,7 +41,6 @@ import com.archimatetool.model.IDiagramModelObject; import com.archimatetool.model.IDiagramModelReference; import com.archimatetool.model.IFolder; -import com.archimatetool.model.IFolderContainer; import com.archimatetool.model.util.ArchimateModelUtils; @@ -63,7 +62,7 @@ public class DeleteCommandHandler { * If deleting elements from more than one model in the tree we need to use the * Command Stack allocated to each model. And then allocate one CompoundCommand per Command Stack. */ - private Hashtable fCommandMap = new Hashtable(); + private Hashtable fCommandMap = new Hashtable<>(); // Treeviewer private TreeModelViewer fViewer; @@ -85,22 +84,22 @@ public class DeleteCommandHandler { * @return True if we can delete this object */ public static boolean canDelete(Object element) { - // Elements, Relations, and Diagrams - if(element instanceof IArchimateConcept || element instanceof IDiagramModel) { - return true; - } - - // Certain Folders - if(element instanceof IFolder) { - IFolder folder = (IFolder)element; - if(folder.getType().equals(FolderType.USER)) { - return true; - } - } - - return false; + // Elements, Relations, Diagrams and user folders + return element instanceof IArchimateConcept || element instanceof IDiagramModel || + (element instanceof IFolder folder && folder.getType().equals(FolderType.USER)); + } + + /** + * @param objects Objects to delete + */ + public DeleteCommandHandler(Object[] objects) { + this(null, objects); } + /** + * @param viewer Tree Viewer in case of selecting a tree node after deletion + * @param objects Objects to delete + */ public DeleteCommandHandler(TreeModelViewer viewer, Object[] objects) { fViewer = viewer; fSelectedObjects = objects; @@ -111,8 +110,7 @@ public DeleteCommandHandler(TreeModelViewer viewer, Object[] objects) { */ public boolean hasDiagramReferences() { for(Object object : fSelectedObjects) { - boolean result = hasDiagramReferences(object); - if(result) { + if(hasDiagramReferences(object)) { return true; } } @@ -124,29 +122,27 @@ public boolean hasDiagramReferences() { * @return True if object has references in a diagram model */ private boolean hasDiagramReferences(Object object) { - if(object instanceof IFolder) { - for(EObject element : ((IFolder)object).getElements()) { - boolean result = hasDiagramReferences(element); - if(result) { + if(object instanceof IFolder folder) { + for(EObject element : folder.getElements()) { + if(hasDiagramReferences(element)) { return true; } } - for(IFolder f : ((IFolder)object).getFolders()) { - boolean result = hasDiagramReferences(f); - if(result) { + for(IFolder subFolder : folder.getFolders()) { + if(hasDiagramReferences(subFolder)) { return true; } } } // Concept - if(object instanceof IArchimateConcept) { - return DiagramModelUtils.isArchimateConceptReferencedInDiagrams((IArchimateConcept)object); + if(object instanceof IArchimateConcept concept) { + return DiagramModelUtils.isArchimateConceptReferencedInDiagrams(concept); } // Diagram Model Reference - if(object instanceof IDiagramModel) { - return DiagramModelUtils.hasDiagramModelReference((IDiagramModel)object); + if(object instanceof IDiagramModel dm) { + return DiagramModelUtils.hasDiagramModelReference(dm); } return false; @@ -188,10 +184,10 @@ private void createCommands() { * any open diagram editors before removing their models from parent folders. */ for(Object object : fObjectsToDelete) { - if(object instanceof IDiagramModel) { - CompoundCommand compoundCommand = getCompoundCommand((IAdapter)object); + if(object instanceof IDiagramModel dm) { + CompoundCommand compoundCommand = getCompoundCommand(dm); if(compoundCommand != null) { - Command cmd = new DeleteDiagramModelCommand((IDiagramModel)object); + Command cmd = new DeleteDiagramModelCommand(dm); compoundCommand.add(cmd); } else { @@ -214,24 +210,24 @@ private void createCommands() { continue; } - if(object instanceof IFolder) { - Command cmd = new DeleteFolderCommand((IFolder)object); + if(object instanceof IFolder folder) { + Command cmd = new DeleteFolderCommand(folder); compoundCommand.add(cmd); } - else if(object instanceof IArchimateElement) { - Command cmd = new DeleteArchimateElementCommand((IArchimateElement)object); + else if(object instanceof IArchimateElement element) { + Command cmd = new DeleteArchimateElementCommand(element); compoundCommand.add(cmd); } - else if(object instanceof IArchimateRelationship) { - Command cmd = new DeleteArchimateRelationshipCommand((IArchimateRelationship)object); + else if(object instanceof IArchimateRelationship relationship) { + Command cmd = new DeleteArchimateRelationshipCommand(relationship); compoundCommand.add(cmd); } - else if(object instanceof IDiagramModelObject) { - Command cmd = DiagramCommandFactory.createDeleteDiagramObjectCommand((IDiagramModelObject)object); + else if(object instanceof IDiagramModelObject dmo) { + Command cmd = DiagramCommandFactory.createDeleteDiagramObjectCommand(dmo); compoundCommand.add(cmd); } - else if(object instanceof IDiagramModelConnection) { - Command cmd = DiagramCommandFactory.createDeleteDiagramConnectionCommand((IDiagramModelConnection)object); + else if(object instanceof IDiagramModelConnection dmc) { + Command cmd = DiagramCommandFactory.createDeleteDiagramConnectionCommand(dmc); compoundCommand.add(cmd); } } @@ -254,13 +250,13 @@ private void getObjectsToDelete() { // Then get the referenced diagram components to be deleted for(IArchimateModelObject object : new ArrayList<>(fObjectsToDelete)) { // Archimate Concept to be deleted - if(object instanceof IArchimateConcept) { - fObjectsToDelete.addAll(((IArchimateConcept)object).getReferencingDiagramComponents()); + if(object instanceof IArchimateConcept concept) { + fObjectsToDelete.addAll(concept.getReferencingDiagramComponents()); } // Diagram Model to be deleted so we also need to delete diagram model references, if any - if(object instanceof IDiagramModel) { - getDiagramModelReferencesToDelete((IDiagramModel)object); + if(object instanceof IDiagramModel dm) { + getDiagramModelReferencesToDelete(dm); } } } @@ -276,8 +272,8 @@ private void getDiagramModelReferencesToDelete(IDiagramModel dm) { if(diagramsFolder != null) { for(Iterator iter = diagramsFolder.eAllContents(); iter.hasNext();) { EObject eObject = iter.next(); - if(eObject instanceof IDiagramModelReference) { - refs.add((IDiagramModelReference)eObject); + if(eObject instanceof IDiagramModelReference ref) { + refs.add(ref); } } } @@ -297,18 +293,18 @@ private void getDiagramModelReferencesToDelete(IDiagramModel dm) { */ private void addFolderChildElements(Object object) { // Folder - if(object instanceof IFolder) { - for(EObject element : ((IFolder)object).getElements()) { + if(object instanceof IFolder folder) { + for(EObject element : folder.getElements()) { addFolderChildElements(element); } // Child folders - for(IFolder f : ((IFolderContainer)object).getFolders()) { - addFolderChildElements(f); + for(IFolder childFolder : folder.getFolders()) { + addFolderChildElements(childFolder); } } - else if(object instanceof IArchimateModelObject) { - fObjectsToDelete.add((IArchimateModelObject)object); + else if(object instanceof IArchimateModelObject amo) { + fObjectsToDelete.add(amo); } } @@ -317,19 +313,19 @@ else if(object instanceof IArchimateModelObject) { */ private void addElementRelationships(Object object) { // Folder - if(object instanceof IFolder) { - for(EObject element : ((IFolder)object).getElements()) { + if(object instanceof IFolder folder) { + for(EObject element : folder.getElements()) { addElementRelationships(element); } // Child folders - for(IFolder f : ((IFolderContainer)object).getFolders()) { - addElementRelationships(f); + for(IFolder childFolder : folder.getFolders()) { + addElementRelationships(childFolder); } } // Element/Relation - else if(object instanceof IArchimateConcept) { - for(IArchimateRelationship relationship : ArchimateModelUtils.getAllRelationshipsForConcept((IArchimateConcept)object)) { + else if(object instanceof IArchimateConcept concept) { + for(IArchimateRelationship relationship : ArchimateModelUtils.getAllRelationshipsForConcept(concept)) { fObjectsToDelete.add(relationship); // Recurse @@ -363,6 +359,10 @@ private CompoundCommand getCompoundCommand(IAdapter object) { * Find the best object to select after the deletion */ private Object findObjectToSelectAfterDeletion() { + if(fViewer == null) { + return null; + } + Object firstObject = fSelectedObjects[0]; TreeItem item = fViewer.findTreeItem(firstObject); @@ -391,8 +391,8 @@ private Object findObjectToSelectAfterDeletion() { } // Was null so select parent of first object - if(selected == null && firstObject instanceof EObject) { - selected = ((EObject)firstObject).eContainer(); + if(selected == null && firstObject instanceof EObject eObject) { + selected = eObject.eContainer(); } return selected; @@ -403,5 +403,7 @@ private void dispose() { fObjectsToDelete = null; fViewer = null; fCommandMap = null; + fDiagramModelReferenceCache = null; + fObjectToSelectAfterDeletion = null; } } diff --git a/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandlerTests.java b/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandlerTests.java index abf087638..601a478c0 100644 --- a/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandlerTests.java +++ b/tests/com.archimatetool.editor.tests/src/com/archimatetool/editor/views/tree/commands/DeleteCommandHandlerTests.java @@ -10,7 +10,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; import java.io.File; import java.io.IOException; @@ -24,7 +23,6 @@ import com.archimatetool.editor.TestSupport; import com.archimatetool.editor.model.DiagramModelUtils; -import com.archimatetool.editor.views.tree.TreeModelViewer; import com.archimatetool.model.FolderType; import com.archimatetool.model.IArchimateElement; import com.archimatetool.model.IArchimateFactory; @@ -44,13 +42,11 @@ public class DeleteCommandHandlerTests { private ArchimateTestModel tm; private IArchimateModel model; - private TreeModelViewer treeModelViewer; @BeforeEach public void runOnceBeforeEachTest() throws IOException { tm = new ArchimateTestModel(TEST_MODEL_FILE); model = tm.loadModelWithCommandStack(); - treeModelViewer = mock(TreeModelViewer.class); } @Test @@ -94,7 +90,7 @@ public void testHasDiagramReferences() { ArchimateModelUtils.getObjectByID(model, "d856464a") }; - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, elements); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(elements); assertTrue(commandHandler.hasDiagramReferences()); // This element isn't in a diagram @@ -102,7 +98,7 @@ public void testHasDiagramReferences() { ArchimateModelUtils.getObjectByID(model, "d856464a") }; - commandHandler = new DeleteCommandHandler(treeModelViewer, elements); + commandHandler = new DeleteCommandHandler(elements); assertFalse(commandHandler.hasDiagramReferences()); } @@ -111,7 +107,7 @@ public void testDelete_CannotDeleteMainFolders() { // Cannot delete the main folders assertEquals(8, model.getFolders().size()); - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, model.getFolders().toArray()); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(model.getFolders().toArray()); commandHandler.delete(); assertEquals(8, model.getFolders().size()); @@ -137,7 +133,7 @@ public void testDelete_Elements() { } // Delete them - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, elements.toArray()); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(elements.toArray()); commandHandler.delete(); // Test that they are all gone in the model and in the referenced diagrams @@ -179,7 +175,7 @@ public void testDelete_Folder_Deleted_SubElements() { } // Delete top folder - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, folder); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(folder); commandHandler.delete(); // Is top folder deleted? @@ -213,7 +209,7 @@ public void testDelete_Elements_Deleted_Attached_Relations() { assertTrue(DiagramModelUtils.isArchimateConceptReferencedInDiagrams(relationship)); // Zap - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, new Object[] { businessActor, businessRole } ); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(new Object[] { businessActor, businessRole } ); commandHandler.delete(); // All gone @@ -231,7 +227,7 @@ public void testDelete_Relations_Deleted_Diagram_Connections() { assertTrue(DiagramModelUtils.isArchimateConceptReferencedInDiagrams(relationship)); // Zap - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, new Object[] { relationship } ); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(new Object[] { relationship } ); commandHandler.delete(); // All gone @@ -248,7 +244,7 @@ public void testDelete_DiagramModel_Deleted_Diagram_Reference() { assertNotNull(dmRef); // Zap - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, new Object[] { dm2 } ); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(new Object[] { dm2 } ); commandHandler.delete(); // All gone @@ -286,7 +282,7 @@ public void testDelete_More_Than_One_Model() throws IOException { assertEquals(8, allElements.size()); // Zap - DeleteCommandHandler commandHandler = new DeleteCommandHandler(treeModelViewer, allElements.toArray() ); + DeleteCommandHandler commandHandler = new DeleteCommandHandler(allElements.toArray() ); commandHandler.delete(); // Test that they are all gone in the models and in the referenced diagrams