refresh internal widget collection on sitemap change (#5523)

* refresh internal widget collection on sitemap change * removed public modifiers from SitemapProvider interface * untangle model update notifiaction order The SitemapSubscriptionService now gets informed by the SitemapProvider about a model change. Before, it was a matter of luck if the SitemapSubscriptionService or the SitemapProvider got informed first about a model change by the ModelRepository. If the SitemapSubscriptionService was so unlucky to be the first one, it got a stale model from the SitemapProvider and hence it again the calculation of the widget IDs was wrong. This partially reverts #3846. fixes #5522 Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
eclipse-archived · May 3, 2018 · 379069a · 379069a
1 parent 644ecb2
commit 379069a
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 16 deletions.
diff --git a/...temap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/SitemapSubscriptionService.java b/...temap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/SitemapSubscriptionService.java
@@ -25,7 +25,6 @@
 import org.eclipse.smarthome.io.rest.sitemap.internal.PageChangeListener;
 import org.eclipse.smarthome.io.rest.sitemap.internal.SitemapEvent;
 import org.eclipse.smarthome.model.core.EventType;
-import org.eclipse.smarthome.model.core.ModelRepository;
 import org.eclipse.smarthome.model.core.ModelRepositoryChangeListener;
 import org.eclipse.smarthome.model.sitemap.LinkableWidget;
 import org.eclipse.smarthome.model.sitemap.Sitemap;
@@ -105,19 +104,13 @@ protected void unsetItemUIRegistry(ItemUIRegistry itemUIRegistry) {
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addSitemapProvider(SitemapProvider provider) {
         sitemapProviders.add(provider);
+        provider.addModelChangeListener(this);
+
     }
 
     protected void removeSitemapProvider(SitemapProvider provider) {
         sitemapProviders.remove(provider);
-    }
-
-    @Reference
-    protected void setModelRepository(ModelRepository modelRepo) {
-        modelRepo.addModelRepositoryChangeListener(this);
-    }
-
-    protected void unsetModelRepository(ModelRepository modelRepo) {
-        modelRepo.removeModelRepositoryChangeListener(this);
+        provider.removeModelChangeListener(this);
     }
 
     /**
@@ -281,9 +274,11 @@ public void modelChanged(String modelName, EventType type) {
         for (Entry<String, PageChangeListener> listenerEntry : pageChangeListeners.entrySet()) {
             String sitemapWithPage = listenerEntry.getKey();
             String sitemapName = extractSitemapName(sitemapWithPage);
+            String pageId = extractPageId(sitemapWithPage);
 
             if (sitemapName.equals(changedSitemapName)) {
-                listenerEntry.getValue().sitemapContentChanged();
+                EList<Widget> widgets = collectWidgets(sitemapName, pageId);
+                listenerEntry.getValue().sitemapContentChanged(widgets);
             }
         }
     }

diff --git a/...emap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/internal/PageChangeListener.java b/...emap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/internal/PageChangeListener.java
@@ -257,7 +257,9 @@ private boolean definesVisibilityOrColor(Widget w, String name) {
         return false;
     }
 
-    public void sitemapContentChanged() {
+    public void sitemapContentChanged(EList<Widget> widgets) {
+        updateItemsAndWidgets(widgets);
+
         SitemapChangedEvent changeEvent = new SitemapChangedEvent();
         changeEvent.pageId = pageId;
         changeEvent.sitemapName = sitemapName;

diff --git a/...ipse.smarthome.model.sitemap/src/org/eclipse/smarthome/model/sitemap/SitemapProvider.java b/...ipse.smarthome.model.sitemap/src/org/eclipse/smarthome/model/sitemap/SitemapProvider.java
@@ -14,21 +14,37 @@
 
 import java.util.Set;
 
+import org.eclipse.smarthome.model.core.ModelRepositoryChangeListener;
 
 public interface SitemapProvider {
 
     /**
      * This method provides access to sitemap model files, loads them and returns the object model tree.
-     * 
+     *
      * @param sitemapName the name of the sitemap to load
      * @return the object model tree, null if it is not found
      */
-    public Sitemap getSitemap(String sitemapName);
+    Sitemap getSitemap(String sitemapName);
 
     /**
      * Returns the names of all available sitemaps
-     * 
+     *
      * @return names of provided sitemaps
      */
-    public Set<String> getSitemapNames();
+    Set<String> getSitemapNames();
+
+    /**
+     * Add a listener which will be informed subsequently once a model has changed
+     *
+     * @param listener
+     */
+    void addModelChangeListener(ModelRepositoryChangeListener listener);
+
+    /**
+     * Remove a model change listener again
+     *
+     * @param listener
+     */
+    void removeModelChangeListener(ModelRepositoryChangeListener listener);
+
 }
diff --git a/...e.model.sitemap/src/org/eclipse/smarthome/model/sitemap/internal/SitemapProviderImpl.java b/...e.model.sitemap/src/org/eclipse/smarthome/model/sitemap/internal/SitemapProviderImpl.java
@@ -15,6 +15,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.stream.Collectors;
 
 import org.apache.commons.lang.StringUtils;
@@ -48,6 +49,8 @@ public class SitemapProviderImpl implements SitemapProvider, ModelRepositoryChan
 
     private final Map<String, Sitemap> sitemapModelCache = new ConcurrentHashMap<>();
 
+    private final Set<ModelRepositoryChangeListener> modelChangeListeners = new CopyOnWriteArraySet<>();
+
     @Reference
     public void setModelRepository(ModelRepository modelRepo) {
         this.modelRepo = modelRepo;
@@ -103,6 +106,9 @@ public void modelChanged(String modelName, EventType type) {
                 sitemapModelCache.put(modelName, (Sitemap) modelRepo.getModel(modelName));
             }
         }
+        for (ModelRepositoryChangeListener listener : modelChangeListeners) {
+            listener.modelChanged(modelName, type);
+        }
     }
 
     private void refreshSitemapModels() {
@@ -113,4 +119,14 @@ private void refreshSitemapModels() {
         }
     }
 
+    @Override
+    public void addModelChangeListener(ModelRepositoryChangeListener listener) {
+        modelChangeListeners.add(listener);
+    }
+
+    @Override
+    public void removeModelChangeListener(ModelRepositoryChangeListener listener) {
+        modelChangeListeners.remove(listener);
+    }
+
 }