Skip to content

Commit

Permalink
fix number deserialization on Json Storage
Browse files Browse the repository at this point in the history
As it turned out, the current Map deserializer was not very effective
in converting numbers to BigDecimals as it took itself out of the game
when deserializing the outer map which JsonStorage adds itself. Hence
a property map within an entity lost its normalization.

This could have been compensated again during read by simply cutting off
the trailing ".0"s, but this still led to the awkward situation that the
numbers in the serialized format were altered and looked different of
what would have been expected.

Therefore this change
* Distinguishes the "outer map" (i.e. the internal data structure of the
  JsonStorage and the inner entity structures
* Uses two different Gson instances for handling those two structures
  individually
* Lets JsonStorage internally keep JsonElement structures _always_
* Registers a custom deserializer for Configuration which handles the
  custom number-to-BigDecimal conversion

fixes eclipse-archived#3774
Signed-off-by: Simon Kaufmann <[email protected]>
  • Loading branch information
Simon Kaufmann committed Jul 5, 2017
1 parent db3a098 commit cfcd58e
Show file tree
Hide file tree
Showing 8 changed files with 339 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Bundle-Vendor: Eclipse.org/SmartHome
Fragment-Host: org.eclipse.smarthome.storage.json
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
Import-Package: groovy.lang,
org.apache.commons.io,
org.codehaus.groovy.reflection,
org.codehaus.groovy.runtime,
org.codehaus.groovy.runtime.callsite,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/**
* Copyright (c) 2014-2017 by the respective copyright holders.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*/
package org.eclipse.smarthome.storage.json;

import static org.junit.Assert.assertEquals;

import java.io.File;
import java.io.IOException;
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import org.apache.commons.io.FileUtils;
import org.eclipse.smarthome.config.core.Configuration;
import org.eclipse.smarthome.storage.json.JsonStorage;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

/**
* This test makes sure that the JSonStorage loads all stored numbers as BigDecimal
*
* @author Stefan Triller - Initial Contribution
*/
public class JSonStorageTest {

private JsonStorage<DummyObject> objectStorage;
private File tmpFile;

@Before
public void setUp() throws IOException {
tmpFile = File.createTempFile("storage-debug", ".json");
tmpFile.deleteOnExit();

objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0);
objectStorage.put("DummyObject", new DummyObject());
}

private void persistAndReadAgain() {
objectStorage.commitDatabase();
objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0);
}

@Test
public void allInsertedNumbersAreLoadedAsBigDecimal_fromCache() {
DummyObject dummy = objectStorage.get("DummyObject");

Assert.assertTrue(dummy.configuration.get("testShort") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testInt") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testLong") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testDouble") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testFloat") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testBigDecimal") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testBoolean") instanceof Boolean);
Assert.assertTrue(dummy.configuration.get("testString") instanceof String);
}

@Test
public void allInsertedNumbersAreLoadedAsBigDecimal_fromDisk() {
persistAndReadAgain();
DummyObject dummy = objectStorage.get("DummyObject");

Assert.assertTrue(dummy.configuration.get("testShort") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testInt") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testLong") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testDouble") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testFloat") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testBigDecimal") instanceof BigDecimal);
Assert.assertTrue(dummy.configuration.get("testBoolean") instanceof Boolean);
Assert.assertTrue(dummy.configuration.get("testString") instanceof String);
}

@Test
public void testIntegerScale_fromCache() {
objectStorage.put("DummyObject", new DummyObject());
DummyObject dummy = objectStorage.get("DummyObject");

Assert.assertEquals(((BigDecimal) dummy.configuration.get("testShort")).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.configuration.get("testInt")).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.configuration.get("testLong")).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.configuration.get("testBigDecimal")).scale(), 0);
}

@SuppressWarnings("unchecked")
@Test
public void testIntegerScale_fromDisk() {
objectStorage.put("DummyObject", new DummyObject());
DummyObject dummy = objectStorage.get("DummyObject");

Assert.assertEquals(((BigDecimal) dummy.configuration.get("testShort")).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.configuration.get("testInt")).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.configuration.get("testLong")).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.configuration.get("testBigDecimal")).scale(), 0);
Assert.assertEquals(((List<BigDecimal>) dummy.configuration.get("multiInt")).get(0).scale(), 0);
Assert.assertEquals(((List<BigDecimal>) dummy.configuration.get("multiInt")).get(1).scale(), 0);
Assert.assertEquals(((List<BigDecimal>) dummy.configuration.get("multiInt")).get(2).scale(), 0);
Assert.assertEquals(((BigDecimal) dummy.channels.get(0).configuration.get("testChildLong")).scale(), 0);
}

@Test
public void testStableOutput() throws IOException {
persistAndReadAgain();
String storageString1 = FileUtils.readFileToString(tmpFile);

objectStorage = new JsonStorage<>(tmpFile, this.getClass().getClassLoader(), 0, 0, 0);
objectStorage.commitDatabase();
String storageString2 = FileUtils.readFileToString(tmpFile);

assertEquals(storageString1, storageString2);
}

