fix(TypeHandlerLibrary): fix error-during-error-logging in GenericMap (#5048)

keturn · web-flow · commit fbc40c3fe2cf · 2022-06-14T15:46:02.000-07:00
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
@@ -1,4 +1,4 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 
 package org.terasology.persistence.typeHandling.coreTypes;
@@ -8,6 +8,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.terasology.persistence.typeHandling.PersistedData;
+import org.terasology.persistence.typeHandling.PersistedDataMap;
 import org.terasology.persistence.typeHandling.PersistedDataSerializer;
 import org.terasology.persistence.typeHandling.TypeHandler;
 
@@ -25,10 +26,10 @@
  */
 public class GenericMapTypeHandler<K, V> extends TypeHandler<Map<K, V>> {
 
-    private static final Logger logger = LoggerFactory.getLogger(GenericMapTypeHandler.class);
+    static final String KEY = "key";
+    static final String VALUE = "value";
 
-    private static final String KEY = "key";
-    private static final String VALUE = "value";
+    private static final Logger logger = LoggerFactory.getLogger(GenericMapTypeHandler.class);
 
     private final TypeHandler<K> keyHandler;
     private final TypeHandler<V> valueHandler;
@@ -67,17 +68,18 @@ public Optional<Map<K, V>> deserialize(PersistedData data) {
         Map<K, V> result = Maps.newLinkedHashMap();
 
         for (PersistedData entry : data.getAsArray()) {
-            final Optional<K> key = keyHandler.deserialize(entry.getAsValueMap().get(KEY));
-            final Optional<V> value = valueHandler.deserialize(entry.getAsValueMap().get(VALUE));
+            PersistedDataMap kvEntry = entry.getAsValueMap();
+            final Optional<K> key = keyHandler.deserialize(kvEntry.get(KEY));
 
             if (key.isPresent()) {
+                final Optional<V> value = valueHandler.deserialize(kvEntry.get(VALUE));
                 if (value.isPresent()) {
                     result.put(key.get(), value.get());
                 } else {
-                    logger.warn("Missing field '{}' for entry '{}'", VALUE, data.getAsString());
+                    logger.warn("Missing value for key '{}' with {} given entry '{}'", key.get(), valueHandler, kvEntry.get(VALUE));
                 }
             } else {
-                logger.warn("Missing field '{}' for entry '{}'", KEY, data.getAsString());
+                logger.warn("Missing field '{}' for entry '{}'", KEY, kvEntry);
             }
         }
 
diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/ObjectFieldMapTypeHandler.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/ObjectFieldMapTypeHandler.java
@@ -1,8 +1,9 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 package org.terasology.persistence.typeHandling.coreTypes;
 
 import com.google.common.base.Defaults;
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.Maps;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -111,4 +112,12 @@ public Optional<T> deserialize(PersistedData data) {
         }
         return Optional.empty();
     }
+
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this)
+                .add("fields", fieldByName.keySet())
+                .add("constructor", constructor)
+                .toString();
+    }
 }
diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/RuntimeDelegatingTypeHandler.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/RuntimeDelegatingTypeHandler.java
@@ -1,7 +1,8 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 package org.terasology.persistence.typeHandling.coreTypes;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.Maps;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,10 +35,10 @@ public class RuntimeDelegatingTypeHandler<T> extends TypeHandler<T> {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(RuntimeDelegatingTypeHandler.class);
 
-    private TypeHandler<T> delegateHandler;
-    private TypeInfo<T> typeInfo;
-    private TypeHandlerLibrary typeHandlerLibrary;
-    private SerializationSandbox sandbox;
+    private final TypeHandler<T> delegateHandler;
+    private final TypeInfo<T> typeInfo;
+    private final TypeHandlerLibrary typeHandlerLibrary;
+    private final SerializationSandbox sandbox;
 
     public RuntimeDelegatingTypeHandler(TypeHandler<T> delegateHandler, TypeInfo<T> typeInfo, TypeHandlerContext context) {
         this.delegateHandler = delegateHandler;
@@ -242,4 +243,11 @@ private Optional<Type> findSubtypeWithName(String runtimeTypeName) {
                    );
     }
 
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this)
+                .add("delegate", delegateHandler)
+                .add("type", typeInfo)
+                .toString();
+    }
 }
diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/inMemory/PersistedMap.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/inMemory/PersistedMap.java
@@ -1,7 +1,8 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 package org.terasology.persistence.typeHandling.inMemory;
 
+import com.google.common.base.MoreObjects;
 import org.terasology.persistence.typeHandling.PersistedData;
 import org.terasology.persistence.typeHandling.PersistedDataArray;
 import org.terasology.persistence.typeHandling.PersistedDataMap;
@@ -80,4 +81,11 @@ public PersistedDataArray getAsArray(String name) {
     public Set<Map.Entry<String, PersistedData>> entrySet() {
         return map.entrySet();
     }
+
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this)
+                .addValue(map)
+                .toString();
+    }
 }
diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/inMemory/PersistedString.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/inMemory/PersistedString.java
@@ -1,10 +1,12 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 package org.terasology.persistence.typeHandling.inMemory;
 
+import com.google.common.base.MoreObjects;
+
 public class PersistedString extends AbstractPersistedData {
 
-    private String data;
+    private final String data;
 
     public PersistedString(String data) {
         this.data = data;
@@ -20,4 +22,10 @@ public boolean isString() {
         return true;
     }
 
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this)
+                .addValue(data)
+                .toString();
+    }
 }
diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/reflection/reflect/ConstructorLibrary.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/reflection/reflect/ConstructorLibrary.java
@@ -1,7 +1,8 @@
-// Copyright 2021 The Terasology Foundation
+// Copyright 2022 The Terasology Foundation
 // SPDX-License-Identifier: Apache-2.0
 package org.terasology.reflection.reflect;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultiset;
@@ -65,6 +66,13 @@ public T construct() {
                             "Register an InstanceCreator for this type to fix this problem.", e);
                 }
             }
+
+            @Override
+            public String toString() {
+                return MoreObjects.toStringHelper(this)
+                        .add("type", typeInfo)
+                        .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
@@ -0,0 +1,78 @@
+// Copyright 2022 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+
+package org.terasology.persistence.typeHandling.coreTypes;
+
+import org.junit.jupiter.api.Test;
+import org.terasology.persistence.typeHandling.PersistedData;
+import org.terasology.persistence.typeHandling.PersistedDataSerializer;
+import org.terasology.persistence.typeHandling.TypeHandler;
+import org.terasology.persistence.typeHandling.inMemory.PersistedLong;
+import org.terasology.persistence.typeHandling.inMemory.PersistedMap;
+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;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+
+class GenericMapTypeHandlerTest {
+
+    private static final String TEST_KEY = "health:baseRegen";
+    private static final long TEST_VALUE = -1;
+
+    private final PersistedData testData = new PersistedValueArray(List.of(
+            new PersistedMap(Map.of(
+                    GenericMapTypeHandler.KEY, new PersistedString(TEST_KEY),
+                    GenericMapTypeHandler.VALUE, new PersistedLong(TEST_VALUE)
+            ))
+    ));
+
+    @Test
+    void testDeserialize() {
+        var th = new GenericMapTypeHandler<>(
+                new StringTypeHandler(),
+                new LongTypeHandler()
+        );
+
+        assertThat(th.deserialize(testData)).isPresent();
+        assertThat(th.deserialize(testData).get()).containsExactly(TEST_KEY, TEST_VALUE);
+    }
+
+    @Test
+    void testDeserializeWithMismatchedValueHandler() {
+        var th = new GenericMapTypeHandler<>(
+                new StringTypeHandler(),
+                new UselessTypeHandler<>()
+        );
+
+        assertThat(th.deserialize(testData)).hasValue(Collections.emptyMap());
+    }
+
+    @Test
+    void testDeserializeWithMismatchedKeyHandler() {
+        var th = new GenericMapTypeHandler<>(
+                new UselessTypeHandler<>(),
+                new LongTypeHandler()
+        );
+
+        assertThat(th.deserialize(testData)).hasValue(Collections.emptyMap());
+    }
+
+    /** Never returns a value. */
+    private static class UselessTypeHandler<T> extends TypeHandler<T> {
+        @Override
+        protected PersistedData serializeNonNull(Object value, PersistedDataSerializer serializer) {
+            return null;
+        }
+
+        @Override
+        public Optional<T> deserialize(PersistedData data) {
+            return Optional.empty();
+        }
+    }
+}
