Hi, Le 08/03/2011 23:51, Jörg Schaible a écrit : > sebb wrote: > >> On 8 March 2011 20:17, Luc Maisonobe <luc.maison...@free.fr> wrote: >>> Le 07/03/2011 11:37, er...@apache.org a écrit : >>>> Author: erans >>>> Date: Mon Mar 7 10:37:52 2011 >>>> New Revision: 1078734 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1078734&view=rev >>>> Log: >>>> MATH-542 >>>> "MathRuntimeException" provides two ways for enhancing the information >>>> content: one for localized messages and one for storing "context" >>>> objects. The additional methods have been added to "MathThrowable". >>>> Consequently, dummy implementations were needed in the old >>>> "MathRuntimeException" and "MathException". >>>> Some parts of the tests for old exceptions were removed as they used >>>> methods that were deleted. A call to "MathUserException" in >>>> "AbstractContinuousDistribution" was simplified for the same reason. >>>> "MathUtils" still contained "String" (instead of "Localizable") as >>>> argument to the constructor of a "MathArithmeticException". >>>> I don't know why a test in "DummyStepInterpolatorTest" stopped working. >>>> It is now commented out; please have a look. >>> >>> [snip] >>> >>> >>>> Modified: >>>> > commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java >>>> URL: >>>> > http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java?rev=1078734&r1=1078733&r2=1078734&view=diff >>>> > ============================================================================== >>>> --- >>>> > commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java >>>> (original) +++ >>>> > commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java >>>> Mon Mar 7 10:37:52 2011 @@ -123,7 +123,9 @@ public class >>>> DummyStepInterpolatorTest { fail("an exception should have been >>>> thrown"); } catch (IOException ioe) { // expected behavior - >>>> assertEquals(0, ioe.getMessage().length()); + // XXX Why was the >>>> message supposed to be empty? + // With the current code it is >>>> "org.apache.commons.math.util.Pair". + // assertEquals(0, >>>> ioe.getMessage().length()); >>> >>> The problem here is that the exception thrown which is an IOException >>> built from the empty string in the BadStepInterpolator below is not the >>> IOException that is caught here. In fact the IOException caught is >>> really a java.io.NotSerializableException which is built with the >>> message "org.apache.commons.math.util.Pair" automatically by the >>> serialization process in the JVM (at least for Oracle implementation). >>> >>> All exceptions must be serializable (this comes from java.lang.Throwable >>> implementing the Serializable interface). This was also one reason why >>> our Localizable interface had to extends Serializable, so it could be >>> used as a field in our exceptions. >>> >>> This change added a List<Pair<Localizable, Object[]>> field in >>> MathRuntimeException. Pair is not serializable. Adding Serializable to >>> the implemented interfaces in Pair does solve the problem (if also the >>> change below is reverted, of course) but I don't think it would be wise >>> unless we also enforce the two generic parameters of Pair to be >>> serializable too. Perhaps we should use a Map<Localizable, Object[]> >>> instead ? >>> >>> The problem also makes me think that we already had a similar bug in >>> another part of the exceptions for a long time : the Object or Object[] >>> parameters of the exceptions are stored and may not be Serializable too. >>> This was never a problem either because the parameters are often simple >>> primitive ones (int, double ...) or arrays so they were serializable. >>> Perhaps we should change the signature from Object and Object[] to >>> Serializable and Serializable[] ? >>> >> >> Or make the fields transient? >> >> That would perhaps cause problems if the Exceptions were ever >> serialised, but can that happen? > > Yes, it's a typical JEE scenario and I cannot say how often I cursed Sun for > stuffing an unserializable object into NamingException.setResolvedObject > within their LDAP implementation ... > > Actually the MathRE should contain a writeObject implementation that > replaces any non-serializable object in that array with some kind of > replacement (e.g. with its String representation). Otherwise the MathRE will > not reach its destination and all the localization was for nothing.
That's a good idea. We could even do that replacement directly at construction and never store the Object themselves, regardless of their status with respect to serialization. Thanks Jörg > > However, with a writeObject/readObject pair, such a field can be made > transparent, if the objects are put directly into the object streams. > > Note, that anything of this requires a change of the serialVersionUID. > > - Jörg > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org