Skip to content

Commit

Permalink
[dynamodb] Changes required after API changes in core framework (open…
Browse files Browse the repository at this point in the history 
…hab#14966)

Changes in openhab/openhab-core#3481 broke this service due to an
breaking API change in NumberItem class (constructor).

Related to openhab/openhab-core#3600

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
  • Loading branch information
@lolodomo @austvik
lolodomo authored and austvik committed Mar 27, 2024
1 parent 70a4e13 commit c1918f9
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 30 deletions.
25 changes: 17 additions & 8 deletions ...b/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBPersistenceService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.common.ThreadPoolManager;
import org.openhab.core.config.core.ConfigurableService;
import org.openhab.core.i18n.UnitProvider;
import org.openhab.core.items.GenericItem;
import org.openhab.core.items.GroupItem;
import org.openhab.core.items.Item;
Expand Down Expand Up @@ -103,7 +104,8 @@ public class DynamoDBPersistenceService implements QueryablePersistenceService {

private static final String DYNAMODB_THREADPOOL_NAME = "dynamodbPersistenceService";

private ItemRegistry itemRegistry;
private final ItemRegistry itemRegistry;
private final UnitProvider unitProvider;
private @Nullable DynamoDbEnhancedAsyncClient client;
private @Nullable DynamoDbAsyncClient lowLevelClient;
private static final Logger logger = LoggerFactory.getLogger(DynamoDBPersistenceService.class);
Expand Down Expand Up @@ -131,15 +133,19 @@ void overrideConfig(ClientOverrideConfiguration.Builder config) {
}

@Activate
public DynamoDBPersistenceService(final @Reference ItemRegistry itemRegistry) {
public DynamoDBPersistenceService(final @Reference ItemRegistry itemRegistry,
final @Reference UnitProvider unitProvider) {
this.itemRegistry = itemRegistry;
this.unitProvider = unitProvider;
}

/**
* For tests
*/
DynamoDBPersistenceService(final ItemRegistry itemRegistry, @Nullable URI endpointOverride) {
DynamoDBPersistenceService(final ItemRegistry itemRegistry, final UnitProvider unitProvider,
@Nullable URI endpointOverride) {
this.itemRegistry = itemRegistry;
this.unitProvider = unitProvider;
this.endpointOverride = endpointOverride;
}

Expand Down Expand Up @@ -414,7 +420,7 @@ public Iterable<HistoricItem> query(FilterCriteria filter) {
item.getClass().getSimpleName(), dtoClass.getSimpleName(), tableName);

QueryEnhancedRequest queryExpression = DynamoDBQueryUtils.createQueryExpression(dtoClass,
localTableNameResolver.getTableSchema(), item, filter);
localTableNameResolver.getTableSchema(), item, filter, unitProvider);

CompletableFuture<List<DynamoDBItem<?>>> itemsFuture = new CompletableFuture<>();
final SdkPublisher<? extends DynamoDBItem<?>> itemPublisher = table.query(queryExpression).items();
Expand Down Expand Up @@ -523,7 +529,7 @@ public void store(Item item, @Nullable String alias) {

// We do not want to rely item.state since async context below can execute much later.
// We 'copy' the item for local use. copyItem also normalizes the unit with NumberItems.
final GenericItem copiedItem = copyItem(itemTemplate, item, effectiveName, null);
final GenericItem copiedItem = copyItem(itemTemplate, item, effectiveName, null, unitProvider);

resolveTableSchema().thenAcceptAsync(resolved -> {
if (!resolved) {
Expand Down Expand Up @@ -613,15 +619,18 @@ private Item getEffectiveItem(Item item) {
* @param item item that is used to acquire name and state
* @param nameOverride name override for the resulting copy
* @param stateOverride state override for the resulting copy
* @param unitProvider the unit provider for number with dimension
* @throws IllegalArgumentException when state is QuantityType and not compatible with item
*/
static GenericItem copyItem(Item itemTemplate, Item item, @Nullable String nameOverride,
@Nullable State stateOverride) {
@Nullable State stateOverride, UnitProvider unitProvider) {
final GenericItem copiedItem;
try {
if (itemTemplate instanceof NumberItem) {
copiedItem = (GenericItem) itemTemplate.getClass().getDeclaredConstructor(String.class, String.class)
.newInstance(itemTemplate.getType(), nameOverride == null ? item.getName() : nameOverride);
copiedItem = (GenericItem) itemTemplate.getClass()
.getDeclaredConstructor(String.class, String.class, UnitProvider.class)
.newInstance(itemTemplate.getType(), nameOverride == null ? item.getName() : nameOverride,
unitProvider);
} else {
copiedItem = (GenericItem) itemTemplate.getClass().getDeclaredConstructor(String.class)
.newInstance(nameOverride == null ? item.getName() : nameOverride);
Expand Down
10 changes: 6 additions & 4 deletions ....dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBQueryUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.i18n.UnitProvider;
import org.openhab.core.items.GenericItem;
import org.openhab.core.items.Item;
import org.openhab.core.persistence.FilterCriteria;
Expand Down Expand Up @@ -45,10 +46,11 @@ public class DynamoDBQueryUtils {
* @param item item corresponding to filter
* @param filter filter for the query
* @return DynamoDBQueryExpression corresponding to the given FilterCriteria
* @param unitProvider the unit provider for number with dimension
* @throws IllegalArgumentException when schema is not fully resolved
*/
public static QueryEnhancedRequest createQueryExpression(Class<? extends DynamoDBItem<?>> dtoClass,
ExpectedTableSchema expectedTableSchema, Item item, FilterCriteria filter) {
ExpectedTableSchema expectedTableSchema, Item item, FilterCriteria filter, UnitProvider unitProvider) {
if (!expectedTableSchema.isFullyResolved()) {
throw new IllegalArgumentException("Schema not resolved");
}
Expand All @@ -59,7 +61,7 @@ public static QueryEnhancedRequest createQueryExpression(Class<? extends DynamoD
throw new IllegalArgumentException("Item name not set");
}
addFilterbyItemAndTimeFilter(queryBuilder, expectedTableSchema, itemName, filter);
addStateFilter(queryBuilder, expectedTableSchema, item, dtoClass, filter);
addStateFilter(queryBuilder, expectedTableSchema, item, dtoClass, filter, unitProvider);
addProjection(dtoClass, expectedTableSchema, queryBuilder);
return queryBuilder.build();
}
Expand Down Expand Up @@ -94,7 +96,7 @@ private static void addProjection(Class<? extends DynamoDBItem<?>> dtoClass,

private static void addStateFilter(QueryEnhancedRequest.Builder queryBuilder,
ExpectedTableSchema expectedTableSchema, Item item, Class<? extends DynamoDBItem<?>> dtoClass,
FilterCriteria filter) {
FilterCriteria filter, UnitProvider unitProvider) {
final Expression expression;
Builder itemStateTypeExpressionBuilder = Expression.builder()
.expression(String.format("attribute_exists(#attr)"));
Expand Down Expand Up @@ -123,7 +125,7 @@ private static void addStateFilter(QueryEnhancedRequest.Builder queryBuilder,
// Following will throw IllegalArgumentException when filter state is not compatible with
// item. This is acceptable.
GenericItem stateToFind = DynamoDBPersistenceService.copyItem(item, item, filter.getItemName(),
filter.getState());
filter.getState(), unitProvider);
acceptAsDTO(stateToFind, legacy, new DynamoDBItemVisitor<@Nullable Void>() {
@Override
public @Nullable Void visit(DynamoDBStringItem serialized) {
Expand Down
21 changes: 3 additions & 18 deletions ...dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/BaseIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.openhab.core.common.registry.RegistryChangeListener;
import org.openhab.core.i18n.UnitProvider;
import org.openhab.core.internal.i18n.I18nProviderImpl;
import org.openhab.core.items.GenericItem;
import org.openhab.core.items.GroupItem;
import org.openhab.core.items.Item;
import org.openhab.core.items.ItemNotFoundException;
Expand Down Expand Up @@ -147,12 +146,12 @@ protected static void populateItems() {
ITEMS.put("dimmer", new DimmerItem("dimmer"));
ITEMS.put("number", new NumberItem("number"));

NumberItem temperatureItem = new NumberItem("Number:Temperature", "numberTemperature");
NumberItem temperatureItem = new NumberItem("Number:Temperature", "numberTemperature", UNIT_PROVIDER);
ITEMS.put("numberTemperature", temperatureItem);
GroupItem groupTemperature = new GroupItem("groupNumberTemperature", temperatureItem);
ITEMS.put("groupNumberTemperature", groupTemperature);

NumberItem dimensionlessItem = new NumberItem("Number:Dimensionless", "numberDimensionless");
NumberItem dimensionlessItem = new NumberItem("Number:Dimensionless", "numberDimensionless", UNIT_PROVIDER);
ITEMS.put("numberDimensionless", dimensionlessItem);
GroupItem groupDimensionless = new GroupItem("groupNumberDimensionless", dimensionlessItem);
ITEMS.put("groupNumberDimensionless", groupDimensionless);
Expand All @@ -170,8 +169,6 @@ protected static void populateItems() {
ITEMS.put("location", new LocationItem("location"));
ITEMS.put("player_playpause", new PlayerItem("player_playpause"));
ITEMS.put("player_rewindfastforward", new PlayerItem("player_rewindfastforward"));

injectItemServices();
}

@BeforeAll
Expand Down Expand Up @@ -248,7 +245,6 @@ public Item getItem(String name) throws ItemNotFoundException {
if (item == null) {
throw new ItemNotFoundException(name);
}
injectItemServices(item);
return item;
}

Expand Down Expand Up @@ -326,23 +322,12 @@ public void addRegistryHook(RegistryHook<Item> hook) {
public void removeRegistryHook(RegistryHook<Item> hook) {
throw new UnsupportedOperationException();
}
}, localEndpointOverride);
}, UNIT_PROVIDER, localEndpointOverride);

service.activate(null, config);
return service;
}

protected static void injectItemServices() {
ITEMS.values().forEach(BaseIntegrationTest::injectItemServices);
}

protected static void injectItemServices(Item item) {
if (item instanceof GenericItem) {
GenericItem genericItem = (GenericItem) item;
genericItem.setUnitProvider(UNIT_PROVIDER);
}
}

private static Map<String, Object> getConfig(@Nullable Boolean legacy, @Nullable String table,
@Nullable String tablePrefix) {
Map<String, Object> config = new HashMap<>();
Expand Down

0 comments on commit c1918f9

Please sign in to comment.