From c3a9a4c7b6416084e587934d2559262c9d224d28 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Mon, 8 May 2023 09:35:36 +0200 Subject: [PATCH] Adjust to core changes (#14952) Signed-off-by: Jan N. Klug --- .../internal/DynamoDBPersistenceService.java | 4 ++ .../dynamodb/internal/DynamoDBQueryUtils.java | 33 +++++++------ .../AbstractTwoItemIntegrationTest.java | 5 +- .../internal/PagingIntegrationTest.java | 15 +++--- ...mplexItemsWithDifferentStateTypesTest.java | 8 ++- .../influxdb/InfluxDBPersistenceService.java | 4 ++ ...fluxDB1FilterCriteriaQueryCreatorImpl.java | 11 +++-- ...fluxDB2FilterCriteriaQueryCreatorImpl.java | 29 +++++------ ...luxFilterCriteriaQueryCreatorImplTest.java | 49 +++++++++---------- .../jdbc/internal/JdbcPersistenceService.java | 4 ++ .../jdbc/internal/db/JdbcBaseDAOTest.java | 24 ++++++--- .../jpa/internal/JpaPersistenceService.java | 4 ++ .../internal/MongoDBPersistenceService.java | 10 +++- .../internal/RRD4jPersistenceService.java | 4 ++ 14 files changed, 124 insertions(+), 80 deletions(-) diff --git a/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBPersistenceService.java b/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBPersistenceService.java index 50706c700..5e58c2e5d 100644 --- a/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBPersistenceService.java +++ b/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBPersistenceService.java @@ -384,6 +384,10 @@ public class DynamoDBPersistenceService implements QueryablePersistenceService { // Proceed with query // String itemName = filter.getItemName(); + if (itemName == null) { + logger.warn("Item name is missing in filter {}", filter); + return List.of(); + } Item item = getItemFromRegistry(itemName); if (item == null) { logger.warn("Could not get item {} from registry! Returning empty query results.", itemName); diff --git a/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBQueryUtils.java b/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBQueryUtils.java index 7a95b36cf..fa86f9a09 100644 --- a/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBQueryUtils.java +++ b/bundles/org.openhab.persistence.dynamodb/src/main/java/org/openhab/persistence/dynamodb/internal/DynamoDBQueryUtils.java @@ -54,7 +54,11 @@ public class DynamoDBQueryUtils { } QueryEnhancedRequest.Builder queryBuilder = QueryEnhancedRequest.builder() .scanIndexForward(filter.getOrdering() == Ordering.ASCENDING); - addFilterbyItemAndTimeFilter(queryBuilder, expectedTableSchema, filter.getItemName(), filter); + String itemName = filter.getItemName(); + if (itemName == null) { + throw new IllegalArgumentException("Item name not set"); + } + addFilterbyItemAndTimeFilter(queryBuilder, expectedTableSchema, itemName, filter); addStateFilter(queryBuilder, expectedTableSchema, item, dtoClass, filter); addProjection(dtoClass, expectedTableSchema, queryBuilder); return queryBuilder.build(); @@ -154,26 +158,25 @@ public class DynamoDBQueryUtils { private static void addFilterbyItemAndTimeFilter(QueryEnhancedRequest.Builder queryBuilder, ExpectedTableSchema expectedTableSchema, String partition, final FilterCriteria filter) { - boolean hasBegin = filter.getBeginDate() != null; - boolean hasEnd = filter.getEndDate() != null; + ZonedDateTime begin = filter.getBeginDate(); + ZonedDateTime end = filter.getEndDate(); boolean legacy = expectedTableSchema == ExpectedTableSchema.LEGACY; AttributeConverter timeConverter = AbstractDynamoDBItem.getTimestampConverter(legacy); - if (!hasBegin && !hasEnd) { - // No need to place time filter filter but we do filter by partition + if (begin == null && end == null) { + // No need to place time filter, but we do filter by partition queryBuilder.queryConditional(QueryConditional.keyEqualTo(k -> k.partitionValue(partition))); - } else if (hasBegin && !hasEnd) { - queryBuilder.queryConditional(QueryConditional.sortGreaterThan( - k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(filter.getBeginDate())))); - } else if (!hasBegin && hasEnd) { - queryBuilder.queryConditional(QueryConditional.sortLessThan( - k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(filter.getEndDate())))); - } else { - assert hasBegin && hasEnd; // invariant + } else if (begin != null && end == null) { + queryBuilder.queryConditional(QueryConditional + .sortGreaterThan(k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(begin)))); + } else if (begin == null && end != null) { + queryBuilder.queryConditional(QueryConditional + .sortLessThan(k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(end)))); + } else if (begin != null && end != null) { queryBuilder.queryConditional(QueryConditional.sortBetween( - k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(filter.getBeginDate())), - k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(filter.getEndDate())))); + k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(begin)), + k -> k.partitionValue(partition).sortValue(timeConverter.transformFrom(end)))); } } diff --git a/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/AbstractTwoItemIntegrationTest.java b/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/AbstractTwoItemIntegrationTest.java index 776cc7276..1c9fc8135 100644 --- a/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/AbstractTwoItemIntegrationTest.java +++ b/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/AbstractTwoItemIntegrationTest.java @@ -317,7 +317,10 @@ public abstract class AbstractTwoItemIntegrationTest extends BaseIntegrationTest FilterCriteria criteria = new FilterCriteria(); criteria.setOperator(Operator.GT); - criteria.setState(getQueryItemStateBetween()); + State filterState = getQueryItemStateBetween(); + if (filterState != null) { + criteria.setState(filterState); + } criteria.setItemName(getItemName()); criteria.setBeginDate(beforeStore); criteria.setEndDate(afterStore2); diff --git a/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/PagingIntegrationTest.java b/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/PagingIntegrationTest.java index c0f942de1..813558bc9 100644 --- a/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/PagingIntegrationTest.java +++ b/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/PagingIntegrationTest.java @@ -19,6 +19,7 @@ import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Objects; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -70,7 +71,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.ASCENDING); criteria.setPageNumber(0); criteria.setPageSize(3); @@ -84,7 +85,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.ASCENDING); criteria.setPageNumber(1); criteria.setPageSize(3); @@ -98,7 +99,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.ASCENDING); criteria.setPageNumber(3); criteria.setPageSize(3); @@ -112,7 +113,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.ASCENDING); criteria.setPageNumber(4); criteria.setPageSize(3); @@ -126,7 +127,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.DESCENDING); criteria.setPageNumber(0); criteria.setPageSize(3); @@ -140,7 +141,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.ASCENDING); criteria.setPageNumber(0); criteria.setPageSize(900); @@ -154,7 +155,7 @@ public class PagingIntegrationTest extends BaseIntegrationTest { waitForAssert(() -> { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(NAME); - criteria.setBeginDate(storeStart); + criteria.setBeginDate(Objects.requireNonNull(storeStart)); criteria.setOrdering(Ordering.ASCENDING); criteria.setPageNumber(0); criteria.setPageSize(3); diff --git a/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/TestComplexItemsWithDifferentStateTypesTest.java b/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/TestComplexItemsWithDifferentStateTypesTest.java index 000662643..e93de0770 100644 --- a/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/TestComplexItemsWithDifferentStateTypesTest.java +++ b/bundles/org.openhab.persistence.dynamodb/src/test/java/org/openhab/persistence/dynamodb/internal/TestComplexItemsWithDifferentStateTypesTest.java @@ -337,8 +337,12 @@ public class TestComplexItemsWithDifferentStateTypesTest extends BaseIntegration FilterCriteria criteria = new FilterCriteria(); criteria.setOrdering(Ordering.ASCENDING); criteria.setItemName(item); - criteria.setOperator(operator); - criteria.setState(state); + if (operator != null) { + criteria.setOperator(operator); + } + if (state != null) { + criteria.setState(state); + } @SuppressWarnings("null") Iterable iterable = BaseIntegrationTest.service.query(criteria); List actualStatesList = new ArrayList<>(); diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/InfluxDBPersistenceService.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/InfluxDBPersistenceService.java index 168bc8e47..783624651 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/InfluxDBPersistenceService.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/InfluxDBPersistenceService.java @@ -214,6 +214,10 @@ public class InfluxDBPersistenceService implements QueryablePersistenceService { "Query-Filter: itemname: {}, ordering: {}, state: {}, operator: {}, getBeginDate: {}, getEndDate: {}, getPageSize: {}, getPageNumber: {}", filter.getItemName(), filter.getOrdering().toString(), filter.getState(), filter.getOperator(), filter.getBeginDate(), filter.getEndDate(), filter.getPageSize(), filter.getPageNumber()); + if (filter.getItemName() == null) { + logger.warn("Item name is missing in filter {}", filter); + return List.of(); + } String query = influxDBRepository.createQueryCreator().createQuery(filter, configuration.getRetentionPolicy()); logger.trace("Query {}", query); diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java index 99620776d..a0ce5df89 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java @@ -16,6 +16,8 @@ import static org.influxdb.querybuilder.BuiltQuery.QueryBuilder.*; import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.*; import static org.openhab.persistence.influxdb.internal.InfluxDBStateConvertUtils.stateToObject; +import java.util.Objects; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.influxdb.dto.Query; @@ -24,6 +26,7 @@ import org.influxdb.querybuilder.Select; import org.influxdb.querybuilder.Where; import org.influxdb.querybuilder.clauses.SimpleClause; import org.openhab.core.persistence.FilterCriteria; +import org.openhab.core.types.State; import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; @@ -48,7 +51,7 @@ public class InfluxDB1FilterCriteriaQueryCreatorImpl implements FilterCriteriaQu @Override public String createQuery(FilterCriteria criteria, String retentionPolicy) { - final String itemName = criteria.getItemName(); + final String itemName = Objects.requireNonNull(criteria.getItemName()); // we checked non-null before final String tableName = getTableName(itemName); final boolean hasCriteriaName = itemName != null; @@ -68,10 +71,10 @@ public class InfluxDB1FilterCriteriaQueryCreatorImpl implements FilterCriteriaQu where.and(BuiltQuery.QueryBuilder.lte(COLUMN_TIME_NAME_V1, criteria.getEndDate().toInstant().toString())); } - if (criteria.getState() != null && criteria.getOperator() != null) { + State filterState = criteria.getState(); + if (filterState != null && criteria.getOperator() != null) { where.and(new SimpleClause(COLUMN_VALUE_NAME_V1, - getOperationSymbol(criteria.getOperator(), InfluxDBVersion.V1), - stateToObject(criteria.getState()))); + getOperationSymbol(criteria.getOperator(), InfluxDBVersion.V1), stateToObject(filterState))); } if (criteria.getOrdering() == FilterCriteria.Ordering.DESCENDING) { diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java index 1f89901dc..711cb69ea 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java @@ -18,9 +18,11 @@ import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.*; import static org.openhab.persistence.influxdb.internal.InfluxDBStateConvertUtils.stateToObject; import java.time.temporal.ChronoUnit; +import java.util.Objects; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.persistence.FilterCriteria; +import org.openhab.core.types.State; import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; @@ -63,23 +65,22 @@ public class InfluxDB2FilterCriteriaQueryCreatorImpl implements FilterCriteriaQu } flux = range; - String itemName = criteria.getItemName(); - if (itemName != null) { - String name = influxDBMetadataService.getMeasurementNameOrDefault(itemName, itemName); - String measurementName = configuration.isReplaceUnderscore() ? name.replace('_', '.') : name; - flux = flux.filter(measurement().equal(measurementName)); - if (!measurementName.equals(itemName)) { - flux = flux.filter(tag(TAG_ITEM_NAME).equal(itemName)); - flux = flux.keep(new String[] { FIELD_MEASUREMENT_NAME, COLUMN_TIME_NAME_V2, COLUMN_VALUE_NAME_V2, - TAG_ITEM_NAME }); - } else { - flux = flux.keep(new String[] { FIELD_MEASUREMENT_NAME, COLUMN_TIME_NAME_V2, COLUMN_VALUE_NAME_V2 }); - } + String itemName = Objects.requireNonNull(criteria.getItemName()); // we checked non-null before + String name = influxDBMetadataService.getMeasurementNameOrDefault(itemName, itemName); + String measurementName = configuration.isReplaceUnderscore() ? name.replace('_', '.') : name; + flux = flux.filter(measurement().equal(measurementName)); + if (!measurementName.equals(itemName)) { + flux = flux.filter(tag(TAG_ITEM_NAME).equal(itemName)); + flux = flux.keep( + new String[] { FIELD_MEASUREMENT_NAME, COLUMN_TIME_NAME_V2, COLUMN_VALUE_NAME_V2, TAG_ITEM_NAME }); + } else { + flux = flux.keep(new String[] { FIELD_MEASUREMENT_NAME, COLUMN_TIME_NAME_V2, COLUMN_VALUE_NAME_V2 }); } - if (criteria.getState() != null && criteria.getOperator() != null) { + State filterState = criteria.getState(); + if (filterState != null && criteria.getOperator() != null) { Restrictions restrictions = Restrictions.and(Restrictions.field().equal(FIELD_VALUE_NAME), - Restrictions.value().custom(stateToObject(criteria.getState()), + Restrictions.value().custom(stateToObject(filterState), getOperationSymbol(criteria.getOperator(), InfluxDBVersion.V2))); flux = flux.filter(restrictions); } diff --git a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxFilterCriteriaQueryCreatorImplTest.java b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxFilterCriteriaQueryCreatorImplTest.java index a6a724307..da7b238bf 100644 --- a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxFilterCriteriaQueryCreatorImplTest.java +++ b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxFilterCriteriaQueryCreatorImplTest.java @@ -77,26 +77,16 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { FilterCriteria criteria = createBaseCriteria(); String queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV1, equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\";")); + assertThat(queryV1, + equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" ORDER BY time DESC;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); assertThat(queryV2, equalTo(""" from(bucket:"origin") \t|> range(start:-100y, stop:100y) \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") - \t|> keep(columns:["_measurement", "_time", "_value"])""")); - } - - @Test - public void testSimpleUnboundedItemWithoutParams() { - FilterCriteria criteria = new FilterCriteria(); - criteria.setOrdering(null); - - String queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV1, equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\"./.*/;")); - - String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y, stop:100y)")); + \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> sort(desc:true, columns:["_time"])""")); } @Test @@ -109,7 +99,7 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { String queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); String expectedQueryV1 = String.format( - "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" WHERE time >= '%s' AND time <= '%s';", + "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" WHERE time >= '%s' AND time <= '%s' ORDER BY time DESC;", now.toInstant(), tomorrow.toInstant()); assertThat(queryV1, equalTo(expectedQueryV1)); @@ -118,8 +108,9 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { from(bucket:"origin") \t|> range(start:%s, stop:%s) \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") - \t|> keep(columns:["_measurement", "_time", "_value"])""", - INFLUX2_DATE_FORMATTER.format(now.toInstant()), INFLUX2_DATE_FORMATTER.format(tomorrow.toInstant())); + \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> sort(desc:true, columns:["_time"])""", INFLUX2_DATE_FORMATTER.format(now.toInstant()), + INFLUX2_DATE_FORMATTER.format(tomorrow.toInstant())); assertThat(queryV2, equalTo(expectedQueryV2)); } @@ -130,8 +121,8 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { criteria.setState(new PercentType(90)); String query = instanceV1.createQuery(criteria, RETENTION_POLICY); - assertThat(query, - equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" WHERE value <= 90;")); + assertThat(query, equalTo( + "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" WHERE value <= 90 ORDER BY time DESC;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); assertThat(queryV2, equalTo(""" @@ -139,7 +130,8 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { \t|> range(start:-100y, stop:100y) \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") \t|> keep(columns:["_measurement", "_time", "_value"]) - \t|> filter(fn: (r) => (r["_field"] == "value" and r["_value"] <= 90))""")); + \t|> filter(fn: (r) => (r["_field"] == "value" and r["_value"] <= 90)) + \t|> sort(desc:true, columns:["_time"])""")); } @Test @@ -149,8 +141,8 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { criteria.setPageSize(10); String query = instanceV1.createQuery(criteria, RETENTION_POLICY); - assertThat(query, - equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" LIMIT 10 OFFSET 20;")); + assertThat(query, equalTo( + "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" ORDER BY time DESC LIMIT 10 OFFSET 20;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); assertThat(queryV2, equalTo(""" @@ -158,6 +150,7 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { \t|> range(start:-100y, stop:100y) \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> sort(desc:true, columns:["_time"]) \t|> limit(n:10, offset:20)""")); } @@ -196,7 +189,6 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { private FilterCriteria createBaseCriteria() { FilterCriteria criteria = new FilterCriteria(); criteria.setItemName(ITEM_NAME); - criteria.setOrdering(null); return criteria; } @@ -210,7 +202,7 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { String queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); assertThat(queryV1, equalTo( - "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"measurementName\" WHERE item = 'sampleItem';")); + "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"measurementName\" WHERE item = 'sampleItem' ORDER BY time DESC;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); assertThat(queryV2, equalTo(""" @@ -218,18 +210,21 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { \t|> range(start:-100y, stop:100y) \t|> filter(fn: (r) => r["_measurement"] == "measurementName") \t|> filter(fn: (r) => r["item"] == "sampleItem") - \t|> keep(columns:["_measurement", "_time", "_value", "item"])""")); + \t|> keep(columns:["_measurement", "_time", "_value", "item"]) + \t|> sort(desc:true, columns:["_time"])""")); when(metadataRegistry.get(metadataKey)) .thenReturn(new Metadata(metadataKey, "", Map.of("key1", "val1", "key2", "val2"))); queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV1, equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\";")); + assertThat(queryV1, + equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" ORDER BY time DESC;")); queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); assertThat(queryV2, equalTo(""" from(bucket:"origin") \t|> range(start:-100y, stop:100y) \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") - \t|> keep(columns:["_measurement", "_time", "_value"])""")); + \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> sort(desc:true, columns:["_time"])""")); } } diff --git a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java index ee7abd778..6ba41ba03 100644 --- a/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java +++ b/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java @@ -193,6 +193,10 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers // Also get the Item object so we can determine the type Item item = null; String itemName = filter.getItemName(); + if (itemName == null) { + logger.warn("Item name is missing in filter {}", filter); + return List.of(); + } logger.debug("JDBC::query: item is {}", itemName); try { item = itemRegistry.getItem(itemName); diff --git a/bundles/org.openhab.persistence.jdbc/src/test/java/org/openhab/persistence/jdbc/internal/db/JdbcBaseDAOTest.java b/bundles/org.openhab.persistence.jdbc/src/test/java/org/openhab/persistence/jdbc/internal/db/JdbcBaseDAOTest.java index 40d0959ae..ae92db9dc 100644 --- a/bundles/org.openhab.persistence.jdbc/src/test/java/org/openhab/persistence/jdbc/internal/db/JdbcBaseDAOTest.java +++ b/bundles/org.openhab.persistence.jdbc/src/test/java/org/openhab/persistence/jdbc/internal/db/JdbcBaseDAOTest.java @@ -22,6 +22,7 @@ import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; +import java.util.Objects; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; @@ -206,8 +207,9 @@ public class JdbcBaseDAOTest { String sql = jdbcBaseDAO.histItemFilterQueryProvider(filter, 0, DB_TABLE_NAME, "TEST", UTC_ZONE_ID); assertThat(sql, is("SELECT time, value FROM " + DB_TABLE_NAME + " WHERE TIME>='" // - + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getBeginDate()) + "'" // - + " AND TIME<='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getEndDate()) + "' ORDER BY time DESC")); + + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getBeginDate())) + "'" // + + " AND TIME<='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getEndDate())) + + "' ORDER BY time DESC")); } @Test @@ -231,8 +233,9 @@ public class JdbcBaseDAOTest { String sql = jdbcBaseDAO.histItemFilterDeleteProvider(filter, DB_TABLE_NAME, UTC_ZONE_ID); assertThat(sql, is("DELETE FROM " + DB_TABLE_NAME + " WHERE TIME>='" // - + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getBeginDate()) + "'" // - + " AND TIME<='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getEndDate()) + "'")); + + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getBeginDate())) + "'" // + + " AND TIME<='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getEndDate())) + + "'")); } @Test @@ -246,7 +249,8 @@ public class JdbcBaseDAOTest { filter.setBeginDate(parseDateTimeString("2022-01-10T15:01:44")); String sql = jdbcBaseDAO.resolveTimeFilter(filter, UTC_ZONE_ID); - assertThat(sql, is(" WHERE TIME>='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getBeginDate()) + "'")); + assertThat(sql, is(" WHERE TIME>='" + + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getBeginDate())) + "'")); } @Test @@ -254,7 +258,8 @@ public class JdbcBaseDAOTest { filter.setEndDate(parseDateTimeString("2022-01-15T15:01:44")); String sql = jdbcBaseDAO.resolveTimeFilter(filter, UTC_ZONE_ID); - assertThat(sql, is(" WHERE TIME<='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getEndDate()) + "'")); + assertThat(sql, is(" WHERE TIME<='" + + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getEndDate())) + "'")); } @Test @@ -263,8 +268,11 @@ public class JdbcBaseDAOTest { filter.setEndDate(parseDateTimeString("2022-01-15T15:01:44")); String sql = jdbcBaseDAO.resolveTimeFilter(filter, UTC_ZONE_ID); - assertThat(sql, is(" WHERE TIME>='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getBeginDate()) + "'" // - + " AND TIME<='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(filter.getEndDate()) + "'")); + assertThat(sql, + is(" WHERE TIME>='" + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getBeginDate())) + + "'" // + + " AND TIME<='" + + JdbcBaseDAO.JDBC_DATE_FORMAT.format(Objects.requireNonNull(filter.getEndDate())) + "'")); } private ZonedDateTime parseDateTimeString(String dts) { diff --git a/bundles/org.openhab.persistence.jpa/src/main/java/org/openhab/persistence/jpa/internal/JpaPersistenceService.java b/bundles/org.openhab.persistence.jpa/src/main/java/org/openhab/persistence/jpa/internal/JpaPersistenceService.java index ebaca5b56..db2de9481 100644 --- a/bundles/org.openhab.persistence.jpa/src/main/java/org/openhab/persistence/jpa/internal/JpaPersistenceService.java +++ b/bundles/org.openhab.persistence.jpa/src/main/java/org/openhab/persistence/jpa/internal/JpaPersistenceService.java @@ -188,6 +188,10 @@ public class JpaPersistenceService implements QueryablePersistenceService { } String itemName = filter.getItemName(); + if (itemName == null) { + logger.warn("Item name is missing in filter {}", filter); + return List.of(); + } Item item = getItemFromRegistry(itemName); if (item == null) { logger.debug("Item '{}' does not exist in the item registry", itemName); diff --git a/bundles/org.openhab.persistence.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBPersistenceService.java b/bundles/org.openhab.persistence.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBPersistenceService.java index 0aee04811..638e65115 100644 --- a/bundles/org.openhab.persistence.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBPersistenceService.java +++ b/bundles/org.openhab.persistence.mongodb/src/main/java/org/openhab/persistence/mongodb/internal/MongoDBPersistenceService.java @@ -331,6 +331,11 @@ public class MongoDBPersistenceService implements QueryablePersistenceService { } String realItemName = filter.getItemName(); + if (realItemName == null) { + logger.warn("Item name is missing in filter {}", filter); + return List.of(); + } + String collectionName = collectionPerItem ? realItemName : this.collection; @Nullable DBCollection collection = connectToCollection(collectionName); @@ -354,7 +359,8 @@ public class MongoDBPersistenceService implements QueryablePersistenceService { if (filter.getItemName() != null) { query.put(FIELD_ITEM, filter.getItemName()); } - if (filter.getState() != null && filter.getOperator() != null) { + State filterState = filter.getState(); + if (filterState != null && filter.getOperator() != null) { @Nullable String op = convertOperator(filter.getOperator()); @@ -363,7 +369,7 @@ public class MongoDBPersistenceService implements QueryablePersistenceService { return Collections.emptyList(); } - Object value = convertValue(filter.getState()); + Object value = convertValue(filterState); query.put(FIELD_VALUE, new BasicDBObject(op, value)); } diff --git a/bundles/org.openhab.persistence.rrd4j/src/main/java/org/openhab/persistence/rrd4j/internal/RRD4jPersistenceService.java b/bundles/org.openhab.persistence.rrd4j/src/main/java/org/openhab/persistence/rrd4j/internal/RRD4jPersistenceService.java index 1cc6eb9c1..c80ef1886 100644 --- a/bundles/org.openhab.persistence.rrd4j/src/main/java/org/openhab/persistence/rrd4j/internal/RRD4jPersistenceService.java +++ b/bundles/org.openhab.persistence.rrd4j/src/main/java/org/openhab/persistence/rrd4j/internal/RRD4jPersistenceService.java @@ -259,6 +259,10 @@ public class RRD4jPersistenceService implements QueryablePersistenceService { } String itemName = filter.getItemName(); + if (itemName == null) { + logger.warn("Item name is missing in filter {}", filter); + return List.of(); + } RrdDb db = null; try {