Use constructor injection to simplify lifecycle (#987)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

bundles/org.openhab.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ChannelCommandDescriptionProvider.java

@@ -28,6 +28,7 @@
 import org.eclipse.smarthome.core.thing.type.ThingTypeRegistry;
 import org.eclipse.smarthome.core.types.CommandDescription;
 import org.eclipse.smarthome.core.types.CommandDescriptionProvider;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
@@ -39,19 +40,25 @@
  * Provides the {@link ChannelType} specific {@link CommandDescription} for the given item name and locale.
  *
  * @author Henning Treu - Initial contribution
- *
  */
-@NonNullByDefault
 @Component
+@NonNullByDefault
 public class ChannelCommandDescriptionProvider implements CommandDescriptionProvider {
 
     private final Logger logger = LoggerFactory.getLogger(ChannelCommandDescriptionProvider.class);
 
-    private @NonNullByDefault({}) ItemChannelLinkRegistry itemChannelLinkRegistry;
-    private @NonNullByDefault({}) ThingTypeRegistry thingTypeRegistry;
-    private @NonNullByDefault({}) ThingRegistry thingRegistry;
-
     private final List<DynamicCommandDescriptionProvider> dynamicCommandDescriptionProviders = new CopyOnWriteArrayList<>();
+    private final ItemChannelLinkRegistry itemChannelLinkRegistry;
+    private final ThingTypeRegistry thingTypeRegistry;
+    private final ThingRegistry thingRegistry;
+
+    @Activate
+    public ChannelCommandDescriptionProvider(final @Reference ItemChannelLinkRegistry itemChannelLinkRegistry,
+            final @Reference ThingTypeRegistry thingTypeRegistry, final @Reference ThingRegistry thingRegistry) {
+        this.itemChannelLinkRegistry = itemChannelLinkRegistry;
+        this.thingTypeRegistry = thingTypeRegistry;
+        this.thingRegistry = thingRegistry;
+    }
 
     @Override
     public @Nullable CommandDescription getCommandDescription(String itemName, @Nullable Locale locale) {
@@ -73,7 +80,6 @@ public class ChannelCommandDescriptionProvider implements CommandDescriptionProv
                 return commandDescription;
             }
         }
-
         return null;
     }
 
@@ -91,37 +97,9 @@ public class ChannelCommandDescriptionProvider implements CommandDescriptionProv
                         dynamicCommandDescriptionProvider.getClass(), e.getLocalizedMessage(), e);
             }
         }
-
         return null;
     }
 
