From 3e3d42f4662ed3d1296e7edd82a3458740f781dd Mon Sep 17 00:00:00 2001 From: Peter Kriens Date: Thu, 1 Feb 2024 16:18:28 +0100 Subject: [PATCH 1/3] Use RecordComponent instead of guessing --- Signed-off-by: Peter Kriens Signed-off-by: Peter Kriens --- .../src/aQute/lib/json/RecordHandler.java | 37 ++++++------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/aQute.libg/src/aQute/lib/json/RecordHandler.java b/aQute.libg/src/aQute/lib/json/RecordHandler.java index a4ed11939b..6dd01986d1 100644 --- a/aQute.libg/src/aQute/lib/json/RecordHandler.java +++ b/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 = 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(component.getName(), accessor); } this.constructor = lookup.findConstructor(c, constructorType); } From c317b0a5e4dd6a72fa5198ec05b57b728a9374ba Mon Sep 17 00:00:00 2001 From: Peter Kriens Date: Thu, 1 Feb 2024 17:56:24 +0100 Subject: [PATCH 2/3] Optimized lookups & handle Java keywords --- Signed-off-by: Peter Kriens Signed-off-by: Peter Kriens --- .../src/aQute/lib/json/CollectionHandler.java | 12 ++++-- aQute.libg/src/aQute/lib/json/Handler.java | 25 +++++++++++- aQute.libg/src/aQute/lib/json/JSONCodec.java | 28 +++++++++++++ aQute.libg/src/aQute/lib/json/MapHandler.java | 11 +++-- .../src/aQute/lib/json/ObjectHandler.java | 17 +++++--- .../src/aQute/lib/json/RecordHandler.java | 4 +- aQute.libg/test/aQute/lib/json/JSONTest.java | 40 +++++++++++++++++++ 7 files changed, 119 insertions(+), 18 deletions(-) diff --git a/aQute.libg/src/aQute/lib/json/CollectionHandler.java b/aQute.libg/src/aQute/lib/json/CollectionHandler.java index 16305534de..e951085f52 100644 --- a/aQute.libg/src/aQute/lib/json/CollectionHandler.java +++ b/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> 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>) newInstanceFunction(rawClass); } @Override @@ -54,7 +58,7 @@ public void encode(Encoder app, Object object, Map 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 visited) throws @Override public Object decodeArray(Decoder r) throws Exception { @SuppressWarnings("unchecked") - Collection c = (Collection) newInstance(rawClass); + Collection c = factory.get(); r.codec.parseArray(c, componentType, r); return c; } diff --git a/aQute.libg/src/aQute/lib/json/Handler.java b/aQute.libg/src/aQute/lib/json/Handler.java index d236134aca..f899be12ad 100644 --- a/aQute.libg/src/aQute/lib/json/Handler.java +++ b/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 visited) throws IOException, Exception; public Object decodeObject(Decoder isr) throws Exception { @@ -41,8 +48,7 @@ public Object decode(Decoder dec) { static T newInstance(Class rawClass) throws Exception { try { - return (T) MethodHandles.publicLookup() - .findConstructor(rawClass, defaultConstructor) + return (T) PUBLIC_LOOKUP.findConstructor(rawClass, defaultConstructor) .invoke(); } catch (Error | Exception e) { throw e; @@ -51,6 +57,21 @@ static T newInstance(Class rawClass) throws Exception { } } + static Supplier newInstanceFunction(Class rawClass) { + try { + MethodHandle constructor = PUBLIC_LOOKUP.findConstructor(rawClass, defaultConstructor); + return () -> { + try { + return (T) constructor.invoke(); + } catch (Throwable e) { + throw Exceptions.duck(e); + } + }; + } catch (Exception e) { + throw Exceptions.duck(e); + } + } + static void setField(Field f, Object targetObject, Object value) throws Exception { try { publicLookup().unreflectSetter(f) diff --git a/aQute.libg/src/aQute/lib/json/JSONCodec.java b/aQute.libg/src/aQute/lib/json/JSONCodec.java index 7aed0863d5..fc9345811c 100644 --- a/aQute.libg/src/aQute/lib/json/JSONCodec.java +++ b/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 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 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; + } + } diff --git a/aQute.libg/src/aQute/lib/json/MapHandler.java b/aQute.libg/src/aQute/lib/json/MapHandler.java index 556a033ed2..9ada15a161 100644 --- a/aQute.libg/src/aQute/lib/json/MapHandler.java +++ b/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; + private 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 map = (Map) newInstance(rawClass); + Map map = (Map) factory.get(); int c = r.next(); while (JSONCodec.START_CHARACTERS.indexOf(c) >= 0) { diff --git a/aQute.libg/src/aQute/lib/json/ObjectHandler.java b/aQute.libg/src/aQute/lib/json/ObjectHandler.java index 0a3dc5b1fb..00b4739e33 100644 --- a/aQute.libg/src/aQute/lib/json/ObjectHandler.java +++ b/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 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 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 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 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) diff --git a/aQute.libg/src/aQute/lib/json/RecordHandler.java b/aQute.libg/src/aQute/lib/json/RecordHandler.java index 6dd01986d1..a8f523c229 100644 --- a/aQute.libg/src/aQute/lib/json/RecordHandler.java +++ b/aQute.libg/src/aQute/lib/json/RecordHandler.java @@ -30,7 +30,7 @@ class Accessor { public Accessor(RecordComponent component, int index) throws IllegalAccessException { Method m = component.getAccessor(); getter = lookup.unreflect(m); - this.name = component.getName(); + this.name = JSONCodec.keyword(component.getName()); this.type = component.getGenericType(); this.index = index; } @@ -55,7 +55,7 @@ public Object get(Object object) { for (RecordComponent component : c.getRecordComponents()) { constructorType = constructorType.appendParameterTypes(component.getType()); Accessor accessor = new Accessor(component, index++); - accessors.put(component.getName(), accessor); + accessors.put(accessor.name, accessor); } this.constructor = lookup.findConstructor(c, constructorType); } diff --git a/aQute.libg/test/aQute/lib/json/JSONTest.java b/aQute.libg/test/aQute/lib/json/JSONTest.java index d19dbcb282..f1e9eed6f4 100644 --- a/aQute.libg/test/aQute/lib/json/JSONTest.java +++ b/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); + } } From 4743fbd43485fb9675c738d9d0c07043fdcacc50 Mon Sep 17 00:00:00 2001 From: Peter Kriens Date: Fri, 2 Feb 2024 14:08:39 +0100 Subject: [PATCH 3/3] Made the factory lazy --- Signed-off-by: Peter Kriens Signed-off-by: Peter Kriens --- aQute.libg/src/aQute/lib/json/Handler.java | 27 +++++++------------ aQute.libg/src/aQute/lib/json/MapHandler.java | 8 +++--- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/aQute.libg/src/aQute/lib/json/Handler.java b/aQute.libg/src/aQute/lib/json/Handler.java index f899be12ad..1c389217c5 100644 --- a/aQute.libg/src/aQute/lib/json/Handler.java +++ b/aQute.libg/src/aQute/lib/json/Handler.java @@ -46,30 +46,21 @@ public Object decode(Decoder dec) { private static final MethodType defaultConstructor = MethodType.methodType(void.class); - static T newInstance(Class rawClass) throws Exception { - try { - return (T) PUBLIC_LOOKUP.findConstructor(rawClass, defaultConstructor) - .invoke(); - } catch (Error | Exception e) { - throw e; - } catch (Throwable e) { - throw new InvocationTargetException(e); - } - } - static Supplier newInstanceFunction(Class rawClass) { - try { - MethodHandle constructor = PUBLIC_LOOKUP.findConstructor(rawClass, defaultConstructor); - return () -> { + return new Supplier() { + 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); } - }; - } catch (Exception e) { - throw Exceptions.duck(e); - } + } + }; } static void setField(Field f, Object targetObject, Object value) throws Exception { diff --git a/aQute.libg/src/aQute/lib/json/MapHandler.java b/aQute.libg/src/aQute/lib/json/MapHandler.java index 9ada15a161..afe91e71f5 100644 --- a/aQute.libg/src/aQute/lib/json/MapHandler.java +++ b/aQute.libg/src/aQute/lib/json/MapHandler.java @@ -13,10 +13,10 @@ import java.util.function.Supplier; public class MapHandler extends Handler { - final Class rawClass; - final Type keyType; - final Type valueType; - private Supplier factory; + final Class rawClass; + final Type keyType; + final Type valueType; + final Supplier factory; MapHandler(Class rawClass, Type keyType, Type valueType) {