From 77d40ab840b1b28c515ca83c0f5ae0f95cb7ce61 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Tue, 31 Jan 2023 18:53:57 +0100 Subject: [PATCH 1/4] [influxdb] code improvements Signed-off-by: Jan N. Klug --- .../org.openhab.persistence.influxdb/pom.xml | 95 ++++++------ .../influxdb/InfluxDBPersistenceService.java | 145 +++++++----------- .../internal/FilterCriteriaQueryCreator.java | 27 ++-- .../internal/InfluxDBConfiguration.java | 12 -- .../internal/InfluxDBMetadataService.java | 70 +++++++++ .../internal/InfluxDBPersistentItemInfo.java | 7 +- .../influxdb/internal/InfluxDBRepository.java | 15 +- .../internal/InfluxDBStateConvertUtils.java | 41 ++--- .../influxdb/internal/InfluxPoint.java | 4 +- .../influxdb/internal/InfluxRow.java | 7 +- .../internal/ItemToStorePointCreator.java | 75 +++------ .../influxdb/internal/RepositoryFactory.java | 54 ------- ...java => UnexpectedConditionException.java} | 10 +- ...luxDB1FilterCriteriaQueryCreatorImpl.java} | 44 +++--- .../influx1/InfluxDB1RepositoryImpl.java | 83 +++++----- ...luxDB2FilterCriteriaQueryCreatorImpl.java} | 38 ++--- .../influx2/InfluxDB2RepositoryImpl.java | 60 ++++---- .../internal/ConfigurationTestHelper.java | 47 ------ .../InfluxDBPersistenceServiceTest.java | 88 ++++++----- ...luxFilterCriteriaQueryCreatorImplTest.java | 31 ++-- .../internal/ItemToStorePointCreatorTest.java | 75 ++++++++- 21 files changed, 484 insertions(+), 544 deletions(-) create mode 100644 bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataService.java delete mode 100644 bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/RepositoryFactory.java rename bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/{UnnexpectedConditionException.java => UnexpectedConditionException.java} (67%) rename bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/{Influx1FilterCriteriaQueryCreatorImpl.java => InfluxDB1FilterCriteriaQueryCreatorImpl.java} (73%) rename bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/{Influx2FilterCriteriaQueryCreatorImpl.java => InfluxDB2FilterCriteriaQueryCreatorImpl.java} (79%) delete mode 100644 bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ConfigurationTestHelper.java diff --git a/bundles/org.openhab.persistence.influxdb/pom.xml b/bundles/org.openhab.persistence.influxdb/pom.xml index 374b41fdf2541..13095df4d6a4f 100644 --- a/bundles/org.openhab.persistence.influxdb/pom.xml +++ b/bundles/org.openhab.persistence.influxdb/pom.xml @@ -16,82 +16,87 @@ - !javax.annotation;!android.*,!com.android.*,!com.google.appengine.*,!dalvik.system,!kotlin.*,!kotlinx.*,!org.conscrypt,!sun.security.ssl,!org.apache.harmony.*,!org.apache.http.*,!rx.*,!org.msgpack.* + !javax.annotation.*;!android.*,!com.android.*,!com.google.appengine.*,!dalvik.system,!kotlin.*,!kotlinx.*,!org.conscrypt,!sun.security.ssl,!org.apache.harmony.*,!org.apache.http.*,!rx.*,!org.msgpack.* + 3.14.9 + 2.7.2 + 1.15.0 + 2.21 - com.influxdb influxdb-client-java - 1.6.0 + ${influx2.version} + com.influxdb influxdb-client-core + ${influx2.version} + + com.influxdb - 1.6.0 + flux-dsl + ${influx2.version} + - converter-gson com.squareup.retrofit2 - 2.5.0 + converter-gson + ${retrofit.version} + com.squareup.retrofit2 converter-scalars + ${retrofit.version} + + + retrofit com.squareup.retrofit2 - 2.5.0 + ${retrofit.version} + + + + com.squareup.okhttp3 + okhttp + ${okhttp3.version} + + + com.squareup.okhttp3 + logging-interceptor + ${okhttp3.version} - gson com.google.code.gson - 2.9.1 + gson + 2.8.5 - gson-fire io.gsonfire - 1.8.0 + gson-fire + 1.8.4 - okio com.squareup.okio - 1.17.3 + okio + 1.17.2 - commons-csv org.apache.commons - 1.6 + commons-csv + 1.8 json org.json - 20180813 - - - okhttp - com.squareup.okhttp3 - ${okhttp.version} - - - retrofit - com.squareup.retrofit2 - 2.6.2 - - - jsr305 - com.google.code.findbugs - 3.0.2 - - - logging-interceptor - com.squareup.okhttp3 - ${okhttp.version} + 20200518 rxjava io.reactivex.rxjava2 - 2.2.17 + 2.2.19 reactive-streams @@ -101,29 +106,20 @@ swagger-annotations io.swagger - 1.5.22 - - - - - - com.influxdb - flux-dsl - 1.6.0 + 1.6.1 - org.influxdb influxdb-java - 2.17 + ${influx1.version} com.squareup.retrofit2 converter-moshi - 2.6.2 + ${retrofit.version} com.squareup.moshi @@ -135,4 +131,5 @@ + 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 2888cb8fdb00f..798f821026fb5 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 @@ -17,6 +17,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -25,7 +26,6 @@ import org.openhab.core.config.core.ConfigurableService; import org.openhab.core.items.Item; import org.openhab.core.items.ItemRegistry; -import org.openhab.core.items.MetadataRegistry; import org.openhab.core.persistence.FilterCriteria; import org.openhab.core.persistence.HistoricItem; import org.openhab.core.persistence.PersistenceItemInfo; @@ -36,18 +36,20 @@ import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; import org.openhab.persistence.influxdb.internal.InfluxDBHistoricItem; +import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBPersistentItemInfo; import org.openhab.persistence.influxdb.internal.InfluxDBRepository; import org.openhab.persistence.influxdb.internal.InfluxDBStateConvertUtils; import org.openhab.persistence.influxdb.internal.InfluxPoint; import org.openhab.persistence.influxdb.internal.InfluxRow; import org.openhab.persistence.influxdb.internal.ItemToStorePointCreator; -import org.openhab.persistence.influxdb.internal.RepositoryFactory; +import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; +import org.openhab.persistence.influxdb.internal.influx1.InfluxDB1RepositoryImpl; +import org.openhab.persistence.influxdb.internal.influx2.InfluxDB2RepositoryImpl; import org.osgi.framework.Constants; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; -import org.osgi.service.component.annotations.Modified; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,47 +87,42 @@ public class InfluxDBPersistenceService implements QueryablePersistenceService { // External dependencies private final ItemRegistry itemRegistry; - private final MetadataRegistry metadataRegistry; + private final InfluxDBMetadataService influxDBMetadataService; - // Internal dependencies/state - private InfluxDBConfiguration configuration = InfluxDBConfiguration.NO_CONFIGURATION; - - // Relax rules because can only be null if component is not active - private @NonNullByDefault({}) ItemToStorePointCreator itemToStorePointCreator; - private @NonNullByDefault({}) InfluxDBRepository influxDBRepository; - - private boolean tryReconnection = false; + private final InfluxDBConfiguration configuration; + private final ItemToStorePointCreator itemToStorePointCreator; + private final InfluxDBRepository influxDBRepository; + private boolean tryReconnection; @Activate public InfluxDBPersistenceService(final @Reference ItemRegistry itemRegistry, - final @Reference MetadataRegistry metadataRegistry) { + final @Reference InfluxDBMetadataService influxDBMetadataService, Map config) { this.itemRegistry = itemRegistry; - this.metadataRegistry = metadataRegistry; - } - - /** - * Connect to database when service is activated - */ - @Activate - public void activate(final @Nullable Map config) { - logger.debug("InfluxDB persistence service is being activated"); - - if (loadConfiguration(config)) { - itemToStorePointCreator = new ItemToStorePointCreator(configuration, metadataRegistry); - influxDBRepository = createInfluxDBRepository(); - influxDBRepository.connect(); + this.influxDBMetadataService = influxDBMetadataService; + this.configuration = new InfluxDBConfiguration(config); + if (configuration.isValid()) { + this.influxDBRepository = createInfluxDBRepository() + .orElseThrow(() -> new IllegalArgumentException("Failed to instantiate repository.")); + this.influxDBRepository.connect(); + this.itemToStorePointCreator = new ItemToStorePointCreator(configuration, influxDBMetadataService); tryReconnection = true; } else { - logger.error("Cannot load configuration, persistence service wont work"); - tryReconnection = false; + throw new IllegalArgumentException("Configuration invalid."); } - logger.debug("InfluxDB persistence service is now activated"); + logger.info("InfluxDB persistence service started."); } // Visible for testing - protected InfluxDBRepository createInfluxDBRepository() { - return RepositoryFactory.createRepository(configuration); + protected Optional createInfluxDBRepository() { + InfluxDBRepository influxDBRepository = null; + switch (configuration.getVersion()) { + case V1 -> influxDBRepository = new InfluxDB1RepositoryImpl(configuration, influxDBMetadataService); + case V2 -> influxDBRepository = new InfluxDB2RepositoryImpl(configuration, influxDBMetadataService); + default -> { + } + } + return Optional.ofNullable(influxDBRepository); } /** @@ -133,48 +130,9 @@ protected InfluxDBRepository createInfluxDBRepository() { */ @Deactivate public void deactivate() { - logger.debug("InfluxDB persistence service deactivated"); + logger.info("InfluxDB persistence service stopped."); + influxDBRepository.disconnect(); tryReconnection = false; - if (influxDBRepository != null) { - influxDBRepository.disconnect(); - influxDBRepository = null; - } - if (itemToStorePointCreator != null) { - itemToStorePointCreator = null; - } - } - - /** - * Rerun deactivation/activation code each time configuration is changed - */ - @Modified - protected void modified(@Nullable Map config) { - if (config != null) { - logger.debug("Config has been modified will deactivate/activate with new config"); - - deactivate(); - activate(config); - } else { - logger.warn("Null configuration, ignoring"); - } - } - - private boolean loadConfiguration(@Nullable Map config) { - boolean configurationIsValid; - if (config != null) { - configuration = new InfluxDBConfiguration(config); - configurationIsValid = configuration.isValid(); - if (configurationIsValid) { - logger.debug("Loaded configuration {}", config); - } else { - logger.warn("Some configuration properties are not valid {}", config); - } - } else { - configuration = InfluxDBConfiguration.NO_CONFIGURATION; - configurationIsValid = false; - logger.warn("Ignoring configuration because it's null"); - } - return configurationIsValid; } @Override @@ -190,8 +148,7 @@ public String getLabel(@Nullable Locale locale) { @Override public Set getItemInfo() { if (checkConnection()) { - return influxDBRepository.getStoredItemsCount().entrySet().stream() - .map(entry -> new InfluxDBPersistentItemInfo(entry.getKey(), entry.getValue())) + return influxDBRepository.getStoredItemsCount().entrySet().stream().map(InfluxDBPersistentItemInfo::new) .collect(Collectors.toUnmodifiableSet()); } else { logger.info("getItemInfo ignored, InfluxDB is not yet connected"); @@ -201,7 +158,7 @@ public Set getItemInfo() { @Override public void store(Item item) { - store(item, item.getName()); + store(item, null); } @Override @@ -209,10 +166,14 @@ public void store(Item item, @Nullable String alias) { if (checkConnection()) { InfluxPoint point = itemToStorePointCreator.convert(item, alias); if (point != null) { - logger.trace("Storing item {} in InfluxDB point {}", item, point); - influxDBRepository.write(point); + try { + influxDBRepository.write(point); + logger.trace("Stored item {} in InfluxDB point {}", item, point); + } catch (UnexpectedConditionException e) { + logger.warn("Failed to store item {} in InfluxDB point {}", point, item); + } } else { - logger.trace("Ignoring item {} as is cannot be converted to an InfluxDB point", item); + logger.trace("Ignoring item {}, conversion to a InfluxDB point failed.", item); } } else { logger.debug("store ignored, InfluxDB is not yet connected"); @@ -228,19 +189,24 @@ public Iterable query(FilterCriteria filter) { "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()); - - String query = RepositoryFactory.createQueryCreator(configuration, metadataRegistry).createQuery(filter, - configuration.getRetentionPolicy()); - logger.trace("Query {}", query); - List results = influxDBRepository.query(query); - return results.stream().map(this::mapRow2HistoricItem).collect(Collectors.toList()); + try { + String query = influxDBRepository.createQueryCreator().createQuery(filter, + configuration.getRetentionPolicy()); + + logger.trace("Query {}", query); + List results = influxDBRepository.query(query); + return results.stream().map(this::mapRowToHistoricItem).collect(Collectors.toList()); + } catch (UnexpectedConditionException e) { + logger.warn("Failed to create query:{}", e.getMessage()); + return List.of(); + } } else { logger.debug("query ignored, InfluxDB is not yet connected"); return List.of(); } } - private HistoricItem mapRow2HistoricItem(InfluxRow row) { + private HistoricItem mapRowToHistoricItem(InfluxRow row) { State state = InfluxDBStateConvertUtils.objectToState(row.getValue(), row.getItemName(), itemRegistry); return new InfluxDBHistoricItem(row.getItemName(), state, ZonedDateTime.ofInstant(row.getTime(), ZoneId.systemDefault())); @@ -257,14 +223,11 @@ public List getDefaultStrategies() { * @return true if connected */ private boolean checkConnection() { - if (influxDBRepository == null) { - return false; - } else if (influxDBRepository.isConnected()) { + if (influxDBRepository.isConnected()) { return true; } else if (tryReconnection) { logger.debug("Connection lost, trying re-connection"); - influxDBRepository.connect(); - return influxDBRepository.isConnected(); + return influxDBRepository.connect(); } return false; } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java index e84f705bba304..3ab9f1867c8e5 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java @@ -28,25 +28,18 @@ public interface FilterCriteriaQueryCreator { * @param criteria Criteria to create query from * @param retentionPolicy Name of the retentionPolicy/bucket to use in query * @return Created query as a String + * @throws UnexpectedConditionException when an error occurs during query creation */ - String createQuery(FilterCriteria criteria, String retentionPolicy); + String createQuery(FilterCriteria criteria, String retentionPolicy) throws UnexpectedConditionException; default String getOperationSymbol(FilterCriteria.Operator operator, InfluxDBVersion version) { - switch (operator) { - case EQ: - return "="; - case LT: - return "<"; - case LTE: - return "<="; - case GT: - return ">"; - case GTE: - return ">="; - case NEQ: - return version == InfluxDBVersion.V1 ? "<>" : "!="; - default: - throw new UnnexpectedConditionException("Not expected operator " + operator); - } + return switch (operator) { + case EQ -> "="; + case LT -> "<"; + case LTE -> "<="; + case GT -> ">"; + case GTE -> ">="; + case NEQ -> version == InfluxDBVersion.V1 ? "<>" : "!="; + }; } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java index d935ca090f093..7d5b9d712642b 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java @@ -12,7 +12,6 @@ */ package org.openhab.persistence.influxdb.internal; -import java.util.Collections; import java.util.Map; import java.util.Optional; import java.util.StringJoiner; @@ -40,7 +39,6 @@ public class InfluxDBConfiguration { public static final String ADD_CATEGORY_TAG_PARAM = "addCategoryTag"; public static final String ADD_LABEL_TAG_PARAM = "addLabelTag"; public static final String ADD_TYPE_TAG_PARAM = "addTypeTag"; - public static InfluxDBConfiguration NO_CONFIGURATION = new InfluxDBConfiguration(Collections.emptyMap()); private final Logger logger = LoggerFactory.getLogger(InfluxDBConfiguration.class); private final String url; private final String user; @@ -49,7 +47,6 @@ public class InfluxDBConfiguration { private final String databaseName; private final String retentionPolicy; private final InfluxDBVersion version; - private final boolean replaceUnderscore; private final boolean addCategoryTag; private final boolean addTypeTag; @@ -63,7 +60,6 @@ public InfluxDBConfiguration(Map config) { databaseName = (String) config.getOrDefault(DATABASE_PARAM, "openhab"); retentionPolicy = (String) config.getOrDefault(RETENTION_POLICY_PARAM, "autogen"); version = parseInfluxVersion((String) config.getOrDefault(VERSION_PARAM, InfluxDBVersion.V1.name())); - replaceUnderscore = getConfigBooleanValue(config, REPLACE_UNDERSCORE_PARAM, false); addCategoryTag = getConfigBooleanValue(config, ADD_CATEGORY_TAG_PARAM, false); addLabelTag = getConfigBooleanValue(config, ADD_LABEL_TAG_PARAM, false); @@ -178,12 +174,4 @@ public String toString() { + addCategoryTag + ", addTypeTag=" + addTypeTag + ", addLabelTag=" + addLabelTag + '}'; return sb; } - - public int getTokenLength() { - return token.length(); - } - - public char[] getTokenAsCharArray() { - return token.toCharArray(); - } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataService.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataService.java new file mode 100644 index 0000000000000..0ca8599c5bab0 --- /dev/null +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataService.java @@ -0,0 +1,70 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * 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.openhab.persistence.influxdb.internal; + +import java.util.Optional; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.openhab.core.items.Metadata; +import org.openhab.core.items.MetadataKey; +import org.openhab.core.items.MetadataRegistry; +import org.openhab.persistence.influxdb.InfluxDBPersistenceService; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; + +/** + * Utility service for using item metadata in InfluxDB + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +@Component(service = InfluxDBMetadataService.class) +public class InfluxDBMetadataService { + private final MetadataRegistry metadataRegistry; + + @Activate + public InfluxDBMetadataService(@Reference MetadataRegistry metadataRegistry) { + this.metadataRegistry = metadataRegistry; + } + + /** + * get the measurement name from the item metadata or return the provided default + * + * @param itemName the item name + * @param defaultName the default measurement name ( + * @return the metadata measurement name if present, defaultName otherwise + */ + public String getMeasurementNameOrDefault(String itemName, String defaultName) { + Optional metadata = getMetaData(itemName); + if (metadata.isPresent()) { + String metaName = metadata.get().getValue(); + if (!metaName.isBlank()) { + return metaName; + } + } + + return defaultName; + } + + /** + * get an Optional of the metadata for an item + * + * @param itemName the item name + * @return Optional with the metadata (may be empty) + */ + public Optional getMetaData(String itemName) { + MetadataKey key = new MetadataKey(InfluxDBPersistenceService.SERVICE_NAME, itemName); + return Optional.ofNullable(metadataRegistry.get(key)); + } +} diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistentItemInfo.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistentItemInfo.java index ba6ccdb058037..4175ed329bc79 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistentItemInfo.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistentItemInfo.java @@ -13,6 +13,7 @@ package org.openhab.persistence.influxdb.internal; import java.util.Date; +import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -28,9 +29,9 @@ public class InfluxDBPersistentItemInfo implements PersistenceItemInfo { private final String name; private final Integer count; - public InfluxDBPersistentItemInfo(String name, Integer count) { - this.name = name; - this.count = count; + public InfluxDBPersistentItemInfo(Map.Entry itemInfo) { + this.name = itemInfo.getKey(); + this.count = itemInfo.getValue(); } @Override diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java index 00ea9146d5b8a..a2c1e16e5f62d 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java @@ -34,7 +34,7 @@ public interface InfluxDBRepository { /** * Connect to InfluxDB server * - * @return True if successful, otherwise false + * @return true if successful, otherwise false */ boolean connect(); @@ -46,7 +46,7 @@ public interface InfluxDBRepository { /** * Check if connection is currently ready * - * @return True if its ready, otherwise false + * @return True if it's ready, otherwise false */ boolean checkConnectionStatus(); @@ -62,6 +62,7 @@ public interface InfluxDBRepository { * * @param query Query * @return Query results + * */ List query(String query); @@ -69,6 +70,14 @@ public interface InfluxDBRepository { * Write point to database * * @param influxPoint Point to write + * @throws UnexpectedConditionException when an error occurs */ - void write(InfluxPoint influxPoint); + void write(InfluxPoint influxPoint) throws UnexpectedConditionException; + + /** + * create a query creator on this repository + * + * @return the query creator for this repository + */ + FilterCriteriaQueryCreator createQueryCreator(); } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBStateConvertUtils.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBStateConvertUtils.java index 0b8aaa3786d4e..5b54ff6de31ac 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBStateConvertUtils.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBStateConvertUtils.java @@ -53,7 +53,7 @@ public class InfluxDBStateConvertUtils { static final Number DIGITAL_VALUE_OFF = 0; // Visible for testing static final Number DIGITAL_VALUE_ON = 1; // Visible for testing - private static Logger logger = LoggerFactory.getLogger(InfluxDBStateConvertUtils.class); + private static final Logger LOGGER = LoggerFactory.getLogger(InfluxDBStateConvertUtils.class); /** * Converts {@link State} to objects fitting into influxdb values. @@ -67,7 +67,7 @@ public static Object stateToObject(State state) { if (state instanceof HSBType) { value = state.toString(); } else if (state instanceof PointType) { - value = point2String((PointType) state); + value = state.toString(); } else if (state instanceof DecimalType) { value = ((DecimalType) state).toBigDecimal(); } else if (state instanceof QuantityType) { @@ -93,22 +93,15 @@ public static Object stateToObject(State state) { * @return the state of the item represented by the itemName parameter, else the string value of * the Object parameter */ - public static State objectToState(@Nullable Object value, String itemName, @Nullable ItemRegistry itemRegistry) { - State state = null; - if (itemRegistry != null) { - try { - Item item = itemRegistry.getItem(itemName); - state = objectToState(value, item); - } catch (ItemNotFoundException e) { - logger.info("Could not find item '{}' in registry", itemName); - } - } - - if (state == null) { - state = new StringType(String.valueOf(value)); + public static State objectToState(Object value, String itemName, ItemRegistry itemRegistry) { + try { + Item item = itemRegistry.getItem(itemName); + return objectToState(value, item); + } catch (ItemNotFoundException e) { + LOGGER.info("Could not find item '{}' in registry", itemName); } - return state; + return new StringType(String.valueOf(value)); } public static State objectToState(@Nullable Object value, Item itemToSetState) { @@ -128,7 +121,7 @@ public static State objectToState(@Nullable Object value, Item itemToSetState) { } else if (item instanceof DimmerItem) { return new PercentType(valueStr); } else if (item instanceof SwitchItem) { - return toBoolean(valueStr) ? OnOffType.ON : OnOffType.OFF; + return OnOffType.from(toBoolean(valueStr)); } else if (item instanceof ContactItem) { return toBoolean(valueStr) ? OpenClosedType.OPEN : OpenClosedType.CLOSED; } else if (item instanceof RollershutterItem) { @@ -149,22 +142,10 @@ private static boolean toBoolean(@Nullable Object object) { if ("1".equals(object) || "1.0".equals(object)) { return true; } else { - return Boolean.valueOf(String.valueOf(object)); + return Boolean.parseBoolean(String.valueOf(object)); } } else { return false; } } - - private static String point2String(PointType point) { - StringBuilder buf = new StringBuilder(); - buf.append(point.getLatitude().toString()); - buf.append(","); - buf.append(point.getLongitude().toString()); - if (!point.getAltitude().equals(DecimalType.ZERO)) { - buf.append(","); - buf.append(point.getAltitude().toString()); - } - return buf.toString(); // latitude, longitude, altitude - } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java index 85c8956e43bc4..63bf15e05e5eb 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java @@ -79,8 +79,8 @@ public Builder withValue(Object val) { return this; } - public Builder withTag(String name, String value) { - tags.put(name, value); + public Builder withTag(String name, Object value) { + tags.put(name, value.toString()); return this; } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java index 8403212e4c4eb..0f774b5118f42 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java @@ -15,7 +15,6 @@ import java.time.Instant; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; /** * Row data returned from database query @@ -26,9 +25,9 @@ public class InfluxRow { private final String itemName; private final Instant time; - private final @Nullable Object value; + private final Object value; - public InfluxRow(Instant time, String itemName, @Nullable Object value) { + public InfluxRow(Instant time, String itemName, Object value) { this.time = time; this.itemName = itemName; this.value = value; @@ -42,7 +41,7 @@ public String getItemName() { return itemName; } - public @Nullable Object getValue() { + public Object getValue() { return value; } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreator.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreator.java index c3c686d77035f..25929d80926f8 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreator.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreator.java @@ -12,20 +12,20 @@ */ package org.openhab.persistence.influxdb.internal; -import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.*; +import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.TAG_CATEGORY_NAME; +import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.TAG_ITEM_NAME; +import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.TAG_LABEL_NAME; +import static org.openhab.persistence.influxdb.internal.InfluxDBConstants.TAG_TYPE_NAME; import java.time.Instant; +import java.util.Objects; import java.util.Optional; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.items.Item; -import org.openhab.core.items.Metadata; -import org.openhab.core.items.MetadataKey; -import org.openhab.core.items.MetadataRegistry; import org.openhab.core.types.State; import org.openhab.core.types.UnDefType; -import org.openhab.persistence.influxdb.InfluxDBPersistenceService; /** * Logic to create an InfluxDB {@link InfluxPoint} from an openHAB {@link Item} @@ -35,11 +35,12 @@ @NonNullByDefault public class ItemToStorePointCreator { private final InfluxDBConfiguration configuration; - private final @Nullable MetadataRegistry metadataRegistry; + private final InfluxDBMetadataService influxDBMetadataService; - public ItemToStorePointCreator(InfluxDBConfiguration configuration, @Nullable MetadataRegistry metadataRegistry) { + public ItemToStorePointCreator(InfluxDBConfiguration configuration, + InfluxDBMetadataService influxDBMetadataService) { this.configuration = configuration; - this.metadataRegistry = metadataRegistry; + this.influxDBMetadataService = influxDBMetadataService; } public @Nullable InfluxPoint convert(Item item, @Nullable String storeAlias) { @@ -53,19 +54,17 @@ public ItemToStorePointCreator(InfluxDBConfiguration configuration, @Nullable Me Object value = InfluxDBStateConvertUtils.stateToObject(state); - InfluxPoint.Builder point = InfluxPoint.newBuilder(measurementName).withTime(Instant.now()).withValue(value) - .withTag(TAG_ITEM_NAME, itemName); + InfluxPoint.Builder pointBuilder = InfluxPoint.newBuilder(measurementName).withTime(Instant.now()) + .withValue(value).withTag(TAG_ITEM_NAME, itemName); - addPointTags(item, point); + addPointTags(item, pointBuilder); - return point.build(); + return pointBuilder.build(); } private String calculateMeasurementName(Item item, @Nullable String storeAlias) { String name = storeAlias != null && !storeAlias.isBlank() ? storeAlias : item.getName(); - - name = InfluxDBMetadataUtils.calculateMeasurementNameFromMetadataIfPresent(metadataRegistry, name, - item.getName()); + name = influxDBMetadataService.getMeasurementNameOrDefault(item.getName(), name); if (configuration.isReplaceUnderscore()) { name = name.replace('_', '.'); @@ -75,19 +74,9 @@ private String calculateMeasurementName(Item item, @Nullable String storeAlias) } private State getItemState(Item item) { - final State state; - final Optional> desiredConversion = calculateDesiredTypeConversionToStore(item); - if (desiredConversion.isPresent()) { - State convertedState = item.getStateAs(desiredConversion.get()); - if (convertedState != null) { - state = convertedState; - } else { - state = item.getState(); - } - } else { - state = item.getState(); - } - return state; + return calculateDesiredTypeConversionToStore(item) + .map(desiredClass -> Objects.requireNonNullElseGet(item.getStateAs(desiredClass), item::getState)) + .orElseGet(item::getState); } private Optional> calculateDesiredTypeConversionToStore(Item item) { @@ -95,36 +84,22 @@ private Optional> calculateDesiredTypeConversionToStore(I .findFirst().map(commandType -> commandType.asSubclass(State.class)); } - private void addPointTags(Item item, InfluxPoint.Builder point) { + private void addPointTags(Item item, InfluxPoint.Builder pointBuilder) { if (configuration.isAddCategoryTag()) { - String categoryName = item.getCategory(); - if (categoryName == null) { - categoryName = "n/a"; - } - point.withTag(TAG_CATEGORY_NAME, categoryName); + String categoryName = Objects.requireNonNullElse(item.getCategory(), "n/a"); + pointBuilder.withTag(TAG_CATEGORY_NAME, categoryName); } if (configuration.isAddTypeTag()) { - point.withTag(TAG_TYPE_NAME, item.getType()); + pointBuilder.withTag(TAG_TYPE_NAME, item.getType()); } if (configuration.isAddLabelTag()) { - String labelName = item.getLabel(); - if (labelName == null) { - labelName = "n/a"; - } - point.withTag(TAG_LABEL_NAME, labelName); + String labelName = Objects.requireNonNullElse(item.getLabel(), "n/a"); + pointBuilder.withTag(TAG_LABEL_NAME, labelName); } - final MetadataRegistry currentMetadataRegistry = metadataRegistry; - if (currentMetadataRegistry != null) { - MetadataKey key = new MetadataKey(InfluxDBPersistenceService.SERVICE_NAME, item.getName()); - Metadata metadata = currentMetadataRegistry.get(key); - if (metadata != null) { - metadata.getConfiguration().forEach((tagName, tagValue) -> { - point.withTag(tagName, tagValue.toString()); - }); - } - } + influxDBMetadataService.getMetaData(item.getName()) + .ifPresent(metadata -> metadata.getConfiguration().forEach(pointBuilder::withTag)); } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/RepositoryFactory.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/RepositoryFactory.java deleted file mode 100644 index 0090d9c6002dd..0000000000000 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/RepositoryFactory.java +++ /dev/null @@ -1,54 +0,0 @@ -/** - * Copyright (c) 2010-2023 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * 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.openhab.persistence.influxdb.internal; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.openhab.core.items.MetadataRegistry; -import org.openhab.persistence.influxdb.internal.influx1.Influx1FilterCriteriaQueryCreatorImpl; -import org.openhab.persistence.influxdb.internal.influx1.InfluxDB1RepositoryImpl; -import org.openhab.persistence.influxdb.internal.influx2.Influx2FilterCriteriaQueryCreatorImpl; -import org.openhab.persistence.influxdb.internal.influx2.InfluxDB2RepositoryImpl; - -/** - * Factory that returns {@link InfluxDBRepository} and - * {@link FilterCriteriaQueryCreator} implementations depending on InfluxDB - * version - * - * @author Joan Pujol Espinar - Initial contribution - */ -@NonNullByDefault -public class RepositoryFactory { - - public static InfluxDBRepository createRepository(InfluxDBConfiguration influxDBConfiguration) { - switch (influxDBConfiguration.getVersion()) { - case V1: - return new InfluxDB1RepositoryImpl(influxDBConfiguration); - case V2: - return new InfluxDB2RepositoryImpl(influxDBConfiguration); - default: - throw new UnnexpectedConditionException("Not expected version " + influxDBConfiguration.getVersion()); - } - } - - public static FilterCriteriaQueryCreator createQueryCreator(InfluxDBConfiguration influxDBConfiguration, - MetadataRegistry metadataRegistry) { - switch (influxDBConfiguration.getVersion()) { - case V1: - return new Influx1FilterCriteriaQueryCreatorImpl(influxDBConfiguration, metadataRegistry); - case V2: - return new Influx2FilterCriteriaQueryCreatorImpl(influxDBConfiguration, metadataRegistry); - default: - throw new UnnexpectedConditionException("Not expected version " + influxDBConfiguration.getVersion()); - } - } -} diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/UnnexpectedConditionException.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/UnexpectedConditionException.java similarity index 67% rename from bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/UnnexpectedConditionException.java rename to bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/UnexpectedConditionException.java index 412b813462792..f96076ae54b23 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/UnnexpectedConditionException.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/UnexpectedConditionException.java @@ -15,19 +15,15 @@ import org.eclipse.jdt.annotation.NonNullByDefault; /** - * Throw to indicate an unnexpected condition that should not have happened (a bug) + * Throw to indicate an unexpected condition that should not have happened (a bug) * * @author Joan Pujol Espinar - Initial contribution */ @NonNullByDefault -public class UnnexpectedConditionException extends RuntimeException { +public class UnexpectedConditionException extends Exception { private static final long serialVersionUID = 1128380327167959556L; - public UnnexpectedConditionException(String message) { + public UnexpectedConditionException(String message) { super(message); } - - public UnnexpectedConditionException(String message, Throwable cause) { - super(message, cause); - } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/Influx1FilterCriteriaQueryCreatorImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java similarity index 73% rename from bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/Influx1FilterCriteriaQueryCreatorImpl.java rename to bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java index c55c62d791b5b..7af2b5cf7ae80 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/Influx1FilterCriteriaQueryCreatorImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1FilterCriteriaQueryCreatorImpl.java @@ -23,12 +23,12 @@ import org.influxdb.querybuilder.Select; import org.influxdb.querybuilder.Where; import org.influxdb.querybuilder.clauses.SimpleClause; -import org.openhab.core.items.MetadataRegistry; import org.openhab.core.persistence.FilterCriteria; import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; -import org.openhab.persistence.influxdb.internal.InfluxDBMetadataUtils; +import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBVersion; +import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; /** * Implementation of {@link FilterCriteriaQueryCreator} for InfluxDB 1.0 @@ -36,24 +36,22 @@ * @author Joan Pujol Espinar - Initial contribution */ @NonNullByDefault -public class Influx1FilterCriteriaQueryCreatorImpl implements FilterCriteriaQueryCreator { +public class InfluxDB1FilterCriteriaQueryCreatorImpl implements FilterCriteriaQueryCreator { - private InfluxDBConfiguration configuration; - private MetadataRegistry metadataRegistry; + private final InfluxDBConfiguration configuration; + private final InfluxDBMetadataService influxDBMetadataService; - public Influx1FilterCriteriaQueryCreatorImpl(InfluxDBConfiguration configuration, - MetadataRegistry metadataRegistry) { + public InfluxDB1FilterCriteriaQueryCreatorImpl(InfluxDBConfiguration configuration, + InfluxDBMetadataService influxDBMetadataService) { this.configuration = configuration; - this.metadataRegistry = metadataRegistry; + this.influxDBMetadataService = influxDBMetadataService; } @Override - public String createQuery(FilterCriteria criteria, String retentionPolicy) { - final String tableName; + public String createQuery(FilterCriteria criteria, String retentionPolicy) throws UnexpectedConditionException { final String itemName = criteria.getItemName(); - boolean hasCriteriaName = itemName != null; - - tableName = calculateTableName(itemName); + final String tableName = getTableName(itemName); + final boolean hasCriteriaName = itemName != null; Select select = select().column("\"" + COLUMN_VALUE_NAME_V1 + "\"::field") .column("\"" + TAG_ITEM_NAME + "\"::tag") @@ -62,20 +60,17 @@ public String createQuery(FilterCriteria criteria, String retentionPolicy) { Where where = select.where(); if (itemName != null && !tableName.equals(itemName)) { - where = where.and(BuiltQuery.QueryBuilder.eq(TAG_ITEM_NAME, itemName)); + where.and(BuiltQuery.QueryBuilder.eq(TAG_ITEM_NAME, itemName)); } - if (criteria.getBeginDate() != null) { - where = where.and( - BuiltQuery.QueryBuilder.gte(COLUMN_TIME_NAME_V1, criteria.getBeginDate().toInstant().toString())); + where.and(BuiltQuery.QueryBuilder.gte(COLUMN_TIME_NAME_V1, criteria.getBeginDate().toInstant().toString())); } if (criteria.getEndDate() != null) { - where = where.and( - BuiltQuery.QueryBuilder.lte(COLUMN_TIME_NAME_V1, criteria.getEndDate().toInstant().toString())); + where.and(BuiltQuery.QueryBuilder.lte(COLUMN_TIME_NAME_V1, criteria.getEndDate().toInstant().toString())); } if (criteria.getState() != null && criteria.getOperator() != null) { - where = where.and(new SimpleClause(COLUMN_VALUE_NAME_V1, + where.and(new SimpleClause(COLUMN_VALUE_NAME_V1, getOperationSymbol(criteria.getOperator(), InfluxDBVersion.V1), stateToObject(criteria.getState()))); } @@ -94,18 +89,15 @@ public String createQuery(FilterCriteria criteria, String retentionPolicy) { } } - final Query query = (Query) select; - return query.getCommand(); + return ((Query) select).getCommand(); } - private String calculateTableName(@Nullable String itemName) { + private String getTableName(@Nullable String itemName) { if (itemName == null) { return "/.*/"; } - String name = itemName; - - name = InfluxDBMetadataUtils.calculateMeasurementNameFromMetadataIfPresent(metadataRegistry, name, itemName); + String name = influxDBMetadataService.getMeasurementNameOrDefault(itemName, itemName); if (configuration.isReplaceUnderscore()) { name = name.replace('_', '.'); diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java index b103657c05f54..d1d9c8482c6a2 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -34,11 +33,13 @@ import org.influxdb.dto.Pong; import org.influxdb.dto.Query; import org.influxdb.dto.QueryResult; +import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; +import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBRepository; import org.openhab.persistence.influxdb.internal.InfluxPoint; import org.openhab.persistence.influxdb.internal.InfluxRow; -import org.openhab.persistence.influxdb.internal.UnnexpectedConditionException; +import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,12 +54,14 @@ @NonNullByDefault public class InfluxDB1RepositoryImpl implements InfluxDBRepository { private final Logger logger = LoggerFactory.getLogger(InfluxDB1RepositoryImpl.class); - private InfluxDBConfiguration configuration; - @Nullable - private InfluxDB client; + private final InfluxDBConfiguration configuration; + private final InfluxDBMetadataService influxDBMetadataService; + private @Nullable InfluxDB client; - public InfluxDB1RepositoryImpl(InfluxDBConfiguration configuration) { + public InfluxDB1RepositoryImpl(InfluxDBConfiguration configuration, + InfluxDBMetadataService influxDBMetadataService) { this.configuration = configuration; + this.influxDBMetadataService = influxDBMetadataService; } @Override @@ -115,7 +118,7 @@ private void handleDatabaseException(Exception e) { } @Override - public void write(InfluxPoint point) { + public void write(InfluxPoint point) throws UnexpectedConditionException { final InfluxDB currentClient = this.client; if (currentClient != null) { Point clientPoint = convertPointToClientFormat(point); @@ -125,15 +128,15 @@ public void write(InfluxPoint point) { } } - private Point convertPointToClientFormat(InfluxPoint point) { + private Point convertPointToClientFormat(InfluxPoint point) throws UnexpectedConditionException { Point.Builder clientPoint = Point.measurement(point.getMeasurementName()).time(point.getTime().toEpochMilli(), TimeUnit.MILLISECONDS); setPointValue(point.getValue(), clientPoint); - point.getTags().entrySet().forEach(e -> clientPoint.tag(e.getKey(), e.getValue())); + point.getTags().forEach(clientPoint::tag); return clientPoint.build(); } - private void setPointValue(@Nullable Object value, Point.Builder point) { + private void setPointValue(@Nullable Object value, Point.Builder point) throws UnexpectedConditionException { if (value instanceof String) { point.addField(FIELD_VALUE_NAME, (String) value); } else if (value instanceof Number) { @@ -141,9 +144,9 @@ private void setPointValue(@Nullable Object value, Point.Builder point) { } else if (value instanceof Boolean) { point.addField(FIELD_VALUE_NAME, (Boolean) value); } else if (value == null) { - point.addField(FIELD_VALUE_NAME, (String) null); + point.addField(FIELD_VALUE_NAME, "null"); } else { - throw new UnnexpectedConditionException("Not expected value type"); + throw new UnexpectedConditionException("Not expected value type"); } } @@ -153,58 +156,47 @@ public List query(String query) { if (currentClient != null) { Query parsedQuery = new Query(query, configuration.getDatabaseName()); List results = currentClient.query(parsedQuery, TimeUnit.MILLISECONDS).getResults(); - return convertClientResutToRepository(results); + return convertClientResultToRepository(results); } else { logger.warn("Returning empty list because queryAPI isn't present"); - return Collections.emptyList(); + return List.of(); } } - private List convertClientResutToRepository(List results) { + private List convertClientResultToRepository(List results) { List rows = new ArrayList<>(); for (QueryResult.Result result : results) { - List seriess = result.getSeries(); + List allSeries = result.getSeries(); if (result.getError() != null) { logger.warn("{}", result.getError()); continue; } - if (seriess == null) { + if (allSeries == null) { logger.debug("query returned no series"); } else { - for (QueryResult.Series series : seriess) { - logger.trace("series {}", series.toString()); - List> valuess = series.getValues(); - if (valuess == null) { + for (QueryResult.Series series : allSeries) { + logger.trace("series {}", series); + String defaultItemName = series.getName(); + List> allValues = series.getValues(); + if (allValues == null) { logger.debug("query returned no values"); } else { List columns = series.getColumns(); logger.trace("columns {}", columns); if (columns != null) { - Integer timestampColumn = null; - Integer valueColumn = null; - Integer itemNameColumn = null; - for (int i = 0; i < columns.size(); i++) { - String columnName = columns.get(i); - if (columnName.equals(COLUMN_TIME_NAME_V1)) { - timestampColumn = i; - } else if (columnName.equals(COLUMN_VALUE_NAME_V1)) { - valueColumn = i; - } else if (columnName.equals(TAG_ITEM_NAME)) { - itemNameColumn = i; - } - } - if (valueColumn == null || timestampColumn == null) { + int timestampColumn = columns.indexOf(COLUMN_TIME_NAME_V1); + int valueColumn = columns.indexOf(COLUMN_VALUE_NAME_V1); + int itemNameColumn = columns.indexOf(TAG_ITEM_NAME); + if (valueColumn == -1 || timestampColumn == -1) { throw new IllegalStateException("missing column"); } - for (int i = 0; i < valuess.size(); i++) { - Double rawTime = (Double) Objects.requireNonNull(valuess.get(i).get(timestampColumn)); + for (List valueObject : allValues) { + Double rawTime = (Double) valueObject.get(timestampColumn); Instant time = Instant.ofEpochMilli(rawTime.longValue()); - @Nullable - Object value = valuess.get(i).get(valueColumn); - var currentI = i; - String itemName = Optional.ofNullable(itemNameColumn) - .flatMap(inc -> Optional.ofNullable((String) valuess.get(currentI).get(inc))) - .orElse(series.getName()); + Object value = valueObject.get(valueColumn); + String itemName = itemNameColumn == -1 ? defaultItemName + : Objects.requireNonNullElse((String) valueObject.get(itemNameColumn), + defaultItemName); logger.trace("adding historic item {}: time {} value {}", itemName, time, value); rows.add(new InfluxRow(time, itemName, value)); } @@ -220,4 +212,9 @@ private List convertClientResutToRepository(List public Map getStoredItemsCount() { return Collections.emptyMap(); } + + @Override + public FilterCriteriaQueryCreator createQueryCreator() { + return new InfluxDB1FilterCriteriaQueryCreatorImpl(configuration, influxDBMetadataService); + } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/Influx2FilterCriteriaQueryCreatorImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java similarity index 79% rename from bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/Influx2FilterCriteriaQueryCreatorImpl.java rename to bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java index ff390529664f8..3157c9d0d6928 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/Influx2FilterCriteriaQueryCreatorImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2FilterCriteriaQueryCreatorImpl.java @@ -20,12 +20,12 @@ import java.time.temporal.ChronoUnit; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.openhab.core.items.MetadataRegistry; import org.openhab.core.persistence.FilterCriteria; import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; -import org.openhab.persistence.influxdb.internal.InfluxDBMetadataUtils; +import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBVersion; +import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import com.influxdb.query.dsl.Flux; import com.influxdb.query.dsl.functions.RangeFlux; @@ -37,43 +37,37 @@ * @author Joan Pujol Espinar - Initial contribution */ @NonNullByDefault -public class Influx2FilterCriteriaQueryCreatorImpl implements FilterCriteriaQueryCreator { +public class InfluxDB2FilterCriteriaQueryCreatorImpl implements FilterCriteriaQueryCreator { + private final InfluxDBConfiguration configuration; + private final InfluxDBMetadataService influxDBMetadataService; - private InfluxDBConfiguration configuration; - private MetadataRegistry metadataRegistry; - - public Influx2FilterCriteriaQueryCreatorImpl(InfluxDBConfiguration configuration, - MetadataRegistry metadataRegistry) { + public InfluxDB2FilterCriteriaQueryCreatorImpl(InfluxDBConfiguration configuration, + InfluxDBMetadataService influxDBMetadataService) { this.configuration = configuration; - this.metadataRegistry = metadataRegistry; + this.influxDBMetadataService = influxDBMetadataService; } @Override - public String createQuery(FilterCriteria criteria, String retentionPolicy) { + public String createQuery(FilterCriteria criteria, String retentionPolicy) throws UnexpectedConditionException { Flux flux = Flux.from(retentionPolicy); RangeFlux range = flux.range(); if (criteria.getBeginDate() != null) { - range = range.withStart(criteria.getBeginDate().toInstant()); + range.withStart(criteria.getBeginDate().toInstant()); } else { range = flux.range(-100L, ChronoUnit.YEARS); // Flux needs a mandatory start range } if (criteria.getEndDate() != null) { - range = range.withStop(criteria.getEndDate().toInstant()); + range.withStop(criteria.getEndDate().toInstant()); } flux = range; String itemName = criteria.getItemName(); if (itemName != null) { - String measurementName = calculateMeasurementName(itemName); - boolean needsToUseItemTagName = !measurementName.equals(itemName); - + String measurementName = getMeasurementName(itemName); flux = flux.filter(measurement().equal(measurementName)); - if (needsToUseItemTagName) { + if (!measurementName.equals(itemName)) { flux = flux.filter(tag(TAG_ITEM_NAME).equal(itemName)); - } - - if (needsToUseItemTagName) { flux = flux.keep(new String[] { FIELD_MEASUREMENT_NAME, COLUMN_TIME_NAME_V2, COLUMN_VALUE_NAME_V2, TAG_ITEM_NAME }); } else { @@ -113,10 +107,8 @@ private Flux applyOrderingAndPageSize(FilterCriteria criteria, Flux flux) { return flux; } - private String calculateMeasurementName(String itemName) { - String name = itemName; - - name = InfluxDBMetadataUtils.calculateMeasurementNameFromMetadataIfPresent(metadataRegistry, name, itemName); + private String getMeasurementName(String itemName) { + String name = influxDBMetadataService.getMeasurementNameOrDefault(itemName, itemName); if (configuration.isReplaceUnderscore()) { name = name.replace('_', '.'); diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java index 950b97ca42a0f..442585d94c0b2 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java @@ -19,17 +19,20 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.persistence.influxdb.internal.FilterCriteriaQueryCreator; import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; import org.openhab.persistence.influxdb.internal.InfluxDBConstants; +import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBRepository; import org.openhab.persistence.influxdb.internal.InfluxPoint; import org.openhab.persistence.influxdb.internal.InfluxRow; -import org.openhab.persistence.influxdb.internal.UnnexpectedConditionException; +import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,16 +54,17 @@ @NonNullByDefault public class InfluxDB2RepositoryImpl implements InfluxDBRepository { private final Logger logger = LoggerFactory.getLogger(InfluxDB2RepositoryImpl.class); - private InfluxDBConfiguration configuration; - @Nullable - private InfluxDBClient client; - @Nullable - private QueryApi queryAPI; - @Nullable - private WriteApi writeAPI; - - public InfluxDB2RepositoryImpl(InfluxDBConfiguration configuration) { + private final InfluxDBConfiguration configuration; + private final InfluxDBMetadataService influxDBMetadataService; + + private @Nullable InfluxDBClient client; + private @Nullable QueryApi queryAPI; + private @Nullable WriteApi writeAPI; + + public InfluxDB2RepositoryImpl(InfluxDBConfiguration configuration, + InfluxDBMetadataService influxDBMetadataService) { this.configuration = configuration; + this.influxDBMetadataService = influxDBMetadataService; } /** @@ -82,7 +86,7 @@ public boolean isConnected() { public boolean connect() { InfluxDBClientOptions.Builder optionsBuilder = InfluxDBClientOptions.builder().url(configuration.getUrl()) .org(configuration.getDatabaseName()).bucket(configuration.getRetentionPolicy()); - char[] token = configuration.getTokenAsCharArray(); + char[] token = configuration.getToken().toCharArray(); if (token.length > 0) { optionsBuilder.authenticateToken(token); } else { @@ -92,9 +96,11 @@ public boolean connect() { final InfluxDBClient createdClient = InfluxDBClientFactory.create(clientOptions); this.client = createdClient; - logger.debug("Succesfully connected to InfluxDB. Instance ready={}", createdClient.ready()); + queryAPI = createdClient.getQueryApi(); writeAPI = createdClient.getWriteApi(); + logger.debug("Successfully connected to InfluxDB. Instance ready={}", createdClient.ready()); + return checkConnectionStatus(); } @@ -133,13 +139,8 @@ public boolean checkConnectionStatus() { } } - /** - * Write point to database - * - * @param point - */ @Override - public void write(InfluxPoint point) { + public void write(InfluxPoint point) throws UnexpectedConditionException { final WriteApi currentWriteAPI = writeAPI; if (currentWriteAPI != null) { currentWriteAPI.writePoint(convertPointToClientFormat(point)); @@ -148,14 +149,14 @@ public void write(InfluxPoint point) { } } - private Point convertPointToClientFormat(InfluxPoint point) { + private Point convertPointToClientFormat(InfluxPoint point) throws UnexpectedConditionException { Point clientPoint = Point.measurement(point.getMeasurementName()).time(point.getTime(), WritePrecision.MS); setPointValue(point.getValue(), clientPoint); - point.getTags().entrySet().forEach(e -> clientPoint.addTag(e.getKey(), e.getValue())); + point.getTags().forEach(clientPoint::addTag); return clientPoint; } - private void setPointValue(@Nullable Object value, Point point) { + private void setPointValue(@Nullable Object value, Point point) throws UnexpectedConditionException { if (value instanceof String) { point.addField(FIELD_VALUE_NAME, (String) value); } else if (value instanceof Number) { @@ -165,7 +166,7 @@ private void setPointValue(@Nullable Object value, Point point) { } else if (value == null) { point.addField(FIELD_VALUE_NAME, (String) null); } else { - throw new UnnexpectedConditionException("Not expected value type"); + throw new UnexpectedConditionException("Not expected value type"); } } @@ -194,9 +195,6 @@ private List convertClientResutToRepository(List clientRes private Stream mapRawResultToHistoric(FluxTable rawRow) { return rawRow.getRecords().stream().map(r -> { String itemName = (String) r.getValueByKey(InfluxDBConstants.TAG_ITEM_NAME); - if (itemName == null) { // use measurement name if item is not tagged - itemName = r.getMeasurement(); - } Object value = r.getValueByKey(COLUMN_VALUE_NAME_V2); Instant time = (Instant) r.getValueByKey(COLUMN_TIME_NAME_V2); return new InfluxRow(time, itemName, value); @@ -221,13 +219,19 @@ public Map getStoredItemsCount() { + " |> group()"; List queryResult = currentQueryAPI.query(query); - queryResult.stream().findFirst().orElse(new FluxTable()).getRecords().forEach(row -> { - result.put((String) row.getValueByKey(TAG_ITEM_NAME), ((Number) row.getValue()).intValue()); - }); + Objects.requireNonNull(queryResult.stream().findFirst().orElse(new FluxTable())).getRecords() + .forEach(row -> { + result.put((String) row.getValueByKey(TAG_ITEM_NAME), ((Number) row.getValue()).intValue()); + }); return result; } else { logger.warn("Returning empty result because queryAPI isn't present"); return Collections.emptyMap(); } } + + @Override + public FilterCriteriaQueryCreator createQueryCreator() { + return new InfluxDB2FilterCriteriaQueryCreatorImpl(configuration, influxDBMetadataService); + } } diff --git a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ConfigurationTestHelper.java b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ConfigurationTestHelper.java deleted file mode 100644 index ecc872ba6a6ad..0000000000000 --- a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ConfigurationTestHelper.java +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Copyright (c) 2010-2023 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * 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.openhab.persistence.influxdb.internal; - -import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.*; - -import java.util.HashMap; -import java.util.Map; - -import org.eclipse.jdt.annotation.NonNullByDefault; - -/** - * @author Joan Pujol Espinar - Initial contribution - */ -@NonNullByDefault -public class ConfigurationTestHelper { - - public static Map createValidConfigurationParameters() { - Map config = new HashMap<>(); - config.put(URL_PARAM, "http://localhost:8086"); - config.put(VERSION_PARAM, InfluxDBVersion.V2.name()); - config.put(TOKEN_PARAM, "sampletoken"); - config.put(DATABASE_PARAM, "openhab"); - config.put(RETENTION_POLICY_PARAM, "default"); - return config; - } - - public static InfluxDBConfiguration createValidConfiguration() { - return new InfluxDBConfiguration(createValidConfigurationParameters()); - } - - public static Map createInvalidConfigurationParameters() { - Map config = createValidConfigurationParameters(); - config.remove(TOKEN_PARAM); - return config; - } -} diff --git a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java index 1c00fc6aecd87..c5749d9c8c878 100644 --- a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java +++ b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java @@ -12,15 +12,21 @@ */ package org.openhab.persistence.influxdb.internal; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.DATABASE_PARAM; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.PASSWORD_PARAM; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.RETENTION_POLICY_PARAM; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.TOKEN_PARAM; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.URL_PARAM; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.USER_PARAM; +import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.VERSION_PARAM; import java.util.Map; +import java.util.Optional; -import org.eclipse.jdt.annotation.DefaultLocation; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -33,74 +39,80 @@ * @author Joan Pujol Espinar - Initial contribution */ @ExtendWith(MockitoExtension.class) -@NonNullByDefault(value = { DefaultLocation.PARAMETER, DefaultLocation.RETURN_TYPE }) +@NonNullByDefault public class InfluxDBPersistenceServiceTest { - private InfluxDBPersistenceService instance; + private static final Map VALID_V1_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", + VERSION_PARAM, InfluxDBVersion.V1.name(), USER_PARAM, "user", PASSWORD_PARAM, "password", DATABASE_PARAM, + "openhab", RETENTION_POLICY_PARAM, "default"); - private @Mock InfluxDBRepository influxDBRepository; + private static final Map VALID_V2_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", + VERSION_PARAM, InfluxDBVersion.V2.name(), TOKEN_PARAM, "sampletoken", DATABASE_PARAM, "openhab", + RETENTION_POLICY_PARAM, "default"); - private Map validConfig; - private Map invalidConfig; + private static final Map INVALID_V1_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", + VERSION_PARAM, InfluxDBVersion.V1.name(), USER_PARAM, "user", DATABASE_PARAM, "openhab", + RETENTION_POLICY_PARAM, "default"); - @BeforeEach - public void before() { - instance = new InfluxDBPersistenceService(mock(ItemRegistry.class), mock(MetadataRegistry.class)) { - @Override - protected InfluxDBRepository createInfluxDBRepository() { - return influxDBRepository; - } - }; + private static final Map INVALID_V2_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", + VERSION_PARAM, InfluxDBVersion.V2.name(), DATABASE_PARAM, "openhab", RETENTION_POLICY_PARAM, "default"); - validConfig = ConfigurationTestHelper.createValidConfigurationParameters(); - invalidConfig = ConfigurationTestHelper.createInvalidConfigurationParameters(); - } + @Mock + private @NonNullByDefault({}) InfluxDBRepository influxDBRepository; + + private final InfluxDBMetadataService influxDBMetadataService = new InfluxDBMetadataService( + mock(MetadataRegistry.class)); - @AfterEach - public void after() { - validConfig = null; - invalidConfig = null; - instance = null; - influxDBRepository = null; + @Test + public void activateWithValidV1ConfigShouldConnectRepository() { + getService(VALID_V1_CONFIGURATION); + verify(influxDBRepository).connect(); } @Test - public void activateWithValidConfigShouldConnectRepository() { - instance.activate(validConfig); + public void activateWithValidV2ConfigShouldConnectRepository() { + getService(VALID_V2_CONFIGURATION); verify(influxDBRepository).connect(); } @Test - public void activateWithInvalidConfigShouldNotConnectRepository() { - instance.activate(invalidConfig); - verify(influxDBRepository, never()).connect(); + public void activateWithInvalidV1ConfigShouldFail() { + assertThrows(IllegalArgumentException.class, () -> getService(INVALID_V1_CONFIGURATION)); } @Test - public void activateWithNullConfigShouldNotConnectRepository() { - instance.activate(null); - verify(influxDBRepository, never()).connect(); + public void activateWithInvalidV2ShouldFail() { + assertThrows(IllegalArgumentException.class, () -> getService(INVALID_V2_CONFIGURATION)); } @Test public void deactivateShouldDisconnectRepository() { - instance.activate(validConfig); + InfluxDBPersistenceService instance = getService(VALID_V2_CONFIGURATION); instance.deactivate(); verify(influxDBRepository).disconnect(); } @Test - public void storeItemWithConnectedRepository() { - instance.activate(validConfig); + public void storeItemWithConnectedRepository() throws UnexpectedConditionException { + InfluxDBPersistenceService instance = getService(VALID_V2_CONFIGURATION); when(influxDBRepository.isConnected()).thenReturn(true); instance.store(ItemTestHelper.createNumberItem("number", 5)); verify(influxDBRepository).write(any()); } @Test - public void storeItemWithDisconnectedRepositoryIsIgnored() { - instance.activate(validConfig); + public void storeItemWithDisconnectedRepositoryIsIgnored() throws UnexpectedConditionException { + InfluxDBPersistenceService instance = getService(VALID_V2_CONFIGURATION); when(influxDBRepository.isConnected()).thenReturn(false); instance.store(ItemTestHelper.createNumberItem("number", 5)); verify(influxDBRepository, never()).write(any()); } + + private InfluxDBPersistenceService getService(Map config) { + return new InfluxDBPersistenceService(mock(ItemRegistry.class), influxDBMetadataService, config) { + @Override + protected Optional createInfluxDBRepository() { + return Optional.of(influxDBRepository); + } + }; + } } 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 f81339859e922..837001ba064a6 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 @@ -36,8 +36,8 @@ import org.openhab.core.library.types.PercentType; import org.openhab.core.persistence.FilterCriteria; import org.openhab.persistence.influxdb.InfluxDBPersistenceService; -import org.openhab.persistence.influxdb.internal.influx1.Influx1FilterCriteriaQueryCreatorImpl; -import org.openhab.persistence.influxdb.internal.influx2.Influx2FilterCriteriaQueryCreatorImpl; +import org.openhab.persistence.influxdb.internal.influx1.InfluxDB1FilterCriteriaQueryCreatorImpl; +import org.openhab.persistence.influxdb.internal.influx2.InfluxDB2FilterCriteriaQueryCreatorImpl; /** * @author Joan Pujol Espinar - Initial contribution @@ -54,13 +54,14 @@ public class InfluxFilterCriteriaQueryCreatorImplTest { private @Mock InfluxDBConfiguration influxDBConfiguration; private @Mock MetadataRegistry metadataRegistry; - private Influx1FilterCriteriaQueryCreatorImpl instanceV1; - private Influx2FilterCriteriaQueryCreatorImpl instanceV2; + private InfluxDB1FilterCriteriaQueryCreatorImpl instanceV1; + private InfluxDB2FilterCriteriaQueryCreatorImpl instanceV2; @BeforeEach public void before() { - instanceV1 = new Influx1FilterCriteriaQueryCreatorImpl(influxDBConfiguration, metadataRegistry); - instanceV2 = new Influx2FilterCriteriaQueryCreatorImpl(influxDBConfiguration, metadataRegistry); + InfluxDBMetadataService influxDBMetadataService = new InfluxDBMetadataService(metadataRegistry); + instanceV1 = new InfluxDB1FilterCriteriaQueryCreatorImpl(influxDBConfiguration, influxDBMetadataService); + instanceV2 = new InfluxDB2FilterCriteriaQueryCreatorImpl(influxDBConfiguration, influxDBMetadataService); } @AfterEach @@ -72,7 +73,7 @@ public void after() { } @Test - public void testSimpleItemQueryWithoutParams() { + public void testSimpleItemQueryWithoutParams() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); String queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); @@ -86,7 +87,7 @@ public void testSimpleItemQueryWithoutParams() { } @Test - public void testSimpleUnboundedItemWithoutParams() { + public void testSimpleUnboundedItemWithoutParams() throws UnexpectedConditionException { FilterCriteria criteria = new FilterCriteria(); criteria.setOrdering(null); @@ -98,7 +99,7 @@ public void testSimpleUnboundedItemWithoutParams() { } @Test - public void testRangeCriteria() { + public void testRangeCriteria() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime tomorrow = now.plus(1, ChronoUnit.DAYS); @@ -121,7 +122,7 @@ public void testRangeCriteria() { } @Test - public void testValueOperator() { + public void testValueOperator() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); criteria.setOperator(FilterCriteria.Operator.LTE); criteria.setState(new PercentType(90)); @@ -139,7 +140,7 @@ public void testValueOperator() { } @Test - public void testPagination() { + public void testPagination() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); criteria.setPageNumber(2); criteria.setPageSize(10); @@ -155,7 +156,7 @@ public void testPagination() { } @Test - public void testOrdering() { + public void testOrdering() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); criteria.setOrdering(FilterCriteria.Ordering.ASCENDING); @@ -168,11 +169,12 @@ public void testOrdering() { equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])\n\t" + + "|> sort(desc:false, columns:[\"_time\"])")); } @Test - public void testPreviousState() { + public void testPreviousState() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); criteria.setOrdering(FilterCriteria.Ordering.DESCENDING); criteria.setPageSize(1); @@ -195,7 +197,7 @@ private FilterCriteria createBaseCriteria(String sampleItem) { } @Test - public void testMeasurementNameFromMetadata() { + public void testMeasurementNameFromMetadata() throws UnexpectedConditionException { FilterCriteria criteria = createBaseCriteria(); MetadataKey metadataKey = new MetadataKey(InfluxDBPersistenceService.SERVICE_NAME, "sampleItem"); @@ -212,7 +214,6 @@ public void testMeasurementNameFromMetadata() { + "|> filter(fn: (r) => r[\"_measurement\"] == \"measurementName\")\n\t" + "|> filter(fn: (r) => r[\"item\"] == \"sampleItem\")\n\t" + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\", \"item\"])")); - when(metadataRegistry.get(metadataKey)) .thenReturn(new Metadata(metadataKey, "", Map.of("key1", "val1", "key2", "val2"))); diff --git a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreatorTest.java b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreatorTest.java index 2f03389ea0d31..364489c5e3361 100644 --- a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreatorTest.java +++ b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreatorTest.java @@ -23,6 +23,7 @@ import org.eclipse.jdt.annotation.DefaultLocation; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -40,7 +41,6 @@ * @author Joan Pujol Espinar - Initial contribution */ @ExtendWith(MockitoExtension.class) -@SuppressWarnings("null") // In case of any NPE it will cause test fail that it's the expected result @NonNullByDefault(value = { DefaultLocation.PARAMETER, DefaultLocation.RETURN_TYPE }) public class ItemToStorePointCreatorTest { @@ -50,12 +50,13 @@ public class ItemToStorePointCreatorTest { @BeforeEach public void before() { + InfluxDBMetadataService influxDBMetadataService = new InfluxDBMetadataService(metadataRegistry); when(influxDBConfiguration.isAddCategoryTag()).thenReturn(false); when(influxDBConfiguration.isAddLabelTag()).thenReturn(false); when(influxDBConfiguration.isAddTypeTag()).thenReturn(false); when(influxDBConfiguration.isReplaceUnderscore()).thenReturn(false); - instance = new ItemToStorePointCreator(influxDBConfiguration, metadataRegistry); + instance = new ItemToStorePointCreator(influxDBConfiguration, influxDBMetadataService); } @AfterEach @@ -71,11 +72,17 @@ public void convertBasicItem(Number number) { NumberItem item = ItemTestHelper.createNumberItem("myitem", number); InfluxPoint point = instance.convert(item, null); + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getMeasurementName(), equalTo(item.getName())); assertThat("Must Store item name", point.getTags(), hasEntry("item", item.getName())); assertThat(point.getValue(), equalTo(new BigDecimal(number.toString()))); } + @SuppressWarnings("unused") private static Stream convertBasicItem() { return Stream.of(5, 5.5, 5L); } @@ -84,6 +91,12 @@ private static Stream convertBasicItem() { public void shouldUseAliasAsMeasurementNameIfProvided() { NumberItem item = ItemTestHelper.createNumberItem("myitem", 5); InfluxPoint point = instance.convert(item, "aliasName"); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getMeasurementName(), is("aliasName")); } @@ -94,10 +107,22 @@ public void shouldStoreCategoryTagIfProvidedAndConfigured() { when(influxDBConfiguration.isAddCategoryTag()).thenReturn(true); InfluxPoint point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), hasEntry(InfluxDBConstants.TAG_CATEGORY_NAME, "categoryValue")); when(influxDBConfiguration.isAddCategoryTag()).thenReturn(false); point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), not(hasKey(InfluxDBConstants.TAG_CATEGORY_NAME))); } @@ -107,10 +132,22 @@ public void shouldStoreTypeTagIfProvidedAndConfigured() { when(influxDBConfiguration.isAddTypeTag()).thenReturn(true); InfluxPoint point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), hasEntry(InfluxDBConstants.TAG_TYPE_NAME, "Number")); when(influxDBConfiguration.isAddTypeTag()).thenReturn(false); point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), not(hasKey(InfluxDBConstants.TAG_TYPE_NAME))); } @@ -121,10 +158,22 @@ public void shouldStoreTypeLabelIfProvidedAndConfigured() { when(influxDBConfiguration.isAddLabelTag()).thenReturn(true); InfluxPoint point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), hasEntry(InfluxDBConstants.TAG_LABEL_NAME, "ItemLabel")); when(influxDBConfiguration.isAddLabelTag()).thenReturn(false); point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), not(hasKey(InfluxDBConstants.TAG_LABEL_NAME))); } @@ -137,6 +186,12 @@ public void shouldStoreMetadataAsTagsIfProvided() { .thenReturn(new Metadata(metadataKey, "", Map.of("key1", "val1", "key2", "val2"))); InfluxPoint point = instance.convert(item, null); + + if (point == null) { + Assertions.fail("'point' is null"); + return; + } + assertThat(point.getTags(), hasEntry("key1", "val1")); assertThat(point.getTags(), hasEntry("key2", "val2")); } @@ -147,9 +202,17 @@ public void shouldUseMeasurementNameFromMetadataIfProvided() { MetadataKey metadataKey = new MetadataKey(InfluxDBPersistenceService.SERVICE_NAME, item.getName()); InfluxPoint point = instance.convert(item, null); + if (point == null) { + Assertions.fail(); + return; + } assertThat(point.getMeasurementName(), equalTo(item.getName())); point = instance.convert(item, null); + if (point == null) { + Assertions.fail(); + return; + } assertThat(point.getMeasurementName(), equalTo(item.getName())); assertThat(point.getTags(), hasEntry("item", item.getName())); @@ -157,6 +220,10 @@ public void shouldUseMeasurementNameFromMetadataIfProvided() { .thenReturn(new Metadata(metadataKey, "measurementName", Map.of("key1", "val1", "key2", "val2"))); point = instance.convert(item, null); + if (point == null) { + Assertions.fail(); + return; + } assertThat(point.getMeasurementName(), equalTo("measurementName")); assertThat(point.getTags(), hasEntry("item", item.getName())); @@ -164,6 +231,10 @@ public void shouldUseMeasurementNameFromMetadataIfProvided() { .thenReturn(new Metadata(metadataKey, "", Map.of("key1", "val1", "key2", "val2"))); point = instance.convert(item, null); + if (point == null) { + Assertions.fail(); + return; + } assertThat(point.getMeasurementName(), equalTo(item.getName())); assertThat(point.getTags(), hasEntry("item", item.getName())); } From 96d9c375b20a4462b6ed2a4a23f08067f9d52894 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Tue, 31 Jan 2023 19:59:58 +0100 Subject: [PATCH 2/4] further improvements Signed-off-by: Jan N. Klug --- .../influxdb/InfluxDBPersistenceService.java | 43 +++----- .../internal/FilterCriteriaQueryCreator.java | 3 +- .../internal/InfluxDBConfiguration.java | 43 +++----- .../internal/InfluxDBHistoricItem.java | 13 +-- .../internal/InfluxDBMetadataUtils.java | 51 --------- .../influxdb/internal/InfluxPoint.java | 12 +-- ...fluxDB1FilterCriteriaQueryCreatorImpl.java | 5 +- ...fluxDB2FilterCriteriaQueryCreatorImpl.java | 3 +- .../InfluxDBPersistenceServiceTest.java | 52 +++++---- ...luxFilterCriteriaQueryCreatorImplTest.java | 102 +++++++++--------- 10 files changed, 128 insertions(+), 199 deletions(-) delete mode 100644 bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataUtils.java 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 798f821026fb5..db4476c0e17c2 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 @@ -17,7 +17,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -101,8 +100,7 @@ public InfluxDBPersistenceService(final @Reference ItemRegistry itemRegistry, this.influxDBMetadataService = influxDBMetadataService; this.configuration = new InfluxDBConfiguration(config); if (configuration.isValid()) { - this.influxDBRepository = createInfluxDBRepository() - .orElseThrow(() -> new IllegalArgumentException("Failed to instantiate repository.")); + this.influxDBRepository = createInfluxDBRepository(); this.influxDBRepository.connect(); this.itemToStorePointCreator = new ItemToStorePointCreator(configuration, influxDBMetadataService); tryReconnection = true; @@ -114,15 +112,12 @@ public InfluxDBPersistenceService(final @Reference ItemRegistry itemRegistry, } // Visible for testing - protected Optional createInfluxDBRepository() { - InfluxDBRepository influxDBRepository = null; - switch (configuration.getVersion()) { - case V1 -> influxDBRepository = new InfluxDB1RepositoryImpl(configuration, influxDBMetadataService); - case V2 -> influxDBRepository = new InfluxDB2RepositoryImpl(configuration, influxDBMetadataService); - default -> { - } - } - return Optional.ofNullable(influxDBRepository); + protected InfluxDBRepository createInfluxDBRepository() throws IllegalArgumentException { + return switch (configuration.getVersion()) { + case V1 -> new InfluxDB1RepositoryImpl(configuration, influxDBMetadataService); + case V2 -> new InfluxDB2RepositoryImpl(configuration, influxDBMetadataService); + default -> throw new IllegalArgumentException("Failed to instantiate repository."); + }; } /** @@ -176,32 +171,24 @@ public void store(Item item, @Nullable String alias) { logger.trace("Ignoring item {}, conversion to a InfluxDB point failed.", item); } } else { - logger.debug("store ignored, InfluxDB is not yet connected"); + logger.debug("store ignored, InfluxDB is not connected"); } } @Override public Iterable query(FilterCriteria filter) { - logger.debug("Got a query for historic points!"); - if (checkConnection()) { logger.trace( - "Filter: itemname: {}, ordering: {}, state: {}, operator: {}, getBeginDate: {}, getEndDate: {}, getPageSize: {}, getPageNumber: {}", + "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()); - try { - String query = influxDBRepository.createQueryCreator().createQuery(filter, - configuration.getRetentionPolicy()); - - logger.trace("Query {}", query); - List results = influxDBRepository.query(query); - return results.stream().map(this::mapRowToHistoricItem).collect(Collectors.toList()); - } catch (UnexpectedConditionException e) { - logger.warn("Failed to create query:{}", e.getMessage()); - return List.of(); - } + String query = influxDBRepository.createQueryCreator().createQuery(filter, + configuration.getRetentionPolicy()); + logger.trace("Query {}", query); + List results = influxDBRepository.query(query); + return results.stream().map(this::mapRowToHistoricItem).collect(Collectors.toList()); } else { - logger.debug("query ignored, InfluxDB is not yet connected"); + logger.debug("Query for persisted data ignored, InfluxDB is not connected"); return List.of(); } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java index 3ab9f1867c8e5..1f72a470e3b99 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/FilterCriteriaQueryCreator.java @@ -28,9 +28,8 @@ public interface FilterCriteriaQueryCreator { * @param criteria Criteria to create query from * @param retentionPolicy Name of the retentionPolicy/bucket to use in query * @return Created query as a String - * @throws UnexpectedConditionException when an error occurs during query creation */ - String createQuery(FilterCriteria criteria, String retentionPolicy) throws UnexpectedConditionException; + String createQuery(FilterCriteria criteria, String retentionPolicy); default String getOperationSymbol(FilterCriteria.Operator operator, InfluxDBVersion version) { return switch (operator) { diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java index 7d5b9d712642b..a779806336cca 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBConfiguration.java @@ -18,6 +18,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.config.core.ConfigParser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,28 +54,17 @@ public class InfluxDBConfiguration { private final boolean addLabelTag; public InfluxDBConfiguration(Map config) { - url = (String) config.getOrDefault(URL_PARAM, "http://127.0.0.1:8086"); - user = (String) config.getOrDefault(USER_PARAM, "openhab"); - password = (String) config.getOrDefault(PASSWORD_PARAM, ""); - token = (String) config.getOrDefault(TOKEN_PARAM, ""); - databaseName = (String) config.getOrDefault(DATABASE_PARAM, "openhab"); - retentionPolicy = (String) config.getOrDefault(RETENTION_POLICY_PARAM, "autogen"); + url = ConfigParser.valueAsOrElse(config.get(URL_PARAM), String.class, "http://127.0.0.1:8086"); + user = ConfigParser.valueAsOrElse(config.get(USER_PARAM), String.class, "openhab"); + password = ConfigParser.valueAsOrElse(config.get(PASSWORD_PARAM), String.class, ""); + token = ConfigParser.valueAsOrElse(config.get(TOKEN_PARAM), String.class, ""); + databaseName = ConfigParser.valueAsOrElse(config.get(DATABASE_PARAM), String.class, "openhab"); + retentionPolicy = ConfigParser.valueAsOrElse(config.get(RETENTION_POLICY_PARAM), String.class, "autogen"); version = parseInfluxVersion((String) config.getOrDefault(VERSION_PARAM, InfluxDBVersion.V1.name())); - replaceUnderscore = getConfigBooleanValue(config, REPLACE_UNDERSCORE_PARAM, false); - addCategoryTag = getConfigBooleanValue(config, ADD_CATEGORY_TAG_PARAM, false); - addLabelTag = getConfigBooleanValue(config, ADD_LABEL_TAG_PARAM, false); - addTypeTag = getConfigBooleanValue(config, ADD_TYPE_TAG_PARAM, false); - } - - private static boolean getConfigBooleanValue(Map config, String key, boolean defaultValue) { - Object object = config.get(key); - if (object instanceof Boolean) { - return (Boolean) object; - } else if (object instanceof String) { - return "true".equalsIgnoreCase((String) object); - } else { - return defaultValue; - } + replaceUnderscore = ConfigParser.valueAsOrElse(config.get(REPLACE_UNDERSCORE_PARAM), Boolean.class, false); + addCategoryTag = ConfigParser.valueAsOrElse(config.get(ADD_CATEGORY_TAG_PARAM), Boolean.class, false); + addLabelTag = ConfigParser.valueAsOrElse(config.get(ADD_LABEL_TAG_PARAM), Boolean.class, false); + addTypeTag = ConfigParser.valueAsOrElse(config.get(ADD_TYPE_TAG_PARAM), Boolean.class, false); } private InfluxDBVersion parseInfluxVersion(@Nullable String value) { @@ -167,11 +157,10 @@ public InfluxDBVersion getVersion() { @Override public String toString() { - String sb = "InfluxDBConfiguration{" + "url='" + url + '\'' + ", user='" + user + '\'' + ", password='" - + password.length() + " chars" + '\'' + ", token='" + token.length() + " chars" + '\'' - + ", databaseName='" + databaseName + '\'' + ", retentionPolicy='" + retentionPolicy + '\'' - + ", version=" + version + ", replaceUnderscore=" + replaceUnderscore + ", addCategoryTag=" - + addCategoryTag + ", addTypeTag=" + addTypeTag + ", addLabelTag=" + addLabelTag + '}'; - return sb; + return "InfluxDBConfiguration{url='" + url + "', user='" + user + "', password='" + password.length() + + " chars', token='" + token.length() + " chars', databaseName='" + databaseName + + "', retentionPolicy='" + retentionPolicy + "', version=" + version + ", replaceUnderscore=" + + replaceUnderscore + ", addCategoryTag=" + addCategoryTag + ", addTypeTag=" + addTypeTag + + ", addLabelTag=" + addLabelTag + '}'; } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBHistoricItem.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBHistoricItem.java index 9dd3a9f2c056b..3e00466e5e02f 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBHistoricItem.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBHistoricItem.java @@ -18,7 +18,6 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.persistence.HistoricItem; import org.openhab.core.types.State; -import org.openhab.core.types.UnDefType; /** * Java bean used to return items queries results from InfluxDB. @@ -30,8 +29,8 @@ public class InfluxDBHistoricItem implements HistoricItem { private String name = ""; - private State state = UnDefType.NULL; - private ZonedDateTime timestamp; + private final State state; + private final ZonedDateTime timestamp; public InfluxDBHistoricItem(String name, State state, ZonedDateTime timestamp) { this.name = name; @@ -53,19 +52,11 @@ public State getState() { return state; } - public void setState(State state) { - this.state = state; - } - @Override public ZonedDateTime getTimestamp() { return timestamp; } - public void setTimestamp(ZonedDateTime timestamp) { - this.timestamp = timestamp; - } - @Override public String toString() { return DateFormat.getDateTimeInstance().format(timestamp) + ": " + name + " -> " + state.toString(); diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataUtils.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataUtils.java deleted file mode 100644 index 7a101541124ba..0000000000000 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBMetadataUtils.java +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Copyright (c) 2010-2023 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * 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.openhab.persistence.influxdb.internal; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; -import org.openhab.core.items.Metadata; -import org.openhab.core.items.MetadataKey; -import org.openhab.core.items.MetadataRegistry; -import org.openhab.persistence.influxdb.InfluxDBPersistenceService; - -/** - * Logic to use items metadata from an openHAB {@link Item} - * - * @author Johannes Ott - Initial contribution - */ -@NonNullByDefault -public class InfluxDBMetadataUtils { - - private InfluxDBMetadataUtils() { - } - - public static String calculateMeasurementNameFromMetadataIfPresent( - final @Nullable MetadataRegistry currentMetadataRegistry, String name, @Nullable String itemName) { - - if (itemName == null || currentMetadataRegistry == null) { - return name; - } - - MetadataKey key = new MetadataKey(InfluxDBPersistenceService.SERVICE_NAME, itemName); - Metadata metadata = currentMetadataRegistry.get(key); - if (metadata != null) { - String metaName = metadata.getValue(); - if (!metaName.isBlank()) { - name = metaName; - } - } - - return name; - } -} diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java index 63bf15e05e5eb..e6e245ee3affe 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxPoint.java @@ -27,10 +27,10 @@ */ @NonNullByDefault({ DefaultLocation.PARAMETER }) public class InfluxPoint { - private String measurementName; - private Instant time; - private Object value; - private Map tags; + private final String measurementName; + private final Instant time; + private final Object value; + private final Map tags; private InfluxPoint(Builder builder) { measurementName = builder.measurementName; @@ -60,10 +60,10 @@ public Map getTags() { } public static final class Builder { - private String measurementName; + private final String measurementName; private Instant time; private Object value; - private Map tags = new HashMap<>(); + private final Map tags = new HashMap<>(); private Builder(String measurementName) { this.measurementName = measurementName; 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 7af2b5cf7ae80..99620776d7a9f 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 @@ -28,7 +28,6 @@ import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBVersion; -import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; /** * Implementation of {@link FilterCriteriaQueryCreator} for InfluxDB 1.0 @@ -48,7 +47,7 @@ public InfluxDB1FilterCriteriaQueryCreatorImpl(InfluxDBConfiguration configurati } @Override - public String createQuery(FilterCriteria criteria, String retentionPolicy) throws UnexpectedConditionException { + public String createQuery(FilterCriteria criteria, String retentionPolicy) { final String itemName = criteria.getItemName(); final String tableName = getTableName(itemName); final boolean hasCriteriaName = itemName != null; @@ -83,7 +82,7 @@ public String createQuery(FilterCriteria criteria, String retentionPolicy) throw if (criteria.getPageSize() != Integer.MAX_VALUE) { if (criteria.getPageNumber() != 0) { - select = select.limit(criteria.getPageSize(), criteria.getPageSize() * criteria.getPageNumber()); + select = select.limit(criteria.getPageSize(), (long) criteria.getPageSize() * criteria.getPageNumber()); } else { select = select.limit(criteria.getPageSize()); } 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 3157c9d0d6928..71a0804469485 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 @@ -25,7 +25,6 @@ import org.openhab.persistence.influxdb.internal.InfluxDBConfiguration; import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBVersion; -import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import com.influxdb.query.dsl.Flux; import com.influxdb.query.dsl.functions.RangeFlux; @@ -48,7 +47,7 @@ public InfluxDB2FilterCriteriaQueryCreatorImpl(InfluxDBConfiguration configurati } @Override - public String createQuery(FilterCriteria criteria, String retentionPolicy) throws UnexpectedConditionException { + public String createQuery(FilterCriteria criteria, String retentionPolicy) { Flux flux = Flux.from(retentionPolicy); RangeFlux range = flux.range(); diff --git a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java index c5749d9c8c878..05ead8fb55e0d 100644 --- a/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java +++ b/bundles/org.openhab.persistence.influxdb/src/test/java/org/openhab/persistence/influxdb/internal/InfluxDBPersistenceServiceTest.java @@ -24,7 +24,6 @@ import static org.openhab.persistence.influxdb.internal.InfluxDBConfiguration.VERSION_PARAM; import java.util.Map; -import java.util.Optional; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; @@ -41,23 +40,34 @@ @ExtendWith(MockitoExtension.class) @NonNullByDefault public class InfluxDBPersistenceServiceTest { - private static final Map VALID_V1_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", - VERSION_PARAM, InfluxDBVersion.V1.name(), USER_PARAM, "user", PASSWORD_PARAM, "password", DATABASE_PARAM, - "openhab", RETENTION_POLICY_PARAM, "default"); + private static final Map VALID_V1_CONFIGURATION = Map.of( // + URL_PARAM, "http://localhost:8086", // + VERSION_PARAM, InfluxDBVersion.V1.name(), // + USER_PARAM, "user", PASSWORD_PARAM, "password", // + DATABASE_PARAM, "openhab", // + RETENTION_POLICY_PARAM, "default"); - private static final Map VALID_V2_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", - VERSION_PARAM, InfluxDBVersion.V2.name(), TOKEN_PARAM, "sampletoken", DATABASE_PARAM, "openhab", + private static final Map VALID_V2_CONFIGURATION = Map.of( // + URL_PARAM, "http://localhost:8086", // + VERSION_PARAM, InfluxDBVersion.V2.name(), // + TOKEN_PARAM, "sampletoken", // + DATABASE_PARAM, "openhab", // RETENTION_POLICY_PARAM, "default"); - private static final Map INVALID_V1_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", - VERSION_PARAM, InfluxDBVersion.V1.name(), USER_PARAM, "user", DATABASE_PARAM, "openhab", + private static final Map INVALID_V1_CONFIGURATION = Map.of(// + URL_PARAM, "http://localhost:8086", // + VERSION_PARAM, InfluxDBVersion.V1.name(), // + USER_PARAM, "user", // + DATABASE_PARAM, "openhab", // RETENTION_POLICY_PARAM, "default"); - private static final Map INVALID_V2_CONFIGURATION = Map.of(URL_PARAM, "http://localhost:8086", - VERSION_PARAM, InfluxDBVersion.V2.name(), DATABASE_PARAM, "openhab", RETENTION_POLICY_PARAM, "default"); + private static final Map INVALID_V2_CONFIGURATION = Map.of( // + URL_PARAM, "http://localhost:8086", // + VERSION_PARAM, InfluxDBVersion.V2.name(), // + DATABASE_PARAM, "openhab", // + RETENTION_POLICY_PARAM, "default"); - @Mock - private @NonNullByDefault({}) InfluxDBRepository influxDBRepository; + private @Mock @NonNullByDefault({}) InfluxDBRepository influxDBRepositoryMock; private final InfluxDBMetadataService influxDBMetadataService = new InfluxDBMetadataService( mock(MetadataRegistry.class)); @@ -65,13 +75,13 @@ public class InfluxDBPersistenceServiceTest { @Test public void activateWithValidV1ConfigShouldConnectRepository() { getService(VALID_V1_CONFIGURATION); - verify(influxDBRepository).connect(); + verify(influxDBRepositoryMock).connect(); } @Test public void activateWithValidV2ConfigShouldConnectRepository() { getService(VALID_V2_CONFIGURATION); - verify(influxDBRepository).connect(); + verify(influxDBRepositoryMock).connect(); } @Test @@ -88,30 +98,30 @@ public void activateWithInvalidV2ShouldFail() { public void deactivateShouldDisconnectRepository() { InfluxDBPersistenceService instance = getService(VALID_V2_CONFIGURATION); instance.deactivate(); - verify(influxDBRepository).disconnect(); + verify(influxDBRepositoryMock).disconnect(); } @Test public void storeItemWithConnectedRepository() throws UnexpectedConditionException { InfluxDBPersistenceService instance = getService(VALID_V2_CONFIGURATION); - when(influxDBRepository.isConnected()).thenReturn(true); + when(influxDBRepositoryMock.isConnected()).thenReturn(true); instance.store(ItemTestHelper.createNumberItem("number", 5)); - verify(influxDBRepository).write(any()); + verify(influxDBRepositoryMock).write(any()); } @Test public void storeItemWithDisconnectedRepositoryIsIgnored() throws UnexpectedConditionException { InfluxDBPersistenceService instance = getService(VALID_V2_CONFIGURATION); - when(influxDBRepository.isConnected()).thenReturn(false); + when(influxDBRepositoryMock.isConnected()).thenReturn(false); instance.store(ItemTestHelper.createNumberItem("number", 5)); - verify(influxDBRepository, never()).write(any()); + verify(influxDBRepositoryMock, never()).write(any()); } private InfluxDBPersistenceService getService(Map config) { return new InfluxDBPersistenceService(mock(ItemRegistry.class), influxDBMetadataService, config) { @Override - protected Optional createInfluxDBRepository() { - return Optional.of(influxDBRepository); + protected InfluxDBRepository createInfluxDBRepository() { + return influxDBRepositoryMock; } }; } 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 837001ba064a6..79f1e504e65fe 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 @@ -73,21 +73,22 @@ public void after() { } @Test - public void testSimpleItemQueryWithoutParams() throws UnexpectedConditionException { + public void testSimpleItemQueryWithoutParams() { FilterCriteria criteria = createBaseCriteria(); String queryV1 = instanceV1.createQuery(criteria, RETENTION_POLICY); assertThat(queryV1, equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\";")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, - equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-100y) + \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") + \t|> keep(columns:["_measurement", "_time", "_value"])""")); } @Test - public void testSimpleUnboundedItemWithoutParams() throws UnexpectedConditionException { + public void testSimpleUnboundedItemWithoutParams() { FilterCriteria criteria = new FilterCriteria(); criteria.setOrdering(null); @@ -99,7 +100,7 @@ public void testSimpleUnboundedItemWithoutParams() throws UnexpectedConditionExc } @Test - public void testRangeCriteria() throws UnexpectedConditionException { + public void testRangeCriteria() { FilterCriteria criteria = createBaseCriteria(); ZonedDateTime now = ZonedDateTime.now(); ZonedDateTime tomorrow = now.plus(1, ChronoUnit.DAYS); @@ -113,16 +114,17 @@ public void testRangeCriteria() throws UnexpectedConditionException { assertThat(queryV1, equalTo(expectedQueryV1)); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - String expectedQueryV2 = String.format( - "from(bucket:\"origin\")\n\t" + "|> range(start:%s, stop:%s)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])", + String expectedQueryV2 = String.format(""" + 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())); assertThat(queryV2, equalTo(expectedQueryV2)); } @Test - public void testValueOperator() throws UnexpectedConditionException { + public void testValueOperator() { FilterCriteria criteria = createBaseCriteria(); criteria.setOperator(FilterCriteria.Operator.LTE); criteria.setState(new PercentType(90)); @@ -132,15 +134,16 @@ public void testValueOperator() throws UnexpectedConditionException { equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" WHERE value <= 90;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, - equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])\n\t" - + "|> filter(fn: (r) => (r[\"_field\"] == \"value\" and r[\"_value\"] <= 90))")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-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))""")); } @Test - public void testPagination() throws UnexpectedConditionException { + public void testPagination() { FilterCriteria criteria = createBaseCriteria(); criteria.setPageNumber(2); criteria.setPageSize(10); @@ -150,13 +153,16 @@ public void testPagination() throws UnexpectedConditionException { equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" LIMIT 10 OFFSET 20;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])\n\t" + "|> limit(n:10, offset:20)")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-100y) + \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") + \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> limit(n:10, offset:20)""")); } @Test - public void testOrdering() throws UnexpectedConditionException { + public void testOrdering() { FilterCriteria criteria = createBaseCriteria(); criteria.setOrdering(FilterCriteria.Ordering.ASCENDING); @@ -165,39 +171,37 @@ public void testOrdering() throws UnexpectedConditionException { equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\" ORDER BY time ASC;")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, - equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])\n\t" - - + "|> sort(desc:false, columns:[\"_time\"])")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-100y) + \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") + \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> sort(desc:false, columns:["_time"])""")); } @Test - public void testPreviousState() throws UnexpectedConditionException { + public void testPreviousState() { FilterCriteria criteria = createBaseCriteria(); criteria.setOrdering(FilterCriteria.Ordering.DESCENDING); criteria.setPageSize(1); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, - equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])\n\t" + "|> last()")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-100y) + \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") + \t|> keep(columns:["_measurement", "_time", "_value"]) + \t|> last()""")); } private FilterCriteria createBaseCriteria() { - return createBaseCriteria(ITEM_NAME); - } - - private FilterCriteria createBaseCriteria(String sampleItem) { FilterCriteria criteria = new FilterCriteria(); - criteria.setItemName(sampleItem); + criteria.setItemName(ITEM_NAME); criteria.setOrdering(null); return criteria; } @Test - public void testMeasurementNameFromMetadata() throws UnexpectedConditionException { + public void testMeasurementNameFromMetadata() { FilterCriteria criteria = createBaseCriteria(); MetadataKey metadataKey = new MetadataKey(InfluxDBPersistenceService.SERVICE_NAME, "sampleItem"); @@ -209,11 +213,12 @@ public void testMeasurementNameFromMetadata() throws UnexpectedConditionExceptio "SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"measurementName\" WHERE item = 'sampleItem';")); String queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, - equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"measurementName\")\n\t" - + "|> filter(fn: (r) => r[\"item\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\", \"item\"])")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-100y) + \t|> filter(fn: (r) => r["_measurement"] == "measurementName") + \t|> filter(fn: (r) => r["item"] == "sampleItem") + \t|> keep(columns:["_measurement", "_time", "_value", "item"])""")); when(metadataRegistry.get(metadataKey)) .thenReturn(new Metadata(metadataKey, "", Map.of("key1", "val1", "key2", "val2"))); @@ -221,9 +226,10 @@ public void testMeasurementNameFromMetadata() throws UnexpectedConditionExceptio assertThat(queryV1, equalTo("SELECT \"value\"::field,\"item\"::tag FROM \"origin\".\"sampleItem\";")); queryV2 = instanceV2.createQuery(criteria, RETENTION_POLICY); - assertThat(queryV2, - equalTo("from(bucket:\"origin\")\n\t" + "|> range(start:-100y)\n\t" - + "|> filter(fn: (r) => r[\"_measurement\"] == \"sampleItem\")\n\t" - + "|> keep(columns:[\"_measurement\", \"_time\", \"_value\"])")); + assertThat(queryV2, equalTo(""" + from(bucket:"origin") + \t|> range(start:-100y) + \t|> filter(fn: (r) => r["_measurement"] == "sampleItem") + \t|> keep(columns:["_measurement", "_time", "_value"])""")); } } From bfd8346e83121380bd820947038bd8ea339ddb99 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Wed, 1 Feb 2023 18:41:26 +0100 Subject: [PATCH 3/4] finalize Signed-off-by: Jan N. Klug --- .../influxdb/InfluxDBPersistenceService.java | 17 ++++--- .../influxdb/internal/InfluxDBRepository.java | 6 ++- .../influxdb/internal/InfluxRow.java | 47 ------------------- .../influx1/InfluxDB1RepositoryImpl.java | 36 ++++++-------- ...fluxDB2FilterCriteriaQueryCreatorImpl.java | 13 +---- .../influx2/InfluxDB2RepositoryImpl.java | 39 +-------------- 6 files changed, 31 insertions(+), 127 deletions(-) delete mode 100644 bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java 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 db4476c0e17c2..18671d18a2775 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 @@ -40,7 +40,6 @@ import org.openhab.persistence.influxdb.internal.InfluxDBRepository; import org.openhab.persistence.influxdb.internal.InfluxDBStateConvertUtils; import org.openhab.persistence.influxdb.internal.InfluxPoint; -import org.openhab.persistence.influxdb.internal.InfluxRow; import org.openhab.persistence.influxdb.internal.ItemToStorePointCreator; import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import org.openhab.persistence.influxdb.internal.influx1.InfluxDB1RepositoryImpl; @@ -125,9 +124,9 @@ protected InfluxDBRepository createInfluxDBRepository() throws IllegalArgumentEx */ @Deactivate public void deactivate() { - logger.info("InfluxDB persistence service stopped."); - influxDBRepository.disconnect(); tryReconnection = false; + influxDBRepository.disconnect(); + logger.info("InfluxDB persistence service stopped."); } @Override @@ -146,7 +145,7 @@ public Set getItemInfo() { return influxDBRepository.getStoredItemsCount().entrySet().stream().map(InfluxDBPersistentItemInfo::new) .collect(Collectors.toUnmodifiableSet()); } else { - logger.info("getItemInfo ignored, InfluxDB is not yet connected"); + logger.info("getItemInfo ignored, InfluxDB is not connected"); return Set.of(); } } @@ -185,7 +184,7 @@ public Iterable query(FilterCriteria filter) { String query = influxDBRepository.createQueryCreator().createQuery(filter, configuration.getRetentionPolicy()); logger.trace("Query {}", query); - List results = influxDBRepository.query(query); + List results = influxDBRepository.query(query); return results.stream().map(this::mapRowToHistoricItem).collect(Collectors.toList()); } else { logger.debug("Query for persisted data ignored, InfluxDB is not connected"); @@ -193,10 +192,10 @@ public Iterable query(FilterCriteria filter) { } } - private HistoricItem mapRowToHistoricItem(InfluxRow row) { - State state = InfluxDBStateConvertUtils.objectToState(row.getValue(), row.getItemName(), itemRegistry); - return new InfluxDBHistoricItem(row.getItemName(), state, - ZonedDateTime.ofInstant(row.getTime(), ZoneId.systemDefault())); + private HistoricItem mapRowToHistoricItem(InfluxDBRepository.InfluxRow row) { + State state = InfluxDBStateConvertUtils.objectToState(row.value(), row.itemName(), itemRegistry); + return new InfluxDBHistoricItem(row.itemName(), state, + ZonedDateTime.ofInstant(row.time(), ZoneId.systemDefault())); } @Override diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java index a2c1e16e5f62d..efb749a8269e6 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxDBRepository.java @@ -12,6 +12,7 @@ */ package org.openhab.persistence.influxdb.internal; +import java.time.Instant; import java.util.List; import java.util.Map; @@ -51,7 +52,7 @@ public interface InfluxDBRepository { boolean checkConnectionStatus(); /** - * Return all stored item names with it's count of stored points + * Return all stored item names with its count of stored points * * @return Map with entries */ @@ -80,4 +81,7 @@ public interface InfluxDBRepository { * @return the query creator for this repository */ FilterCriteriaQueryCreator createQueryCreator(); + + record InfluxRow(Instant time, String itemName, Object value) { + } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java deleted file mode 100644 index 0f774b5118f42..0000000000000 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/InfluxRow.java +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Copyright (c) 2010-2023 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * 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.openhab.persistence.influxdb.internal; - -import java.time.Instant; - -import org.eclipse.jdt.annotation.NonNullByDefault; - -/** - * Row data returned from database query - * - * @author Joan Pujol Espinar - Initial contribution - */ -@NonNullByDefault -public class InfluxRow { - private final String itemName; - private final Instant time; - private final Object value; - - public InfluxRow(Instant time, String itemName, Object value) { - this.time = time; - this.itemName = itemName; - this.value = value; - } - - public Instant getTime() { - return time; - } - - public String getItemName() { - return itemName; - } - - public Object getValue() { - return value; - } -} diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java index d1d9c8482c6a2..dda799785e65d 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx1/InfluxDB1RepositoryImpl.java @@ -38,7 +38,6 @@ import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBRepository; import org.openhab.persistence.influxdb.internal.InfluxPoint; -import org.openhab.persistence.influxdb.internal.InfluxRow; import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -82,12 +81,15 @@ public boolean connect() { @Override public void disconnect() { + final InfluxDB currentClient = client; + if (currentClient != null) { + currentClient.close(); + } this.client = null; } @Override public boolean checkConnectionStatus() { - boolean dbStatus = false; final InfluxDB currentClient = client; if (currentClient != null) { try { @@ -95,26 +97,19 @@ public boolean checkConnectionStatus() { String version = pong.getVersion(); // may be check for version >= 0.9 if (version != null && !version.contains("unknown")) { - dbStatus = true; logger.debug("database status is OK, version is {}", version); + return true; } else { logger.warn("database ping error, version is: \"{}\" response time was \"{}\"", version, pong.getResponseTime()); - dbStatus = false; } } catch (RuntimeException e) { - dbStatus = false; - logger.error("database connection failed", e); - handleDatabaseException(e); + logger.warn("database error: {}", e.getMessage(), e); } } else { logger.warn("checkConnection: database is not connected"); } - return dbStatus; - } - - private void handleDatabaseException(Exception e) { - logger.warn("database error: {}", e.getMessage(), e); + return false; } @Override @@ -131,23 +126,20 @@ public void write(InfluxPoint point) throws UnexpectedConditionException { private Point convertPointToClientFormat(InfluxPoint point) throws UnexpectedConditionException { Point.Builder clientPoint = Point.measurement(point.getMeasurementName()).time(point.getTime().toEpochMilli(), TimeUnit.MILLISECONDS); - setPointValue(point.getValue(), clientPoint); - point.getTags().forEach(clientPoint::tag); - return clientPoint.build(); - } - - private void setPointValue(@Nullable Object value, Point.Builder point) throws UnexpectedConditionException { + Object value = point.getValue(); if (value instanceof String) { - point.addField(FIELD_VALUE_NAME, (String) value); + clientPoint.addField(FIELD_VALUE_NAME, (String) value); } else if (value instanceof Number) { - point.addField(FIELD_VALUE_NAME, (Number) value); + clientPoint.addField(FIELD_VALUE_NAME, (Number) value); } else if (value instanceof Boolean) { - point.addField(FIELD_VALUE_NAME, (Boolean) value); + clientPoint.addField(FIELD_VALUE_NAME, (Boolean) value); } else if (value == null) { - point.addField(FIELD_VALUE_NAME, "null"); + clientPoint.addField(FIELD_VALUE_NAME, "null"); } else { throw new UnexpectedConditionException("Not expected value type"); } + point.getTags().forEach(clientPoint::tag); + return clientPoint.build(); } @Override 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 71a0804469485..76c1528c549ff 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 @@ -63,7 +63,8 @@ public String createQuery(FilterCriteria criteria, String retentionPolicy) { String itemName = criteria.getItemName(); if (itemName != null) { - String measurementName = getMeasurementName(itemName); + 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)); @@ -105,14 +106,4 @@ private Flux applyOrderingAndPageSize(FilterCriteria criteria, Flux flux) { } return flux; } - - private String getMeasurementName(String itemName) { - String name = influxDBMetadataService.getMeasurementNameOrDefault(itemName, itemName); - - if (configuration.isReplaceUnderscore()) { - name = name.replace('_', '.'); - } - - return name; - } } diff --git a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java index 442585d94c0b2..045bc7577c81b 100644 --- a/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java +++ b/bundles/org.openhab.persistence.influxdb/src/main/java/org/openhab/persistence/influxdb/internal/influx2/InfluxDB2RepositoryImpl.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -31,7 +30,6 @@ import org.openhab.persistence.influxdb.internal.InfluxDBMetadataService; import org.openhab.persistence.influxdb.internal.InfluxDBRepository; import org.openhab.persistence.influxdb.internal.InfluxPoint; -import org.openhab.persistence.influxdb.internal.InfluxRow; import org.openhab.persistence.influxdb.internal.UnexpectedConditionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,21 +65,11 @@ public InfluxDB2RepositoryImpl(InfluxDBConfiguration configuration, this.influxDBMetadataService = influxDBMetadataService; } - /** - * Returns if the client has been successfully connected to server - * - * @return True if it's connected, otherwise false - */ @Override public boolean isConnected() { return client != null; } - /** - * Connect to InfluxDB server - * - * @return True if successful, otherwise false - */ @Override public boolean connect() { InfluxDBClientOptions.Builder optionsBuilder = InfluxDBClientOptions.builder().url(configuration.getUrl()) @@ -104,9 +92,6 @@ public boolean connect() { return checkConnectionStatus(); } - /** - * Disconnect from InfluxDB server - */ @Override public void disconnect() { final InfluxDBClient currentClient = this.client; @@ -116,11 +101,6 @@ public void disconnect() { this.client = null; } - /** - * Check if connection is currently ready - * - * @return True if its ready, otherwise false - */ @Override public boolean checkConnectionStatus() { final InfluxDBClient currentClient = client; @@ -170,28 +150,18 @@ private void setPointValue(@Nullable Object value, Point point) throws Unexpecte } } - /** - * Executes Flux query - * - * @param query Query - * @return Query results - */ @Override public List query(String query) { final QueryApi currentQueryAPI = queryAPI; if (currentQueryAPI != null) { List clientResult = currentQueryAPI.query(query); - return convertClientResutToRepository(clientResult); + return clientResult.stream().flatMap(this::mapRawResultToHistoric).toList(); } else { logger.warn("Returning empty list because queryAPI isn't present"); - return Collections.emptyList(); + return List.of(); } } - private List convertClientResutToRepository(List clientResult) { - return clientResult.stream().flatMap(this::mapRawResultToHistoric).collect(Collectors.toList()); - } - private Stream mapRawResultToHistoric(FluxTable rawRow) { return rawRow.getRecords().stream().map(r -> { String itemName = (String) r.getValueByKey(InfluxDBConstants.TAG_ITEM_NAME); @@ -201,11 +171,6 @@ private Stream mapRawResultToHistoric(FluxTable rawRow) { }); } - /** - * Return all stored item names with it's count of stored points - * - * @return Map with entries - */ @Override public Map getStoredItemsCount() { final QueryApi currentQueryAPI = queryAPI; From 60bb9ec27dd5f8914aa9055ea54e385b4bea6058 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Mon, 13 Feb 2023 14:38:45 +0100 Subject: [PATCH 4/4] address review comments Signed-off-by: Jan N. Klug --- bundles/org.openhab.persistence.influxdb/pom.xml | 4 ++-- .../persistence/influxdb/InfluxDBPersistenceService.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bundles/org.openhab.persistence.influxdb/pom.xml b/bundles/org.openhab.persistence.influxdb/pom.xml index 13095df4d6a4f..45456ef2717ed 100644 --- a/bundles/org.openhab.persistence.influxdb/pom.xml +++ b/bundles/org.openhab.persistence.influxdb/pom.xml @@ -71,7 +71,7 @@ com.google.code.gson gson - 2.8.5 + 2.9.1 io.gsonfire @@ -81,7 +81,7 @@ com.squareup.okio okio - 1.17.2 + 1.17.3 org.apache.commons 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 18671d18a2775..31dff022172b6 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 @@ -167,7 +167,7 @@ public void store(Item item, @Nullable String alias) { logger.warn("Failed to store item {} in InfluxDB point {}", point, item); } } else { - logger.trace("Ignoring item {}, conversion to a InfluxDB point failed.", item); + logger.trace("Ignoring item {}, conversion to an InfluxDB point failed.", item); } } else { logger.debug("store ignored, InfluxDB is not connected");