-    @Reference
-    protected void setThingTypeRegistry(ThingTypeRegistry thingTypeRegistry) {
-        this.thingTypeRegistry = thingTypeRegistry;
-    }
-
-    protected void unsetThingTypeRegistry(ThingTypeRegistry thingTypeRegistry) {
-        this.thingTypeRegistry = null;
-    }
-
-    @Reference
-    protected void setItemChannelLinkRegistry(ItemChannelLinkRegistry itemChannelLinkRegistry) {
-        this.itemChannelLinkRegistry = itemChannelLinkRegistry;
-    }
-
-    protected void unsetItemChannelLinkRegistry(ItemChannelLinkRegistry itemChannelLinkRegistry) {
-        this.itemChannelLinkRegistry = null;
-    }
-
-    @Reference
-    protected void setThingRegistry(ThingRegistry thingRegistry) {
-        this.thingRegistry = thingRegistry;
-    }
-
-    protected void unsetThingRegistry(ThingRegistry thingRegistry) {
-        this.thingRegistry = null;
-    }
-
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addDynamicCommandDescriptionProvider(
             DynamicCommandDescriptionProvider dynamicCommandDescriptionProvider) {

bundles/org.openhab.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ChannelStateDescriptionProvider.java

@@ -55,11 +55,19 @@ public class ChannelStateDescriptionProvider implements StateDescriptionFragment
     private final Logger logger = LoggerFactory.getLogger(ChannelStateDescriptionProvider.class);
 
     private final List<DynamicStateDescriptionProvider> dynamicStateDescriptionProviders = new CopyOnWriteArrayList<>();
-    private @NonNullByDefault({}) ItemChannelLinkRegistry itemChannelLinkRegistry;
-    private @NonNullByDefault({}) ThingTypeRegistry thingTypeRegistry;
-    private @NonNullByDefault({}) ThingRegistry thingRegistry;
+    private final ItemChannelLinkRegistry itemChannelLinkRegistry;
+    private final ThingTypeRegistry thingTypeRegistry;
+    private final ThingRegistry thingRegistry;
     private Integer rank = 0;
 
+    @Activate
+    public ChannelStateDescriptionProvider(final @Reference ItemChannelLinkRegistry itemChannelLinkRegistry,
+            final @Reference ThingTypeRegistry thingTypeRegistry, final @Reference ThingRegistry thingRegistry) {
+        this.itemChannelLinkRegistry = itemChannelLinkRegistry;
+        this.thingTypeRegistry = thingTypeRegistry;
+        this.thingRegistry = thingRegistry;
+    }
+
     @Activate
     protected void activate(Map<String, Object> properties) {
         Object serviceRanking = properties.get(Constants.SERVICE_RANKING);
@@ -131,33 +139,6 @@ public Integer getRank() {
         return null;
     }
 
-    @Reference
-    protected void setThingTypeRegistry(ThingTypeRegistry thingTypeRegistry) {
-        this.thingTypeRegistry = thingTypeRegistry;
-    }
-
-    protected void unsetThingTypeRegistry(ThingTypeRegistry thingTypeRegistry) {
-        this.thingTypeRegistry = null;
-    }
-
-    @Reference
-    protected void setItemChannelLinkRegistry(ItemChannelLinkRegistry itemChannelLinkRegistry) {
-        this.itemChannelLinkRegistry = itemChannelLinkRegistry;
-    }
-
-    protected void unsetItemChannelLinkRegistry(ItemChannelLinkRegistry itemChannelLinkRegistry) {
-        this.itemChannelLinkRegistry = null;
-    }
-
-    @Reference
-    protected void setThingRegistry(ThingRegistry thingRegistry) {
-        this.thingRegistry = thingRegistry;
-    }
-
-    protected void unsetThingRegistry(ThingRegistry thingRegistry) {
-        this.thingRegistry = null;
-    }
-
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addDynamicStateDescriptionProvider(DynamicStateDescriptionProvider dynamicStateDescriptionProvider) {
         this.dynamicStateDescriptionProviders.add(dynamicStateDescriptionProvider);

bundles/org.openhab.core.thing/src/main/java/org/eclipse/smarthome/core/thing/link/ItemChannelLinkRegistry.java

@@ -25,6 +25,7 @@
 import org.eclipse.smarthome.core.thing.ThingUID;
 import org.eclipse.smarthome.core.thing.UID;
 import org.eclipse.smarthome.core.thing.link.events.LinkEventFactory;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
@@ -37,16 +38,19 @@
  * @author Dennis Nobel - Initial contribution
  * @author Markus Rathgeb - Linked items returns only existing items
  * @author Markus Rathgeb - Rewrite collection handling to improve performance
- *
  */
 @Component(immediate = true, service = ItemChannelLinkRegistry.class)
 public class ItemChannelLinkRegistry extends AbstractLinkRegistry<ItemChannelLink, ItemChannelLinkProvider> {
 
-    private ThingRegistry thingRegistry;
-    private ItemRegistry itemRegistry;
+    private final ThingRegistry thingRegistry;
+    private final ItemRegistry itemRegistry;
 
-    public ItemChannelLinkRegistry() {
+    @Activate
+    public ItemChannelLinkRegistry(final @Reference ThingRegistry thingRegistry,
+            final @Reference ItemRegistry itemRegistry) {
         super(ItemChannelLinkProvider.class);
+        this.thingRegistry = thingRegistry;
+        this.itemRegistry = itemRegistry;
     }
 
     /**
@@ -82,24 +86,6 @@ public Set<Thing> getBoundThings(final String itemName) {
                 .collect(Collectors.toSet());
     }
 
-    @Reference
-    protected void setThingRegistry(final ThingRegistry thingRegistry) {
-        this.thingRegistry = thingRegistry;
-    }
-
-    protected void unsetThingRegistry(final ThingRegistry thingRegistry) {
-        this.thingRegistry = null;
-    }
-
-    @Reference
-    protected void setItemRegistry(final ItemRegistry itemRegistry) {
-        this.itemRegistry = itemRegistry;
-    }
-
-    protected void unsetItemRegistry(final ItemRegistry itemRegistry) {
-        this.itemRegistry = null;
-    }
-
     @Reference
     protected void setManagedProvider(final ManagedItemChannelLinkProvider provider) {
         super.setManagedProvider(provider);

bundles/org.openhab.core.thing/src/main/java/org/eclipse/smarthome/core/thing/type/ThingTypeRegistry.java

@@ -18,10 +18,13 @@
 import java.util.Locale;
 import java.util.concurrent.CopyOnWriteArrayList;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.smarthome.core.thing.Channel;
 import org.eclipse.smarthome.core.thing.ChannelUID;
 import org.eclipse.smarthome.core.thing.ThingTypeUID;
 import org.eclipse.smarthome.core.thing.binding.ThingTypeProvider;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
@@ -34,18 +37,24 @@
  * @author Dennis Nobel - Added locale support
  */
 @Component(immediate = true, service = ThingTypeRegistry.class)
+@NonNullByDefault
 public class ThingTypeRegistry {
 
     private final List<ThingTypeProvider> thingTypeProviders = new CopyOnWriteArrayList<>();
-    private ChannelTypeRegistry channelTypeRegistry;
+    private final ChannelTypeRegistry channelTypeRegistry;
+
+    @Activate
+    public ThingTypeRegistry(final @Reference ChannelTypeRegistry channelTypeRegistry) {
+        this.channelTypeRegistry = channelTypeRegistry;
+    }
 
     /**
      * Returns all thing types.
      *
      * @param locale locale (can be null)
      * @return all thing types
      */
-    public List<ThingType> getThingTypes(Locale locale) {
+    public List<ThingType> getThingTypes(@Nullable Locale locale) {
         List<ThingType> thingTypes = new ArrayList<>();
         for (ThingTypeProvider thingTypeProvider : thingTypeProviders) {
             thingTypes.addAll(thingTypeProvider.getThingTypes(locale));
@@ -69,15 +78,13 @@ public List<ThingType> getThingTypes() {
      * @param locale locale (can be null)
      * @return thing types for given binding id
      */
-    public List<ThingType> getThingTypes(String bindingId, Locale locale) {
+    public List<ThingType> getThingTypes(String bindingId, @Nullable Locale locale) {
         List<ThingType> thingTypesForBinding = new ArrayList<>();
-
         for (ThingType thingType : getThingTypes()) {
             if (thingType.getBindingId().equals(bindingId)) {
                 thingTypesForBinding.add(thingType);
             }
         }
-
         return Collections.unmodifiableList(thingTypesForBinding);
     }
 
@@ -99,14 +106,13 @@ public List<ThingType> getThingTypes(String bindingId) {
      * @return thing type for given UID or null if no thing type with this UID
      *         was found
      */
-    public ThingType getThingType(ThingTypeUID thingTypeUID, Locale locale) {
+    public @Nullable ThingType getThingType(ThingTypeUID thingTypeUID, @Nullable Locale locale) {
         for (ThingTypeProvider thingTypeProvider : thingTypeProviders) {
             ThingType thingType = thingTypeProvider.getThingType(thingTypeUID, locale);
             if (thingType != null) {
                 return thingType;
             }
         }
-
         return null;
     }
 
@@ -117,7 +123,7 @@ public ThingType getThingType(ThingTypeUID thingTypeUID, Locale locale) {
      * @return thing type for given UID or null if no thing type with this UID
      *         was found
      */
-    public ThingType getThingType(ThingTypeUID thingTypeUID) {
+    public @Nullable ThingType getThingType(ThingTypeUID thingTypeUID) {
         return getThingType(thingTypeUID, null);
     }
 
@@ -133,7 +139,7 @@ public ThingType getThingType(ThingTypeUID thingTypeUID) {
      * @param channel channel
      * @return channel type or null if no channel type was found
      */
-    public ChannelType getChannelType(Channel channel) {
+    public @Nullable ChannelType getChannelType(Channel channel) {
         return getChannelType(channel, null);
     }
 
@@ -150,7 +156,7 @@ public ChannelType getChannelType(Channel channel) {
      * @param locale locale (can be null)
      * @return channel type or null if no channel type was found
      */
-    public ChannelType getChannelType(Channel channel, Locale locale) {
+    public @Nullable ChannelType getChannelType(Channel channel, @Nullable Locale locale) {
         ChannelTypeUID channelTypeUID = channel.getChannelTypeUID();
         if (channelTypeUID != null) {
             return channelTypeRegistry.getChannelType(channelTypeUID, locale);
@@ -160,24 +166,11 @@ public ChannelType getChannelType(Channel channel, Locale locale) {
 
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addThingTypeProvider(ThingTypeProvider thingTypeProvider) {
-        if (thingTypeProvider != null) {
-            this.thingTypeProviders.add(thingTypeProvider);
-        }
+        this.thingTypeProviders.add(thingTypeProvider);
     }
 
     protected void removeThingTypeProvider(ThingTypeProvider thingTypeProvider) {
-        if (thingTypeProvider != null) {
-            this.thingTypeProviders.remove(thingTypeProvider);
-        }
-    }
-
-    @Reference
-    protected void setChannelTypeRegistry(ChannelTypeRegistry channelTypeRegistry) {
-        this.channelTypeRegistry = channelTypeRegistry;
-    }
-
-    protected void unsetChannelTypeRegistry(ChannelTypeRegistry channelTypeRegistry) {
-        this.channelTypeRegistry = null;
+        this.thingTypeProviders.remove(thingTypeProvider);
     }
 
 }

itests/org.openhab.core.thing.tests/src/main/java/org/eclipse/smarthome/core/thing/internal/CommunicationManagerOSGiTest.java

@@ -85,6 +85,10 @@
 public class CommunicationManagerOSGiTest extends JavaOSGiTest {
 
     private class ItemChannelLinkRegistryAdvanced extends ItemChannelLinkRegistry {
+        public ItemChannelLinkRegistryAdvanced(ThingRegistry thingRegistry, ItemRegistry itemRegistry) {
+            super(thingRegistry, itemRegistry);
+        }
+
         @Override
         protected void addProvider(Provider<ItemChannelLink> provider) {
             super.addProvider(provider);
@@ -201,7 +205,7 @@ public void setup() {
         manager.addProfileFactory(mockProfileFactory);
         manager.addProfileAdvisor(mockProfileAdvisor);
 
-        ItemChannelLinkRegistryAdvanced iclRegistry = new ItemChannelLinkRegistryAdvanced();
+        ItemChannelLinkRegistryAdvanced iclRegistry = new ItemChannelLinkRegistryAdvanced(thingRegistry, itemRegistry);
         iclRegistry.addProvider(new ItemChannelLinkProvider() {
             @Override
             public void addProviderChangeListener(ProviderChangeListener<ItemChannelLink> listener) {