From 8135eef45acdb01efebf37185fe29d74592f4ef9 Mon Sep 17 00:00:00 2001 From: Simon Kaufmann Date: Mon, 12 Jun 2017 16:04:36 +0200 Subject: [PATCH] fix orphaned items ...which were automatically created by the ChannelItemProvider but it lost track of them and therefore they remained as orphans in the registry. fixes #3607 Signed-off-by: Simon Kaufmann --- .../.classpath | 1 + .../META-INF/MANIFEST.MF | 6 + .../build.properties | 3 +- ...g.eclipse.smarthome.core.thing.test.launch | 8 +- .../internal/ChannelItemProviderTest.java | 136 ++++++++++++++++++ .../thing/internal/ChannelItemProvider.java | 11 +- 6 files changed, 158 insertions(+), 7 deletions(-) create mode 100644 bundles/core/org.eclipse.smarthome.core.thing.test/src/test/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProviderTest.java diff --git a/bundles/core/org.eclipse.smarthome.core.thing.test/.classpath b/bundles/core/org.eclipse.smarthome.core.thing.test/.classpath index 3a01c6281c7..6eb354ccdb2 100644 --- a/bundles/core/org.eclipse.smarthome.core.thing.test/.classpath +++ b/bundles/core/org.eclipse.smarthome.core.thing.test/.classpath @@ -4,6 +4,7 @@ + diff --git a/bundles/core/org.eclipse.smarthome.core.thing.test/META-INF/MANIFEST.MF b/bundles/core/org.eclipse.smarthome.core.thing.test/META-INF/MANIFEST.MF index 84b381efb99..3db1c52ca56 100644 --- a/bundles/core/org.eclipse.smarthome.core.thing.test/META-INF/MANIFEST.MF +++ b/bundles/core/org.eclipse.smarthome.core.thing.test/META-INF/MANIFEST.MF @@ -13,6 +13,7 @@ Import-Package: com.google.gson, org.codehaus.groovy.runtime.callsite, org.codehaus.groovy.runtime.typehandling, org.eclipse.smarthome.config.core, + org.eclipse.smarthome.core.items, org.eclipse.smarthome.core.library.items, org.eclipse.smarthome.core.library.types, org.eclipse.smarthome.core.storage, @@ -21,5 +22,10 @@ Import-Package: com.google.gson, org.hamcrest;core=split, org.junit;version="4.0.0", org.junit.rules, + org.mockito, + org.mockito.invocation, + org.mockito.stubbing, + org.mockito.verification, org.osgi.framework, org.osgi.service.cm +Require-Bundle: org.hamcrest diff --git a/bundles/core/org.eclipse.smarthome.core.thing.test/build.properties b/bundles/core/org.eclipse.smarthome.core.thing.test/build.properties index 3d32d47f045..cb410c1e2fa 100644 --- a/bundles/core/org.eclipse.smarthome.core.thing.test/build.properties +++ b/bundles/core/org.eclipse.smarthome.core.thing.test/build.properties @@ -1,5 +1,6 @@ source.. = src/test/groovy,\ - src/test/resources + src/test/resources,\ + src/test/java/ output.. = target/test-classes/ bin.includes = META-INF/,\ .,\ diff --git a/bundles/core/org.eclipse.smarthome.core.thing.test/org.eclipse.smarthome.core.thing.test.launch b/bundles/core/org.eclipse.smarthome.core.thing.test/org.eclipse.smarthome.core.thing.test.launch index 72d0c3b6c4d..d3cffcd6384 100644 --- a/bundles/core/org.eclipse.smarthome.core.thing.test/org.eclipse.smarthome.core.thing.test.launch +++ b/bundles/core/org.eclipse.smarthome.core.thing.test/org.eclipse.smarthome.core.thing.test.launch @@ -16,12 +16,12 @@ - + - + - + @@ -34,7 +34,7 @@ - + diff --git a/bundles/core/org.eclipse.smarthome.core.thing.test/src/test/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProviderTest.java b/bundles/core/org.eclipse.smarthome.core.thing.test/src/test/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProviderTest.java new file mode 100644 index 00000000000..f76a16e88f7 --- /dev/null +++ b/bundles/core/org.eclipse.smarthome.core.thing.test/src/test/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProviderTest.java @@ -0,0 +1,136 @@ +/** + * 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.core.thing.internal; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; +import static org.mockito.MockitoAnnotations.initMocks; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +import org.eclipse.smarthome.core.common.registry.ProviderChangeListener; +import org.eclipse.smarthome.core.i18n.LocaleProvider; +import org.eclipse.smarthome.core.items.Item; +import org.eclipse.smarthome.core.items.ItemFactory; +import org.eclipse.smarthome.core.items.ItemRegistry; +import org.eclipse.smarthome.core.library.items.NumberItem; +import org.eclipse.smarthome.core.thing.Channel; +import org.eclipse.smarthome.core.thing.ChannelUID; +import org.eclipse.smarthome.core.thing.ThingRegistry; +import org.eclipse.smarthome.core.thing.link.ItemChannelLink; +import org.eclipse.smarthome.core.thing.link.ItemChannelLinkRegistry; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; + +/** + * + * @author Simon Kaufmann - initial contribution and API + * + */ +public class ChannelItemProviderTest { + + private static final String ITEM_NAME = "test"; + private static final ChannelUID CHANNEL_UID = new ChannelUID("test:test:test:test"); + private static final NumberItem ITEM = new NumberItem(ITEM_NAME); + + @Mock + private ItemRegistry itemRegistry; + @Mock + private ThingRegistry thingRegistry; + @Mock + private ItemFactory itemFactory; + @Mock + private ProviderChangeListener listener; + @Mock + private LocaleProvider localeProvider; + @Mock + private ItemChannelLinkRegistry linkRegistry; + + private ChannelItemProvider provider; + + @Before + public void setup() throws Exception { + initMocks(this); + + provider = new ChannelItemProvider(); + provider.setItemRegistry(itemRegistry); + provider.setThingRegistry(thingRegistry); + provider.setItemChannelLinkRegistry(linkRegistry); + provider.addItemFactory(itemFactory); + provider.setLocaleProvider(localeProvider); + provider.addProviderChangeListener(listener); + + Map props = new HashMap<>(); + props.put("enable", "true"); + provider.activate(props); + Thread.sleep(2500); + + when(thingRegistry.getChannel(same(CHANNEL_UID))).thenReturn(new Channel(CHANNEL_UID, "Number")); + when(itemFactory.createItem("Number", ITEM_NAME)).thenReturn(ITEM); + when(localeProvider.getLocale()).thenReturn(Locale.ENGLISH); + } + + @Test + public void testItemCreation_notThere() throws Exception { + provider.linkRegistryListener.added(new ItemChannelLink(ITEM_NAME, CHANNEL_UID)); + verify(listener, only()).added(same(provider), same(ITEM)); + } + + @Test + public void testItemCreation_alreadyExists() throws Exception { + when(itemRegistry.get(eq(ITEM_NAME))).thenReturn(ITEM); + + provider.linkRegistryListener.added(new ItemChannelLink(ITEM_NAME, CHANNEL_UID)); + verify(listener, never()).added(same(provider), same(ITEM)); + } + + @Test + public void testItemRemoval_linkRemoved() throws Exception { + provider.linkRegistryListener.added(new ItemChannelLink(ITEM_NAME, CHANNEL_UID)); + + resetAndPrepareListener(); + + provider.linkRegistryListener.removed(new ItemChannelLink(ITEM_NAME, CHANNEL_UID)); + verify(listener, never()).added(same(provider), same(ITEM)); + verify(listener, only()).removed(same(provider), same(ITEM)); + } + + @Test + public void testItemRemoval_itemFromOtherProvider() throws Exception { + provider.linkRegistryListener.added(new ItemChannelLink(ITEM_NAME, CHANNEL_UID)); + + resetAndPrepareListener(); + + provider.itemRegistryListener.added(new NumberItem(ITEM_NAME)); + verify(listener, only()).removed(same(provider), same(ITEM)); + verify(listener, never()).added(same(provider), same(ITEM)); + } + + @SuppressWarnings("unchecked") + private void resetAndPrepareListener() { + reset(listener); + doAnswer(invocation -> { + // this is crucial as it mimicks the real ItemRegistry's behavior + provider.itemRegistryListener.removed((Item) invocation.getArguments()[1]); + return null; + }).when(listener).removed(same(provider), any(Item.class)); + doAnswer(invocation -> { + // this is crucial as it mimicks the real ItemRegistry's behavior + provider.itemRegistryListener.added((Item) invocation.getArguments()[1]); + return null; + }).when(listener).added(same(provider), any(Item.class)); + when(linkRegistry.getBoundChannels(eq(ITEM_NAME))).thenReturn(Collections.singleton(CHANNEL_UID)); + when(linkRegistry.getLinks(eq(CHANNEL_UID))) + .thenReturn(Collections.singleton(new ItemChannelLink(ITEM_NAME, CHANNEL_UID))); + } + +} diff --git a/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProvider.java b/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProvider.java index dc9478ae146..095e1b05bd6 100644 --- a/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProvider.java +++ b/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ChannelItemProvider.java @@ -263,7 +263,6 @@ private void removeItem(String key) { } Item item = items.get(key); if (item != null) { - items.remove(key); for (ProviderChangeListener listener : listeners) { listener.removed(this, item); } @@ -329,17 +328,25 @@ public void added(Item element) { } } // it is from some other provider, so remove ours, if we have one - Item oldElement = items.remove(element.getName()); + Item oldElement = items.get(element.getName()); if (oldElement != null) { for (ProviderChangeListener listener : listeners) { listener.removed(ChannelItemProvider.this, oldElement); } + items.remove(element.getName()); } lastUpdate = System.nanoTime(); } @Override public void removed(Item element) { + // check, if it is our own item + for (Item item : items.values()) { + if (item == element) { + return; + } + } + // it is from some other provider, so create one ourselves if needed for (ChannelUID uid : linkRegistry.getBoundChannels(element.getName())) { for (ItemChannelLink link : linkRegistry.getLinks(uid)) { if (itemRegistry.get(link.getItemName()) == null) {