changeset 2557:61a28b7c3719

Fix sorting of plugins review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-January/021952.html reviewed-by: aazores
author Mario Torre <neugens.limasoftware@gmail.com>
date Mon, 16 Jan 2017 10:24:54 +0100
parents 252478620e7a
children d9c7b3581420
files client/core/src/main/java/com/redhat/thermostat/client/core/views/HostInformationView.java client/core/src/main/java/com/redhat/thermostat/client/core/views/UIPluginInfo.java client/core/src/main/java/com/redhat/thermostat/client/ui/HostInformationController.java client/core/src/main/java/com/redhat/thermostat/client/ui/PluginInfo.java client/core/src/main/java/com/redhat/thermostat/client/ui/VmInformationController.java client/core/src/test/java/com/redhat/thermostat/client/ui/HostInformationControllerTest.java client/core/src/test/java/com/redhat/thermostat/client/ui/VmInformationControllerTest.java client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java platform/swing/compat/src/main/java/com/redhat/thermostat/compat/platform/internal/DynamicHostPluginProviderImpl.java platform/swing/compat/src/main/java/com/redhat/thermostat/compat/platform/internal/DynamicVMPluginProviderImpl.java
diffstat 11 files changed, 156 insertions(+), 65 deletions(-) [+]
line wrap: on
line diff
--- a/client/core/src/main/java/com/redhat/thermostat/client/core/views/HostInformationView.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/core/src/main/java/com/redhat/thermostat/client/core/views/HostInformationView.java	Mon Jan 16 10:24:54 2017 +0100
@@ -38,6 +38,8 @@
 
 import com.redhat.thermostat.shared.locale.LocalizedString;
 
+import java.util.List;
+
 /**
  * A {@link View} that shows host information. It does not display
  * this information directly. It relies on child views.
@@ -52,5 +54,12 @@
 
     public abstract int getNumChildren();
 
+    public void addChildViews(List<UIPluginInfo> plugins) {
+        for (UIPluginInfo plugin : plugins) {
+            addChildView(plugin.getLocalizedName(), plugin.getView());
+        }
+    }
+
+    public void clear() {}
 }
 
--- a/client/core/src/main/java/com/redhat/thermostat/client/core/views/UIPluginInfo.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/core/src/main/java/com/redhat/thermostat/client/core/views/UIPluginInfo.java	Mon Jan 16 10:24:54 2017 +0100
@@ -37,12 +37,13 @@
 package com.redhat.thermostat.client.core.views;
 
 import com.redhat.thermostat.client.core.views.UIComponent;
+import com.redhat.thermostat.common.Ordered;
 import com.redhat.thermostat.shared.locale.LocalizedString;
 
 /**
  * A visual component plugin.
  */
