diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/gson/AbstractGSONParser.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/gson/AbstractGSONParser.java index a030be2595a..d74eb6405da 100644 --- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/gson/AbstractGSONParser.java +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/gson/AbstractGSONParser.java @@ -13,6 +13,7 @@ package org.openhab.core.automation.internal.parser.gson; import java.io.OutputStreamWriter; +import java.util.Map; import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -23,6 +24,8 @@ import org.openhab.core.config.core.Configuration; import org.openhab.core.config.core.ConfigurationDeserializer; import org.openhab.core.config.core.ConfigurationSerializer; +import org.openhab.core.config.core.OrderingMapSerializer; +import org.openhab.core.config.core.OrderingSetSerializer; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -32,6 +35,7 @@ * * @author Kai Kreuzer - Initial contribution * @author Ana Dimova - add Instance Creators + * @author Sami Salonen - add sorting for maps and sets for minimal diffs * * @param the type of the entities to parse */ @@ -45,6 +49,8 @@ public abstract class AbstractGSONParser implements Parser { .registerTypeAdapter(CompositeTriggerType.class, new TriggerInstanceCreator()) // .registerTypeAdapter(Configuration.class, new ConfigurationDeserializer()) // .registerTypeAdapter(Configuration.class, new ConfigurationSerializer()) // + .registerTypeHierarchyAdapter(Map.class, new OrderingMapSerializer()) // + .registerTypeHierarchyAdapter(Set.class, new OrderingSetSerializer()) // .create(); @Override diff --git a/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigurationSerializer.java b/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigurationSerializer.java index 9bbdb083bca..e9cafbe86e3 100644 --- a/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigurationSerializer.java +++ b/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigurationSerializer.java @@ -28,6 +28,7 @@ * * @author Yordan Mihaylov - Initial contribution * @author Ana Dimova - provide serialization of multiple configuration values. + * @author Sami Salonen - property names are sorted for serialization for minimal diffs */ public class ConfigurationSerializer implements JsonSerializer { @@ -35,7 +36,7 @@ public class ConfigurationSerializer implements JsonSerializer { @Override public JsonElement serialize(Configuration src, Type typeOfSrc, JsonSerializationContext context) { JsonObject result = new JsonObject(); - for (String propName : src.keySet()) { + src.keySet().stream().sorted().forEachOrdered((String propName) -> { Object value = src.get(propName); if (value instanceof List) { JsonArray array = new JsonArray(); @@ -46,7 +47,7 @@ public JsonElement serialize(Configuration src, Type typeOfSrc, JsonSerializatio } else { result.add(propName, serializePrimitive(value)); } - } + }); return result; } diff --git a/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/OrderingMapSerializer.java b/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/OrderingMapSerializer.java new file mode 100644 index 00000000000..bf624963f0f --- /dev/null +++ b/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/OrderingMapSerializer.java @@ -0,0 +1,62 @@ +/** + * Copyright (c) 2010-2021 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.config.core; + +import java.lang.reflect.Type; +import java.util.Comparator; +import java.util.Map; +import java.util.Map.Entry; +import java.util.stream.Stream; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; + +/** + * Serializes map data by ordering the keys + * + * @author Sami Salonen - Initial contribution + */ +@NonNullByDefault +public class OrderingMapSerializer implements JsonSerializer> { + + @SuppressWarnings({ "rawtypes", "unchecked", "null" }) + @Override + public JsonElement serialize(Map<@Nullable Object, @Nullable Object> src, Type typeOfSrc, + JsonSerializationContext context) { + JsonObject ordered = new JsonObject(); + final Stream> possiblySortedStream; + if (OrderingSetSerializer.allSameClassAndComparable(src.keySet())) { + // Map keys are comparable as verified above so casting to plain Comparator is safe + possiblySortedStream = src.entrySet().stream().sorted((Comparator) Map.Entry.comparingByKey()); + } else { + possiblySortedStream = src.entrySet().stream(); + } + possiblySortedStream.forEachOrdered(entry -> { + Object key = entry.getKey(); + if (key instanceof String) { + ordered.add((String) key, context.serialize(entry.getValue())); + } else { + JsonElement serialized = context.serialize(key); + ordered.add(serialized.isJsonPrimitive() ? serialized.getAsString() : serialized.toString(), + context.serialize(entry.getValue())); + } + + }); + return ordered; + } +} diff --git a/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/OrderingSetSerializer.java b/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/OrderingSetSerializer.java new file mode 100644 index 00000000000..eebb3358649 --- /dev/null +++ b/bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/OrderingSetSerializer.java @@ -0,0 +1,64 @@ +/** + * Copyright (c) 2010-2021 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.config.core; + +import java.lang.reflect.Type; +import java.util.Set; +import java.util.stream.Stream; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; + +/** + * Serializes set by ordering the elements + * + * @author Sami Salonen - Initial contribution + */ +@NonNullByDefault +public class OrderingSetSerializer implements JsonSerializer> { + + public static boolean allSameClassAndComparable(Set<@Nullable Object> src) { + Class expectedClass = null; + for (Object object : src) { + if (!(object instanceof Comparable)) { + // not comparable or simply null + return false; + } else if (expectedClass == null) { + // first item + expectedClass = object.getClass(); + } else if (!object.getClass().equals(expectedClass)) { + // various classes in the Set, let's not try to sort + return false; + } + } + return true; + } + + @Override + public JsonElement serialize(Set<@Nullable Object> src, Type typeOfSrc, JsonSerializationContext context) { + JsonArray ordered = new JsonArray(); + final Stream<@Nullable Object> possiblySortedStream; + if (allSameClassAndComparable(src)) { + possiblySortedStream = src.stream().sorted(); + } else { + possiblySortedStream = src.stream(); + } + possiblySortedStream.map(context::serialize).forEachOrdered(ordered::add); + return ordered; + } +} 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 ca14e4e6653..e5d86348e80 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 @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; @@ -29,6 +30,8 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.config.core.Configuration; 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.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,6 +54,8 @@ * @author Stefan Triller - Removed dependency to internal GSon packages * @author Simon Kaufmann - Distinguish between inner and outer * de-/serialization, keep json structures in map + * @author Sami Salonen - ordered inner and outer serialization of Maps, + * Sets and properties of Configuration */ @NonNullByDefault public class JsonStorage implements Storage { @@ -88,11 +93,18 @@ public JsonStorage(File file, @Nullable ClassLoader classLoader, int maxBackupFi this.writeDelay = writeDelay; this.maxDeferredPeriod = maxDeferredPeriod; - this.internalMapper = new GsonBuilder() - .registerTypeHierarchyAdapter(Map.class, new StorageEntryMapDeserializer()).setPrettyPrinting() + this.internalMapper = new GsonBuilder() // + .registerTypeHierarchyAdapter(Map.class, new OrderingMapSerializer())// + .registerTypeHierarchyAdapter(Set.class, new OrderingSetSerializer())// + .registerTypeHierarchyAdapter(Map.class, new StorageEntryMapDeserializer()) // + .setPrettyPrinting() // + .create(); + this.entityMapper = new GsonBuilder() // + .registerTypeHierarchyAdapter(Map.class, new OrderingMapSerializer())// + .registerTypeHierarchyAdapter(Set.class, new OrderingSetSerializer())// + .registerTypeAdapter(Configuration.class, new ConfigurationDeserializer()) // + .setPrettyPrinting() // .create(); - this.entityMapper = new GsonBuilder().registerTypeAdapter(Configuration.class, new ConfigurationDeserializer()) - .setPrettyPrinting().create(); commitTimer = new Timer(); 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 76a57226aaf..8deb7ac5d7b 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 @@ -12,24 +12,38 @@ */ package org.openhab.core.storage.json.internal; +import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.*; import java.io.File; import java.io.IOException; import java.math.BigDecimal; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.file.Files; import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openhab.core.config.core.Configuration; import org.openhab.core.test.java.JavaTest; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonObject; +import com.google.gson.reflect.TypeToken; + /** * This test makes sure that the JsonStorage loads all stored numbers as BigDecimal * * @author Stefan Triller - Initial contribution + * @author Samie Salonen - test for ensuring ordering of keys in json */ public class JsonStorageTest extends JavaTest { @@ -129,8 +143,82 @@ public void testStableOutput() throws IOException { assertEquals(storageString1, storageString2); } + @SuppressWarnings({ "null", "unchecked" }) + @Test + public void testOrdering() throws IOException { + objectStorage.put("DummyObject", new DummyObject()); + { + objectStorage.put("a", new DummyObject()); + objectStorage.put("b", new DummyObject()); + persistAndReadAgain(); + } + String storageStringAB = Files.readString(tmpFile.toPath()); + + { + objectStorage.remove("a"); + objectStorage.remove("b"); + objectStorage.put("b", new DummyObject()); + objectStorage.put("a", new DummyObject()); + persistAndReadAgain(); + } + String storageStringBA = Files.readString(tmpFile.toPath()); + assertEquals(storageStringAB, storageStringBA); + + { + objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0); + objectStorage.flush(); + } + String storageStringReserialized = Files.readString(tmpFile.toPath()); + assertEquals(storageStringAB, storageStringReserialized); + Gson gson = new GsonBuilder().create(); + + // Parse json. Gson preserves json object key ordering when we parse only JsonObject + JsonObject orderedMap = gson.fromJson(storageStringAB, JsonObject.class); + // Assert ordering of top level keys (uppercase first in alphabetical order, then lowercase items in + // alphabetical order) + assertArrayEquals(new String[] { "DummyObject", "a", "b" }, orderedMap.keySet().toArray()); + // Ordering is ensured also for sub-keys of Configuration object + assertArrayEquals( + new String[] { "multiInt", "testBigDecimal", "testBoolean", "testDouble", "testFloat", "testInt", + "testLong", "testShort", "testString" }, + orderedMap.getAsJsonObject("DummyObject").getAsJsonObject("value").getAsJsonObject("configuration") + .getAsJsonObject("properties").keySet().toArray()); + // Set having non-comparable items remains unordered + assertArrayEquals( + new String[] { "http://www.example.com/key2", "http://www.example.com/key1", + "http://www.example.com/key3" }, + gson.fromJson(orderedMap.getAsJsonObject("DummyObject").getAsJsonObject("value") + .getAsJsonArray("innerSetWithNonComparableElements"), LinkedList.class).toArray()); + // ...while Set having all Comparable keys is ordered: + assertArrayEquals(new Object[] { -5, 0, 3, 50 }, + ((LinkedList) gson.fromJson( + orderedMap.getAsJsonObject("DummyObject").getAsJsonObject("value") + .getAsJsonArray("innerSetWithComparableElements"), + TypeToken.getParameterized(LinkedList.class, Integer.class).getType())).toArray()); + + // Map having non-comparable items remains unordered + assertArrayEquals( + new String[] { "http://www.example.com/key2", "http://www.example.com/key1", + "http://www.example.com/key3" }, + gson.fromJson(orderedMap.getAsJsonObject("DummyObject").getAsJsonObject("value") + .getAsJsonObject("innerMapWithNonComparableKeys"), LinkedHashMap.class).keySet().toArray()); + // ...while Map with Comparable keys is ordered + assertArrayEquals(new Integer[] { -5, 0, 3, 50 }, + ((LinkedHashMap) gson.fromJson( + orderedMap.getAsJsonObject("DummyObject").getAsJsonObject("value") + .getAsJsonObject("innerMapWithComparableKeys"), + TypeToken.getParameterized(LinkedHashMap.class, Integer.class, Object.class).getType())) + .keySet().toArray()); + } + private static class DummyObject { + // For the test here we use Linked variants of Map and Set which preserve the insertion order + // In tests we verify that collections having Comparable items (keys) are ordered on serialization + private final Map innerMapWithNonComparableKeys = new LinkedHashMap<>(); + private final Map innerMapWithComparableKeys = new LinkedHashMap<>(); + private final Set innerSetWithNonComparableElements = new LinkedHashSet<>(); + private final Set innerSetWithComparableElements = new LinkedHashSet<>(); private final Configuration configuration = new Configuration(); public List channels = new ArrayList<>(); @@ -148,6 +236,27 @@ public DummyObject() { InnerObject inner = new InnerObject(); inner.configuration.put("testChildLong", Long.valueOf("12")); channels.add(inner); + innerMapWithComparableKeys.put(3, 1); + innerMapWithComparableKeys.put(0, 2); + innerMapWithComparableKeys.put(50, 3); + innerMapWithComparableKeys.put(-5, 4); + + innerSetWithComparableElements.add(3); + innerSetWithComparableElements.add(0); + innerSetWithComparableElements.add(50); + innerSetWithComparableElements.add(-5); + + try { + innerMapWithNonComparableKeys.put(new URL("http://www.example.com/key2"), 1); + innerMapWithNonComparableKeys.put(new URL("http://www.example.com/key1"), 2); + innerMapWithNonComparableKeys.put(new URL("http://www.example.com/key3"), 3); + + innerSetWithNonComparableElements.add(new URL("http://www.example.com/key2")); + innerSetWithNonComparableElements.add(new URL("http://www.example.com/key1")); + innerSetWithNonComparableElements.add(new URL("http://www.example.com/key3")); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } } }