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

Commit

Permalink
Extract ItemBuilderFactory from ItemRegistry (#6556)
Browse files Browse the repository at this point in the history
The ItemRegistry did offer `newItemBuilder` methods which depended on a list of ItemFactories. In the ItemRegisryImpl nothing else did depend on the ItemFactories. With ESH in the Felix OSGi runtime there are circumstances where a circular dependency is detected for an ItemProvider implementation which depends on the ItemRegistry for ItemBuilder access.
This extracts the new `ItemBuilderFactory` service from the ItemRegistry and provides the same API. The now deprecated API from ItemRegistry provides a fallback implementation based on a single instance of CoreItemFactory.

This of cause API breaking.

Signed-off-by: Henning Treu <henning.treu@googlemail.com>
  • Loading branch information
htreu authored and kaikreuzer committed Nov 27, 2018
1 parent c6c40b5 commit 4733152
Show file tree
Hide file tree
Showing 9 changed files with 186 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,23 @@
*/
public class ItemBuilderTest {

private ItemRegistryImpl itemRegistry;
private ItemBuilderFactoryImpl itemBuilderFactory;
private @Mock ItemFactory mockFactory;
private @Mock ActiveItem mockItem;
private @Mock Item originalItem;

@Before
public void setup() {
initMocks(this);
itemRegistry = new ItemRegistryImpl();
itemRegistry.addItemFactory(mockFactory);
itemBuilderFactory = new ItemBuilderFactoryImpl();
itemBuilderFactory.addItemFactory(mockFactory);
}

@Test
public void testMinimal() {
when(mockFactory.createItem(anyString(), anyString())).thenReturn(mockItem);

Item res = itemRegistry.newItemBuilder("String", "test").build();
Item res = itemBuilderFactory.newItemBuilder("String", "test").build();

assertSame(mockItem, res);
verify(mockFactory).createItem(eq("String"), eq("test"));
Expand All @@ -63,7 +63,7 @@ public void testMinimal() {

@Test
public void testMinimalGroupItem() {
Item resItem = itemRegistry.newItemBuilder("Group", "test").build();
Item resItem = itemBuilderFactory.newItemBuilder("Group", "test").build();

assertEquals(GroupItem.class, resItem.getClass());
GroupItem res = (GroupItem) resItem;
Expand All @@ -79,7 +79,7 @@ public void testMinimalGroupItem() {
public void testFull() {
when(mockFactory.createItem(anyString(), anyString())).thenReturn(mockItem);

Item res = itemRegistry.newItemBuilder("String", "test") //
Item res = itemBuilderFactory.newItemBuilder("String", "test") //
.withCategory("category") //
.withGroups(Arrays.asList("a", "b")) //
.withLabel("label") //
Expand All @@ -97,7 +97,7 @@ public void testFullGroupItem() {
Item baseItem = mock(Item.class);
GroupFunction mockFunction = mock(GroupFunction.class);

Item resItem = itemRegistry.newItemBuilder("Group", "test") //
Item resItem = itemBuilderFactory.newItemBuilder("Group", "test") //
.withCategory("category") //
.withGroups(Arrays.asList("a", "b")) //
.withLabel("label") //
Expand Down Expand Up @@ -125,7 +125,7 @@ public void testClone() {

when(mockFactory.createItem(anyString(), anyString())).thenReturn(mockItem);

Item res = itemRegistry.newItemBuilder(originalItem).build();
Item res = itemBuilderFactory.newItemBuilder(originalItem).build();

assertSame(mockItem, res);
verify(mockFactory).createItem(eq("type"), eq("name"));
Expand All @@ -143,7 +143,7 @@ public void testCloneGroupItem() {
originalItem.setLabel("label");
originalItem.addGroupNames("a", "b");

Item resItem = itemRegistry.newItemBuilder(originalItem).build();
Item resItem = itemBuilderFactory.newItemBuilder(originalItem).build();

assertEquals(GroupItem.class, resItem.getClass());
GroupItem res = (GroupItem) resItem;
Expand All @@ -158,19 +158,19 @@ public void testCloneGroupItem() {
@Test(expected = IllegalStateException.class)
public void testNoFactory() {
when(mockFactory.createItem(anyString(), anyString())).thenReturn(null);
itemRegistry.newItemBuilder("String", "test").build();
itemBuilderFactory.newItemBuilder("String", "test").build();
}

@Test(expected = IllegalArgumentException.class)
public void testFunctionOnNonGroupItem() {
GroupFunction mockFunction = mock(GroupFunction.class);
itemRegistry.newItemBuilder("String", "test").withGroupFunction(mockFunction);
itemBuilderFactory.newItemBuilder("String", "test").withGroupFunction(mockFunction);
}

@Test(expected = IllegalArgumentException.class)
public void testBaseItemOnNonGroupItem() {
Item mockItem = mock(Item.class);
itemRegistry.newItemBuilder("String", "test").withBaseItem(mockItem);
itemBuilderFactory.newItemBuilder("String", "test").withBaseItem(mockItem);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ public void setUp() {

setManagedProvider(itemProvider);
setEventPublisher(ItemRegistryImplTest.this.eventPublisher);
setCoreItemFactory(coreItemFactory);
setStateDescriptionService(mock(StateDescriptionService.class));
setUnitProvider(mock(UnitProvider.class));
setItemStateConverter(mock(ItemStateConverter.class));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* Copyright (c) 2014,2018 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipse.smarthome.core.internal.items;

import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.smarthome.core.items.Item;
import org.eclipse.smarthome.core.items.ItemBuilder;
import org.eclipse.smarthome.core.items.ItemBuilderFactory;
import org.eclipse.smarthome.core.items.ItemFactory;
import org.eclipse.smarthome.core.library.CoreItemFactory;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;

/**
* Provides an {@link ItemBuilder} with all available {@link ItemFactory}s set. The {@link CoreItemFactory} will always
* be present.
*
* @author Henning Treu - initial contribution and API
*
*/
@NonNullByDefault
@Component
public class ItemBuilderFactoryImpl implements ItemBuilderFactory {

private final @NonNullByDefault({}) Set<ItemFactory> itemFactories = new CopyOnWriteArraySet<>();

@Override
public ItemBuilder newItemBuilder(Item item) {
return new ItemBuilderImpl(itemFactories, item);
}

@Override
public ItemBuilder newItemBuilder(String itemType, String itemName) {
return new ItemBuilderImpl(itemFactories, itemType, itemName);
}

@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
protected void addItemFactory(ItemFactory itemFactory) {
itemFactories.add(itemFactory);
}

protected void removeItemFactory(ItemFactory itemFactory) {
itemFactories.remove(itemFactory);
}

@Reference(target = "(component.name=org.eclipse.smarthome.core.library.CoreItemFactory)")
protected void setCoreItemFactory(ItemFactory coreItemFactory) {
itemFactories.add(coreItemFactory);
}

protected void unsetCoreItemFactory(ItemFactory coreItemFactory) {
itemFactories.remove(coreItemFactory);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;

import org.eclipse.smarthome.core.common.registry.AbstractRegistry;
import org.eclipse.smarthome.core.common.registry.Provider;
Expand All @@ -28,8 +26,6 @@
import org.eclipse.smarthome.core.items.GenericItem;
import org.eclipse.smarthome.core.items.GroupItem;
import org.eclipse.smarthome.core.items.Item;
import org.eclipse.smarthome.core.items.ItemBuilder;
import org.eclipse.smarthome.core.items.ItemFactory;
import org.eclipse.smarthome.core.items.ItemNotFoundException;
import org.eclipse.smarthome.core.items.ItemNotUniqueException;
import org.eclipse.smarthome.core.items.ItemProvider;
Expand Down Expand Up @@ -68,7 +64,6 @@ public class ItemRegistryImpl extends AbstractRegistry<Item, String, ItemProvide
private final List<RegistryHook<Item>> registryHooks = new CopyOnWriteArrayList<>();
private StateDescriptionService stateDescriptionService;
private MetadataRegistry metadataRegistry;
private final Set<ItemFactory> itemFactories = new CopyOnWriteArraySet<>();

private UnitProvider unitProvider;
private ItemStateConverter itemStateConverter;
Expand Down Expand Up @@ -429,16 +424,6 @@ public void removeRegistryHook(RegistryHook<Item> hook) {
registryHooks.remove(hook);
}

@Override
public ItemBuilder newItemBuilder(Item item) {
return new ItemBuilderImpl(itemFactories, item);
}

@Override
public ItemBuilder newItemBuilder(String itemType, String itemName) {
return new ItemBuilderImpl(itemFactories, itemType, itemName);
}

@Activate
protected void activate(final ComponentContext componentContext) {
super.activate(componentContext.getBundleContext());
Expand Down Expand Up @@ -484,22 +469,4 @@ protected void unsetMetadataRegistry(MetadataRegistry metadataRegistry) {
this.metadataRegistry = null;
}

@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
protected void addItemFactory(ItemFactory itemFactory) {
itemFactories.add(itemFactory);
}

protected void removeItemFactory(ItemFactory itemFactory) {
itemFactories.remove(itemFactory);
}

@Reference(target = "(component.name=org.eclipse.smarthome.core.library.CoreItemFactory)")
protected void setCoreItemFactory(ItemFactory coreItemFactory) {
itemFactories.add(coreItemFactory);
}

protected void unsetCoreItemFactory(ItemFactory coreItemFactory) {
itemFactories.remove(coreItemFactory);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Copyright (c) 2014,2018 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipse.smarthome.core.items;

import org.eclipse.jdt.annotation.NonNullByDefault;

/**
* Creates a new {@link ItemBuilder} which is based on all available {@link ItemFactory}s.
*
* @author Henning Treu - initial contribution and API
*
*/
@NonNullByDefault
public interface ItemBuilderFactory {

/**
* Create a new {@link ItemBuilder}, which is initialized by the given item.
*
* @param item the template to initialize the builder with
*
* @return an ItemBuilder instance
*/
ItemBuilder newItemBuilder(Item item);

/**
* Create a new {@link ItemBuilder}, which is initialized by the given item.
*
* @param itemType the item type to create
* @param itemName the name of the item to create
*
* @return an ItemBuilder instance
*/
ItemBuilder newItemBuilder(String itemType, String itemName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@
package org.eclipse.smarthome.core.items;

import java.util.Collection;
import java.util.Collections;

import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.common.registry.Registry;
import org.eclipse.smarthome.core.internal.items.ItemBuilderImpl;
import org.eclipse.smarthome.core.library.CoreItemFactory;
import org.slf4j.LoggerFactory;

/**
* The ItemRegistry is the central place, where items are kept in memory and their state
Expand Down Expand Up @@ -131,16 +135,30 @@ public interface ItemRegistry extends Registry<Item, String> {
*
* @param item the template to initialize the builder with
* @return an ItemBuilder instance
*
* @deprecated Use the {@link ItemBuilderFactory} service instead.
*/
ItemBuilder newItemBuilder(Item item);
@Deprecated
default ItemBuilder newItemBuilder(Item item) {
LoggerFactory.getLogger(getClass())
.warn("Deprecation: You are using a deprecated API. Please use the ItemBuilder OSGi service instead.");
return new ItemBuilderImpl(Collections.singleton(new CoreItemFactory()), item);
}

/**
* Create a new {@link ItemBuilder}, which is initialized by the given item.
*
* @param itemType the item type to create
* @param itemName the name of the item to create
* @return an ItemBuilder instance
*
* @deprecated Use the {@link ItemBuilderFactory} service instead.
*/
ItemBuilder newItemBuilder(String itemType, String itemName);
@Deprecated
default ItemBuilder newItemBuilder(String itemType, String itemName) {
LoggerFactory.getLogger(getClass())
.warn("Deprecation: You are using a deprecated API. Please use the ItemBuilder OSGi service instead.");
return new ItemBuilderImpl(Collections.singleton(new CoreItemFactory()), itemType, itemName);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.eclipse.smarthome.core.items.GroupItem;
import org.eclipse.smarthome.core.items.Item;
import org.eclipse.smarthome.core.items.ItemBuilder;
import org.eclipse.smarthome.core.items.ItemRegistry;
import org.eclipse.smarthome.core.items.ItemBuilderFactory;
import org.eclipse.smarthome.core.library.items.NumberItem;
import org.eclipse.smarthome.core.types.State;
import org.eclipse.smarthome.core.types.TypeParser;
Expand All @@ -47,25 +47,25 @@ public class ItemDTOMapper {
* Maps item DTO into item object.
*
* @param itemDTO the DTO
* @param itemRegistry the item registry
* @param itemBuilderFactory the item registry
* @return the item object
*/
public static @Nullable Item map(ItemDTO itemDTO, ItemRegistry itemRegistry) {
public static @Nullable Item map(ItemDTO itemDTO, ItemBuilderFactory itemBuilderFactory) {
if (itemDTO == null) {
throw new IllegalArgumentException("The argument 'itemDTO' must no be null.");
}
if (itemRegistry == null) {
throw new IllegalArgumentException("The argument 'itemRegistry' must no be null.");
if (itemBuilderFactory == null) {
throw new IllegalArgumentException("The argument 'itemBuilderFactory' must no be null.");
}

if (itemDTO.type != null) {
ItemBuilder builder = itemRegistry.newItemBuilder(itemDTO.type, itemDTO.name);
ItemBuilder builder = itemBuilderFactory.newItemBuilder(itemDTO.type, itemDTO.name);

if (itemDTO instanceof GroupItemDTO && GroupItem.TYPE.equals(itemDTO.type)) {
GroupItemDTO groupItemDTO = (GroupItemDTO) itemDTO;
Item baseItem = null;
if (!StringUtils.isEmpty(groupItemDTO.groupType)) {
baseItem = itemRegistry.newItemBuilder(groupItemDTO.groupType, itemDTO.name).build();
baseItem = itemBuilderFactory.newItemBuilder(groupItemDTO.groupType, itemDTO.name).build();
builder.withBaseItem(baseItem);
}
GroupFunction function = new GroupFunction.Equality();
Expand Down
Loading

0 comments on commit 4733152

Please sign in to comment.