From b739f4be6ec5cb6997c6c0e292867f72c4b6f905 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Wed, 3 Oct 2012 10:12:53 -0400 Subject: [PATCH] Make ConfigException serializable Done with a custom serialization because we don't want ConfigOrigin to be serializable; if ConfigOrigin were serializable we'd have to support its serialization remaining compatible across releases. And there's no known use for ConfigOrigin being serializable. Not intending to support ConfigException serialization back compat across releases, because that probably isn't useful or expected. --- .../com/typesafe/config/ConfigException.java | 40 ++++++++++++++++++- .../typesafe/config/impl/ConfigImplUtil.java | 14 +++++++ .../config/impl/SerializedConfigValue.java | 6 ++- .../typesafe/config/impl/PublicApiTest.scala | 12 ++++++ .../com/typesafe/config/impl/TestUtils.scala | 20 +++++++--- 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/config/src/main/java/com/typesafe/config/ConfigException.java b/config/src/main/java/com/typesafe/config/ConfigException.java index d913b794..0e9f895f 100644 --- a/config/src/main/java/com/typesafe/config/ConfigException.java +++ b/config/src/main/java/com/typesafe/config/ConfigException.java @@ -3,15 +3,20 @@ */ package com.typesafe.config; +import java.io.IOException; +import java.io.Serializable; +import java.lang.reflect.Field; + +import com.typesafe.config.impl.ConfigImplUtil; /** * All exceptions thrown by the library are subclasses of * ConfigException. */ -public abstract class ConfigException extends RuntimeException { +public abstract class ConfigException extends RuntimeException implements Serializable { private static final long serialVersionUID = 1L; - final private ConfigOrigin origin; + final private transient ConfigOrigin origin; protected ConfigException(ConfigOrigin origin, String message, Throwable cause) { @@ -45,6 +50,37 @@ public abstract class ConfigException extends RuntimeException { return origin; } + // we customize serialization because ConfigOrigin isn't + // serializable and we don't want it to be (don't want to + // support it) + private void writeObject(java.io.ObjectOutputStream out) throws IOException { + out.defaultWriteObject(); + ConfigImplUtil.writeOrigin(out, origin); + } + + private void readObject(java.io.ObjectInputStream in) throws IOException, + ClassNotFoundException { + in.defaultReadObject(); + ConfigOrigin origin = ConfigImplUtil.readOrigin(in); + // circumvent "final" + Field f; + try { + f = ConfigException.class.getDeclaredField("origin"); + } catch (NoSuchFieldException e) { + throw new IOException("ConfigException has no origin field?", e); + } catch (SecurityException e) { + throw new IOException("unable to fill out origin field in ConfigException", e); + } + f.setAccessible(true); + try { + f.set(this, origin); + } catch (IllegalArgumentException e) { + throw new IOException("unable to set origin field", e); + } catch (IllegalAccessException e) { + throw new IOException("unable to set origin field", e); + } + } + /** * Exception indicating that the type of a value does not match the type you * requested. diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigImplUtil.java b/config/src/main/java/com/typesafe/config/impl/ConfigImplUtil.java index be15993c..c778525c 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigImplUtil.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigImplUtil.java @@ -3,13 +3,18 @@ */ package com.typesafe.config.impl; +import java.io.DataOutputStream; import java.io.File; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.net.URISyntaxException; import java.net.URL; import java.util.ArrayList; import java.util.List; import com.typesafe.config.ConfigException; +import com.typesafe.config.ConfigOrigin; /** This is public just for the "config" package to use, don't touch it */ @@ -186,4 +191,13 @@ final public class ConfigImplUtil { } return elements; } + + public static ConfigOrigin readOrigin(ObjectInputStream in) throws IOException { + return SerializedConfigValue.readOrigin(in, null); + } + + public static void writeOrigin(ObjectOutputStream out, ConfigOrigin origin) throws IOException { + SerializedConfigValue.writeOrigin(new DataOutputStream(out), (SimpleConfigOrigin) origin, + null); + } } diff --git a/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java b/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java index b5323d79..55970857 100644 --- a/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java +++ b/config/src/main/java/com/typesafe/config/impl/SerializedConfigValue.java @@ -196,7 +196,8 @@ class SerializedConfigValue extends AbstractConfigValue implements Externalizabl } } - private static void writeOrigin(DataOutput out, SimpleConfigOrigin origin, + // not private because we use it to serialize ConfigException + static void writeOrigin(DataOutput out, SimpleConfigOrigin origin, SimpleConfigOrigin baseOrigin) throws IOException { Map m = origin.toFieldsDelta(baseOrigin); for (Map.Entry e : m.entrySet()) { @@ -208,7 +209,8 @@ class SerializedConfigValue extends AbstractConfigValue implements Externalizabl writeEndMarker(out); } - private static SimpleConfigOrigin readOrigin(DataInput in, SimpleConfigOrigin baseOrigin) + // not private because we use it to deserialize ConfigException + static SimpleConfigOrigin readOrigin(DataInput in, SimpleConfigOrigin baseOrigin) throws IOException { Map m = new EnumMap(SerializedField.class); while (true) { diff --git a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala index a57e1a99..aa735797 100644 --- a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala @@ -844,4 +844,16 @@ class PublicApiTest extends TestUtils { None } } + + @Test + def exceptionSerializable() { + // ArrayList is a serialization problem so we want to cover it in tests + val comments = new java.util.ArrayList(List("comment 1", "comment 2").asJava) + val e = new ConfigException.WrongType(SimpleConfigOrigin.newSimple("an origin").setComments(comments), + "this is a message", new RuntimeException("this is a cause")) + val eCopy = checkSerializableNoMeaningfulEquals(e) + assertTrue("messages equal after deserialize", e.getMessage.equals(eCopy.getMessage)) + assertTrue("cause messages equal after deserialize", e.getCause().getMessage.equals(eCopy.getCause().getMessage)) + assertTrue("origins equal after deserialize", e.origin().equals(eCopy.origin())) + } } diff --git a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala index d4cd18f4..4ae03e93 100644 --- a/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala +++ b/config/src/test/scala/com/typesafe/config/impl/TestUtils.scala @@ -229,9 +229,7 @@ abstract trait TestUtils { t } - protected def checkSerializable[T: Manifest](o: T): T = { - checkEqualObjects(o, o) - + protected def checkSerializableNoMeaningfulEquals[T: Manifest](o: T): T = { assertTrue(o.getClass.getSimpleName + " not an instance of Serializable", o.isInstanceOf[java.io.Serializable]) val a = o.asInstanceOf[java.io.Serializable] @@ -251,12 +249,22 @@ abstract trait TestUtils { assertTrue("deserialized type " + b.getClass.getSimpleName + " doesn't match serialized type " + a.getClass.getSimpleName, manifest[T].erasure.isAssignableFrom(b.getClass)) - checkEqualObjects(a, b) - checkEqualOrigins(a, b) - b.asInstanceOf[T] } + protected def checkSerializable[T: Manifest](o: T): T = { + checkEqualObjects(o, o) + + assertTrue(o.getClass.getSimpleName + " not an instance of Serializable", o.isInstanceOf[java.io.Serializable]) + + val b = checkSerializableNoMeaningfulEquals(o) + + checkEqualObjects(o, b) + checkEqualOrigins(o, b) + + b + } + // origin() is not part of value equality but is serialized, so // we check it separately protected def checkEqualOrigins[T](a: T, b: T): Unit = {