From 1f5332eb7a9d2382d76b1a771ca75cdfe699c59c Mon Sep 17 00:00:00 2001 From: Sebastian <89982771+sschulz92@users.noreply.github.com> Date: Fri, 17 Sep 2021 17:11:57 +0200 Subject: [PATCH] [HK] Use MapPart in 'ActiveMapTracker' (#478) * [HK] Use MapPart in 'ActiveMapTracker' * Add JUnit test of class 'ActiveMapTracker' * uses ConcurrentLinkedDeque for openMaps Signed-off-by: Sebastian Schulz bastie92_spam@gmx.de Co-authored-by: Frank Gasdorf --- .../ui/internal/ActiveMapTrackerTest.java | 160 ++++++++++++++++++ .../project/ui/internal/ActiveMapTracker.java | 114 +++++++++---- 2 files changed, 242 insertions(+), 32 deletions(-) create mode 100644 plugins/org.locationtech.udig.project.ui.tests/src/org/locationtech/udig/project/ui/internal/ActiveMapTrackerTest.java diff --git a/plugins/org.locationtech.udig.project.ui.tests/src/org/locationtech/udig/project/ui/internal/ActiveMapTrackerTest.java b/plugins/org.locationtech.udig.project.ui.tests/src/org/locationtech/udig/project/ui/internal/ActiveMapTrackerTest.java new file mode 100644 index 000000000..46780e3bc --- /dev/null +++ b/plugins/org.locationtech.udig.project.ui.tests/src/org/locationtech/udig/project/ui/internal/ActiveMapTrackerTest.java @@ -0,0 +1,160 @@ +/** + * uDig - User Friendly Desktop Internet GIS client + * https://locationtech.org/projects/technology.udig + * (C) 2021, Eclipse Foundation + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * (http://www.eclipse.org/legal/epl-v10.html), and the Eclipse Distribution + * License v1.0 (http://www.eclipse.org/org/documents/edl-v10.html). + */ +package org.locationtech.udig.project.ui.internal; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.eclipse.ui.IEditorReference; +import org.eclipse.ui.IViewPart; +import org.eclipse.ui.IViewReference; +import org.eclipse.ui.IWorkbenchPage; +import org.eclipse.ui.IWorkbenchPartReference; +import org.eclipse.ui.IWorkbenchWindow; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.locationtech.udig.project.internal.Map; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class ActiveMapTrackerTest { + + @Mock + private IWorkbenchWindow workbenchWindow; + + @Mock + private IWorkbenchPage workbenchPage; + + @Mock + private IEditorReference editorReference; + + @Mock + private IWorkbenchPartReference partRef; + + @Test + public void registersPageListenerOnWindowOpen() { + final ActiveMapTracker instance = new ActiveMapTracker(); + when(workbenchWindow.getPages()).thenReturn(new IWorkbenchPage[] {}); + + workbenchWindow.addPageListener(instance); + + instance.windowOpened(workbenchWindow); + verify(workbenchWindow, atLeastOnce()).addPageListener(instance); + } + + @Test + public void removesPageListenerOnWindowClose() { + final ActiveMapTracker instance = new ActiveMapTracker(); + when(workbenchWindow.getPages()).thenReturn(new IWorkbenchPage[] {}); + + workbenchWindow.removePageListener(instance); + + instance.windowClosed(workbenchWindow); + verify(workbenchWindow, atLeastOnce()).removePageListener(instance); + } + + @Test + public void registersOpenPartOnWindowOpen() { + final ActiveMapTracker instance = new ActiveMapTracker(); + + final Map map = mock(Map.class); + + final MapEditorPart mEditor = mock(MapEditorPart.class); + + when(mEditor.getMap()).thenReturn(map); + + when(editorReference.getPart(false)).thenReturn(mEditor); + + when(workbenchWindow.getPages()).thenReturn(new IWorkbenchPage[] { workbenchPage }); + + when(workbenchPage.getEditorReferences()) + .thenReturn(new IEditorReference[] { editorReference }); + + when(workbenchPage.getViewReferences()).thenReturn(new IViewReference[] {}); + + instance.windowOpened(workbenchWindow); + + assertTrue(instance.getOpenMapParts().contains(mEditor)); + assertTrue(instance.getOpenMaps().contains(map)); + + verify(mEditor, times(1)).getMap(); + verify(editorReference, times(1)).getPart(false); + verify(workbenchWindow, times(1)).getPages(); + verify(workbenchPage, times(1)).getEditorReferences(); + verify(workbenchPage, times(1)).getViewReferences(); + } + + @Test(expected = UnsupportedOperationException.class) + public void openMapsCollectionIsReadOnly() { + final Map map = mock(Map.class); + new ActiveMapTracker().getOpenMaps().remove(map); + } + + @Test + public void trackMapPartOnlyOnPartOpened() { + final ActiveMapTracker instance = new ActiveMapTracker(); + final MapEditorPart mapPart = mock(MapEditorPart.class); + final IViewPart anotherPart = mock(IViewPart.class); + + when(partRef.getPart(false)).thenReturn(mapPart); + + assertEquals(0, instance.getOpenMapParts().size()); + instance.partOpened(partRef); + + assertTrue(instance.getOpenMapParts().contains(mapPart)); + assertEquals(1, instance.getOpenMapParts().size()); + + // another View + when(partRef.getPart(false)).thenReturn(anotherPart); + + instance.partOpened(partRef); + + assertTrue(instance.getOpenMapParts().contains(mapPart)); + assertEquals(1, instance.getOpenMapParts().size()); + assertEquals(mapPart, instance.getMostRecentOpenedPart()); + + verify(partRef, times(2)).getPart(false); + } + + @Test + public void mostRecentOpenedMapFirst() { + final ActiveMapTracker instance = new ActiveMapTracker(); + IWorkbenchPartReference partRef = mock(IWorkbenchPartReference.class); + final MapEditorPart mapPart1 = mock(MapEditorPart.class); + final MapEditorPart mapPart2 = mock(MapEditorPart.class); + + when(partRef.getPart(false)).thenReturn(mapPart1); + + assertEquals(0, instance.getOpenMapParts().size()); + instance.partOpened(partRef); + + assertTrue(instance.getOpenMapParts().contains(mapPart1)); + assertEquals(mapPart1, instance.getMostRecentOpenedPart()); + assertEquals(1, instance.getOpenMapParts().size()); + + // second View + when(partRef.getPart(false)).thenReturn(mapPart2); + + instance.partOpened(partRef); + assertTrue(instance.getOpenMapParts().contains(mapPart1)); + assertTrue(instance.getOpenMapParts().contains(mapPart2)); + + assertEquals(2, instance.getOpenMapParts().size()); + assertEquals(mapPart2, instance.getMostRecentOpenedPart()); + verify(partRef, times(2)).getPart(false); + } +} diff --git a/plugins/org.locationtech.udig.project.ui/src/org/locationtech/udig/project/ui/internal/ActiveMapTracker.java b/plugins/org.locationtech.udig.project.ui/src/org/locationtech/udig/project/ui/internal/ActiveMapTracker.java index 232dbbf1d..ca75ce3b4 100644 --- a/plugins/org.locationtech.udig.project.ui/src/org/locationtech/udig/project/ui/internal/ActiveMapTracker.java +++ b/plugins/org.locationtech.udig.project.ui/src/org/locationtech/udig/project/ui/internal/ActiveMapTracker.java @@ -11,9 +11,11 @@ import java.util.Collection; import java.util.Collections; +import java.util.Deque; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CopyOnWriteArraySet; @@ -37,7 +39,7 @@ /** * Tracks which map is the active map. Essentially just listens to the workbench for part changes * and when a part is an MapEditorPart it marks that as the active map. - * + * * @author jesse * @since 1.1.0 */ @@ -45,7 +47,7 @@ public class ActiveMapTracker implements IStartup, IPartListener2, IWindowListen /** * All the parts that are have been active in the order of activation. - * + * */ private List activeParts = new CopyOnWriteArrayList<>(); @@ -57,7 +59,7 @@ public class ActiveMapTracker implements IStartup, IPartListener2, IWindowListen /** * All the maps that are currently open */ - private Set openMaps = new CopyOnWriteArraySet<>(); + private Deque openMaps = new ConcurrentLinkedDeque<>(); /** * Returns the set of the visible maps. @@ -79,15 +81,26 @@ public Collection getVisibleMaps() { * @return */ public Map getActiveMap() { + MapPart activePart = getActiveMapPartInternal(); + if (activePart != null) { + return activePart.getMap(); + } - if (Display.getCurrent() != null) { - MapPart part = getActivePart(); - if (part != null && !activeParts.isEmpty() && part != activeParts.get(0)) { - addActivePart(part); - return part.getMap(); - } + MapPart visibleMapPart = getFirstVisibleMapPart(); + return (visibleMapPart == null ? ApplicationGIS.NO_MAP : visibleMapPart.getMap()); + } + + /** + * @return most recent opened MapPart or null. + */ + public MapPart getMostRecentOpenedPart() { + if (!openMaps.isEmpty()) { + return openMaps.peek(); } + return null; + } + private MapPart getFirstVisibleMapPart() { // lets first look at activeParts MapPart mapPart = null; if (!activeParts.isEmpty()) { @@ -103,15 +116,28 @@ public Map getActiveMap() { } } } + return mapPart; + } + + public MapPart getActiveMapPart() { + return getActiveMapPartInternal(); + } - if (mapPart == null || mapPart.getMap() == null) { - // nothing found - return ApplicationGIS.NO_MAP; + private MapPart getActiveMapPartInternal() { + if (Display.getCurrent() != null) { + MapPart part = getActiveMapPartFromWorkbench(); + if (part == null) { + part = getMostRecentOpenedPart(); + } + if (!activeParts.isEmpty() && part != activeParts.get(0)) { + addActivePart(part); + } + return part; } - return mapPart.getMap(); + return null; } - private MapPart getActivePart() { + private MapPart getActiveMapPartFromWorkbench() { IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow(); if (window == null) { return null; @@ -124,7 +150,6 @@ private MapPart getActivePart() { return (MapPart) part; } } - return null; } @@ -142,22 +167,28 @@ public Collection getOpenMaps() { } private void addActivePart(MapPart part) { - while (activeParts.remove(part)) + while (activeParts.remove(part)) { ; + } activeParts.add(0, part); } private void removePart(MapPart part) { - while (activeParts.remove(part)) + while (activeParts.remove(part)) { ; + } visibleMaps.remove(part); openMaps.remove(part); } - private boolean addOpenMap(IWorkbenchPart part) { - return openMaps.add((MapPart) part); + private void addOpenMap(MapPart part) { + while (openMaps.remove(part)) { + ; + } + openMaps.push(part); } + @Override public void windowClosed(IWorkbenchWindow window) { // stop listening to pages and parts IWorkbenchPage[] pages = window.getPages(); @@ -167,6 +198,7 @@ public void windowClosed(IWorkbenchWindow window) { window.removePageListener(this); } + @Override public void windowOpened(IWorkbenchWindow window) { // start listening to pages and parts // stop listening to pages and parts @@ -177,38 +209,39 @@ public void windowOpened(IWorkbenchWindow window) { window.addPageListener(this); } + @Override public void pageClosed(IWorkbenchPage page) { page.removePartListener(this); } + @Override public void pageOpened(IWorkbenchPage page) { page.addPartListener(this); IEditorReference[] editors = page.getEditorReferences(); for (IEditorReference reference : editors) { - IWorkbenchPart workpart = reference.getPart(false); - if (reference.getPart(false) instanceof MapPart) { - MapPart part = (MapPart) reference.getPart(false); - openMaps.add(part); + IWorkbenchPart workbenchPart = reference.getPart(false); + if (workbenchPart instanceof MapPart) { + addOpenMap((MapPart) workbenchPart); - if (page.isPartVisible(workpart)) { - visibleMaps.add(part); + if (page.isPartVisible(workbenchPart)) { + visibleMaps.add((MapPart) workbenchPart); } } } IViewReference[] views = page.getViewReferences(); for (IViewReference reference : views) { - IWorkbenchPart workpart = reference.getPart(false); - if (workpart instanceof MapPart) { - MapPart part = (MapPart) reference.getPart(false); - openMaps.add(part); + IWorkbenchPart workbenchPart = reference.getPart(false); + if (workbenchPart instanceof MapPart) { + addOpenMap((MapPart) workbenchPart); - if (page.isPartVisible(workpart)) { - visibleMaps.add(part); + if (page.isPartVisible(workbenchPart)) { + visibleMaps.add((MapPart) workbenchPart); } } } } + @Override public void partActivated(IWorkbenchPartReference partRef) { // make this the active map(if it is a MapPart) IWorkbenchPart part = partRef.getPart(false); @@ -217,6 +250,7 @@ public void partActivated(IWorkbenchPartReference partRef) { } } + @Override public void partClosed(IWorkbenchPartReference partRef) { // if active map then make previous map be the active map IWorkbenchPart part = partRef.getPart(false); @@ -225,6 +259,7 @@ public void partClosed(IWorkbenchPartReference partRef) { } } + @Override public void partVisible(IWorkbenchPartReference partRef) { // if no active map then make this the active map (if it is a MapPart) IWorkbenchPart part = partRef.getPart(false); @@ -236,6 +271,7 @@ public void partVisible(IWorkbenchPartReference partRef) { } } + @Override public void partHidden(IWorkbenchPartReference partRef) { IWorkbenchPart part = partRef.getPart(false); if (part instanceof MapPart) { @@ -243,33 +279,40 @@ public void partHidden(IWorkbenchPartReference partRef) { } } + @Override public void partOpened(IWorkbenchPartReference partRef) { IWorkbenchPart part = partRef.getPart(false); if (part instanceof MapPart) { - addOpenMap(part); + addOpenMap((MapPart) part); } } + @Override public void partBroughtToTop(IWorkbenchPartReference partRef) { // do nothing } + @Override public void partDeactivated(IWorkbenchPartReference partRef) { // do nothing } + @Override public void partInputChanged(IWorkbenchPartReference partRef) { // do nothing } + @Override public void windowActivated(IWorkbenchWindow window) { // do nothing } + @Override public void windowDeactivated(IWorkbenchWindow window) { // do nothing } + @Override public void pageActivated(IWorkbenchPage page) { // do nothing } @@ -288,4 +331,11 @@ public void earlyStartup() { } } + /** + * @return Collection of open MapParts. + */ + public Collection getOpenMapParts() { + return Collections.unmodifiableCollection(openMaps); + } + }