-public interface UIPluginInfo {
+public interface UIPluginInfo extends Ordered {
     UIComponent getView();
     LocalizedString getLocalizedName();
 }
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/HostInformationController.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/HostInformationController.java	Mon Jan 16 10:24:54 2017 +0100
@@ -36,6 +36,7 @@
 
 package com.redhat.thermostat.client.ui;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
@@ -60,15 +61,14 @@
 
     private static class PluginAction implements UIPluginAction {
 
-        private HostInformationView view;
-        PluginAction(HostInformationView view) {
-            this.view = view;
+        private List<UIPluginInfo> plugins;
+        PluginAction(List<UIPluginInfo> plugins) {
+            this.plugins = plugins;
         }
 
         @Override
         public void execute(UIPluginInfo info) {
-            LocalizedString name = info.getLocalizedName();
-            view.addChildView(name, info.getView());
+            plugins.add(info);
         }
     }
 
@@ -84,20 +84,25 @@
     }
 
     public void rebuild() {
-        Collections.sort(hostInfoServices, new OrderedComparator<InformationService<HostRef>>());
+        List<UIPluginInfo> plugins = new ArrayList<>();
+        view.clear();
+
         for (InformationService<HostRef> hostInfoService : hostInfoServices) {
             if (hostInfoService.getFilter().matches(ref)) {
                 InformationServiceController<HostRef> ctrl = hostInfoService.getInformationServiceController(ref);
-                LocalizedString name = ctrl.getLocalizedName();
-                view.addChildView(name, ctrl.getView());
+                plugins.add(new PluginInfo(ctrl.getLocalizedName(), ctrl.getView(), hostInfoService.getOrderValue()));
             }
         }
 
-        PluginAction action = new PluginAction(view);
+        PluginAction action = new PluginAction(plugins);
 
         for (DynamicHostPluginProvider dynamicProvider : dynamicProviders) {
             dynamicProvider.forEach(ref, action);
         }
+
+        Collections.sort(plugins, new OrderedComparator<UIPluginInfo>());
+
+        view.addChildViews(plugins);
     }
 
     public BasicView getView() {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/PluginInfo.java	Mon Jan 16 10:24:54 2017 +0100
@@ -0,0 +1,70 @@
+/*
+ * 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.UIComponent;
+import com.redhat.thermostat.client.core.views.UIPluginInfo;
+import com.redhat.thermostat.shared.locale.LocalizedString;
+
+/**
+ */
+class PluginInfo implements UIPluginInfo {
+    private final UIComponent view;
+    private final LocalizedString name;
+    private final int order;
+
+    public PluginInfo(LocalizedString name, UIComponent view, int order) {
+        this.view = view;
+        this.name = name;
+        this.order = order;
+    }
+
+    @Override
+    public UIComponent getView() {
+        return view;
+    }
+
+    @Override
+    public LocalizedString getLocalizedName() {
+        return name;
+    }
+
+    @Override
+    public int getOrderValue() {
+        return order;
+    }
+}
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/VmInformationController.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/VmInformationController.java	Mon Jan 16 10:24:54 2017 +0100
@@ -45,12 +45,10 @@
 import com.redhat.thermostat.client.core.internal.platform.DynamicVMPluginProvider;
 import com.redhat.thermostat.client.core.internal.platform.UIPluginAction;
 import com.redhat.thermostat.client.core.views.BasicView;
-import com.redhat.thermostat.client.core.views.UIComponent;
 import com.redhat.thermostat.client.core.views.UIPluginInfo;
 import com.redhat.thermostat.client.core.views.VmInformationView;
 import com.redhat.thermostat.client.core.views.VmInformationViewProvider;
 import com.redhat.thermostat.common.OrderedComparator;
-import com.redhat.thermostat.shared.locale.LocalizedString;
 import com.redhat.thermostat.storage.core.VmRef;
 
 public class VmInformationController implements ContentProvider {
@@ -76,26 +74,6 @@
         }
     }
 
-    private class PluginInfo implements UIPluginInfo {
-        UIComponent view;
-        LocalizedString name;
-
-        public PluginInfo(LocalizedString name, UIComponent view) {
-            this.view = view;
-            this.name = name;
-        }
-
-        @Override
-        public UIComponent getView() {
-            return view;
-        }
-
-        @Override
-        public LocalizedString getLocalizedName() {
-            return name;
-        }
-    }
-
     public VmInformationController(List<InformationService<VmRef>> vmInfoServices,
                                    VmRef vmRef,
                                    VmInformationViewProvider provider,
@@ -114,11 +92,10 @@
 
         view.clear();
 
-        Collections.sort(vmInfoServices, new OrderedComparator<InformationService<VmRef>>());
         for (InformationService<VmRef> vmInfoService : vmInfoServices) {
             if (vmInfoService.getFilter().matches(vmRef)) {
                 InformationServiceController<VmRef> ctrl = vmInfoService.getInformationServiceController(vmRef);
-                plugins.add(new PluginInfo(ctrl.getLocalizedName(), ctrl.getView()));
+                plugins.add(new PluginInfo(ctrl.getLocalizedName(), ctrl.getView(), vmInfoService.getOrderValue()));
             }
         }
 
@@ -127,6 +104,8 @@
             dynamicProvider.forEach(vmRef, action);
         }
 
+        Collections.sort(plugins, new OrderedComparator<UIPluginInfo>());
+
         view.addChildViews(plugins);
 
         view.selectChildID(selectedID);
@@ -149,6 +128,5 @@
         rebuild();
         return view;
     }
