diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java index c6e47c58ae7..0c75399f6de 100644 --- a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java +++ b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java @@ -3,6 +3,7 @@ package org.terasology.persistence.typeHandling.coreTypes; +import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import org.slf4j.Logger; @@ -61,7 +62,8 @@ private PersistedData serializeEntry(Map.Entry entry, PersistedDataSeriali @Override public Optional> deserialize(PersistedData data) { - if (!data.isArray()) { + if (!data.isArray() || data.isValueMap()) { + logger.warn("Incorrect map format detected: object instead of array.\n" + getUsageInfo(data)); return Optional.empty(); } @@ -69,20 +71,44 @@ public Optional> deserialize(PersistedData data) { for (PersistedData entry : data.getAsArray()) { PersistedDataMap kvEntry = entry.getAsValueMap(); - final Optional key = keyHandler.deserialize(kvEntry.get(KEY)); - - if (key.isPresent()) { - final Optional value = valueHandler.deserialize(kvEntry.get(VALUE)); - if (value.isPresent()) { - result.put(key.get(), value.get()); - } else { - logger.warn("Missing value for key '{}' with {} given entry '{}'", key.get(), valueHandler, kvEntry.get(VALUE)); - } - } else { - logger.warn("Missing field '{}' for entry '{}'", KEY, kvEntry); + PersistedData rawKey = kvEntry.get(KEY); + PersistedData rawValue = kvEntry.get(VALUE); + if (rawKey == null || rawValue == null) { + logger.warn("Incorrect map format detected: missing map entry with \"key\" or \"value\" key.\n" + getUsageInfo(data)); + return Optional.empty(); } + + final Optional key = keyHandler.deserialize(rawKey); + if (key.isEmpty()) { + logger.warn("Could not deserialize key '{}' as '{}'", rawKey, keyHandler.getClass().getSimpleName()); + return Optional.empty(); + } + + final Optional value = valueHandler.deserialize(kvEntry.get(VALUE)); + if (value.isEmpty()) { + logger.warn("Could not deserialize value '{}' as '{}'", rawValue, valueHandler.getClass().getSimpleName()); + return Optional.empty(); + } + + result.put(key.get(), value.get()); } return Optional.of(result); } + + private String getUsageInfo(PersistedData data) { + return "Expected\n" + + " \"mapName\": [\n" + + " { \"key\": \"...\", \"value\": \"...\" }\n" + + " ]\n" + + "but found \n'{}'" + data + "'"; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("key", keyHandler) + .add("value", valueHandler) + .toString(); + } } diff --git a/subsystems/TypeHandlerLibrary/src/test/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandlerTest.java b/subsystems/TypeHandlerLibrary/src/test/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandlerTest.java index 0419524eab8..0cb9ec45bd6 100644 --- a/subsystems/TypeHandlerLibrary/src/test/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandlerTest.java +++ b/subsystems/TypeHandlerLibrary/src/test/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandlerTest.java @@ -3,6 +3,7 @@ package org.terasology.persistence.typeHandling.coreTypes; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.terasology.persistence.typeHandling.PersistedData; import org.terasology.persistence.typeHandling.PersistedDataSerializer; @@ -12,7 +13,6 @@ import org.terasology.persistence.typeHandling.inMemory.PersistedString; import org.terasology.persistence.typeHandling.inMemory.arrays.PersistedValueArray; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; @@ -25,6 +25,17 @@ class GenericMapTypeHandlerTest { private static final String TEST_KEY = "health:baseRegen"; private static final long TEST_VALUE = -1; + /** + * JSON equivalent: + *

+     * [
+     *   {
+     *       "key": "health:baseRegen",
+     *       "value": -1
+     *   }
+     * ]
+     * 
+ */ private final PersistedData testData = new PersistedValueArray(List.of( new PersistedMap(Map.of( GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY), @@ -32,7 +43,84 @@ GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE) )) )); + /** + * JSON equivalent: + *

+     * {
+     *   "health:baseRegen": -1
+     * }
+     * 
+ */ + private final PersistedData testDataMalformatted = new PersistedValueArray(List.of( + new PersistedMap(Map.of( + TEST_KEY, new PersistedLong(TEST_VALUE) + )) + )); + + /** + * JSON equivalent: + *

+     * [
+     *   {
+     *       "not key": "health:baseRegen",
+     *       "value": -1
+     *   }
+     * ]
+     * 
+ */ + private final PersistedData testDataMissingKeyEntry = new PersistedValueArray(List.of( + new PersistedMap(Map.of( + "not key", new PersistedString(TEST_KEY), + GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE) + )) + )); + + /** + * JSON equivalent: + *

+     * [
+     *   {
+     *       "key": "health:baseRegen",
+     *       "not value": -1
+     *   }
+     * ]
+     * 
+ */ + private final PersistedData testDataMissingValueEntry = new PersistedValueArray(List.of( + new PersistedMap(Map.of( + GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY), + "not value", new PersistedLong(TEST_VALUE) + )) + )); + + /** + * JSON equivalent: + *

+     * [
+     *   {
+     *       "key": "health:baseRegen",
+     *       "value": -1
+     *   },
+     *   {
+     *       "not key": "health:baseRegen",
+     *       "not value": -1
+     *   },
+     * ]
+     * 
+ */ + private final PersistedData testDataValidAndInvalidMix = new PersistedValueArray(List.of( + new PersistedMap(Map.of( + GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY), + GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE) + )), + new PersistedMap(Map.of( + "not key", new PersistedString(TEST_KEY), + "not value", new PersistedLong(TEST_VALUE) + )) + )); + @Test + @DisplayName("Data with valid formatting can be deserialized successfully.") void testDeserialize() { var th = new GenericMapTypeHandler<>( new StringTypeHandler(), @@ -44,23 +132,69 @@ void testDeserialize() { } @Test + @DisplayName("Deserializing valid data with a mismatching value type handler fails deserialization (returns empty `Optional`)") void testDeserializeWithMismatchedValueHandler() { var th = new GenericMapTypeHandler<>( new StringTypeHandler(), new UselessTypeHandler<>() ); - assertThat(th.deserialize(testData)).hasValue(Collections.emptyMap()); + assertThat(th.deserialize(testData)).isEmpty(); } @Test + @DisplayName("Deserializing valid data with a mismatching key type handler fails deserialization (returns empty `Optional`)") void testDeserializeWithMismatchedKeyHandler() { var th = new GenericMapTypeHandler<>( new UselessTypeHandler<>(), new LongTypeHandler() ); - assertThat(th.deserialize(testData)).hasValue(Collections.emptyMap()); + assertThat(th.deserialize(testData)).isEmpty(); + } + + @Test + @DisplayName("Incorrectly formatted data (without an outer array) fails deserialization (returns empty `Optional`)") + void testDeserializeWithObjectInsteadOfArray() { + var th = new GenericMapTypeHandler<>( + new StringTypeHandler(), + new LongTypeHandler() + ); + + assertThat(th.deserialize(testDataMalformatted)).isEmpty(); + } + + @Test + @DisplayName("Incorrectly formatted data (without a map entry with key \"key\") fails deserialization (returns empty `Optional`)") + void testDeserializeWithMissingKeyEntry() { + var th = new GenericMapTypeHandler<>( + new StringTypeHandler(), + new LongTypeHandler() + ); + + assertThat(th.deserialize(testDataMissingKeyEntry)).isEmpty(); + } + + @Test + @DisplayName("Incorrectly formatted data (without a map entry with key \"value\") fails deserialization (returns empty `Optional`)") + void testDeserializeWithMissingValueEntry() { + var th = new GenericMapTypeHandler<>( + new StringTypeHandler(), + new LongTypeHandler() + ); + + assertThat(th.deserialize(testDataMissingValueEntry)).isEmpty(); + } + + @Test + @DisplayName("A map containing both, correctly and incorrectly formatted data, fails deserialization (returns empty `Optional`)") + void testDeserializeWithValidAndInvalidEntries() { + var th = new GenericMapTypeHandler<>( + new StringTypeHandler(), + new LongTypeHandler() + ); + + assertThat(th.deserialize(testDataValidAndInvalidMix)).isEmpty(); } /** Never returns a value. */