From f82223eb45aa28f604b72852ca00d9f3b26a8936 Mon Sep 17 00:00:00 2001 From: Fangchen Li Date: Fri, 30 Jan 2026 13:21:48 -0800 Subject: [PATCH] Modernize chill-java for Java 17 - Use pattern matching instanceof (Java 16+) to simplify equals() methods in ClassRegistrar, ReflectingRegistrar, ReflectingDefaultRegistrar, and ReflectingInstantiator - Use multi-catch exception blocks to reduce boilerplate in ConfiguredInstantiator and ReflectingInstantiator - Convert CachedKryoInstantiator inner class to a record (Java 16+) - Add proper generics to JavaMapConfig (Map instead of raw Map) - Use diamond operator and var for local variable type inference Co-Authored-By: Claude Opus 4.5 --- .../com/twitter/chill/ClassRegistrar.java | 11 +--- .../chill/ReflectingDefaultRegistrar.java | 12 +--- .../twitter/chill/ReflectingRegistrar.java | 12 +--- .../chill/config/ConfiguredInstantiator.java | 25 ++----- .../twitter/chill/config/JavaMapConfig.java | 23 ++++--- .../chill/config/ReflectingInstantiator.java | 65 ++++++++----------- .../java/IterableRegistrarSerializer.java | 2 +- 7 files changed, 52 insertions(+), 98 deletions(-) diff --git a/chill-java/src/main/java/com/twitter/chill/ClassRegistrar.java b/chill-java/src/main/java/com/twitter/chill/ClassRegistrar.java index 9a1aac9d..b2e75b96 100644 --- a/chill-java/src/main/java/com/twitter/chill/ClassRegistrar.java +++ b/chill-java/src/main/java/com/twitter/chill/ClassRegistrar.java @@ -35,14 +35,9 @@ public ClassRegistrar(Class cls) { @Override public boolean equals(Object that) { - if(null == that) { - return false; - } - else if(that instanceof ClassRegistrar) { - return klass.equals(((ClassRegistrar)that).klass); - } - else { - return false; + if (that instanceof ClassRegistrar other) { + return klass.equals(other.klass); } + return false; } } diff --git a/chill-java/src/main/java/com/twitter/chill/ReflectingDefaultRegistrar.java b/chill-java/src/main/java/com/twitter/chill/ReflectingDefaultRegistrar.java index 7ace7507..83404aca 100644 --- a/chill-java/src/main/java/com/twitter/chill/ReflectingDefaultRegistrar.java +++ b/chill-java/src/main/java/com/twitter/chill/ReflectingDefaultRegistrar.java @@ -39,15 +39,9 @@ public ReflectingDefaultRegistrar(Class cls, Class> s @Override public boolean equals(Object that) { - if(null == that) { - return false; - } - else if(that instanceof ReflectingDefaultRegistrar) { - return klass.equals(((ReflectingDefaultRegistrar)that).klass) && - serializerKlass.equals(((ReflectingDefaultRegistrar)that).serializerKlass); - } - else { - return false; + if (that instanceof ReflectingDefaultRegistrar other) { + return klass.equals(other.klass) && serializerKlass.equals(other.serializerKlass); } + return false; } } diff --git a/chill-java/src/main/java/com/twitter/chill/ReflectingRegistrar.java b/chill-java/src/main/java/com/twitter/chill/ReflectingRegistrar.java index f6ce3f0c..a2c0ec6c 100644 --- a/chill-java/src/main/java/com/twitter/chill/ReflectingRegistrar.java +++ b/chill-java/src/main/java/com/twitter/chill/ReflectingRegistrar.java @@ -50,15 +50,9 @@ public void apply(Kryo k) { @Override public boolean equals(Object that) { - if(null == that) { - return false; - } - else if(that instanceof ReflectingRegistrar) { - return klass.equals(((ReflectingRegistrar)that).klass) && - serializerKlass.equals(((ReflectingRegistrar)that).serializerKlass); - } - else { - return false; + if (that instanceof ReflectingRegistrar other) { + return klass.equals(other.klass) && serializerKlass.equals(other.serializerKlass); } + return false; } } diff --git a/chill-java/src/main/java/com/twitter/chill/config/ConfiguredInstantiator.java b/chill-java/src/main/java/com/twitter/chill/config/ConfiguredInstantiator.java index 833eedfd..dcab20e0 100644 --- a/chill-java/src/main/java/com/twitter/chill/config/ConfiguredInstantiator.java +++ b/chill-java/src/main/java/com/twitter/chill/config/ConfiguredInstantiator.java @@ -103,16 +103,8 @@ static KryoInstantiator reflect(Class instClass, Con return instClass.getDeclaredConstructor().newInstance(); } } - catch(NoSuchMethodException x) { - throw new ConfigurationException(x); - } - catch(InstantiationException x) { - throw new ConfigurationException(x); - } - catch(IllegalAccessException x) { - throw new ConfigurationException(x); - } - catch(InvocationTargetException x) { + catch (NoSuchMethodException | InstantiationException | + IllegalAccessException | InvocationTargetException x) { throw new ConfigurationException(x); } } @@ -153,17 +145,10 @@ protected static String serialize(Kryo k, KryoInstantiator ki) { return Base64.encodeBytes(out.toBytes()); } - /** Simple class to hold the cached copy of the latest kryo instantiator. - * As well as its corresponding base64 encoded data. + /** Simple record to hold the cached copy of the latest kryo instantiator + * along with its corresponding base64 encoded data. */ - private static class CachedKryoInstantiator { - public final KryoInstantiator kryoInstantiator; - public final String base64Value; - public CachedKryoInstantiator(KryoInstantiator ki, String bv) { - kryoInstantiator = ki; - base64Value = bv; - } - } + private record CachedKryoInstantiator(KryoInstantiator kryoInstantiator, String base64Value) {} private static CachedKryoInstantiator cachedKryoInstantiator = null; diff --git a/chill-java/src/main/java/com/twitter/chill/config/JavaMapConfig.java b/chill-java/src/main/java/com/twitter/chill/config/JavaMapConfig.java index 8fcf98f6..25184b63 100644 --- a/chill-java/src/main/java/com/twitter/chill/config/JavaMapConfig.java +++ b/chill-java/src/main/java/com/twitter/chill/config/JavaMapConfig.java @@ -19,29 +19,28 @@ import java.util.Map; /** - * This takes a raw Map and calls toString on the objects before returning them as values + * This takes a Map and calls toString on the objects before returning them as values */ public class JavaMapConfig extends Config { - final Map conf; - public JavaMapConfig(Map conf) { - this.conf = conf; + final Map conf; + + public JavaMapConfig(Map conf) { + // We need to store as Map to allow put() with String values + this.conf = new java.util.HashMap<>(conf); } + public JavaMapConfig() { - this(new java.util.HashMap()); + this.conf = new java.util.HashMap<>(); } - /** Return null if this key is undefined */ + /** Return null if this key is undefined */ @Override public String get(String key) { Object value = conf.get(key); - if(null != value) { - return value.toString(); - } - else { - return null; - } + return value != null ? value.toString() : null; } + @Override public void set(String key, String value) { conf.put(key, value); diff --git a/chill-java/src/main/java/com/twitter/chill/config/ReflectingInstantiator.java b/chill-java/src/main/java/com/twitter/chill/config/ReflectingInstantiator.java index 971e0349..63a8ee92 100644 --- a/chill-java/src/main/java/com/twitter/chill/config/ReflectingInstantiator.java +++ b/chill-java/src/main/java/com/twitter/chill/config/ReflectingInstantiator.java @@ -59,11 +59,10 @@ public ReflectingInstantiator(Config conf) throws ConfigurationException { // Make sure we can make a newKryo, this throws a runtime exception if not. newKryoWithEx(); } - catch(ClassNotFoundException x) { throw new ConfigurationException(x); } - catch(InstantiationException x) { throw new ConfigurationException(x); } - catch(IllegalAccessException x) { throw new ConfigurationException(x); } - catch(NoSuchMethodException x) { throw new ConfigurationException(x); } - catch(java.lang.reflect.InvocationTargetException x) { throw new ConfigurationException(x); } + catch (ClassNotFoundException | InstantiationException | IllegalAccessException | + NoSuchMethodException | InvocationTargetException x) { + throw new ConfigurationException(x); + } } /** Create an instance using the defaults for non-listed params */ @@ -92,12 +91,12 @@ public ReflectingInstantiator(Class kryoClass, this.regRequired = regRequired; this.skipMissing = skipMissing; - this.registrations = new ArrayList(); - for(IKryoRegistrar cr: classRegistrations) { this.registrations.add(cr); } - for(IKryoRegistrar rr: registrations) { this.registrations.add(rr); } + this.registrations = new ArrayList<>(); + for (IKryoRegistrar cr : classRegistrations) { this.registrations.add(cr); } + for (IKryoRegistrar rr : registrations) { this.registrations.add(rr); } - defaultRegistrations = new ArrayList(); - for(ReflectingDefaultRegistrar rdr: defaults) { defaultRegistrations.add(rdr); } + defaultRegistrations = new ArrayList<>(); + for (ReflectingDefaultRegistrar rdr : defaults) { defaultRegistrations.add(rdr); } } public void set(Config conf) throws ConfigurationException { @@ -131,10 +130,10 @@ public Kryo newKryo() { try { return newKryoWithEx(); } - catch(InstantiationException x) { throw new RuntimeException(x); } - catch(IllegalAccessException x) { throw new RuntimeException(x); } - catch(NoSuchMethodException x) { throw new RuntimeException(x); } - catch(java.lang.reflect.InvocationTargetException x) { throw new RuntimeException(x); } + catch (InstantiationException | IllegalAccessException | + NoSuchMethodException | InvocationTargetException x) { + throw new RuntimeException(x); + } } /** All keys are prefixed with this string */ @@ -192,7 +191,7 @@ public Kryo newKryo() { protected List buildRegistrars(String base, boolean isAddDefault) throws ConfigurationException { - List builder = new ArrayList(); + List builder = new ArrayList<>(); if (base == null) return builder; @@ -240,19 +239,13 @@ protected String registrarsToString(Iterable registrar builder.append(":"); isFirst = false; String part = null; - if(reg instanceof ClassRegistrar) { - ClassRegistrar r = (ClassRegistrar)reg; + if (reg instanceof ClassRegistrar r) { part = r.getRegisteredClass().getName(); - } - else if(reg instanceof ReflectingRegistrar) { - ReflectingRegistrar r = (ReflectingRegistrar)reg; + } else if (reg instanceof ReflectingRegistrar r) { part = r.getRegisteredClass().getName() + "," + r.getSerializerClass().getName(); - } - else if(reg instanceof ReflectingDefaultRegistrar) { - ReflectingDefaultRegistrar r = (ReflectingDefaultRegistrar)reg; + } else if (reg instanceof ReflectingDefaultRegistrar r) { part = r.getRegisteredClass().getName() + "," + r.getSerializerClass().getName(); - } - else { + } else { throw new ConfigurationException("Unknown type of reflecting registrar: " + reg.getClass().getName()); } builder.append(part); @@ -269,21 +262,15 @@ public int hashCode() { @Override public boolean equals(Object that) { - if(null == that) { - return false; - } - else if(that instanceof ReflectingInstantiator) { - ReflectingInstantiator thatri = (ReflectingInstantiator)that; - return (regRequired == thatri.regRequired) && - (skipMissing == thatri.skipMissing) && - kryoClass.equals(thatri.kryoClass) && - instStratClass.equals(thatri.instStratClass) && - registrations.equals(thatri.registrations) && - defaultRegistrations.equals(thatri.defaultRegistrations); - } - else { - return false; + if (that instanceof ReflectingInstantiator other) { + return regRequired == other.regRequired && + skipMissing == other.skipMissing && + kryoClass.equals(other.kryoClass) && + instStratClass.equals(other.instStratClass) && + registrations.equals(other.registrations) && + defaultRegistrations.equals(other.defaultRegistrations); } + return false; } } diff --git a/chill-java/src/main/java/com/twitter/chill/java/IterableRegistrarSerializer.java b/chill-java/src/main/java/com/twitter/chill/java/IterableRegistrarSerializer.java index de61fe8b..c982d99b 100644 --- a/chill-java/src/main/java/com/twitter/chill/java/IterableRegistrarSerializer.java +++ b/chill-java/src/main/java/com/twitter/chill/java/IterableRegistrarSerializer.java @@ -33,7 +33,7 @@ public void write(Kryo kryo, Output output, IterableRegistrar obj) { kryo.writeClassAndObject(output, null); } public IterableRegistrar read(Kryo kryo, Input input, Class type) { - ArrayList krs = new ArrayList(); + var krs = new ArrayList(); IKryoRegistrar thisKr = (IKryoRegistrar)kryo.readClassAndObject(input); while(thisKr != null) { krs.add(thisKr);