-
 }
 
--- a/client/core/src/test/java/com/redhat/thermostat/client/ui/HostInformationControllerTest.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/core/src/test/java/com/redhat/thermostat/client/ui/HostInformationControllerTest.java	Mon Jan 16 10:24:54 2017 +0100
@@ -36,16 +36,21 @@
 
 package com.redhat.thermostat.client.ui;
 
+import static junit.framework.Assert.assertEquals;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import com.redhat.thermostat.client.core.internal.platform.DynamicHostPluginProvider;
+import com.redhat.thermostat.client.core.views.UIPluginInfo;
+import com.redhat.thermostat.shared.locale.LocalizedString;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 
 import com.redhat.thermostat.common.Filter;
@@ -87,25 +92,31 @@
                                               dynamicProviders);
         controller.rebuild();
 
-        InOrder order = inOrder(services.get(0), services.get(1),
-                services.get(2), services.get(3), services.get(4));
-        verifyService(services.get(2), order);
-        verifyService(services.get(1), order);
-        verifyService(services.get(0), order);
-        verifyService(services.get(4), order);
-        verifyService(services.get(3), order);
+        ArgumentCaptor captor = ArgumentCaptor.forClass(List.class);
+        verify(view).addChildViews((List<UIPluginInfo>) captor.capture());
+
+        List<UIPluginInfo> plugins = (List<UIPluginInfo>) captor.getValue();
+        assertEquals(5, plugins.size());
+        assertEquals("2", plugins.get(0).getLocalizedName().getContents());
+        assertEquals("1", plugins.get(1).getLocalizedName().getContents());
+        assertEquals("0", plugins.get(2).getLocalizedName().getContents());
+        assertEquals("4", plugins.get(3).getLocalizedName().getContents());
+        assertEquals("3", plugins.get(4).getLocalizedName().getContents());
     }
 
     private List<InformationService<HostRef>> mockServices(int[] orderValues) {
         List<InformationService<HostRef>> services = new ArrayList<>();
+        int id = 0;
         for (int order : orderValues) {
             InformationService<HostRef> service = mock(InformationService.class);
             InformationServiceController<HostRef> controller = mock(InformationServiceController.class);
+            when(controller.getLocalizedName()).thenReturn(new LocalizedString("" + id));
             when(service.getFilter()).thenReturn(FILTER);
             when(service.getInformationServiceController(ref)).thenReturn(
                     controller);
             when(service.getOrderValue()).thenReturn(order);
             services.add(service);
+            id++;
         }
         return services;
     }
--- a/client/core/src/test/java/com/redhat/thermostat/client/ui/VmInformationControllerTest.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/core/src/test/java/com/redhat/thermostat/client/ui/VmInformationControllerTest.java	Mon Jan 16 10:24:54 2017 +0100
@@ -36,17 +36,21 @@
 
 package com.redhat.thermostat.client.ui;
 
-import static org.mockito.Mockito.inOrder;
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import com.redhat.thermostat.client.core.internal.platform.DynamicVMPluginProvider;
+import com.redhat.thermostat.client.core.views.UIPluginInfo;
+import com.redhat.thermostat.shared.locale.LocalizedString;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.InOrder;
+import org.mockito.ArgumentCaptor;
 
 import com.redhat.thermostat.client.core.NameMatchingRefFilter;
 import com.redhat.thermostat.common.Filter;
@@ -88,32 +92,33 @@
 
         controller.rebuild();
 
