Alexander Wels has uploaded a new change for review.

Change subject: userportal,webadmin: synchronize grid refresh
......................................................................

userportal,webadmin: synchronize grid refresh

- Updated code to have refresh rates across tabs and sub
  tabs be identical. So if you switch the refresh rate on
  lets say the VM tab, the same refresh rate is used for
  all other tabs. As well as affecting the refresh rate of
  the events/alerts and tasks list.
- Fixed duplicate timers being created for EventListModels
  and its sub classes.
- Fixed each GridTimer having its own EventBus, which then
  didn't actually communicate with the rest of the system.
  Now all the GridTimers have the correct EventBus.

Change-Id: I8742785cb9b3d890c39859586b03cd53c64b31e5
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=879662
Signed-off-by: Alexander Wels <[email protected]>
---
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
M 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
8 files changed, 147 insertions(+), 72 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/13/24213/1

diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
index ed37e50..b466424 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/AbstractRefreshManager.java
@@ -19,6 +19,7 @@
 import com.google.gwt.event.logical.shared.ValueChangeHandler;
 import com.google.gwt.event.shared.EventBus;
 import com.google.gwt.event.shared.GwtEvent;
+import com.google.gwt.event.shared.HandlerRegistration;
 import com.google.gwt.event.shared.HasHandlers;
 
 /**
@@ -35,8 +36,8 @@
 
     }
 
-    // Prefix for keys used to store refresh rates of individual data grids
-    private static final String GRID_REFRESH_RATE_PREFIX = "GridRefreshRate"; 
//$NON-NLS-1$
+    // Key used to store refresh rate of all data grids
+    private static final String GRID_REFRESH_RATE_KEY = "GridRefreshRate"; 
//$NON-NLS-1$
 
     private static final Integer DEFAULT_REFRESH_RATE = 
GridTimer.DEFAULT_NORMAL_RATE;
     private static final Integer OUT_OF_FOCUS_REFRESH_RATE = 
Integer.valueOf(60000);
@@ -62,6 +63,7 @@
     private final T refreshPanel;
     private final EventBus eventBus;
     private ManualRefreshCallback manualRefreshCallback;
+    private HandlerRegistration statusUpdateHandlerRegistration;
 
     private GridController controller;
 
@@ -95,15 +97,22 @@
     private void updateController() {
         this.controller = modelProvider.getModel();
 
-        GridTimer modelTimer = getModelTimer();
-        modelTimer.setRefreshRate(readRefreshRate());
+        updateTimer();
+    }
 
-        modelTimer.addValueChangeHandler(new ValueChangeHandler<String>() {
+    private void updateTimer() {
+        final GridTimer modelTimer = getModelTimer();
+
+        if (statusUpdateHandlerRegistration != null) {
+            statusUpdateHandlerRegistration.removeHandler();
+        }
+        statusUpdateHandlerRegistration = modelTimer.addValueChangeHandler(new 
ValueChangeHandler<Integer>() {
             @Override
-            public void onValueChange(ValueChangeEvent<String> event) {
-                onRefresh(event.getValue());
+            public void onValueChange(ValueChangeEvent<Integer> event) {
+                onRefresh(modelTimer.getTimerRefreshStatus());
             }
         });
+        modelTimer.resume();
     }
 
     /**
@@ -156,8 +165,9 @@
     }
 
     public void setCurrentRefreshRate(int newRefreshRate) {
-        getModelTimer().setRefreshRate(newRefreshRate);
         saveRefreshRate(newRefreshRate);
+        getModelTimer().setRefreshRate(readRefreshRate());
+        updateTimer();
     }
 
     public int getCurrentRefreshRate() {
@@ -165,7 +175,7 @@
     }
 
     String getRefreshRateItemKey() {
-        return GRID_REFRESH_RATE_PREFIX + "_" + controller.getId(); 
//$NON-NLS-1$
+        return GRID_REFRESH_RATE_KEY;
     }
 
     void saveRefreshRate(int newRefreshRate) {
diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
index 1966cd5..cc699c3 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java
@@ -177,7 +177,7 @@
 
     private ToggleButton refreshMenuButton;
 
-    private Image separator;
+    private final Image separator;
 
     private final RefreshRateOptionsMenu refreshOptionsMenu;
 
@@ -290,11 +290,7 @@
     }
 
     public void showStatus(String status) {
-        // for debugging
-        // statusLabel.setText(status);
-        if (refreshManager.getModelTimer().isActive()) {
-            setTitle(status);
-        }
+        setTitle(status);
     }
 
     private void createRefreshButton() {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
index d955a32..048b83b 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridController.java
@@ -7,11 +7,13 @@
 
     /**
      * Returns the refresh timer used by the Grid.
+     * @return The Grid timer.
      */
     GridTimer getTimer();
 
     /**
      * Returns the controller ID.
+     * @return The controller ID.
      */
     String getId();
 
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
index 8e66fa5..81cf77d 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/GridTimer.java
@@ -9,9 +9,9 @@
 import com.google.gwt.event.logical.shared.HasValueChangeHandlers;
 import com.google.gwt.event.logical.shared.ValueChangeEvent;
 import com.google.gwt.event.logical.shared.ValueChangeHandler;
