From 6a75130355f7ba55954dbee97e785ae6501308b2 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Sat, 9 Apr 2022 15:49:38 +0200 Subject: [PATCH] Allow type migrations in JSONStorage (#2784) Signed-off-by: Jan N. Klug --- .../storage/json/internal/JsonStorage.java | 68 ++++++-- .../json/internal/JsonStorageService.java | 13 +- .../migration/RenamingTypeMigrator.java | 42 +++++ .../migration/TypeMigrationException.java | 29 ++++ .../json/internal/migration/TypeMigrator.java | 53 ++++++ .../json/internal/JsonStorageTest.java | 8 +- .../storage/json/internal/MigrationTest.java | 156 ++++++++++++++++++ 7 files changed, 344 insertions(+), 25 deletions(-) create mode 100644 bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/RenamingTypeMigrator.java create mode 100644 bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrationException.java create mode 100644 bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrator.java create mode 100644 bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/MigrationTest.java diff --git a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorage.java b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorage.java index 2ed4a1f25..d0f67082a 100644 --- a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorage.java +++ b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorage.java @@ -26,6 +26,7 @@ import java.util.Set; import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -34,6 +35,8 @@ import org.openhab.core.config.core.ConfigurationDeserializer; import org.openhab.core.config.core.OrderingMapSerializer; import org.openhab.core.config.core.OrderingSetSerializer; import org.openhab.core.storage.Storage; +import org.openhab.core.storage.json.internal.migration.TypeMigrationException; +import org.openhab.core.storage.json.internal.migration.TypeMigrator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,19 +83,21 @@ public class JsonStorage implements Storage { private final File file; private final @Nullable ClassLoader classLoader; private final Map map = new ConcurrentHashMap<>(); + private final Map typeMigrators; - private transient Gson internalMapper; - private transient Gson entityMapper; + private final transient Gson internalMapper; + private final transient Gson entityMapper; private boolean dirty; public JsonStorage(File file, @Nullable ClassLoader classLoader, int maxBackupFiles, int writeDelay, - int maxDeferredPeriod) { + int maxDeferredPeriod, List typeMigrators) { this.file = file; this.classLoader = classLoader; this.maxBackupFiles = maxBackupFiles; this.writeDelay = writeDelay; this.maxDeferredPeriod = maxDeferredPeriod; + this.typeMigrators = typeMigrators.stream().collect(Collectors.toMap(e -> e.getOldType(), e -> e)); this.internalMapper = new GsonBuilder() // .registerTypeHierarchyAdapter(Map.class, new OrderingMapSerializer())// @@ -156,7 +161,7 @@ public class JsonStorage implements Storage { if (previousValue == null) { return null; } - return deserialize(previousValue); + return deserialize(previousValue, null); } @Override @@ -166,7 +171,7 @@ public class JsonStorage implements Storage { if (removedElement == null) { return null; } - return deserialize(removedElement); + return deserialize(removedElement, null); } @Override @@ -180,7 +185,7 @@ public class JsonStorage implements Storage { if (value == null) { return null; } - return deserialize(value); + return deserialize(value, key); } @Override @@ -201,33 +206,58 @@ public class JsonStorage implements Storage { * Deserializes and instantiates an object of type {@code T} out of the given * JSON String. A special classloader (other than the one of the JSON bundle) is * used in order to load the classes in the context of the calling bundle. + * + * The {@code key} must only be specified if the requested object stays in storage (i.e. only when called from + * {@link #get(String)} action). If specified on other actions, the old or removed value will be persisted. + * + * @param entry the entry that needs deserialization + * @param key the key for this element if storage after type migration is requested + * @return the deserialized type */ - @SuppressWarnings("unchecked") - private @Nullable T deserialize(@Nullable StorageEntry entry) { + @SuppressWarnings({ "unchecked", "null" }) + private @Nullable T deserialize(@Nullable StorageEntry entry, @Nullable String key) { if (entry == null) { // nothing to deserialize return null; } try { - // load required class within the given bundle context - Class loadedValueType; - if (classLoader != null) { - loadedValueType = (Class) classLoader.loadClass(entry.getEntityClassName()); - } else { - loadedValueType = (Class) Class.forName(entry.getEntityClassName()); + String entityClassName = entry.getEntityClassName(); + JsonElement entityValue = (JsonElement) entry.getValue(); + + TypeMigrator migrator = typeMigrators.get(entityClassName); + if (migrator != null) { + entityClassName = migrator.getNewType(); + entityValue = migrator.migrate(entityValue); + if (key != null) { + map.put(key, new StorageEntry(entityClassName, entityValue)); + deferredCommit(); + } } - T value = entityMapper.fromJson((JsonElement) entry.getValue(), loadedValueType); + // load required class within the given bundle context + Class loadedValueType; + + if (classLoader != null) { + loadedValueType = (Class) classLoader.loadClass(entityClassName); + } else { + loadedValueType = (Class) Class.forName(entityClassName); + } + + T value = entityMapper.fromJson(entityValue, loadedValueType); logger.trace("deserialized value '{}' from Json", value); return value; } catch (JsonSyntaxException | JsonIOException | ClassNotFoundException e) { logger.error("Couldn't deserialize value '{}'. Root cause is: {}", entry, e.getMessage()); return null; + } catch (TypeMigrationException e) { + logger.error("Type '{}' needs migration but migration failed: '{}'", entry.getEntityClassName(), + e.getMessage()); + return null; } } - @SuppressWarnings("unchecked") + @SuppressWarnings({ "unchecked", "null" }) private @Nullable Map readDatabase(File inputFile) { if (inputFile.length() == 0) { logger.warn("Json storage file at '{}' is empty - ignoring corrupt file.", inputFile.getAbsolutePath()); @@ -304,9 +334,10 @@ public class JsonStorage implements Storage { */ public synchronized void flush() { // Stop any existing timer + TimerTask commitTimerTask = this.commitTimerTask; if (commitTimerTask != null) { commitTimerTask.cancel(); - commitTimerTask = null; + this.commitTimerTask = null; } if (dirty) { @@ -357,9 +388,10 @@ public class JsonStorage implements Storage { dirty = true; // Stop any existing timer + TimerTask commitTimerTask = this.commitTimerTask; if (commitTimerTask != null) { commitTimerTask.cancel(); - commitTimerTask = null; + this.commitTimerTask = null; } // Handle a maximum time for deferring the commit. diff --git a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorageService.java b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorageService.java index 69e6fef07..84719cc75 100644 --- a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorageService.java +++ b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorageService.java @@ -16,6 +16,7 @@ import java.io.File; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -24,6 +25,7 @@ import org.openhab.core.OpenHAB; import org.openhab.core.config.core.ConfigurableService; import org.openhab.core.storage.Storage; import org.openhab.core.storage.StorageService; +import org.openhab.core.storage.json.internal.migration.TypeMigrator; import org.osgi.framework.Constants; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; @@ -46,6 +48,11 @@ public class JsonStorageService implements StorageService { private static final int MAX_FILENAME_LENGTH = 127; + /** + * Contains a map of needed migrations, key is the storage name + */ + private static final Map> MIGRATORS = Map.of(); + private final Logger logger = LoggerFactory.getLogger(JsonStorageService.class); /** the folder name to store database ({@code jsondb} by default) */ @@ -120,9 +127,8 @@ public class JsonStorageService implements StorageService { @Override public Storage getStorage(String name, @Nullable ClassLoader classLoader) { File legacyFile = new File(dbFolderName, name + ".json"); - File escapedFile = new File(dbFolderName, urlEscapeUnwantedChars(name) + ".json"); + File file = new File(dbFolderName, urlEscapeUnwantedChars(name) + ".json"); - File file = escapedFile; if (legacyFile.exists()) { file = legacyFile; } @@ -132,7 +138,8 @@ public class JsonStorageService implements StorageService { oldStorage.flush(); } - JsonStorage newStorage = new JsonStorage<>(file, classLoader, maxBackupFiles, writeDelay, maxDeferredPeriod); + JsonStorage newStorage = new JsonStorage<>(file, classLoader, maxBackupFiles, writeDelay, maxDeferredPeriod, + MIGRATORS.getOrDefault(name, List.of())); storageList.put(name, (JsonStorage) newStorage); return newStorage; diff --git a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/RenamingTypeMigrator.java b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/RenamingTypeMigrator.java new file mode 100644 index 000000000..fc43aa882 --- /dev/null +++ b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/RenamingTypeMigrator.java @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2010-2022 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.core.storage.json.internal.migration; + +import org.eclipse.jdt.annotation.NonNullByDefault; + +/** + * The {@link RenamingTypeMigrator} is an {@link TypeMigrator} for renaming types + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +public class RenamingTypeMigrator implements TypeMigrator { + + private final String oldType; + private final String newType; + + public RenamingTypeMigrator(String oldType, String newType) { + this.oldType = oldType; + this.newType = newType; + } + + @Override + public String getOldType() { + return oldType; + } + + @Override + public String getNewType() { + return newType; + } +} diff --git a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrationException.java b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrationException.java new file mode 100644 index 000000000..9f42e7d06 --- /dev/null +++ b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrationException.java @@ -0,0 +1,29 @@ +/** + * Copyright (c) 2010-2022 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.core.storage.json.internal.migration; + +import org.eclipse.jdt.annotation.NonNullByDefault; + +/** + * The {@link TypeMigrationException} is thrown if a migration fails + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +public class TypeMigrationException extends Exception { + private static final long serialVersionUID = 1L; + + public TypeMigrationException(String message) { + super(message); + } +} diff --git a/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrator.java b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrator.java new file mode 100644 index 000000000..1abdf7e6c --- /dev/null +++ b/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/migration/TypeMigrator.java @@ -0,0 +1,53 @@ +/** + * Copyright (c) 2010-2022 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.core.storage.json.internal.migration; + +import org.eclipse.jdt.annotation.NonNullByDefault; + +import com.google.gson.JsonElement; + +/** + * The {@link TypeMigrator} interface allows the implementation of JSON storage type migrations + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +public interface TypeMigrator { + + /** + * Get the name of the old (stored) type + * + * @return Full class name + */ + String getOldType(); + + /** + * Get the name of the new type + * + * @return Full class name + */ + String getNewType(); + + /** + * Migrate the old type to the new type + * + * The default implementation can be used if type is renamed only. + * + * @param oldValue The {@link JsonElement} representation of the old type + * @return The corresponding {@link JsonElement} representation of the new type + * @throws TypeMigrationException if an error occurs + */ + default JsonElement migrate(JsonElement oldValue) throws TypeMigrationException { + return oldValue; + } +} diff --git a/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/JsonStorageTest.java b/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/JsonStorageTest.java index 099b0a472..902f9b40d 100644 --- a/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/JsonStorageTest.java +++ b/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/JsonStorageTest.java @@ -55,13 +55,13 @@ public class JsonStorageTest extends JavaTest { public void setUp() throws IOException { tmpFile = File.createTempFile("storage-debug", ".json"); tmpFile.deleteOnExit(); - objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0); + objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, List.of()); } private void persistAndReadAgain() { objectStorage.flush(); waitForAssert(() -> { - objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0); + objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, List.of()); DummyObject dummy = objectStorage.get("DummyObject"); assertNotNull(dummy); assertNotNull(dummy.configuration); @@ -137,7 +137,7 @@ public class JsonStorageTest extends JavaTest { persistAndReadAgain(); String storageString1 = Files.readString(tmpFile.toPath()); - objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0); + objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, List.of()); objectStorage.flush(); String storageString2 = Files.readString(tmpFile.toPath()); @@ -166,7 +166,7 @@ public class JsonStorageTest extends JavaTest { assertEquals(storageStringAB, storageStringBA); { - objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0); + objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, List.of()); objectStorage.flush(); } String storageStringReserialized = Files.readString(tmpFile.toPath()); diff --git a/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/MigrationTest.java b/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/MigrationTest.java new file mode 100644 index 000000000..a3ae253b7 --- /dev/null +++ b/bundles/org.openhab.core.storage.json/src/test/java/org/openhab/core/storage/json/internal/MigrationTest.java @@ -0,0 +1,156 @@ +/** + * Copyright (c) 2010-2022 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.core.storage.json.internal; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import java.io.File; +import java.io.IOException; +import java.util.List; +import java.util.Objects; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openhab.core.storage.json.internal.migration.RenamingTypeMigrator; +import org.openhab.core.storage.json.internal.migration.TypeMigrationException; +import org.openhab.core.storage.json.internal.migration.TypeMigrator; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; + +/** + * The {@link MigrationTest} is a + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +public class MigrationTest { + private static final String OBJECT_KEY = "foo"; + private static final String OBJECT_VALUE = "bar"; + + private @NonNullByDefault({}) File tmpFile; + + @BeforeEach + public void setup() throws IOException { + tmpFile = File.createTempFile("storage-debug", ".json"); + tmpFile.deleteOnExit(); + + // store old class + OldNameClass oldNameInstance = new OldNameClass(OBJECT_VALUE); + JsonStorage storage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, + List.of()); + storage.put(OBJECT_KEY, oldNameInstance); + storage.flush(); + } + + @Test + public void testRenameClassMigration() throws TypeMigrationException { + TypeMigrator typeMigrator = spy( + new RenamingTypeMigrator(OldNameClass.class.getName(), NewNameClass.class.getName())); + + // read new class + JsonStorage storage1 = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, + List.of(typeMigrator)); + + NewNameClass newNameInstance = storage1.get(OBJECT_KEY); + + verify(typeMigrator).getOldType(); + verify(typeMigrator).getNewType(); + verify(typeMigrator).migrate(any()); + + Objects.requireNonNull(newNameInstance); + + assertThat(OBJECT_VALUE, is(newNameInstance.value)); + + // ensure type migrations are stored + storage1.flush(); + newNameInstance = storage1.get(OBJECT_KEY); + verifyNoMoreInteractions(typeMigrator); + } + + @Test + public void testRenameFieldMigration() throws TypeMigrationException { + TypeMigrator typeMigrator = spy(new OldToNewFieldMigrator()); + // read new class + JsonStorage storage1 = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0, + List.of(typeMigrator)); + NewFieldClass newNameInstance = storage1.get(OBJECT_KEY); + + verify(typeMigrator).getOldType(); + verify(typeMigrator).getNewType(); + verify(typeMigrator).migrate(any()); + + Objects.requireNonNull(newNameInstance); + + assertThat(OBJECT_VALUE, is(newNameInstance.val)); + + // ensure type migrations are stored + storage1.flush(); + newNameInstance = storage1.get(OBJECT_KEY); + verifyNoMoreInteractions(typeMigrator); + } + + @SuppressWarnings("unused") + private static class OldNameClass { + public String value; + + public OldNameClass(String value) { + this.value = value; + } + } + + @SuppressWarnings("unused") + private static class NewNameClass { + public String value; + + public NewNameClass(String value) { + this.value = value; + } + } + + @SuppressWarnings("unused") + private static class NewFieldClass { + public String val; + + public NewFieldClass(String value) { + this.val = value; + } + } + + private static class OldToNewFieldMigrator implements TypeMigrator { + + @Override + public String getOldType() { + return OldNameClass.class.getName(); + } + + @Override + public String getNewType() { + return NewFieldClass.class.getName(); + } + + @Override + public JsonElement migrate(JsonElement oldValue) throws TypeMigrationException { + JsonObject newElement = oldValue.getAsJsonObject(); + JsonElement element = newElement.remove("value"); + newElement.add("val", element); + return newElement; + } + } +}