-        InOrder order = inOrder(services.get(0), services.get(1),
-                services.get(2), services.get(3), services.get(4));
-        verifyService(services.get(2), order);
-        verifyService(services.get(1), order);
-        verifyService(services.get(0), order);
-        verifyService(services.get(4), order);
-        verifyService(services.get(3), order);
+        ArgumentCaptor captor = ArgumentCaptor.forClass(List.class);
+        verify(view).addChildViews((List<UIPluginInfo>) captor.capture());
+
+        List<UIPluginInfo> plugins = (List<UIPluginInfo>) captor.getValue();
+        assertEquals(5, plugins.size());
+        assertEquals("2", plugins.get(0).getLocalizedName().getContents());
+        assertEquals("1", plugins.get(1).getLocalizedName().getContents());
+        assertEquals("0", plugins.get(2).getLocalizedName().getContents());
+        assertEquals("4", plugins.get(3).getLocalizedName().getContents());
+        assertEquals("3", plugins.get(4).getLocalizedName().getContents());
     }
 
     private List<InformationService<VmRef>> mockServices(int[] orderValues) {
         List<InformationService<VmRef>> services = new ArrayList<>();
+        int id = 0;
         for (int order : orderValues) {
             InformationService<VmRef> service = mock(InformationService.class);
             InformationServiceController<VmRef> controller = mock(InformationServiceController.class);
+            when(controller.getLocalizedName()).thenReturn(new LocalizedString("" + id));
             when(service.getFilter()).thenReturn(FILTER);
             when(service.getInformationServiceController(ref)).thenReturn(
                     controller);
             when(service.getOrderValue()).thenReturn(order);
             services.add(service);
+            id++;
         }
         return services;
     }
-
-    private void verifyService(InformationService<VmRef> service, InOrder order) {
-        order.verify(service).getInformationServiceController(ref);
-    }
-
 }
 
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java	Mon Jan 16 10:24:54 2017 +0100
@@ -501,7 +501,7 @@
             }
         });
     }
-    
+
     @Override
     public void setContent(final ContentProvider provider) {
 
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java	Mon Jan 16 10:24:54 2017 +0100
@@ -568,24 +568,28 @@
                 id = lastSelectedVM.getSelectedChildID();
             }
 
+            VmInformationController currentSelection = null;
+
             if (cachedControllers.containsKey(vmRef)) {
-                lastSelectedVM = cachedControllers.get(vmRef);
+                currentSelection = cachedControllers.get(vmRef);
             } else {
-                lastSelectedVM = createVmController(vmRef);
-                cachedControllers.put(vmRef, lastSelectedVM);
+                currentSelection = createVmController(vmRef);
+                cachedControllers.put(vmRef, currentSelection);
             }
 
-            lastSelectedVM.rebuild();
+            currentSelection.rebuild();
 
-            if (!lastSelectedVM.selectChildID(id)) {
+            if (!currentSelection.selectChildID(id)) {
                 Integer _id = selectedForVM.get(vmRef);
                 id = _id != null ? _id : 0;
-                lastSelectedVM.selectChildID(id);
+                currentSelection.selectChildID(id);
             }
 
             selectedForVM.put(vmRef, id);
 
-            return lastSelectedVM;
+            lastSelectedVM = currentSelection;
+
+            return currentSelection;
         }
     }
     
--- a/platform/swing/compat/src/main/java/com/redhat/thermostat/compat/platform/internal/DynamicHostPluginProviderImpl.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/platform/swing/compat/src/main/java/com/redhat/thermostat/compat/platform/internal/DynamicHostPluginProviderImpl.java	Mon Jan 16 10:24:54 2017 +0100
@@ -77,7 +77,11 @@
         public LocalizedString getLocalizedName() {
             return plugin.getController().getName();
         }
-        
+
+        @Override
+        public int getOrderValue() {
+            return plugin.getOrderValue();
+        }
     }
     
     private DynamicHostPluginRegistry registry;
--- a/platform/swing/compat/src/main/java/com/redhat/thermostat/compat/platform/internal/DynamicVMPluginProviderImpl.java	Fri Jan 13 12:41:31 2017 -0500
+++ b/platform/swing/compat/src/main/java/com/redhat/thermostat/compat/platform/internal/DynamicVMPluginProviderImpl.java	Mon Jan 16 10:24:54 2017 +0100
@@ -77,6 +77,10 @@
             return plugin.getController().getName();
         }
 
+        @Override
+        public int getOrderValue() {
+            return plugin.getOrderValue();
+        }
     }
 
     private DynamicVMPluginRegistry registry;