+import com.google.gwt.event.shared.EventBus;
 import com.google.gwt.event.shared.GwtEvent;
 import com.google.gwt.event.shared.HandlerRegistration;
-import com.google.gwt.event.shared.SimpleEventBus;
 import com.google.gwt.user.client.Timer;
 
 /**
@@ -27,7 +27,7 @@
  *     refresh rate). This mode is triggered by the fastForward() method. each 
call reset the cycle
  *     to the start point.
  */
-public abstract class GridTimer extends Timer implements 
HasValueChangeHandlers<String> {
+public abstract class GridTimer extends Timer implements 
HasValueChangeHandlers<Integer> {
 
     private enum RATE {
         FAST {
@@ -111,7 +111,7 @@
 
     private int currentRate = 0;
 
-    private final SimpleEventBus eventBus;
+    private final EventBus eventBus;
 
     private final String name;
 
@@ -125,13 +125,14 @@
 
     private int repetitions;
 
-    public GridTimer(String name) {
+    public GridTimer(String name, final EventBus eventBus) {
         this.name = name;
-        eventBus = new SimpleEventBus();
+        assert(eventBus != null);
+        this.eventBus = eventBus;
     }
 
     @Override
-    public HandlerRegistration 
addValueChangeHandler(ValueChangeHandler<String> handler) {
+    public HandlerRegistration 
addValueChangeHandler(ValueChangeHandler<Integer> handler) {
         return eventBus.addHandler(ValueChangeEvent.getType(), handler);
     }
 
@@ -202,14 +203,13 @@
         logger.fine("GridTimer[" + name + "]: Refresh Rate set to: " + 
interval); //$NON-NLS-1$ //$NON-NLS-2$
         // set the NORMAL interval
         normalInterval = interval;
-        start();
+        ValueChangeEvent.fire(this, getRefreshRate());
     }
 
     public void start() {
         logger.fine("GridTimer[" + name + "].start()"); //$NON-NLS-1$ 
//$NON-NLS-2$
         active = true;
         scheduleRepeating(getRefreshRate());
-        ValueChangeEvent.fire(this, getValue());
     }
 
     public void stop() {
@@ -242,7 +242,7 @@
         return active;
     }
 
-    private String getValue() {
+    public String getTimerRefreshStatus() {
         logger.fine((isActive() ? "Refresh Status: Active(" : "Inactive(") + 
(isPaused() ? "paused)" : "running)") + ":" //$NON-NLS-1$ //$NON-NLS-2$ 
//$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
                 + " Rate: " + rateCycle[currentRate] + "(" + getRefreshRate() 
/ 1000 + " sec)"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$); }
         return 
ConstantsManager.getInstance().getMessages().refreshInterval(getRefreshRate() / 
1000);
@@ -251,7 +251,7 @@
     private void doStop() {
         reset();
         cancel();
-        ValueChangeEvent.fire(this, getValue());
+        ValueChangeEvent.fire(this, getRefreshRate());
     }
 
     private void cycleRate() {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
index 922839f..94ca787 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
@@ -42,6 +42,12 @@
 import org.ovirt.engine.ui.uicompat.NotifyCollectionChangedEventArgs;
 import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs;
 
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
+import com.google.gwt.event.logical.shared.ValueChangeEvent;
+import com.google.gwt.event.logical.shared.ValueChangeHandler;
+import com.google.gwt.event.shared.HandlerRegistration;
+
 /**
  * Represents a list model with ability to fetch items both sync and async.
  */
@@ -54,6 +60,7 @@
     private static final String PAGE_NUMBER_REGEX = "[1-9]+[0-9]*$"; 
//$NON-NLS-1$
 
     private UICommand privateSearchCommand;
+    private HandlerRegistration timerChangeHandler;
 
     public UICommand getSearchCommand()
     {
@@ -296,35 +303,84 @@
         return true;
     }
 
-    private GridTimer privatetimer;
+    /**
+     * Grid refresh timer associated with this list model.
+     */
+    private GridTimer timer;
 
-    public GridTimer gettimer()
-    {
-        return privatetimer;
-    }
-
-    public void settimer(GridTimer value)
-    {
-        privatetimer = value;
+    /**
+     * Setter for the grid timer.
+     * @param value The new {@code GridTimer}.
+     */
+    private void setTimer(final GridTimer value) {
+        timer = value;
     }
 
     @Override
-    public GridTimer getTimer()
-    {
-        if (gettimer() == null)
-        {
-            settimer(new GridTimer(getListName()) {
+    public GridTimer getTimer() {
+        if (timer == null && getEventBus() != null) {
+            // The timer doesn't exist yet, and we have an event bus, create 
the timer and pass in the bus.
+            setTimer(new GridTimer(getListName(), getEventBus()) {
 
                 @Override
                 public void execute() {
-                    logger.fine(SearchableListModel.this.getClass().getName() 
+ ": Executing search"); //$NON-NLS-1$
-                    syncSearch();
+                    // Execute the code, sub classes can override this method 
to get their own code run.
+                    doGridTimerExecute();
                 }
 
             });
-            
gettimer().setRefreshRate(getConfigurator().getPollingTimerInterval());
+            //Always add a change handler, so we can properly synchronize the 
interval on all GridTimers.
+            replaceTimerChangeHandler();
         }
-        return gettimer();
+        return timer;
+    }
+
+    /**
+     * Sub classes can override this method if they need to do something 
different when the timer
+     * expires.
+     */
+    protected void doGridTimerExecute() {
+        logger.fine(SearchableListModel.this.getClass().getName() + ": 
Executing search"); //$NON-NLS-1$
+        syncSearch();
+    }
+
+    /**
+     * Add a {@code ValueChangeHandler} to the timer associated with this 
{@code SearchableListModel}.
+     * The handler is used to update the refresh rate based on changes of 
other timers. So if another timer changes
+     * from lets say 5 seconds to 30 seconds interval. It will fire a {@code 
ValueChangeEvent} which this timer
+     * receives.
+     *
+     * If this timer is currently active (active tab/always active). It will 
stop this timer, change the interval,
+     * and start the timer again. If it is inactive, it will just update the 
interval so that the interval is correct
+     * for when the timer does become active (changing main tabs).
+     */
+    private void addTimerChangeHandler() {
+        timerChangeHandler = timer.addValueChangeHandler(new 
ValueChangeHandler<Integer>() {
+            @Override
+            public void onValueChange(ValueChangeEvent<Integer> event) {
+                int newInterval = event.getValue();
+                if (timer.isActive()) {
+                    //Immediately adjust timer and restart if it was active.
+                    if (newInterval != timer.getRefreshRate()) {
+                        timer.stop();
+                        timer.setRefreshRate(newInterval);
+                        timer.start();
+                    }
+                } else {
+                    //Update the timer interval for inactive timers, so they 
are correct when they become active
+                    timer.setRefreshRate(newInterval);
+                }
+            }
+        });
+    }
+
+    protected void replaceTimerChangeHandler() {
+        if (timerChangeHandler == null) {
+            addTimerChangeHandler();
+        } else {
+            removeTimerChangeHandler();
+            addTimerChangeHandler();
+        }
     }
 
     @Override
@@ -390,7 +446,20 @@
                 onPropertyChanged(new 
PropertyChangedEventArgs(PropertyChangedEventArgs.Args.PROGRESS.toString()));
                 syncSearch();
                 setIsQueryFirstTime(false);
-                getTimer().start();
+                if (getTimer() != null) {
+                    //Timer can be null if the event bus hasn't been set yet 
(model hasn't been fully initialized)
+                    startRefresh();
+                } else {
+                    //Defer the start of the timer until after the event bus 
has been added to this model. Then we
+                    //can pass the event bus to the timer and the timer can 
become active.
+                    Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+
+                        @Override
+                        public void execute() {
+                            startRefresh();
+                        }
+                    });
+                }
             }
             else
             {
@@ -399,15 +468,21 @@
         }
     }
 
+    private void startRefresh() {
+        if (getTimer() != null) {
+            getTimer().start();
+        }
+    }
+
     public void forceRefresh()
     {
-        getTimer().stop();
+        stopRefresh();
         setIsQueryFirstTime(true);
         syncSearch();
 
         if (!getIsTimerDisabled())
         {
-            getTimer().start();
+            startRefresh();
         }
     }
 
@@ -771,7 +846,17 @@
 
     public void stopRefresh()
     {
-        getTimer().stop();
+        if (getTimer() != null) {
+            //Timer can be null if the event bus hasn't been set yet. If the 
timer is null we can't stop it.
+            getTimer().stop();
+        }
+    }
+
+    protected void removeTimerChangeHandler() {
+        if (timerChangeHandler != null) {
+            timerChangeHandler.removeHandler();
+            timerChangeHandler = null;
+        }
     }
 
     @Override
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
index 96bee5c..2ed1e80 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
@@ -19,7 +19,6 @@
 import org.ovirt.engine.ui.frontend.communication.RefreshActiveModelEvent;
 import org.ovirt.engine.ui.uicommonweb.Linq;
 import org.ovirt.engine.ui.uicommonweb.UICommand;
-import org.ovirt.engine.ui.uicommonweb.models.GridTimer;
 import org.ovirt.engine.ui.uicommonweb.models.ListWithDetailsModel;
 import org.ovirt.engine.ui.uicompat.ConstantsManager;
 import org.ovirt.engine.ui.uicompat.EventArgs;
@@ -28,8 +27,6 @@
 
 public class EventListModel extends ListWithDetailsModel
 {
-    private final GridTimer timer;
-
     private UICommand privateRefreshCommand;
 
     public UICommand getRefreshCommand()
@@ -106,16 +103,11 @@
         getSearchPreviousPageCommand().setIsAvailable(true);
 
         setIsTimerDisabled(true);
+    }
 
-        timer = new GridTimer(getListName()) {
-
-            @Override
-            public void execute() {
-                getRefreshCommand().execute();
-            }
-        };
-
-        timer.setRefreshRate(getConfigurator().getPollingTimerInterval());
+    @Override
+    protected void doGridTimerExecute() {
+        getRefreshCommand().execute();
     }
 
     @Override
@@ -132,8 +124,6 @@
         requestingData = true;
         setItems(new ObservableCollection<AuditLog>());
         setLastEvent(null);
-
-        timer.start();
     }
 
     @Override
@@ -219,14 +209,6 @@
     @Override
     public UICommand getDefaultCommand() {
         return getDetailsCommand();
-    }
-
-    @Override
-    public void stopRefresh()
-    {
-        super.stopRefresh();
-
-        timer.stop();
     }
 
     private void updateItems(ArrayList<AuditLog> source)
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
index e857036..ea70661 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmMonitorModel.java
@@ -80,7 +80,7 @@
     {
         if (refreshTimer == null)
         {
-            refreshTimer = new GridTimer("VmMonitorModel") { //$NON-NLS-1$
+            refreshTimer = new GridTimer("VmMonitorModel", getEventBus()) { 
//$NON-NLS-1$
                         @Override
                         public void execute() {
                             refresh();
diff --git 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
index 15b2bd9..1ac529b 100644
--- 
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
+++ 
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/host/SubTabHostInterfacePresenter.java
@@ -70,9 +70,9 @@
                 }
             }
         }));
-        getModelProvider().getModel().getTimer().addValueChangeHandler(new 
ValueChangeHandler<String>() {
+        getModelProvider().getModel().getTimer().addValueChangeHandler(new 
ValueChangeHandler<Integer>() {
             @Override
-            public void onValueChange(ValueChangeEvent<String> event) {
+            public void onValueChange(ValueChangeEvent<Integer> event) {
                 
getView().setRefreshButtonVisibility(!getModelProvider().getModel().getTimer().isActive());
             }
         });


-- 
To view, visit http://gerrit.ovirt.org/24213
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8742785cb9b3d890c39859586b03cd53c64b31e5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Alexander Wels <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to