From 0bbd276a240d14237f55aaf63591f2f117b8f089 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Thu, 2 Jun 2022 18:18:58 +0200 Subject: [PATCH] Refactor scale transformation to configuration registry (#12862) Signed-off-by: Jan N. Klug --- .../internal/ScaleTransformationService.java | 200 +++++++++++------- .../internal/ScaleTransformServiceTest.java | 63 ++++-- 2 files changed, 171 insertions(+), 92 deletions(-) diff --git a/bundles/org.openhab.transform.scale/src/main/java/org/openhab/transform/scale/internal/ScaleTransformationService.java b/bundles/org.openhab.transform.scale/src/main/java/org/openhab/transform/scale/internal/ScaleTransformationService.java index cbbb30ffc..48b422b6b 100644 --- a/bundles/org.openhab.transform.scale/src/main/java/org/openhab/transform/scale/internal/ScaleTransformationService.java +++ b/bundles/org.openhab.transform.scale/src/main/java/org/openhab/transform/scale/internal/ScaleTransformationService.java @@ -12,8 +12,8 @@ */ package org.openhab.transform.scale.internal; -import java.io.FileReader; import java.io.IOException; +import java.io.StringReader; import java.math.BigDecimal; import java.net.URI; import java.util.Collection; @@ -26,19 +26,25 @@ import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.common.registry.RegistryChangeListener; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ParameterOption; import org.openhab.core.library.types.QuantityType; -import org.openhab.core.transform.AbstractFileTransformationService; +import org.openhab.core.transform.TransformationConfiguration; +import org.openhab.core.transform.TransformationConfigurationRegistry; import org.openhab.core.transform.TransformationException; import org.openhab.core.transform.TransformationService; +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.Reference; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,17 +57,18 @@ import org.slf4j.LoggerFactory; */ @Component(service = { TransformationService.class, ConfigOptionProvider.class }, property = { "openhab.transform=SCALE" }) -public class ScaleTransformationService extends AbstractFileTransformationService> - implements ConfigOptionProvider { +@NonNullByDefault +public class ScaleTransformationService + implements TransformationService, ConfigOptionProvider, RegistryChangeListener { private final Logger logger = LoggerFactory.getLogger(ScaleTransformationService.class); private static final String PROFILE_CONFIG_URI = "profile:transform:SCALE"; private static final String CONFIG_PARAM_FUNCTION = "function"; - private static final String[] FILE_NAME_EXTENSIONS = { "scale" }; + private static final Set SUPPORTED_CONFIGURATION_TYPES = Set.of("scale"); /** RegEx to extract a scale definition */ - private static final Pattern LIMITS_PATTERN = Pattern.compile("(\\[|\\])(.*)\\.\\.(.*)(\\[|\\])"); + private static final Pattern LIMITS_PATTERN = Pattern.compile("(\\[|])(.*)\\.\\.(.*)(\\[|])"); private static final String NON_NUMBER = "NaN"; private static final String FORMAT = "format"; @@ -70,6 +77,39 @@ public class ScaleTransformationService extends AbstractFileTransformationServic /** Inaccessible range used to store presentation format ]0..0[ */ private static final Range FORMAT_RANGE = Range.range(BigDecimal.ZERO, false, BigDecimal.ZERO, false); + private final TransformationConfigurationRegistry transformationConfigurationRegistry; + + private final Map> cachedTransformations = new ConcurrentHashMap<>(); + + @Activate + public ScaleTransformationService( + @Reference TransformationConfigurationRegistry transformationConfigurationRegistry) { + this.transformationConfigurationRegistry = transformationConfigurationRegistry; + transformationConfigurationRegistry.addRegistryChangeListener(this); + } + + @Deactivate + public void deactivate() { + transformationConfigurationRegistry.removeRegistryChangeListener(this); + } + + @Override + public void added(TransformationConfiguration element) { + // do nothing, configurations are added to cache if needed + } + + @Override + public void removed(TransformationConfiguration element) { + cachedTransformations.remove(element.getUID()); + } + + @Override + public void updated(TransformationConfiguration oldElement, TransformationConfiguration element) { + if (cachedTransformations.remove(oldElement.getUID()) != null) { + // import only if it was present before + importConfiguration(element); + } + } /** * The implementation of {@link OrderedProperties} that let access @@ -83,125 +123,129 @@ public class ScaleTransformationService extends AbstractFileTransformationServic */ static class OrderedProperties extends Properties { private static final long serialVersionUID = 3860553217028220119L; - private final HashSet keys = new LinkedHashSet<>(); + private final HashSet<@Nullable Object> keys = new LinkedHashSet<>(); - Set orderedKeys() { + Set<@Nullable Object> orderedKeys() { return keys; } @Override - public Enumeration keys() { - return Collections. enumeration(keys); + public @NonNullByDefault({}) Enumeration keys() { + return Collections.enumeration(keys); } @Override - public Object put(Object key, Object value) { + public @Nullable Object put(@Nullable Object key, @Nullable Object value) { keys.add(key); return super.put(key, value); } } - /** - * Performs transformation of the input source - * - * The method transforms the input source by matching searching - * the range where it fits i.e. [min..max]=value or ]min..max]=value - * - * @param properties the list of properties defining all the available ranges - * @param source the input to transform - * @return the transformed result or null if the transformation couldn't be completed for any reason. - */ @Override - protected @Nullable String internalTransform(Map data, String source) - throws TransformationException { - try { - final BigDecimal value = new BigDecimal(source); - return formatResult(data, source, value); - } catch (NumberFormatException e) { - // Scale can only be used with numeric inputs, so lets try to see if ever its a valid quantity type - try { - final QuantityType quantity = new QuantityType<>(source); - return formatResult(data, source, quantity.toBigDecimal()); - } catch (IllegalArgumentException e2) { - String nonNumeric = data.get(null); - if (nonNumeric != null) { - return nonNumeric; - } else { - throw new TransformationException( - "Scale must be used with numeric inputs, valid quantity types or a 'NaN' entry."); + public @Nullable String transform(String function, String source) throws TransformationException { + // always get a configuration from the registry to account for changed system locale + TransformationConfiguration transformationConfiguration = transformationConfigurationRegistry.get(function, + null); + + if (transformationConfiguration != null) { + if (!cachedTransformations.containsKey(transformationConfiguration.getUID())) { + importConfiguration(transformationConfiguration); + } + Map<@Nullable Range, String> data = cachedTransformations.get(function); + + if (data != null) { + String target; + + try { + final BigDecimal value = new BigDecimal(source); + target = formatResult(data, source, value); + } catch (NumberFormatException e) { + // Scale can only be used with numeric inputs, so lets try to see if ever its a valid quantity type + try { + final QuantityType quantity = new QuantityType<>(source); + return formatResult(data, source, quantity.toBigDecimal()); + } catch (IllegalArgumentException e2) { + String nonNumeric = data.get(null); + if (nonNumeric != null) { + target = nonNumeric; + } else { + throw new TransformationException( + "Scale must be used with numeric inputs, valid quantity types or a 'NaN' entry."); + } + } } + logger.debug("Transformation resulted in '{}'", target); + return target; } } + + throw new TransformationException("Could not find configuration '" + function + "' or failed to parse it."); } - private String formatResult(Map data, String source, final BigDecimal value) + private String formatResult(Map<@Nullable Range, String> data, String source, final BigDecimal value) throws TransformationException { String format = data.get(FORMAT_RANGE); String result = getScaleResult(data, source, value); return format.replaceAll(FORMAT_VALUE, source).replaceAll(FORMAT_LABEL, result); } - private String getScaleResult(Map data, String source, final BigDecimal value) + private String getScaleResult(Map<@Nullable Range, String> data, String source, final BigDecimal value) throws TransformationException { return data.entrySet().stream().filter(entry -> entry.getKey() != null && entry.getKey().contains(value)) .findFirst().map(Map.Entry::getValue) .orElseThrow(() -> new TransformationException("No matching range for '" + source + "'")); } - @Override - protected Map internalLoadTransform(String filename) throws TransformationException { - try (FileReader reader = new FileReader(filename)) { - final Map data = new LinkedHashMap<>(); - data.put(FORMAT_RANGE, FORMAT_LABEL); - final OrderedProperties properties = new OrderedProperties(); - properties.load(reader); + private void importConfiguration(@Nullable TransformationConfiguration configuration) { + if (configuration != null) { + try { + final Map<@Nullable Range, String> data = new LinkedHashMap<>(); + data.put(FORMAT_RANGE, FORMAT_LABEL); + final OrderedProperties properties = new OrderedProperties(); + properties.load(new StringReader(configuration.getContent())); - for (Object orderedKey : properties.orderedKeys()) { - final String entry = (String) orderedKey; - final String value = properties.getProperty(entry); - final Matcher matcher = LIMITS_PATTERN.matcher(entry); - if (matcher.matches() && (matcher.groupCount() == 4)) { - final boolean lowerInclusive = matcher.group(1).equals("["); - final boolean upperInclusive = matcher.group(4).equals("]"); + for (Object orderedKey : properties.orderedKeys()) { + final String entry = (String) orderedKey; + final String value = properties.getProperty(entry); + final Matcher matcher = LIMITS_PATTERN.matcher(entry); + if (matcher.matches() && (matcher.groupCount() == 4)) { + final boolean lowerInclusive = matcher.group(1).equals("["); + final boolean upperInclusive = matcher.group(4).equals("]"); - final String lowLimit = matcher.group(2); - final String highLimit = matcher.group(3); + final String lowLimit = matcher.group(2); + final String highLimit = matcher.group(3); - try { final BigDecimal lowValue = lowLimit.isEmpty() ? null : new BigDecimal(lowLimit); final BigDecimal highValue = highLimit.isEmpty() ? null : new BigDecimal(highLimit); final Range range = Range.range(lowValue, lowerInclusive, highValue, upperInclusive); data.put(range, value); - } catch (NumberFormatException ex) { - throw new TransformationException("Error parsing bounds: " + lowLimit + ".." + highLimit); - } - } else { - if (NON_NUMBER.equals(entry)) { - data.put(null, value); - } else if (FORMAT.equals(entry)) { - data.put(FORMAT_RANGE, value); } else { - logger.warn("Scale transform file '{}' does not comply with syntax for entry : '{}', '{}'", - filename, entry, value); + if (NON_NUMBER.equals(entry)) { + data.put(null, value); + } else if (FORMAT.equals(entry)) { + data.put(FORMAT_RANGE, value); + } else { + logger.warn( + "Scale transformation configuration '{}' does not comply with syntax for entry : '{}', '{}'", + configuration.getUID(), entry, value); + } } } - } - return data; - } catch (final IOException ex) { - throw new TransformationException("An error occurred while opening file.", ex); + cachedTransformations.put(configuration.getUID(), data); + } catch (IOException | NumberFormatException ignored) { + } } } @Override - public @Nullable Collection<@NonNull ParameterOption> getParameterOptions(URI uri, String param, - @Nullable String context, @Nullable Locale locale) { + public @Nullable Collection getParameterOptions(URI uri, String param, @Nullable String context, + @Nullable Locale locale) { if (PROFILE_CONFIG_URI.equals(uri.toString())) { - switch (param) { - case CONFIG_PARAM_FUNCTION: - return getFilenames(FILE_NAME_EXTENSIONS).stream().map(f -> new ParameterOption(f, f)) - .collect(Collectors.toList()); + if (CONFIG_PARAM_FUNCTION.equals(param)) { + return transformationConfigurationRegistry.getConfigurations(SUPPORTED_CONFIGURATION_TYPES).stream() + .map(c -> new ParameterOption(c.getUID(), c.getLabel())).collect(Collectors.toList()); } } return null; diff --git a/bundles/org.openhab.transform.scale/src/test/java/org/openhab/transform/scale/internal/ScaleTransformServiceTest.java b/bundles/org.openhab.transform.scale/src/test/java/org/openhab/transform/scale/internal/ScaleTransformServiceTest.java index bb703c056..d198307d2 100644 --- a/bundles/org.openhab.transform.scale/src/test/java/org/openhab/transform/scale/internal/ScaleTransformServiceTest.java +++ b/bundles/org.openhab.transform.scale/src/test/java/org/openhab/transform/scale/internal/ScaleTransformServiceTest.java @@ -13,30 +13,68 @@ package org.openhab.transform.scale.internal; import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; -import java.util.Locale; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; import javax.measure.quantity.Dimensionless; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.mockito.stubbing.Answer; import org.openhab.core.library.types.QuantityType; +import org.openhab.core.transform.TransformationConfiguration; +import org.openhab.core.transform.TransformationConfigurationRegistry; import org.openhab.core.transform.TransformationException; /** * @author Gaël L'hopital - Initial contribution */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +@NonNullByDefault public class ScaleTransformServiceTest { - private ScaleTransformationService processor; + private static final String SRC_FOLDER = "conf" + File.separator + "transform"; + + @Mock + private @NonNullByDefault({}) TransformationConfigurationRegistry transformationConfigurationRegistry; + private final Map configurationMap = new HashMap<>(); + private @NonNullByDefault({}) ScaleTransformationService processor; @BeforeEach - public void init() { - processor = new ScaleTransformationService() { - @Override - protected Locale getLocale() { - return Locale.US; + public void init() throws IOException { + configurationMap.clear(); + Files.walk(Path.of(SRC_FOLDER)).filter(Files::isRegularFile).forEach(file -> { + try { + String content = new String(Files.readAllBytes(file), StandardCharsets.UTF_8); + String uid = Path.of(SRC_FOLDER).relativize(file).toString(); + TransformationConfiguration transformationConfiguration = new TransformationConfiguration(uid, uid, + "scale", null, content); + configurationMap.put(uid, transformationConfiguration); + } catch (IOException ignored) { } - }; + }); + + Mockito.when(transformationConfigurationRegistry.get(anyString(), eq(null))) + .thenAnswer((Answer) invocation -> { + Object[] args = invocation.getArguments(); + return configurationMap.get(args[0]); + }); + processor = new ScaleTransformationService(transformationConfigurationRegistry); } @Test @@ -80,8 +118,7 @@ public class ScaleTransformServiceTest { // Issue #1107 String existingscale = "scale/humidex_fr.scale"; String source = "-"; - String transformedResponse = processor.transform(existingscale, source); - assertEquals("", transformedResponse); + assertThrows(TransformationException.class, () -> processor.transform(existingscale, source)); } @Test @@ -104,8 +141,7 @@ public class ScaleTransformServiceTest { // checks that an error is raised when trying to scale an erroneous value String existingscale = "scale/evaluationorder.scale"; String source = "azerty"; - String transformedResponse = processor.transform(existingscale, source); - assertEquals("", transformedResponse); + assertThrows(TransformationException.class, () -> processor.transform(existingscale, source)); } @Test @@ -150,7 +186,6 @@ public class ScaleTransformServiceTest { public void testValueExceedsRange() throws TransformationException { String existingscale = "scale/humidex.scale"; String source = "200"; - String transformedResponse = processor.transform(existingscale, source); - assertEquals("", transformedResponse); + assertThrows(TransformationException.class, () -> processor.transform(existingscale, source)); } }