Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Maps in ReflectiveConfigGroup #2828

Closed
wants to merge 2 commits into from
Closed

Support Maps in ReflectiveConfigGroup #2828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2661e3
Support simple maps in ReflectiveConfigGroup
marecabo Oct 9, 2023
3be97c6
Support EnumMaps
marecabo Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions matsim/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,10 @@
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-jsr310</artifactId>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
50 changes: 49 additions & 1 deletion matsim/src/main/java/org/matsim/core/config/ReflectiveConfigGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, Method> setters;
Expand Down Expand Up @@ -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<Class<?>> 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<String> and Set<String>, LocalTime, LocalDate, LocalDateTime"
+ " enumerations, List<String> and Set<String>, Maps<?,?>, LocalTime, LocalDate, LocalDateTime"
+ " Other types are fine as parameters, but you will need to implement conversion strategies"
+ " in corresponding StringGetters andStringSetters.";

Expand All @@ -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))
Expand Down Expand Up @@ -406,6 +427,22 @@ 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]);
Class<? extends Map> 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) {
throw new RuntimeException(e);
}
}
throw new RuntimeException("Unsupported map field");
} else if (type.equals(Class.class)) {
try {
return ClassLoader.getSystemClassLoader().loadClass(value);
Expand Down Expand Up @@ -484,6 +521,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 + "";
}
Expand Down
51 changes: 51 additions & 0 deletions matsim/src/test/java/org/matsim/core/config/ReflectiveConfigGroupTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
import java.time.LocalDateTime;
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;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -69,6 +72,11 @@ 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.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);
Expand All @@ -87,6 +95,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);
}

Expand All @@ -97,14 +108,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
Expand All @@ -114,14 +134,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) {
Expand Down Expand Up @@ -167,6 +198,10 @@ 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");
expectedComments.put("enumMapField", "map of Enum");

assertThat(new MyModule().getComments()).isEqualTo(expectedComments);
}
Expand Down Expand Up @@ -472,6 +507,22 @@ private static class MyModule extends ReflectiveConfigGroup {
@Parameter
private Set<MyEnum> enumSetField;

@Comment("map of Integer")
@Parameter
private Map<String, Integer> integerMapField;

@Comment("map of LocalDate")
@Parameter
private Map<Character, LocalDate> localDateMapField;

@Comment("map of Boolean")
@Parameter
private Map<Double, Boolean> booleanMapField;

@Comment("map of Enum")
@Parameter
private Map<MyEnum, String> enumMapField;

// Object fields:
// Id: string representation is toString
private Id<Link> idField;
Expand Down
Loading