From 320dd8ab008e47b523b1fa58f97d8172f5c92d35 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Thu, 14 Apr 2022 00:04:16 +0200 Subject: [PATCH] [transform.map] Refactor service to use TransformationConfigurationRegistry (#12433) This is the reference implementation for using the new `TransformationConfigurationRegistry`. The localization tests can be omitted as this is covered by tests in the core component. Signed-off-by: Jan N. Klug --- .../internal/MapTransformationService.java | 119 ++++++--- .../MapTransformationServiceTest.java | 247 ++++++++---------- 2 files changed, 183 insertions(+), 183 deletions(-) diff --git a/bundles/org.openhab.transform.map/src/main/java/org/openhab/transform/map/internal/MapTransformationService.java b/bundles/org.openhab.transform.map/src/main/java/org/openhab/transform/map/internal/MapTransformationService.java index 00f5c4f5f..f97394b00 100644 --- a/bundles/org.openhab.transform.map/src/main/java/org/openhab/transform/map/internal/MapTransformationService.java +++ b/bundles/org.openhab.transform.map/src/main/java/org/openhab/transform/map/internal/MapTransformationService.java @@ -12,22 +12,30 @@ */ package org.openhab.transform.map.internal; -import java.io.FileReader; import java.io.IOException; +import java.io.StringReader; import java.net.URI; import java.util.Collection; import java.util.Locale; +import java.util.Map; import java.util.Properties; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; 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.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; @@ -37,65 +45,100 @@ import org.slf4j.LoggerFactory; * * @author Kai Kreuzer - Initial contribution and API * @author Gaël L'hopital - Make it localizable + * @author Jan N. Klug - Refactored to use {@link TransformationConfigurationRegistry} */ @NonNullByDefault @Component(service = { TransformationService.class, ConfigOptionProvider.class }, property = { "openhab.transform=MAP" }) -public class MapTransformationService extends AbstractFileTransformationService - implements ConfigOptionProvider { - +public class MapTransformationService + implements TransformationService, ConfigOptionProvider, RegistryChangeListener { private final Logger logger = LoggerFactory.getLogger(MapTransformationService.class); private static final String PROFILE_CONFIG_URI = "profile:transform:MAP"; private static final String CONFIG_PARAM_FUNCTION = "function"; - private static final String[] FILE_NAME_EXTENSIONS = { "map" }; + private static final Set SUPPORTED_CONFIGURATION_TYPES = Set.of("map"); - /** - *

- * Transforms the input source by mapping it to another string. It expects the mappings to be read from - * a file which is stored under the 'configurations/transform' folder. This file should be in property syntax, i.e. - * simple lines with "key=value" pairs. To organize the various transformations one might use subfolders. - * - * @param properties the list of properties which contains the key value pairs for the mapping. - * @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(Properties properties, String source) throws TransformationException { - String target = properties.getProperty(source); + private final TransformationConfigurationRegistry transformationConfigurationRegistry; + private final Map cachedTransformations = new ConcurrentHashMap<>(); - if (target == null) { - target = properties.getProperty(""); - if (target == null) { - throw new TransformationException("Target value not found in map for '" + source + "'"); - } - } + @Activate + public MapTransformationService( + @Reference TransformationConfigurationRegistry transformationConfigurationRegistry) { + this.transformationConfigurationRegistry = transformationConfigurationRegistry; + transformationConfigurationRegistry.addRegistryChangeListener(this); + } - logger.debug("Transformation resulted in '{}'", target); - return target; + @Deactivate + public void deactivate() { + transformationConfigurationRegistry.removeRegistryChangeListener(this); } @Override - protected Properties internalLoadTransform(String filename) throws TransformationException { - Properties result = new Properties(); - try (FileReader reader = new FileReader(filename)) { - result.load(reader); - return result; - } catch (IOException e) { - throw new TransformationException("An error occurred while opening file.", e); + 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); + } + Properties properties = cachedTransformations.get(function); + if (properties != null) { + String target = properties.getProperty(source); + + if (target == null) { + target = properties.getProperty(""); + if (target == null) { + throw new TransformationException("Target value not found in map for '" + source + "'"); + } + } + + logger.debug("Transformation resulted in '{}'", target); + return target; + } } + throw new TransformationException("Could not find configuration '" + function + "' or failed to parse it."); } @Override 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; } + + @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); + } + } + + private void importConfiguration(@Nullable TransformationConfiguration configuration) { + if (configuration != null) { + try { + Properties properties = new Properties(); + properties.load(new StringReader(configuration.getContent())); + cachedTransformations.put(configuration.getUID(), properties); + } catch (IOException ignored) { + } + } + } } diff --git a/bundles/org.openhab.transform.map/src/test/java/org/openhab/transform/map/internal/MapTransformationServiceTest.java b/bundles/org.openhab.transform.map/src/test/java/org/openhab/transform/map/internal/MapTransformationServiceTest.java index d934987c4..fdc271233 100644 --- a/bundles/org.openhab.transform.map/src/test/java/org/openhab/transform/map/internal/MapTransformationServiceTest.java +++ b/bundles/org.openhab.transform.map/src/test/java/org/openhab/transform/map/internal/MapTransformationServiceTest.java @@ -13,189 +13,146 @@ package org.openhab.transform.map.internal; import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; -import java.io.File; -import java.io.FileReader; -import java.io.FileWriter; import java.io.IOException; -import java.io.PrintStream; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.Comparator; -import java.util.Locale; -import java.util.Properties; -import java.util.concurrent.Callable; -import java.util.stream.Stream; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; -import org.junit.jupiter.api.AfterEach; +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.osgi.framework.BundleContext; +import org.mockito.stubbing.Answer; +import org.openhab.core.test.java.JavaTest; +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 + * @author Jan N. Klug - Refactored to use {@link TransformationConfigurationRegistry} */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) -public class MapTransformationServiceTest { - +@NonNullByDefault +public class MapTransformationServiceTest extends JavaTest { private static final String SOURCE_CLOSED = "CLOSED"; private static final String SOURCE_UNKNOWN = "UNKNOWN"; - private static final String EXISTING_FILENAME_DE = "map/doorstatus_de.map"; - private static final String SHOULD_BE_LOCALIZED_FILENAME = "map/doorstatus.map"; - private static final String DEFAULTED_FILENAME = "map/doorstatus_defaulted.map"; - private static final String INEXISTING_FILENAME = "map/de.map"; - private static final String BASE_FOLDER = "target"; - private static final String SRC_FOLDER = "conf"; - private static final String CONFIG_FOLDER = BASE_FOLDER + File.separator + SRC_FOLDER; - private static final String USED_FILENAME = CONFIG_FOLDER + File.separator + "transform/" + EXISTING_FILENAME_DE; - private @Mock BundleContext bundleContext; + private static final String NON_DEFAULTED_TRANSFORMATION_DE = "map/doorstatus_de.map"; + private static final String NON_DEFAULTED_TRANSFORMATION_FR = "map/doorstatus_fr.map"; + private static final String DEFAULTED_TRANSFORMATION = "map/doorstatus_defaulted.map"; + private static final String UNKNOWN_TRANSFORMATION = "map/de.map"; - private TestableMapTransformationService processor; + private static final String SRC_FOLDER = "conf/transform"; - private class TestableMapTransformationService extends MapTransformationService { - @Override - protected String getSourcePath() { - return BASE_FOLDER + File.separator + super.getSourcePath(); - } + @Mock + private @NonNullByDefault({}) TransformationConfigurationRegistry transformationConfigurationRegistry; - @Override - protected Locale getLocale() { - return Locale.US; - } - - @Override - public void activate(BundleContext context) { - super.activate(context); - } - - @Override - public void deactivate() { - super.deactivate(); - } - }; + private @NonNullByDefault({}) MapTransformationService processor; + private final Map configurationMap = new HashMap<>(); @BeforeEach public void setUp() throws IOException { - processor = new TestableMapTransformationService(); - processor.activate(bundleContext); - copyDirectory(SRC_FOLDER, CONFIG_FOLDER); - } - - @AfterEach - public void tearDown() throws IOException { - processor.deactivate(); - - try (Stream walk = Files.walk(Path.of(CONFIG_FOLDER))) { - walk.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete); - } - } - - private void copyDirectory(String from, String to) throws IOException { - Files.walk(Paths.get(from)).forEach(fromPath -> { - Path toPath = Paths.get(to, fromPath.toString().substring(from.length())); + configurationMap.clear(); + Files.walk(Path.of(SRC_FOLDER)).filter(Files::isRegularFile).forEach(file -> { try { - Files.copy(fromPath, toPath); - } catch (IOException e) { + String content = new String(Files.readAllBytes(file)); + String uid = Path.of(SRC_FOLDER).relativize(file).toString(); + TransformationConfiguration transformationConfiguration = new TransformationConfiguration(uid, uid, + "map", 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((String) args[0]); + }); + + processor = new MapTransformationService(transformationConfigurationRegistry); } @Test - public void testTransformByMap() throws Exception { - // Test that we find a translation in an existing file - String transformedResponse = processor.transform(EXISTING_FILENAME_DE, SOURCE_CLOSED); + public void testTransformSucceeds() throws TransformationException { + String transformedResponse = processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_CLOSED); assertEquals("zu", transformedResponse); - - Properties properties = new Properties(); - try (FileReader reader = new FileReader(USED_FILENAME); FileWriter writer = new FileWriter(USED_FILENAME)) { - properties.load(reader); - properties.setProperty(SOURCE_CLOSED, "changevalue"); - properties.store(writer, ""); - - // This tests that the requested transformation file has been removed from - // the cache - waitForAssert(new Callable() { - @Override - public Void call() throws Exception { - final String transformedResponse = processor.transform(EXISTING_FILENAME_DE, SOURCE_CLOSED); - assertEquals("changevalue", transformedResponse); - return null; - } - }, 10000, 100); - - properties.setProperty(SOURCE_CLOSED, "zu"); - properties.store(writer, ""); - - waitForAssert(new Callable() { - @Override - public Void call() throws Exception { - final String transformedResponse = processor.transform(EXISTING_FILENAME_DE, SOURCE_CLOSED); - assertEquals("zu", transformedResponse); - return null; - } - }, 10000, 100); - } catch (IOException e1) { - PrintStream err = System.err; - if (err != null) { - e1.printStackTrace(err); - } - } - - // Checks that an unknown input in an existing file give the expected - // transformed response that shall be empty string (Issue #1107) if not found in the file - transformedResponse = processor.transform(EXISTING_FILENAME_DE, SOURCE_UNKNOWN); - assertEquals("", transformedResponse); - - // Test that an inexisting file raises correct exception as expected - try { - transformedResponse = processor.transform(INEXISTING_FILENAME, SOURCE_CLOSED); - fail(); - } catch (Exception e) { - // That's what we expect. - } - - // Test that we find a localized version of desired file - transformedResponse = processor.transform(SHOULD_BE_LOCALIZED_FILENAME, SOURCE_CLOSED); - // as we don't know the real locale at the moment the - // test is run, we test that the string has actually been transformed - assertNotEquals(SOURCE_CLOSED, transformedResponse); - transformedResponse = processor.transform(SHOULD_BE_LOCALIZED_FILENAME, SOURCE_CLOSED); - assertNotEquals(SOURCE_CLOSED, transformedResponse); } @Test - public void testTransformByMapWithDefault() throws Exception { - // Standard behaviour with no default value - String transformedResponse = processor.transform(SHOULD_BE_LOCALIZED_FILENAME, "toBeDefaulted"); - assertEquals("", transformedResponse); - // Modified behaviour with a file containing default value definition - transformedResponse = processor.transform(DEFAULTED_FILENAME, "toBeDefaulted"); - assertEquals("Default Value", transformedResponse); + public void testTransformFailsWithoutDefault() { + assertThrows(TransformationException.class, + () -> processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_UNKNOWN)); } - protected void waitForAssert(Callable assertion, int timeout, int sleepTime) throws Exception { - int waitingTime = 0; - while (waitingTime < timeout) { - try { - assertion.call(); - return; - } catch (AssertionError error) { - waitingTime += sleepTime; - try { - Thread.sleep(sleepTime); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - } - } - assertion.call(); + @Test + public void testTransformSucceedsWithDefault() throws TransformationException { + assertEquals("Default Value", processor.transform(DEFAULTED_TRANSFORMATION, SOURCE_UNKNOWN)); + } + + @Test + public void testTransformFailsOnUnknownTransformation() { + assertThrows(TransformationException.class, () -> processor.transform(UNKNOWN_TRANSFORMATION, SOURCE_CLOSED)); + } + + @Test + public void setTransformationConfigurationIsRemoved() throws TransformationException { + assertEquals("zu", processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_CLOSED)); + + TransformationConfiguration transformationConfiguration = configurationMap + .remove(NON_DEFAULTED_TRANSFORMATION_DE); + processor.removed(Objects.requireNonNull(transformationConfiguration)); + + assertThrows(TransformationException.class, + () -> processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_CLOSED)); + } + + @Test + public void setTransformationConfigurationIsNotUpdatedIfOldElementMissing() throws TransformationException { + // update configuration + TransformationConfiguration transformationConfigurationDE = Objects + .requireNonNull(configurationMap.get(NON_DEFAULTED_TRANSFORMATION_DE)); + TransformationConfiguration transformationConfigurationFR = Objects + .requireNonNull(configurationMap.get(NON_DEFAULTED_TRANSFORMATION_FR)); + TransformationConfiguration transformationConfigurationModified = new TransformationConfiguration( + transformationConfigurationDE.getUID(), transformationConfigurationDE.getLabel(), + transformationConfigurationDE.getType(), transformationConfigurationDE.getLanguage(), + transformationConfigurationFR.getContent()); + processor.updated(transformationConfigurationDE, transformationConfigurationModified); + + // assert there is no modified cached version + assertEquals("zu", processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_CLOSED)); + } + + @Test + public void setTransformationConfigurationIsUpdatedIfOldElementPresent() throws TransformationException { + // ensure old transformation is cached + processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_CLOSED); + + // update configuration + TransformationConfiguration transformationConfigurationDE = Objects + .requireNonNull(configurationMap.get(NON_DEFAULTED_TRANSFORMATION_DE)); + TransformationConfiguration transformationConfigurationFR = Objects + .requireNonNull(configurationMap.get(NON_DEFAULTED_TRANSFORMATION_FR)); + TransformationConfiguration transformationConfigurationModified = new TransformationConfiguration( + transformationConfigurationDE.getUID(), transformationConfigurationDE.getLabel(), + transformationConfigurationDE.getType(), transformationConfigurationDE.getLanguage(), + transformationConfigurationFR.getContent()); + processor.updated(transformationConfigurationDE, transformationConfigurationModified); + + // ensure modified configuration is applied + assertEquals("fermé", processor.transform(NON_DEFAULTED_TRANSFORMATION_DE, SOURCE_CLOSED)); } }