private static class DummyObject {

private Configuration configuration = new Configuration();
public List<InnerObject> channels = new ArrayList<>();

public DummyObject() {
configuration.put("testShort", Short.valueOf("12"));
configuration.put("testInt", Integer.valueOf("12"));
configuration.put("testLong", Long.valueOf("12"));
configuration.put("testDouble", Double.valueOf("12.12"));
configuration.put("testFloat", Float.valueOf("12.12"));
configuration.put("testBigDecimal", new BigDecimal(12));
configuration.put("testBoolean", true);
configuration.put("testString", "hello world");
configuration.put("multiInt", Arrays.asList(1, 2, 3));

InnerObject inner = new InnerObject();
inner.configuration.put("testChildLong", Long.valueOf("12"));
channels.add(inner);
}
}

private static class InnerObject {
private Configuration configuration = new Configuration();

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@
import java.io.IOException;
import java.math.BigDecimal;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.apache.commons.io.FileUtils;
import org.eclipse.smarthome.config.core.Configuration;
import org.eclipse.smarthome.core.storage.Storage;
import org.eclipse.smarthome.core.storage.StorageService;
import org.eclipse.smarthome.test.java.JavaOSGiTest;
Expand Down Expand Up @@ -85,27 +84,19 @@ public void testClassloader() {

@Test
public void testConfiguration() {
Storage<MockConfiguration> storageWithoutClassloader = storageService.getStorage("storage");
Storage<DummyObject> storageWithoutClassloader = storageService.getStorage("storage");

MockConfiguration configuration = new MockConfiguration();
configuration.put("bigDecimal", new BigDecimal(3));
DummyObject dummy = new DummyObject();
dummy.configuration.put("bigDecimal", new BigDecimal(3));

storageWithoutClassloader.put("configuration", configuration);
storageWithoutClassloader.put("configuration", dummy);

Object bigDecimal = storageWithoutClassloader.get("configuration").get("bigDecimal");
Object bigDecimal = storageWithoutClassloader.get("configuration").configuration.get("bigDecimal");
assertThat(bigDecimal instanceof BigDecimal, is(true));
}

private class MockConfiguration {
private Map<String, Object> configuration = new HashMap<String, Object>();

public void put(String key, Object value) {
configuration.put(key, value);
}

public Object get(String key) {
return configuration.get(key);
}
private class DummyObject {
private Configuration configuration = new Configuration();
}

private class PersistedItem {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* Copyright (c) 2014-2016 by the respective copyright holders.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*/
package org.eclipse.smarthome.storage.json;

import java.lang.reflect.Type;
import java.math.BigDecimal;
import java.util.LinkedList;
import java.util.List;
import java.util.Map.Entry;

import org.eclipse.smarthome.config.core.Configuration;

import com.google.gson.JsonArray;
import com.google.gson.JsonDeserializationContext;
import com.google.gson.JsonDeserializer;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParseException;
import com.google.gson.JsonPrimitive;

/**
* Deserializes a {@link Configuration} object.
*
* As opposed to Gson's default behavior, it ensures that all numbers are represented as {@link BigDecimal}s.
*
* @author Simon Kaufmann - initial contribution and API
*
*/
public class ConfigurationDeserializer implements JsonDeserializer<Configuration> {

@Override
public Configuration deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context)
throws JsonParseException {
Configuration configuration = new Configuration();
JsonObject configurationObject = json.getAsJsonObject();
JsonObject propertiesObject = configurationObject.get("properties").getAsJsonObject();
for (Entry<String, JsonElement> entry : propertiesObject.entrySet()) {
JsonElement value = entry.getValue();
String key = entry.getKey();
if (value.isJsonPrimitive()) {
JsonPrimitive primitive = value.getAsJsonPrimitive();
configuration.put(key, deserialize(primitive));
} else if (value.isJsonArray()) {
JsonArray array = value.getAsJsonArray();
configuration.put(key, deserialize(array));
} else {
throw new IllegalArgumentException(
"Configuration parameters must be primitives or arrays of primities only but was " + value);
}
}
return configuration;
}

private Object deserialize(JsonPrimitive primitive) {
if (primitive.isString()) {
return primitive.getAsString();
} else if (primitive.isNumber()) {
return primitive.getAsBigDecimal();
} else if (primitive.isBoolean()) {
return primitive.getAsBoolean();
} else {
throw new IllegalArgumentException("Unsupported primitive: " + primitive);
}
}

private Object deserialize(JsonArray array) {
List<Object> list = new LinkedList<>();
for (JsonElement element : array) {
if (element.isJsonPrimitive()) {
JsonPrimitive primitive = element.getAsJsonPrimitive();
list.add(deserialize(primitive));
} else {
throw new IllegalArgumentException("Multiples must only contain primitives but was " + element);
}
}
return list;
}

}
Loading

0 comments on commit cfcd58e

Please sign in to comment.