Merge pull request bndtools#6012 from pkriens/issue/json-codec-record

Use RecordComponent instead of guessing

aQute.libg/src/aQute/lib/json/CollectionHandler.java

@@ -12,11 +12,14 @@
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.function.Supplier;
 
 public class CollectionHandler extends Handler {
-	Class<?>	rawClass;
-	Type		componentType;
+	Class<?>							rawClass;
+	Type								componentType;
+	final Supplier<Collection<Object>>	factory;
 
+	@SuppressWarnings("unchecked")
 	CollectionHandler(Class<?> rawClass, Type componentType) {
 		this.componentType = componentType;
 		if (rawClass.isInterface()) {
@@ -40,6 +43,7 @@ else if (rawClass.isAssignableFrom(CopyOnWriteArraySet.class))
 				throw new IllegalArgumentException("Unknown interface type for collection: " + rawClass);
 		}
 		this.rawClass = rawClass;
+		this.factory = (Supplier<Collection<Object>>) newInstanceFunction(rawClass);
 	}
 
 	@Override
@@ -54,7 +58,7 @@ public void encode(Encoder app, Object object, Map<Object, Type> visited) throws
 			try {
 				app.append(del);
 				if (!del.isEmpty()) {
-				app.linebreak();
+					app.linebreak();
 				}
 
 				app.encode(o, componentType, visited);
@@ -70,7 +74,7 @@ public void encode(Encoder app, Object object, Map<Object, Type> visited) throws
 	@Override
 	public Object decodeArray(Decoder r) throws Exception {
 		@SuppressWarnings("unchecked")
-		Collection<Object> c = (Collection<Object>) newInstance(rawClass);
+		Collection<Object> c = factory.get();
 		r.codec.parseArray(c, componentType, r);
 		return c;
 	}

aQute.libg/src/aQute/lib/json/Handler.java

@@ -3,14 +3,21 @@
 import static java.lang.invoke.MethodHandles.publicLookup;
 
 import java.io.IOException;
+import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Type;
 import java.util.Map;
+import java.util.function.Supplier;
+
+import aQute.bnd.exceptions.Exceptions;
 
 public abstract class Handler {
+	static final Lookup PUBLIC_LOOKUP = MethodHandles.publicLookup();
+
 	public abstract void encode(Encoder app, Object object, Map<Object, Type> visited) throws IOException, Exception;
 
 	public Object decodeObject(Decoder isr) throws Exception {
@@ -39,16 +46,21 @@ public Object decode(Decoder dec) {
 
 	private static final MethodType defaultConstructor = MethodType.methodType(void.class);
 
-	static <T> T newInstance(Class<T> rawClass) throws Exception {
-		try {
-			return (T) MethodHandles.publicLookup()
-				.findConstructor(rawClass, defaultConstructor)
-				.invoke();
-		} catch (Error | Exception e) {
-			throw e;
-		} catch (Throwable e) {
-			throw new InvocationTargetException(e);
-		}
+	static <T> Supplier<T> newInstanceFunction(Class<T> rawClass) {
+		return new Supplier<T>() {
+			volatile MethodHandle constructor = null;
+
+			@Override
+			public T get() {
+				try {
+					if (constructor == null)
+						constructor = PUBLIC_LOOKUP.findConstructor(rawClass, defaultConstructor);
+					return (T) constructor.invoke();
+				} catch (Throwable e) {
+					throw Exceptions.duck(e);
+				}
+			}
+		};
 	}
 
 	static void setField(Field f, Object targetObject, Object value) throws Exception {

aQute.libg/src/aQute/lib/json/JSONCodec.java

@@ -17,6 +17,7 @@
 import java.util.Hashtable;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
 import java.util.WeakHashMap;
 import java.util.concurrent.ConcurrentHashMap;
@@ -51,10 +52,20 @@
  * Will now use hex for encoding byte arrays
  */
 public class JSONCodec {
+	final static Set<String>						keywords			= Set.of("abstract", "assert", "boolean",
+		"break", "byte", "case", "catch", "char", "class", "const", "continue", "default", "do", "double", "else",
+		"enum", "exports", "extends", "final", "finally", "float", "for", "goto", "if", "implements", "import",
+		"instanceof", "int", "interface", "long", "module", "native", "new", "package", "private", "protected",
+		"public", "requires", "return", "short", "static", "strictfp", "super", "switch", "synchronized", "this",
+		"throw", "throws", "transient", "try", "var", "void", "volatile", "while", "true", "false", "null", "_",
+		"record", "sealed", "non-sealed", "permits");
+	public static final String						KEYWORD_SUFFIX		= "__";
+
 	final static String								START_CHARACTERS	= "[{\"-0123456789tfn";
 
 	// Handlers
 	private final static WeakHashMap<Type, Handler>	handlers			= new WeakHashMap<>();
+
 	private static StringHandler					sh					= new StringHandler();
 	private static BooleanHandler					bh					= new BooleanHandler();
 	private static CharacterHandler					ch					= new CharacterHandler();
@@ -542,4 +553,21 @@ public JSONCodec addHandler(Type type, Handler handler) {
 		return this;
 	}
 
+	/**
+	 * This maps a name of a Java construct, which cannot contain Java keywords,
+	 * to a keyword if it ends with a {@link #KEYWORD_SUFFIX} and the name
+	 * without the suffix maps to a Java keyword.
+	 *
+	 * @param name the name
+	 * @return either the name when it wasn't a keyword or a keyword
+	 */
+	public static String keyword(String name) {
+		if (name.endsWith(KEYWORD_SUFFIX)) {
+			String keyword = name.substring(0, name.length() - KEYWORD_SUFFIX.length());
+			if (keywords.contains(keyword))
+				return keyword;
+		}
+		return name;
+	}
+
 }

aQute.libg/src/aQute/lib/json/MapHandler.java

@@ -10,11 +10,13 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.TreeMap;
+import java.util.function.Supplier;
 
 public class MapHandler extends Handler {
-	final Class<?>	rawClass;
-	final Type		keyType;
-	final Type		valueType;
+	final Class<?>		rawClass;
+	final Type			keyType;
+	final Type			valueType;
+	final Supplier<?>	factory;
 
 	MapHandler(Class<?> rawClass, Type keyType, Type valueType) {
 
@@ -42,6 +44,7 @@ else if (rawClass.isAssignableFrom(Dictionary.class))
 				throw new IllegalArgumentException("Unknown map interface: " + rawClass);
 		}
 		this.rawClass = rawClass;
+		this.factory = newInstanceFunction(rawClass);
 	}
 
 	private Type resolve(Type type) {
@@ -109,7 +112,7 @@ public Object decodeObject(Decoder r) throws Exception {
 		assert r.current() == '{';
 
 		@SuppressWarnings("unchecked")
-		Map<Object, Object> map = (Map<Object, Object>) newInstance(rawClass);
+		Map<Object, Object> map = (Map<Object, Object>) factory.get();
 
 		int c = r.next();
 		while (JSONCodec.START_CHARACTERS.indexOf(c) >= 0) {

aQute.libg/src/aQute/lib/json/ObjectHandler.java

@@ -8,6 +8,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 
 public class ObjectHandler extends Handler {
 	@SuppressWarnings("rawtypes")
@@ -16,10 +17,11 @@ public class ObjectHandler extends Handler {
 	final Type		types[];
 	final Object	defaults[];
 	final Field		extra;
+	final Supplier<?>	factory;
 
 	ObjectHandler(JSONCodec codec, Class<?> c) throws Exception {
 		rawClass = c;
-
+		factory = newInstanceFunction(c);
 		List<Field> fields = new ArrayList<>();
 		for (Field f : c.getFields()) {
 			if (Modifier.isStatic(f.getModifiers()))
@@ -49,7 +51,7 @@ public class ObjectHandler extends Handler {
 			extra = null;
 
 		try {
-			Object template = newInstance(c);
+			Object template = factory.get();
 
 			for (int i = 0; i < this.fields.length; i++) {
 				defaults[i] = getField(this.fields[i], template);
@@ -66,8 +68,9 @@ public void encode(Encoder app, Object object, Map<Object, Type> visited) throws
 		String del = "";
 		for (int i = 0; i < fields.length; i++)
 			try {
-				if (fields[i].getName()
-					.startsWith("__"))
+				Field field = fields[i];
+				String actualName = JSONCodec.keyword(field.getName());
+				if (actualName.startsWith("__"))
 					continue;
 
 				Object value = getField(fields[i], object);
@@ -83,7 +86,7 @@ public void encode(Encoder app, Object object, Map<Object, Type> visited) throws
 				if (!del.isEmpty()) {
 					app.linebreak();
 				}
-				StringHandler.string(app, fields[i].getName());
+				StringHandler.string(app, actualName);
 				app.append(":");
 				app.encode(value, types[i], visited);
 				del = ",";
@@ -98,7 +101,7 @@ public void encode(Encoder app, Object object, Map<Object, Type> visited) throws
 	public Object decodeObject(Decoder r) throws Exception {
 		assert r.current() == '{';
 		@SuppressWarnings("unchecked")
-		Object targetObject = newInstance(rawClass);
+		Object targetObject = factory.get();
 
 		int c = r.next();
 		while (JSONCodec.START_CHARACTERS.indexOf(c) >= 0) {
@@ -165,6 +168,8 @@ public Object decodeObject(Decoder r) throws Exception {
 	}
 
 	private Field getField(String key) {
+		if (JSONCodec.keywords.contains(key))
+			key = key + JSONCodec.KEYWORD_SUFFIX;
 		for (Field field : fields) {
 			int n = key.compareTo(field.getName());
 			if (n == 0)

aQute.libg/src/aQute/lib/json/RecordHandler.java

@@ -4,9 +4,8 @@
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
-import java.lang.reflect.Field;
 import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
+import java.lang.reflect.RecordComponent;
 import java.lang.reflect.Type;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -28,10 +27,11 @@ class Accessor {
 		final Type			type;
 		final int			index;
 
-		public Accessor(Method m, int index) throws IllegalAccessException {
+		public Accessor(RecordComponent component, int index) throws IllegalAccessException {
+			Method m = component.getAccessor();
 			getter = lookup.unreflect(m);
-			this.name = m.getName();
-			this.type = m.getGenericReturnType();
+			this.name = JSONCodec.keyword(component.getName());
+			this.type = component.getGenericType();
 			this.index = index;
 		}
 
@@ -48,29 +48,14 @@ public Object get(Object object) {
 	RecordHandler(JSONCodec codec, Class<?> c) throws Exception {
 		this.codec = codec;
 		assert c.getSuperclass() == Record.class;
+		assert c.isRecord();
+
 		MethodType constructorType = MethodType.methodType(void.class);
 		int index = 0;
-		for (Field f : c.getDeclaredFields()) {
-			int modifiers = f.getModifiers();
-			if (Modifier.isStatic(modifiers) || !Modifier.isFinal(modifiers) || !Modifier.isPrivate(modifiers))
-				continue;
-			try {
-				String name = f.getName();
-				if (name.startsWith("__") && !name.equals("__extra")) {
-					continue;
-				}
-				Method method = c.getMethod(name);
-				if (method == null || method.getReturnType() != f.getType())
-					continue;
-
-				constructorType = constructorType.appendParameterTypes(f.getType());
-
-				Accessor accessor = new Accessor(method, index++);
-				accessors.put(name, accessor);
-
-			} catch (NoSuchMethodException nsme) {
-				// ignore
-			}
+		for (RecordComponent component : c.getRecordComponents()) {
+			constructorType = constructorType.appendParameterTypes(component.getType());
+			Accessor accessor = new Accessor(component, index++);
+			accessors.put(accessor.name, accessor);
 		}
 		this.constructor = lookup.findConstructor(c, constructorType);
 	}

aQute.libg/test/aQute/lib/json/JSONTest.java

@@ -1243,4 +1243,44 @@ record ARecord(int a, String b, long c) {}
 		assertThat(x.b).isEqualTo("1");
 		assertThat(x.c).isEqualTo(1L);
 	}
+
+	@Test
+	public void testRecordWithKeywordName() throws Exception {
+		record ARecord(int if__, String while__, long ___) {}
+		ARecord a = new ARecord(1, "1", 1L);
+		assertThat(new JSONCodec().enc()
+			.put(a)
+			.toString()).isEqualTo("{\"_\":1,\"if\":1,\"while\":\"1\"}");
+
+		ARecord x = new JSONCodec().dec()
+			.from("{\"_\":1,\"if\":1,\"while\":\"1\"}")
+			.get(ARecord.class);
+		assertThat(x.if__).isEqualTo(1);
+		assertThat(x.while__).isEqualTo("1");
+		assertThat(x.___).isEqualTo(1L);
+	}
+
+	public static class KeywordDTO {
+		public Integer	if__;
+		public long		___;
+		public String	while__;
+	}
+
+	@Test
+	public void testDTOWithKeywords() throws Exception {
+		KeywordDTO a = new KeywordDTO();
+		a.___ = 1L;
+		a.while__ = "1";
+		a.if__ = 1;
+		assertThat(new JSONCodec().enc()
+			.put(a)
+			.toString()).isEqualTo("{\"_\":1,\"if\":1,\"while\":\"1\"}");
+
+		KeywordDTO x = new JSONCodec().dec()
+			.from("{\"_\":1,\"if\":1,\"while\":\"1\"}")
+			.get(KeywordDTO.class);
+		assertThat(x.if__).isEqualTo(1);
+		assertThat(x.while__).isEqualTo("1");
+		assertThat(x.___).isEqualTo(1L);
+	}
 }