Mercurial > hg > thermostat
changeset 2510:6f13c7edf6a5
Add some caching logic to VMInformationControllers/Views
review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-October/021467.html
reviewed-by: aazores
line wrap: on
line diff
--- a/client/core/src/main/java/com/redhat/thermostat/client/core/views/VmInformationView.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/core/src/main/java/com/redhat/thermostat/client/core/views/VmInformationView.java Thu Nov 03 18:23:07 2016 +0100 @@ -50,5 +50,6 @@ public abstract boolean selectChildID(int id); public abstract int getSelectedChildID(); + public abstract void clear(); }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/ContentProvider.java Thu Nov 03 18:23:07 2016 +0100 @@ -0,0 +1,46 @@ +/* + * Copyright 2012-2016 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.client.ui; + +import com.redhat.thermostat.client.core.views.BasicView; + +/** + */ +public interface ContentProvider { + + BasicView getView(); +}
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/HostInformationController.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/HostInformationController.java Thu Nov 03 18:23:07 2016 +0100 @@ -48,14 +48,23 @@ import com.redhat.thermostat.shared.locale.LocalizedString; import com.redhat.thermostat.storage.core.HostRef; -public class HostInformationController { +public class HostInformationController implements ContentProvider { + private final List<InformationService<HostRef>> hostInfoServices; + private final HostRef ref; private final HostInformationView view; public HostInformationController(List<InformationService<HostRef>> hostInfoServices, - HostRef ref, HostInformationViewProvider provider) { + HostRef ref, + HostInformationViewProvider provider) + { + this.hostInfoServices = hostInfoServices; + this.ref = ref; view = provider.createView(); + rebuild(); + } + public void rebuild() { Collections.sort(hostInfoServices, new OrderedComparator<InformationService<HostRef>>()); for (InformationService<HostRef> hostInfoService : hostInfoServices) { if (hostInfoService.getFilter().matches(ref)) { @@ -67,8 +76,8 @@ } public BasicView getView() { + rebuild(); return view; } - }
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/VersionAndInfoController.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/VersionAndInfoController.java Thu Nov 03 18:23:07 2016 +0100 @@ -41,7 +41,7 @@ import com.redhat.thermostat.client.core.views.VersionAndInfoViewProvider; import com.redhat.thermostat.common.ApplicationInfo; -public class VersionAndInfoController { +public class VersionAndInfoController implements ContentProvider { private final VersionAndInfoView view; @@ -56,6 +56,4 @@ public BasicView getView() { return view; } - } -
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/VmInformationController.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/VmInformationController.java Thu Nov 03 18:23:07 2016 +0100 @@ -48,13 +48,29 @@ import com.redhat.thermostat.shared.locale.LocalizedString; import com.redhat.thermostat.storage.core.VmRef; -public class VmInformationController { +public class VmInformationController implements ContentProvider { + private final List<InformationService<VmRef>> vmInfoServices; + private final VmRef vmRef; private final VmInformationView view; - public VmInformationController(List<InformationService<VmRef>> vmInfoServices, - VmRef vmRef, VmInformationViewProvider provider) { + private int selectedID; + + public VmInformationController(List<InformationService<VmRef>> vmInfoServices, + VmRef vmRef, + VmInformationViewProvider provider) + { + this.vmInfoServices = vmInfoServices; + this.vmRef = vmRef; + this.selectedID = 0; + view = provider.createView(); + rebuild(); + } + + void rebuild() { + + view.clear(); Collections.sort(vmInfoServices, new OrderedComparator<InformationService<VmRef>>()); for (InformationService<VmRef> vmInfoService : vmInfoServices) { @@ -64,6 +80,8 @@ view.addChildView(name, ctrl.getView()); } } + + view.selectChildID(selectedID); } public int getSelectedChildID() { @@ -71,6 +89,7 @@ } public boolean selectChildID(int id) { + selectedID = id; return view.selectChildID(id); } @@ -79,6 +98,7 @@ } public BasicView getView() { + rebuild(); return view; }
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/IssueViewController.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/IssueViewController.java Thu Nov 03 18:23:07 2016 +0100 @@ -48,6 +48,7 @@ import com.redhat.thermostat.client.core.Severity; import com.redhat.thermostat.client.swing.internal.vmlist.controller.DecoratorManager; +import com.redhat.thermostat.client.ui.ContentProvider; import com.redhat.thermostat.client.ui.ReferenceFieldLabelDecorator; import com.redhat.thermostat.common.ActionNotifier; import com.redhat.thermostat.storage.core.HostRef; @@ -76,7 +77,7 @@ import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.dao.VmInfoDAO; -public class IssueViewController { +public class IssueViewController implements ContentProvider { private static final Logger logger = LoggingUtils.getLogger(IssueViewController.IssueDiagnoserTracker.class);
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainView.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainView.java Thu Nov 03 18:23:07 2016 +0100 @@ -44,6 +44,7 @@ import com.redhat.thermostat.client.swing.internal.vmlist.controller.ContextActionController; import com.redhat.thermostat.client.swing.internal.vmlist.controller.DecoratorManager; import com.redhat.thermostat.client.swing.internal.vmlist.controller.HostTreeController; +import com.redhat.thermostat.client.ui.ContentProvider; import com.redhat.thermostat.client.ui.MenuAction; import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.shared.config.CommonPaths; @@ -71,7 +72,7 @@ void hideMainWindow(); - void setSubView(BasicView view); + void setContent(ContentProvider content); void setStatusBarPrimaryStatus(LocalizedString primaryStatus);
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java Thu Nov 03 18:23:07 2016 +0100 @@ -68,6 +68,7 @@ import javax.swing.SwingUtilities; import javax.swing.tree.TreeNode; +import com.redhat.thermostat.client.ui.ContentProvider; import com.redhat.thermostat.shared.config.CommonPaths; import sun.misc.Signal; @@ -108,7 +109,7 @@ public static final String MAIN_WINDOW_NAME = "Thermostat_mainWindo_JFrame_parent#1"; - private static final Logger logger = LoggingUtils.getLogger(MainWindow.class); + private static Logger logger = LoggingUtils.getLogger(MainWindow.class); private static final Translate<LocaleResources> translator = LocaleResources.createLocalizer(); @@ -481,30 +482,39 @@ } @Override - public void setSubView(final BasicView view) { - if (view instanceof SwingComponent) { - final SwingComponent swingComp = (SwingComponent)view; - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { + public void setContent(final ContentProvider provider) { + + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + + BasicView view = provider.getView(); + if (view instanceof SwingComponent) { + SwingComponent swingComp = (SwingComponent) view; + contentArea.removeAll(); Component toAdd = swingComp.getUiComponent(); contentArea.add(toAdd); contentArea.revalidate(); contentArea.repaint(); + + return; } - }); - } else { - String message = "" - + "There's a non-swing view registered: '" + view.toString() - + "'. The swing client can not use these views. This is " - + "most likely a developer mistake. If this is meant to " - + "be a swing-based view, it must implement the " - + "'SwingComponent' interface. If it's not meant to be a " - + "swing-based view, it should not have been registered."; - logger.severe(message); - throw new AssertionError(message); - } + + final String message = "" + + "There's a non-swing view registered: '" + view.toString() + + "'. The swing client can not use these views. This is " + + "most likely a developer mistake. If this is meant to " + + "be a swing-based view, it must implement the " + + "'SwingComponent' interface. If it's not meant to be a " + + "swing-based view, it should not have been registered."; + logger.severe(message); + } + }); + } + + static void __test__setLogger(Logger logger) { + MainWindow.logger = logger; } @Override
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Thu Nov 03 18:23:07 2016 +0100 @@ -506,21 +506,21 @@ void updateView(Ref ref) { if (ref == null) { VersionAndInfoController controller = createSummaryController(); - view.setSubView(controller.getView()); + view.setContent(controller); } else if (ref == ISSUES_REF) { view.getHostTreeController().clearSelection(); - view.setSubView(issuesController.getView()); + view.setContent(issuesController); } else if (ref instanceof HostRef) { HostRef hostRef = (HostRef) ref; HostInformationController hostController = createHostInformationController(hostRef); - view.setSubView(hostController.getView()); + view.setContent(hostController); view.setStatusBarPrimaryStatus(t.localize(LocaleResources.HOST_PRIMARY_STATUS, hostRef.getHostName(), hostRef.getAgentId())); } else if (ref instanceof VmRef) { VmRef vmRef = (VmRef) ref; VmInformationController vmInformation = vmInfoControllerProvider.getVmInfoController(vmRef); - view.setSubView(vmInformation.getView()); + view.setContent(vmInformation); view.setStatusBarPrimaryStatus(t.localize(LocaleResources.VM_PRIMARY_STATUS, vmRef.getName(), String.valueOf(vmRef.getPid()), vmRef.getHostRef().getHostName())); } else { @@ -531,22 +531,29 @@ private class VmInformationControllerProvider { private VmInformationController lastSelectedVM; private Map<VmRef, Integer> selectedForVM = new ConcurrentHashMap<>(); - + private Map<VmRef, VmInformationController> cachedControllers = new ConcurrentHashMap<>(); + VmInformationController getVmInfoController(VmRef vmRef) { int id = 0; if (lastSelectedVM != null) { id = lastSelectedVM.getSelectedChildID(); } - - lastSelectedVM = createVmController(vmRef); + + if (cachedControllers.containsKey(vmRef)) { + lastSelectedVM = cachedControllers.get(vmRef); + } else { + lastSelectedVM = createVmController(vmRef); + cachedControllers.put(vmRef, lastSelectedVM); + } + if (!lastSelectedVM.selectChildID(id)) { Integer _id = selectedForVM.get(vmRef); - id = _id != null? _id : 0; + id = _id != null ? _id : 0; lastSelectedVM.selectChildID(id); } selectedForVM.put(vmRef, id); - + return lastSelectedVM; } } @@ -570,5 +577,9 @@ Desktop.getDesktop().browse(uri); } } + + void __test__forceSetIssueController(IssueViewController issuesController) { + this.issuesController = issuesController; + } }
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/HostInformationPanel.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/HostInformationPanel.java Thu Nov 03 18:23:07 2016 +0100 @@ -68,7 +68,7 @@ tabPane = new JTabbedPane(); visiblePanel.add(tabPane); } - + @Override public void addChildView(final LocalizedString title, final UIComponent view) { if (view instanceof SwingComponent) {
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/VmInformationPanel.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/VmInformationPanel.java Thu Nov 03 18:23:07 2016 +0100 @@ -69,6 +69,21 @@ } @Override + public void clear() { + try { + edtHelper.callAndWait(new Runnable() { + @Override + public void run() { + tabPane.removeAll(); + } + }); + + } catch (InvocationTargetException | InterruptedException e) { + logger.severe(e.getLocalizedMessage()); + } + } + + @Override public void addChildView(final LocalizedString title, final UIComponent view) { if (view instanceof SwingComponent) { try {
--- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java Thu Nov 03 18:23:07 2016 +0100 @@ -57,6 +57,7 @@ import java.util.concurrent.CountDownLatch; import com.redhat.thermostat.client.swing.internal.search.ReferenceFieldSearchFilter; +import com.redhat.thermostat.client.ui.ContentProvider; import com.redhat.thermostat.common.Filter; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; @@ -227,7 +228,6 @@ doNothing().when(treeController).addReferenceSelectionChangeListener(hostTreeCaptor.capture()); issueViewController = mock(IssueViewController.class); - when(issueViewController.getView()).thenReturn(issueView); ProgressNotifier notifier = mock(ProgressNotifier.class); when(view.getNotifier()).thenReturn(notifier); @@ -290,116 +290,6 @@ verify(uriOpener).open(isA(URI.class)); } -// @Test -// @Bug(id="954", -// summary="Thermostat GUI client should remember my last panel selected", -// url="http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=954") -// public void verifyOpenSameHostVMTab() throws Exception { -// -// VmRef vmRef = mock(VmRef.class); -// when(vmRef.getName()).thenReturn("testvm"); -// when(vmRef.getVmId()).thenReturn("testvmid"); -// HostRef ref = mock(HostRef.class); -// when(ref.getAgentId()).thenReturn("agentId"); -// when(vmRef.getHostRef()).thenReturn(ref); -// -// when(view.getSelectedHostOrVm()).thenReturn(vmRef); -// -// when(vmInfoView.getSelectedChildID()).thenReturn(3); -// when(vmInfoView.selectChildID(anyInt())).thenReturn(true); -// -// l.actionPerformed(new ActionEvent<MainView.Action>(view, MainView.Action.HOST_VM_SELECTION_CHANGED)); -// -// ArgumentCaptor<Integer> arg = ArgumentCaptor.forClass(Integer.class); -// verify(vmInfoView).selectChildID(arg.capture()); -// verify(vmInfoView, times(0)).getSelectedChildID(); -// -// int id = arg.getValue(); -// -// assertEquals(0, id); -// -// l.actionPerformed(new ActionEvent<MainView.Action>(view, MainView.Action.HOST_VM_SELECTION_CHANGED)); -// -// arg = ArgumentCaptor.forClass(Integer.class); -// verify(vmInfoView, times(1)).getSelectedChildID(); -// verify(vmInfoView, times(2)).selectChildID(arg.capture()); -// id = arg.getValue(); -// -// assertEquals(3, id); -// } -// -// @Test -// public void verifyOpenSameHostVMTab2() { -// -// VmRef vmRef1 = mock(VmRef.class); -// VmRef vmRef2 = mock(VmRef.class); -// when(view.getSelectedHostOrVm()).thenReturn(vmRef1).thenReturn(vmRef1).thenReturn(vmRef2).thenReturn(vmRef1); -// -// when(vmRef1.getName()).thenReturn("testvm"); -// when(vmRef1.getVmId()).thenReturn("testvmid"); -// HostRef ref = mock(HostRef.class); -// when(ref.getAgentId()).thenReturn("agentId"); -// when(vmRef1.getHostRef()).thenReturn(ref); -// -// when(vmRef2.getName()).thenReturn("testvm"); -// when(vmRef2.getVmId()).thenReturn("testvmid"); -// when(vmRef2.getHostRef()).thenReturn(ref); -// -// VmInformationView vmInfoView2 = mock(VmInformationView.class); -// -// when(vmInfoView.getSelectedChildID()).thenReturn(2).thenReturn(2); -// when(vmInfoView2.getSelectedChildID()).thenReturn(3); -// -// when(vmInfoView.selectChildID(0)).thenReturn(true); -// when(vmInfoView.selectChildID(2)).thenReturn(true); -// when(vmInfoView.selectChildID(3)).thenReturn(false); -// -// when(vmInfoView2.selectChildID(0)).thenReturn(true); -// when(vmInfoView2.selectChildID(2)).thenReturn(true); -// when(vmInfoView2.selectChildID(3)).thenReturn(true); -// -// when(vmInfoViewProvider.createView()).thenReturn(vmInfoView) -// .thenReturn(vmInfoView2).thenReturn(vmInfoView2) -// .thenReturn(vmInfoView); -// -// l.actionPerformed(new ActionEvent<MainView.Action>(view, MainView.Action.HOST_VM_SELECTION_CHANGED)); -// -// ArgumentCaptor<Integer> arg = ArgumentCaptor.forClass(Integer.class); -// verify(vmInfoView).selectChildID(arg.capture()); -// verify(vmInfoView, times(0)).getSelectedChildID(); -// -// int id = arg.getValue(); -// -// assertEquals(0, id); -// -// l.actionPerformed(new ActionEvent<MainView.Action>(view, MainView.Action.HOST_VM_SELECTION_CHANGED)); -// -// arg = ArgumentCaptor.forClass(Integer.class); -// verify(vmInfoView).getSelectedChildID(); -// verify(vmInfoView2, times(1)).selectChildID(arg.capture()); -// id = arg.getValue(); -// -// assertEquals(2, id); -// -// l.actionPerformed(new ActionEvent<MainView.Action>(view, MainView.Action.HOST_VM_SELECTION_CHANGED)); -// -// arg = ArgumentCaptor.forClass(Integer.class); -// verify(vmInfoView2, times(1)).getSelectedChildID(); -// verify(vmInfoView2, times(2)).selectChildID(arg.capture()); -// id = arg.getValue(); -// -// assertEquals(3, id); -// -// l.actionPerformed(new ActionEvent<MainView.Action>(view, MainView.Action.HOST_VM_SELECTION_CHANGED)); -// -// arg = ArgumentCaptor.forClass(Integer.class); -// verify(vmInfoView2, times(2)).getSelectedChildID(); -// verify(vmInfoView, times(3)).selectChildID(arg.capture()); -// id = arg.getValue(); -// -// assertEquals(2, id); -// } - @Test public void verifyMenuItems() { @@ -430,11 +320,13 @@ @Test public void testSelectionIssuesViewClearsHostTreeSelectionAndDisplaysIssuesView() { + controller.__test__forceSetIssueController(issueViewController); + controller.updateView(ISSUES_REF); InOrder inOrder = inOrder(view, treeController); inOrder.verify(view).getHostTreeController(); inOrder.verify(treeController).clearSelection(); - inOrder.verify(view).setSubView(issueView); + inOrder.verify(view).setContent(issueViewController); } }
--- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowTest.java Wed Nov 02 10:53:48 2016 -0400 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowTest.java Thu Nov 03 18:23:07 2016 +0100 @@ -38,6 +38,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -45,6 +46,7 @@ import javax.swing.JCheckBoxMenuItem; import javax.swing.JRadioButtonMenuItem; +import com.redhat.thermostat.client.ui.ContentProvider; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.shared.config.CommonPaths; import net.java.openjdk.cacio.ctc.junit.CacioFESTRunner; @@ -70,7 +72,10 @@ import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.shared.locale.LocalizedString; +import java.awt.AWTException; +import java.awt.Robot; import java.io.File; +import java.util.logging.Logger; @Category(CacioTest.class) @RunWith(CacioFESTRunner.class) @@ -268,48 +273,23 @@ } @GUITest - @Test(expected=AssertionError.class) - public void verifyThatAddingNonSwingSubViewFails() { + @Test + public void verifyThatAddingNonSwingSubViewFails() throws AWTException { + + Logger logger = mock(Logger.class); + window.__test__setLogger(logger); + BasicView subView = mock(BasicView.class); + ContentProvider subViewProvider = mock(ContentProvider.class); + when(subViewProvider.getView()).thenReturn(subView); frameFixture.show(); - window.setSubView(subView); - } + window.setContent(subViewProvider); -// @GUITest -// @Test -// public void verifyContextMenu() { -// List<ContextAction> actions = new ArrayList<>(); -// -// HostContextAction action = mock(HostContextAction.class); -// when(action.getName()).thenReturn(new LocalizedString("action")); -// when(action.getDescription()).thenReturn(new LocalizedString("description of action")); -// Filter allMatchingFilter = mock(Filter.class); -// when(allMatchingFilter.matches(any(HostRef.class))).thenReturn(true); -// -// actions.add(action); -// -// frameFixture.show(); -// -// // add a second action listener to discard the 'show' event invoked on the first -// l = mock(ActionListener.class); -// window.addActionListener(l); -// -// MouseEvent e = new MouseEvent(window, MouseEvent.MOUSE_CLICKED, System.currentTimeMillis(), MouseEvent.BUTTON2_MASK, 0, 0, 0, 0, 1, true, MouseEvent.BUTTON2); -// -// window.showContextActions(actions, e); -// -// JMenuItemFixture hostActionMenuItem = frameFixture.menuItem("action"); -// hostActionMenuItem.click(); -// -// ArgumentCaptor<ActionEvent> actionEventCaptor = ArgumentCaptor.forClass(ActionEvent.class); -// verify(l).actionPerformed(actionEventCaptor.capture()); -// -// ActionEvent actionEvent = actionEventCaptor.getValue(); -// assertEquals(window, actionEvent.getSource()); -// assertEquals(MainView.Action.HOST_VM_CONTEXT_ACTION, actionEvent.getActionId()); -// assertEquals(action, actionEvent.getPayload()); -// } + Robot robot = new Robot(); + robot.waitForIdle(); + verify(logger).severe(anyString()); + } }