From a2661e3163ccdc21fcb2efce5b8ec6cfcf8a89a1 Mon Sep 17 00:00:00 2001 From: marecabo <23156476+marecabo@users.noreply.github.com> Date: Mon, 9 Oct 2023 16:24:41 +0200 Subject: [PATCH 1/2] Support simple maps in ReflectiveConfigGroup --- matsim/pom.xml | 4 ++ .../core/config/ReflectiveConfigGroup.java | 49 ++++++++++++++++++- .../config/ReflectiveConfigGroupTest.java | 43 ++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/matsim/pom.xml b/matsim/pom.xml index 4dc0348e536..45a3e54f201 100644 --- a/matsim/pom.xml +++ b/matsim/pom.xml @@ -257,6 +257,10 @@ com.fasterxml.jackson.core jackson-core + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + org.mockito mockito-core diff --git a/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java b/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java index 1301c6daeb1..391c1577394 100644 --- a/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java +++ b/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java @@ -39,7 +39,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.EnumMap; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -48,11 +50,17 @@ import javax.annotation.Nullable; +import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.matsim.api.core.v01.Id; import org.matsim.core.api.internal.MatsimExtensionPoint; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.type.TypeFactory; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; @@ -105,6 +113,12 @@ public abstract class ReflectiveConfigGroup extends ConfigGroup implements MatsimExtensionPoint { private static final Logger log = LogManager.getLogger(ReflectiveConfigGroup.class); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + static { + OBJECT_MAPPER.registerModule(new JavaTimeModule()); + } + private final boolean storeUnknownParameters; private final Map setters; @@ -240,8 +254,10 @@ private static void checkParamFieldValidity(Field field) { Integer.TYPE, Long.TYPE, Boolean.TYPE, Character.TYPE, Byte.TYPE, Short.TYPE, LocalTime.class, LocalDate.class, LocalDateTime.class); + private static final Set> ALLOWED_MAP_PARAMETER_TYPES = ALLOWED_PARAMETER_TYPES; // currently identical + private static final String HINT = " Valid types are String, primitive types and their wrapper classes," - + " enumerations, List and Set, LocalTime, LocalDate, LocalDateTime" + + " enumerations, List and Set, Maps, LocalTime, LocalDate, LocalDateTime" + " Other types are fine as parameters, but you will need to implement conversion strategies" + " in corresponding StringGetters andStringSetters."; @@ -255,6 +271,11 @@ private static boolean checkType(Type type) { if (rawType.equals(List.class) || rawType.equals(Set.class)) { var typeArgument = pType.getActualTypeArguments()[0]; return typeArgument.equals(String.class) || (typeArgument instanceof Class && ((Class) typeArgument).isEnum()); + } else if (rawType.equals(Map.class)) { + var keyType = pType.getActualTypeArguments()[0]; + var valueType = pType.getActualTypeArguments()[1]; + return (ALLOWED_MAP_PARAMETER_TYPES.contains(keyType) || (keyType instanceof Class && ((Class) keyType).isEnum())) && + (ALLOWED_MAP_PARAMETER_TYPES.contains(valueType) || (valueType instanceof Class && ((Class) valueType).isEnum())); } if (rawType.equals(Class.class)) @@ -406,6 +427,21 @@ private Object fromString(String value, Class type, @Nullable Field paramFiel return stream.map(s -> stringToEnumValue(s, enumConstants)).toList(); } return stream.toList(); + } else if (type.equals(Map.class) && !type.equals(EnumMap.class)) { + if (value.isBlank()) { + return Collections.emptyMap(); + } else if (paramField != null && paramField.getGenericType() instanceof ParameterizedType pType) { + Class keyClass = TypeFactory.rawClass(pType.getActualTypeArguments()[0]); + Class valueClass = TypeFactory.rawClass(pType.getActualTypeArguments()[1]); + JavaType mapType = OBJECT_MAPPER.getTypeFactory().constructMapType( + LinkedHashMap.class, keyClass, valueClass); + try { + return OBJECT_MAPPER.readValue(value, mapType); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + throw new RuntimeException("Unsupported map field"); } else if (type.equals(Class.class)) { try { return ClassLoader.getSystemClassLoader().loadClass(value); @@ -484,6 +520,17 @@ private String toString(Object result) { Preconditions.checkArgument(collection.stream().noneMatch(String::isBlank), "Collection %s contains blank elements. Only non-blank elements are supported.", collection); return String.join(", ", collection); + } else if (result instanceof Map mapResult) { + Preconditions.checkArgument(mapResult.keySet().stream() + .filter(String.class::isInstance) + .map(String.class::cast) + .noneMatch(String::isBlank), + "Map %s contains blank string keys. Only non-blank string keys are supported.", mapResult); + try { + return new StringEscapeUtils().escapeXml11(OBJECT_MAPPER.writeValueAsString(result)); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } } else { return result + ""; } diff --git a/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java b/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java index a662f53d5ad..e410b659522 100644 --- a/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java +++ b/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java @@ -26,8 +26,10 @@ import java.time.LocalDateTime; import java.time.LocalTime; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -69,6 +71,9 @@ public void testDumpAndRead() { dumpedModule.localDateTimeField = LocalDateTime.of(2022, 12, 31, 23, 59, 59); dumpedModule.enumListField = List.of(MyEnum.VALUE1, MyEnum.VALUE2); dumpedModule.enumSetField = Set.of(MyEnum.VALUE2); + dumpedModule.integerMapField = Map.of("a\"b", 1, "b", 2); + dumpedModule.localDateMapField = Map.of('d', LocalDate.of(2023, 10, 9)); + dumpedModule.booleanMapField = Map.of(0.1, true); dumpedModule.setField = ImmutableSet.of("a", "b", "c"); dumpedModule.listField = List.of("1", "2", "3"); assertEqualAfterDumpAndRead(dumpedModule); @@ -87,6 +92,9 @@ public void testDumpAndReadEmptyCollections() { dumpedModule.setField = ImmutableSet.of(); dumpedModule.enumListField = List.of(); dumpedModule.enumSetField = ImmutableSet.of(); + dumpedModule.integerMapField = Collections.emptyMap(); + dumpedModule.localDateMapField = Collections.emptyMap(); + dumpedModule.booleanMapField = Collections.emptyMap(); assertEqualAfterDumpAndRead(dumpedModule); } @@ -97,14 +105,23 @@ public void testDumpAndReadCollectionsWithExactlyOneEmptyString() { //fail on list dumpedModule.listField = List.of(""); dumpedModule.setField = null; + dumpedModule.integerMapField = null; assertThatThrownBy(() -> assertEqualAfterDumpAndRead(dumpedModule)).isInstanceOf(IllegalArgumentException.class) .hasMessage("Collection [] contains blank elements. Only non-blank elements are supported."); //fail on set dumpedModule.listField = null; dumpedModule.setField = ImmutableSet.of(""); + dumpedModule.integerMapField = null; assertThatThrownBy(() -> assertEqualAfterDumpAndRead(dumpedModule)).isInstanceOf(IllegalArgumentException.class) .hasMessage("Collection [] contains blank elements. Only non-blank elements are supported."); + + // fail on map + dumpedModule.listField = null; + dumpedModule.setField = null; + dumpedModule.integerMapField = Map.of("", 2); + assertThatThrownBy(() -> assertEqualAfterDumpAndRead(dumpedModule)).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Map {=2} contains blank string keys. Only non-blank string keys are supported."); } @Test @@ -114,14 +131,25 @@ public void testDumpAndReadCollectionsIncludingEmptyString() { //fail on list dumpedModule.listField = List.of("non-empty", ""); dumpedModule.setField = ImmutableSet.of("non-empty"); + dumpedModule.integerMapField = Map.of("a", 1, "b", 2); assertThatThrownBy(() -> assertEqualAfterDumpAndRead(dumpedModule)).isInstanceOf(IllegalArgumentException.class) .hasMessage("Collection [non-empty, ] contains blank elements. Only non-blank elements are supported."); //fail on set dumpedModule.listField = List.of("non-empty"); dumpedModule.setField = ImmutableSet.of("non-empty", ""); + dumpedModule.integerMapField = Map.of("a", 1, "b", 2); assertThatThrownBy(() -> assertEqualAfterDumpAndRead(dumpedModule)).isInstanceOf(IllegalArgumentException.class) .hasMessage("Collection [non-empty, ] contains blank elements. Only non-blank elements are supported."); + + // fail on map + dumpedModule.listField = List.of("non-empty"); + dumpedModule.setField = ImmutableSet.of("non-empty"); + dumpedModule.integerMapField = new LinkedHashMap<>(); + dumpedModule.integerMapField.put("a", 1); + dumpedModule.integerMapField.put("", 2); + assertThatThrownBy(() -> assertEqualAfterDumpAndRead(dumpedModule)).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Map {a=1, =2} contains blank string keys. Only non-blank string keys are supported."); } private void assertEqualAfterDumpAndRead(MyModule dumpedModule) { @@ -167,6 +195,9 @@ public void testComments() { expectedComments.put("enumListField", "list of enum"); expectedComments.put("enumSetField", "set of enum"); expectedComments.put("setField", "set"); + expectedComments.put("integerMapField", "map of Integer"); + expectedComments.put("localDateMapField", "map of LocalDate"); + expectedComments.put("booleanMapField", "map of Boolean"); assertThat(new MyModule().getComments()).isEqualTo(expectedComments); } @@ -472,6 +503,18 @@ private static class MyModule extends ReflectiveConfigGroup { @Parameter private Set enumSetField; + @Comment("map of Integer") + @Parameter + private Map integerMapField; + + @Comment("map of LocalDate") + @Parameter + private Map localDateMapField; + + @Comment("map of Boolean") + @Parameter + private Map booleanMapField; + // Object fields: // Id: string representation is toString private Id idField; From 3be97c61805361b46bfbf0695ef483ae3a3f4685 Mon Sep 17 00:00:00 2001 From: marecabo <23156476+marecabo@users.noreply.github.com> Date: Tue, 10 Oct 2023 09:42:35 +0200 Subject: [PATCH 2/2] Support EnumMaps --- .../org/matsim/core/config/ReflectiveConfigGroup.java | 5 +++-- .../org/matsim/core/config/ReflectiveConfigGroupTest.java | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java b/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java index 391c1577394..7d6d86f0a4d 100644 --- a/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java +++ b/matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java @@ -433,8 +433,9 @@ private Object fromString(String value, Class type, @Nullable Field paramFiel } else if (paramField != null && paramField.getGenericType() instanceof ParameterizedType pType) { Class keyClass = TypeFactory.rawClass(pType.getActualTypeArguments()[0]); Class valueClass = TypeFactory.rawClass(pType.getActualTypeArguments()[1]); - JavaType mapType = OBJECT_MAPPER.getTypeFactory().constructMapType( - LinkedHashMap.class, keyClass, valueClass); + Class mapClass = keyClass.isEnum() ? EnumMap.class : LinkedHashMap.class; + JavaType mapType = OBJECT_MAPPER.getTypeFactory().constructMapType(mapClass, keyClass, + valueClass); try { return OBJECT_MAPPER.readValue(value, mapType); } catch (JsonProcessingException e) { diff --git a/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java b/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java index e410b659522..b19c1552100 100644 --- a/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java +++ b/matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java @@ -27,6 +27,7 @@ import java.time.LocalTime; import java.util.Collection; import java.util.Collections; +import java.util.EnumMap; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -74,6 +75,8 @@ public void testDumpAndRead() { dumpedModule.integerMapField = Map.of("a\"b", 1, "b", 2); dumpedModule.localDateMapField = Map.of('d', LocalDate.of(2023, 10, 9)); dumpedModule.booleanMapField = Map.of(0.1, true); + dumpedModule.enumMapField = new EnumMap<>(MyEnum.class); + dumpedModule.enumMapField.put(MyEnum.VALUE1, "abc"); dumpedModule.setField = ImmutableSet.of("a", "b", "c"); dumpedModule.listField = List.of("1", "2", "3"); assertEqualAfterDumpAndRead(dumpedModule); @@ -198,6 +201,7 @@ public void testComments() { expectedComments.put("integerMapField", "map of Integer"); expectedComments.put("localDateMapField", "map of LocalDate"); expectedComments.put("booleanMapField", "map of Boolean"); + expectedComments.put("enumMapField", "map of Enum"); assertThat(new MyModule().getComments()).isEqualTo(expectedComments); } @@ -515,6 +519,10 @@ private static class MyModule extends ReflectiveConfigGroup { @Parameter private Map booleanMapField; + @Comment("map of Enum") + @Parameter + private Map enumMapField; + // Object fields: // Id: string representation is toString private Id idField;