improve handling of links related to model-items

bundles/model/org.eclipse.smarthome.model.item/src/org/eclipse/smarthome/model/item/internal/GenericItemProvider.java

@@ -160,7 +160,7 @@ private Collection<Item> getItemsFromModel(String modelName) {
         return items;
     }
 
-    private void processBindingConfigsFromModel(String modelName) {
+    private void processBindingConfigsFromModel(String modelName, EventType type) {
         logger.debug("Processing binding configs for items from model '{}'", modelName);
 
         if (modelRepository != null) {
@@ -175,10 +175,12 @@ private void processBindingConfigsFromModel(String modelName) {
             }
 
             // create items and read new binding configuration
-            for (ModelItem modelItem : model.getItems()) {
-                Item item = createItemFromModelItem(modelItem);
-                if (item != null) {
-                    internalDispatchBindings(modelName, item, modelItem.getBindings());
+            if (!EventType.REMOVED.equals(type)) {
+                for (ModelItem modelItem : model.getItems()) {
+                    Item item = createItemFromModelItem(modelItem);
+                    if (item != null) {
+                        internalDispatchBindings(modelName, item, modelItem.getBindings());
+                    }
                 }
             }
 
@@ -341,15 +343,14 @@ public void modelChanged(String modelName, EventType type) {
         if (modelName.endsWith("items")) {
             switch (type) {
                 case ADDED:
-                    processBindingConfigsFromModel(modelName);
                     Collection<Item> allNewItems = getItemsFromModel(modelName);
                     itemsMap.put(modelName, allNewItems);
                     for (Item item : allNewItems) {
                         notifyListenersAboutAddedElement(item);
                     }
+                    processBindingConfigsFromModel(modelName, type);
                     break;
                 case MODIFIED:
-                    processBindingConfigsFromModel(modelName);
                     Map<String, Item> oldItems = toItemMap(itemsMap.get(modelName));
                     Map<String, Item> newItems = toItemMap(getAll());
                     itemsMap.put(modelName, newItems.values());
@@ -363,13 +364,15 @@ public void modelChanged(String modelName, EventType type) {
                             notifyListenersAboutAddedElement(newItem);
                         }
                     }
+                    processBindingConfigsFromModel(modelName, type);
                     for (Item oldItem : oldItems.values()) {
                         if (!newItems.containsKey(oldItem.getName())) {
                             notifyListenersAboutRemovedElement(oldItem);
                         }
                     }
                     break;
                 case REMOVED:
+                    processBindingConfigsFromModel(modelName, type);
                     Collection<Item> itemsFromModel = getItemsFromModel(modelName);
                     itemsMap.remove(modelName);
                     for (Item item : itemsFromModel) {

bundles/model/org.eclipse.smarthome.model.thing.tests/META-INF/MANIFEST.MF

@@ -4,7 +4,8 @@ Bundle-Name: Eclipse SmartHome Thing Model Tests
 Bundle-Vendor: Eclipse.org/SmartHome
 Bundle-Version: 0.9.0.qualifier
 Bundle-SymbolicName: org.eclipse.smarthome.model.thing.tests; singleton:=true
-Require-Bundle: org.eclipse.core.runtime
+Require-Bundle: org.eclipse.core.runtime,
+ org.hamcrest
 Import-Package: groovy.lang,
  org.apache.log4j,
  org.codehaus.groovy.reflection,
@@ -16,6 +17,7 @@ Import-Package: groovy.lang,
  org.eclipse.smarthome.core.thing.util,
  org.eclipse.smarthome.core.types,
  org.eclipse.smarthome.test,
+ org.eclipse.smarthome.test.java,
  org.hamcrest;core=split,
  org.hamcrest.core,
  org.junit;version="4.0.0",
@@ -24,6 +26,8 @@ Import-Package: groovy.lang,
  org.junit.runner.notification;version="4.0.0",
  org.junit.runners;version="4.0.0",
  org.junit.runners.model;version="4.0.0",
+ org.mockito,
+ org.mockito.verification,
  org.osgi.service.component;version="1.2.2"
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
 Fragment-Host: org.eclipse.smarthome.model.thing

bundles/model/org.eclipse.smarthome.model.thing.tests/org.eclipse.smarthome.model.thing.tests.launch.launch

@@ -34,7 +34,7 @@
 <stringAttribute key="pde.version" value="3.3"/>
 <stringAttribute key="product" value="org.eclipse.sdk.ide"/>
 <booleanAttribute key="run_in_ui_thread" value="false"/>
-<stringAttribute key="selected_target_plugins" value="ch.qos.logback.classic@default:default,ch.qos.logback.core@default:default,ch.qos.logback.slf4j@default:false,com.google.gson*2.2.4.v201311231704@default:default,com.google.gson*2.5.0@default:default,com.google.guava@default:default,com.google.inject@default:default,com.ibm.icu.base@default:default,javax.inject@default:default,javax.transaction@default:false,javax.xml@default:default,org.antlr.runtime@default:default,org.apache.ant@default:default,org.apache.commons.collections@default:default,org.apache.commons.io@default:default,org.apache.commons.lang@default:default,org.codehaus.groovy@default:default,org.eclipse.ant.core@default:default,org.eclipse.compare.core@default:default,org.eclipse.core.contenttype@default:default,org.eclipse.core.expressions@default:default,org.eclipse.core.filesystem.macosx@default:false,org.eclipse.core.filesystem@default:default,org.eclipse.core.jobs@default:default,org.eclipse.core.resources@default:default,org.eclipse.core.runtime.compatibility.registry@default:false,org.eclipse.core.runtime@default:true,org.eclipse.core.variables@default:default,org.eclipse.emf.common@default:default,org.eclipse.emf.ecore.xmi@default:default,org.eclipse.emf.ecore@default:default,org.eclipse.equinox.app@default:default,org.eclipse.equinox.common@2:true,org.eclipse.equinox.ds@1:true,org.eclipse.equinox.event@default:default,org.eclipse.equinox.preferences@default:default,org.eclipse.equinox.registry@default:default,org.eclipse.equinox.transforms.hook@default:false,org.eclipse.equinox.util@default:default,org.eclipse.equinox.weaving.hook@default:false,org.eclipse.osgi.services@default:default,org.eclipse.osgi@-1:true,org.eclipse.team.core@default:default,org.eclipse.xtend.lib.macro@default:default,org.eclipse.xtend.lib@default:default,org.eclipse.xtext.builder.standalone@default:default,org.eclipse.xtext.common.types@default:default,org.eclipse.xtext.junit4@default:default,org.eclipse.xtext.smap@default:default,org.eclipse.xtext.util@default:default,org.eclipse.xtext.xbase.lib@default:default,org.eclipse.xtext.xbase@default:default,org.eclipse.xtext@default:default,org.hamcrest.core@default:default,org.junit@default:default,org.objectweb.asm@default:default,org.slf4j.api@default:default,org.slf4j.log4j@default:default"/>
+<stringAttribute key="selected_target_plugins" value="ch.qos.logback.classic@default:default,ch.qos.logback.core@default:default,ch.qos.logback.slf4j@default:false,com.google.gson*2.2.4.v201311231704@default:default,com.google.gson*2.5.0@default:default,com.google.guava@default:default,com.google.inject@default:default,com.ibm.icu.base@default:default,javax.inject@default:default,javax.transaction@default:false,javax.xml@default:default,org.antlr.runtime@default:default,org.apache.ant@default:default,org.apache.commons.collections@default:default,org.apache.commons.io@default:default,org.apache.commons.lang@default:default,org.codehaus.groovy@default:default,org.eclipse.ant.core@default:default,org.eclipse.compare.core@default:default,org.eclipse.core.contenttype@default:default,org.eclipse.core.expressions@default:default,org.eclipse.core.filesystem.macosx@default:false,org.eclipse.core.filesystem@default:default,org.eclipse.core.jobs@default:default,org.eclipse.core.resources@default:default,org.eclipse.core.runtime.compatibility.registry@default:false,org.eclipse.core.runtime@default:true,org.eclipse.core.variables@default:default,org.eclipse.emf.common@default:default,org.eclipse.emf.ecore.xmi@default:default,org.eclipse.emf.ecore@default:default,org.eclipse.equinox.app@default:default,org.eclipse.equinox.common@2:true,org.eclipse.equinox.ds@1:true,org.eclipse.equinox.event@default:default,org.eclipse.equinox.preferences@default:default,org.eclipse.equinox.registry@default:default,org.eclipse.equinox.transforms.hook@default:false,org.eclipse.equinox.util@default:default,org.eclipse.equinox.weaving.hook@default:false,org.eclipse.osgi.services@default:default,org.eclipse.osgi@-1:true,org.eclipse.team.core@default:default,org.eclipse.xtend.lib.macro@default:default,org.eclipse.xtend.lib@default:default,org.eclipse.xtext.builder.standalone@default:default,org.eclipse.xtext.common.types@default:default,org.eclipse.xtext.junit4@default:default,org.eclipse.xtext.smap@default:default,org.eclipse.xtext.util@default:default,org.eclipse.xtext.xbase.lib@default:default,org.eclipse.xtext.xbase@default:default,org.eclipse.xtext@default:default,org.hamcrest.core@default:default,org.hamcrest.integration@default:default,org.hamcrest.library@default:default,org.hamcrest.text@default:default,org.hamcrest@default:default,org.junit@default:default,org.mockito@default:default,org.objectweb.asm@default:default,org.objenesis@default:default,org.slf4j.api@default:default,org.slf4j.log4j@default:default"/>
 <stringAttribute key="selected_workspace_plugins" value="org.eclipse.smarthome.config.core.test@default:false,org.eclipse.smarthome.config.core@default:default,org.eclipse.smarthome.config.xml@default:default,org.eclipse.smarthome.core.autoupdate@default:default,org.eclipse.smarthome.core.thing@3:default,org.eclipse.smarthome.core@default:default,org.eclipse.smarthome.io.console@default:default,org.eclipse.smarthome.model.core@default:default,org.eclipse.smarthome.model.item.runtime@default:true,org.eclipse.smarthome.model.item@default:true,org.eclipse.smarthome.model.thing.runtime@default:true,org.eclipse.smarthome.model.thing.tests@default:false,org.eclipse.smarthome.model.thing@default:true,org.eclipse.smarthome.test@default:default"/>
 <booleanAttribute key="show_selected_only" value="false"/>
 <booleanAttribute key="tracing" value="false"/>

bundles/model/org.eclipse.smarthome.model.thing.tests/src/org/eclipse/smarthome/model/thing/tests/GenericItemChannelLinkProviderJavaTest.java

@@ -0,0 +1,176 @@
+/**
+ * Copyright (c) 2014-2017 by the respective copyright holders.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.smarthome.model.thing.tests;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Matchers.*;
+import static org.mockito.Mockito.*;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+import java.io.ByteArrayInputStream;
+import java.util.Collection;
+
+import org.eclipse.smarthome.core.common.registry.ProviderChangeListener;
+import org.eclipse.smarthome.core.items.ItemRegistry;
+import org.eclipse.smarthome.core.thing.ChannelUID;
+import org.eclipse.smarthome.core.thing.Thing;
+import org.eclipse.smarthome.core.thing.ThingRegistry;
+import org.eclipse.smarthome.core.thing.link.ItemChannelLink;
+import org.eclipse.smarthome.core.thing.link.ItemChannelLinkProvider;
+import org.eclipse.smarthome.core.thing.link.ItemChannelLinkRegistry;
+import org.eclipse.smarthome.model.core.ModelRepository;
+import org.eclipse.smarthome.model.thing.internal.GenericItemChannelLinkProvider;
+import org.eclipse.smarthome.test.java.JavaOSGiTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+
+/**
+ *
+ * @author Simon Kaufmann - initial contribution and API
+ *
+ */
+public class GenericItemChannelLinkProviderJavaTest extends JavaOSGiTest {
+
+    private final static String THINGS_TESTMODEL_NAME = "test.things";
+    private final static String ITEMS_TESTMODEL_NAME = "test.items";
+
+    private static final String ITEM = "test";
+    private static final String CHANNEL = "test:test:test:test";
+    private static final String LINK = ITEM + " -> " + CHANNEL;
+
+    private ModelRepository modelRepository;
+    private ThingRegistry thingRegistry;
+    private ItemRegistry itemRegistry;
+    private ItemChannelLinkRegistry itemChannelLinkRegistry;
+    private ItemChannelLinkProvider itemChannelLinkProvider;
+
+    @Mock
+    private ProviderChangeListener<ItemChannelLink> listener;
+
+    @Before
+    public void setUp() {
+        initMocks(this);
+
+        thingRegistry = getService(ThingRegistry.class);
+        assertThat(thingRegistry, is(notNullValue()));
+        modelRepository = getService(ModelRepository.class);
+        assertThat(modelRepository, is(notNullValue()));
+        itemRegistry = getService(ItemRegistry.class);
+        assertThat(itemRegistry, is(notNullValue()));
+        itemChannelLinkRegistry = getService(ItemChannelLinkRegistry.class);
+        assertThat(itemChannelLinkRegistry, is(notNullValue()));
+        itemChannelLinkProvider = getService(ItemChannelLinkProvider.class);
+        assertThat(itemChannelLinkProvider, is(notNullValue()));
+        modelRepository.removeModel(THINGS_TESTMODEL_NAME);
+        modelRepository.removeModel(ITEMS_TESTMODEL_NAME);
+    }
+
+    @After
+    public void tearDown() {
+        modelRepository.removeModel(THINGS_TESTMODEL_NAME);
+        modelRepository.removeModel(ITEMS_TESTMODEL_NAME);
+
+        assertThat(itemRegistry.getItems().size(), is(0));
+        assertThat(itemChannelLinkRegistry.getAll().size(), is(0));
+    }
+
+    @Test
+    public void testIntegrationWithGenericItemProvider() throws Exception {
+        Thread.sleep(2500); // Wait for the ChannelItemProvider to join the game
+
+        Collection<Thing> things = thingRegistry.getAll();
+        assertThat(things.size(), is(0));
+
+        String thingsModel =
+
+                "Bridge hue:bridge:huebridge [ ipAddress = \"192.168.3.84\", userName = \"19fc3fa6fc870a4280a55f21315631f\" ] {"
+                        + "LCT001 bulb3 [ lightId = \"3\" ]" + "LCT001 bulb4 [ lightId = \"3\" ]" + "}";
+        modelRepository.addOrRefreshModel(THINGS_TESTMODEL_NAME, new ByteArrayInputStream(thingsModel.getBytes()));
+        Collection<Thing> actualThings = thingRegistry.getAll();
+
+        assertThat(actualThings.size(), is(3));
+
+        assertThat(itemRegistry.getItems().size(), is(0));
+        assertThat(itemChannelLinkRegistry.getAll().size(), is(0));
+
+        String itemsModel = "Color Light3Color \"Light3 Color\" { channel=\"hue:LCT001:huebridge:bulb3:color\" }"
+                + "Group:Switch:MAX TestSwitches";
+
+        // Initially load the model
+        modelRepository.addOrRefreshModel(ITEMS_TESTMODEL_NAME, new ByteArrayInputStream(itemsModel.getBytes()));
+        assertThat(itemRegistry.getItems().size(), is(2));
+        assertThat(itemChannelLinkRegistry.getAll().size(), is(1));
+
+        // Update the model to run into GenericItemChannelLinkProvider's amnesia
+        modelRepository.addOrRefreshModel(ITEMS_TESTMODEL_NAME, new ByteArrayInputStream(itemsModel.getBytes()));
+        assertThat(itemRegistry.getItems().size(), is(2));
+        assertThat(itemChannelLinkRegistry.getAll().size(), is(1));
+
+        // Remove the model (-> the link and therefore the item is kept)
+        modelRepository.removeModel(ITEMS_TESTMODEL_NAME);
+        assertThat(itemRegistry.getItems().size(), is(0));
+        assertThat(itemChannelLinkRegistry.getAll().size(), is(0));
+
+        // Now add the model again
+        modelRepository.addOrRefreshModel(ITEMS_TESTMODEL_NAME, new ByteArrayInputStream(itemsModel.getBytes()));
+        assertThat(itemRegistry.getItems().size(), is(2)); // -> ensure ChannelItemProvider cleans up properly
+        assertThat(itemChannelLinkRegistry.getAll().size(), is(1));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testTransaction_requiredStart() {
+        GenericItemChannelLinkProvider provider = new GenericItemChannelLinkProvider();
+        provider.stopConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testTransaction_requiredProcess() throws Exception {
+        GenericItemChannelLinkProvider provider = new GenericItemChannelLinkProvider();
+        provider.processBindingConfiguration(ITEMS_TESTMODEL_NAME, "Number", ITEM, CHANNEL);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testTransaction_duplicate() {
+        GenericItemChannelLinkProvider provider = new GenericItemChannelLinkProvider();
+        provider.startConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        provider.startConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+    }
+
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testNoAmnesia() throws Exception {
+        GenericItemChannelLinkProvider provider = new GenericItemChannelLinkProvider();
+        provider.addProviderChangeListener(listener);
+
+        provider.startConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        provider.processBindingConfiguration(ITEMS_TESTMODEL_NAME, "Number", ITEM, CHANNEL);
+        provider.stopConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        assertThat(provider.getAll().size(), is(1));
+        assertThat(provider.getAll().iterator().next().toString(), is(LINK));
+        verify(listener, only()).added(same(provider), eq(new ItemChannelLink(ITEM, new ChannelUID(CHANNEL))));
+
+        reset(listener);
+        provider.startConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        provider.processBindingConfiguration(ITEMS_TESTMODEL_NAME, "Number", ITEM, CHANNEL);
+        provider.stopConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        assertThat(provider.getAll().size(), is(1));
+        assertThat(provider.getAll().iterator().next().toString(), is(LINK));
+        verify(listener, only()).updated(same(provider), eq(new ItemChannelLink(ITEM, new ChannelUID(CHANNEL))),
+                eq(new ItemChannelLink(ITEM, new ChannelUID(CHANNEL))));
+
+        reset(listener);
+        provider.startConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        provider.stopConfigurationUpdate(ITEMS_TESTMODEL_NAME);
+        assertThat(provider.getAll().size(), is(0));
+        verify(listener, only()).removed(same(provider), eq(new ItemChannelLink(ITEM, new ChannelUID(CHANNEL))));
+    }
+
+}

bundles/model/org.eclipse.smarthome.model.thing/OSGI-INF/genericitemchannellinkprovider.xml

@@ -10,7 +10,6 @@
 -->
 <scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" name="org.eclipse.smarthome.model.thing.internal.genericitemchannellinkprovider">
    <implementation class="org.eclipse.smarthome.model.thing.internal.GenericItemChannelLinkProvider"/>
-   <reference bind="setModelRepository" cardinality="1..1" interface="org.eclipse.smarthome.model.core.ModelRepository" name="ModelRepository" policy="dynamic" unbind="unsetModelRepository"/>
    <service>
       <provide interface="org.eclipse.smarthome.model.item.BindingConfigReader"/>
       <provide interface="org.eclipse.smarthome.core.thing.link.ItemChannelLinkProvider"/>