Skip to content

Commit

Permalink
Don't update unchanged things in .things file
Browse files Browse the repository at this point in the history
There were two problems:
- The old things weren't removed, resulting in accumulation of duplicate things and comparing the new one against the old one resulting in erroneous update
-  Numeric values (usually entered as integer) in a newly loaded Channel Configuration properties are stored as BigDecimal with Scale 0, but subsequent normalization changed it to scale 1. This made equals() return false when it shouldn't. This leads to calling notifyListenersAboutUpdatedElement unnecessarily.

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
  • Loading branch information
jimtng committed Feb 1, 2024
1 parent 9b5e19e commit 8a6ed12
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,12 @@ public static Object normalizeType(Object value, @Nullable ConfigDescriptionPara
if (configDescriptionParameter != null) {
Normalizer normalizer = NormalizerFactory.getNormalizer(configDescriptionParameter);
return normalizer.normalize(value);
} else if (value instanceof Boolean || value instanceof String || value instanceof BigDecimal) {
return value;
} else if (value instanceof Boolean) {
return NormalizerFactory.getNormalizer(Type.BOOLEAN).normalize(value);
} else if (value instanceof String) {
return NormalizerFactory.getNormalizer(Type.TEXT).normalize(value);
} else if (value instanceof Number) {
return new BigDecimal(value.toString());
return NormalizerFactory.getNormalizer(Type.DECIMAL).normalize(value);
} else if (value instanceof Collection collection) {
return normalizeCollection(collection);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@ final class DecimalNormalizer extends AbstractNormalizer {
@Override
public Object doNormalize(Object value) {
try {
if (value instanceof BigDecimal) {
return stripTrailingZeros((BigDecimal) value);
if (value instanceof BigDecimal bd) {
return stripTrailingZeros(bd);
}
if (value instanceof String) {
return stripTrailingZeros(new BigDecimal((String) value));
if (value instanceof String strValue) {
return stripTrailingZeros(new BigDecimal(strValue));
}
if (value instanceof Byte) {
return new BigDecimal((Byte) value).setScale(1);
}
if (value instanceof Short) {
return new BigDecimal((Short) value).setScale(1);
}
if (value instanceof Integer) {
return new BigDecimal((Integer) value).setScale(1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,14 @@ public static Normalizer getNormalizer(ConfigDescriptionParameter configDescript
Normalizer ret = NORMALIZERS.get(configDescriptionParameter.getType());
return configDescriptionParameter.isMultiple() ? new ListNormalizer(ret) : ret;
}

/**
* Returns the {@link Normalizer} for the given ConfigDescriptionParameter type.
*
* @param type the type
* @return the corresponding {@link Normalizer} (not null)
*/
public static Normalizer getNormalizer(Type type) {
return NORMALIZERS.get(type);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void assertPropertiesCanBeRemoved() {

Configuration configuration = new Configuration(orgProperties);

assertThat(configuration.get("intField"), is(equalTo(BigDecimal.ONE)));
assertThat(configuration.get("intField"), is(equalTo(BigDecimal.ONE.setScale(1))));

configuration.setProperties(newProperties);

Expand All @@ -160,18 +160,42 @@ public void assertToStringHandlesNullValuesGracefully() {
@Test
public void assertNormalizationInSetProperties() {
Map<String, Object> properties = new HashMap<>();
properties.put("byteField", Byte.valueOf("1"));
properties.put("shortField", Short.valueOf("1"));
properties.put("intField", 1);
properties.put("longField", Long.valueOf("1"));
properties.put("doubleField", Double.valueOf("1"));
properties.put("floatField", Float.valueOf("1"));
properties.put("bigDecimalField", BigDecimal.ONE);

Configuration configuration = new Configuration();
configuration.setProperties(properties);
assertThat(configuration.get("intField"), is(equalTo(BigDecimal.ONE)));
assertThat(configuration.get("byteField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("shortField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("intField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("longField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("doubleField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("floatField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("bigDecimalField"), is(equalTo(BigDecimal.ONE.setScale(1))));
}

@Test
public void assertNormalizationInPut() {
Configuration configuration = new Configuration();
configuration.put("byteField", Byte.valueOf("1"));
configuration.put("shortField", Short.valueOf("1"));
configuration.put("intField", 1);
assertThat(configuration.get("intField"), is(equalTo(BigDecimal.ONE)));
configuration.put("longField", Long.valueOf("1"));
configuration.put("doubleField", Double.valueOf("1"));
configuration.put("floatField", Float.valueOf("1"));
configuration.put("bigDecimalField", BigDecimal.ONE);
assertThat(configuration.get("byteField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("shortField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("intField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("longField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("doubleField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("floatField"), is(equalTo(BigDecimal.ONE.setScale(1))));
assertThat(configuration.get("bigDecimalField"), is(equalTo(BigDecimal.ONE.setScale(1))));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,8 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
if (!loadedXmlThingTypes.contains(factory.bundleName) || modelRepository == null) {
return
}
val oldThings = thingsMap.get(modelName).clone
val newThings = newArrayList()
val oldThings = thingsMap.put(modelName, newThings)

val model = modelRepository.getModel(modelName) as ThingModel
if (model !== null) {
Expand All @@ -616,7 +616,6 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
}

newThings.forEach [ newThing |
thingsMap.get(modelName).add(newThing)
val oldThing = oldThings.findFirst[it.UID == newThing.UID]
if (oldThing !== null) {
if (!ThingHelper.equals(oldThing, newThing)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ public void testIntegerScaleFromCache() {
DummyObject dummy = objectStorage.get("DummyObject");

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

@SuppressWarnings("unchecked")
Expand All @@ -121,14 +121,14 @@ public void testIntegerScaleFromDisk() {
DummyObject dummy = objectStorage.get("DummyObject");

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

@Test
Expand Down

0 comments on commit 8a6ed12

Please sign in to comment.