Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Commit

Permalink
improve handling of links related to model-items (#3641)
Browse files Browse the repository at this point in the history
* change the order of how the GenericItemProvider registers links and items
* enable the GenericItemChannelLinkProvider to handle deletions correctly
* call GenericItemChannelLinkProvider from within GenericItemProvider only
* enforce correct transaction handling in GenericItemChannelLinkProvider

fixes #2491
relates to #3607
Signed-off-by: Simon Kaufmann <[email protected]>
  • Loading branch information
Simon Kaufmann authored and kaikreuzer committed Jun 13, 2017
1 parent 607d424 commit 9d96c90
Show file tree
Hide file tree
Showing 6 changed files with 218 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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());
}
}
}

Expand Down Expand Up @@ -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());
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -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))));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
Loading

0 comments on commit 9d96c90

Please sign in to comment.