Am 07.09.2011 19:51, schrieb Sean Mullan:
On 9/3/11 1:04 PM, Sebastian Sickelmann wrote:
Am 02.09.2011 21:58, schrieb Sean Mullan:
On 9/2/11 1:43 AM, Sebastian Sickelmann wrote:
Here is the updated webrev:
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/7011804_0/
Hmm, the main problem I have with this change is that the printStackTrace
methods will no longer print the stack trace of the cause because it will always
be null. That doesn't seem right to me, as it could be considered an
incompatible change, and it will make it harder to debug issues.
The printStackTrace in Throwable calls the overridden getCause().
Maybe we should add @Override to it.
Updated the webrev to:
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/7011804_1/
In that case, my main concern is addressed then. I would probably want someone
from our TCK team to also review it with respect to JSR 105 compatibility, so
I'll see if I can find someone.
Fine, that would be good.
But first, can you expand your webrev to include the other Exception classes in
javax.xml.crypto.**?
The new webrev is here:
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/7011804_2/
In some classes the initCause comment is misspelled as initCaus. What about a
test case, for example testing to make sure initCause throws an exception? Can
you write one?
While creating an test(suggested code below) for this, i thought a
little bit about if it is really good to change the behavoir of the
ctors without a cause (defaultctor, ctor with message).
What is the best behavoir (see DEFAULT and MESSAGE_ONLY cases below) .
Should in the cases DEFAULT and MESSAGE_ONLY the cause mutable? I
actually think this would the better solution, cause it is what the
users can actually do with the exceptions in javax/xml/crypto. Or should
the test check on imutability in all cases?
Also, I have asked someone from the TCK team to look at this and he said he will
do that by Friday. It might require a CCC change because the behavior of
initCause is different. I am hoping it doesn't require a JSR 105 maintenance
revision though.
--Sean
public class PreventOverridingOfChaining {
private static final Class[] DEFAULT = new Class[]{};
private static final Class[] MESSAGE_ONLY = new Class[]{String.class};
private static final Class[] CAUSE_ONLY = new Class[]{Throwable.class};
private static final Class[] BOTH = new Class[]{String.class,
Throwable.class};
private static final Class[] URI_REFERENCE_SPECIAL = new
Class[]{String.class, Throwable.class, URIReference.class};
public static void main(String args[]) throws Exception {
check(NoSuchMechanismException.class);
// all other exceptions.
}
private static void check(Class<? extends Throwable> exClass)
throws ReflectiveOperationException {
Constructor<? extends Throwable>[] constructors =
(Constructor<? extends Throwable>[]) exClass.getConstructors();
for (Constructor<? extends Throwable> ctor : constructors) {
Class[] parameterTypes = ctor.getParameterTypes();
if (Arrays.equals(DEFAULT, parameterTypes)) {
checkMutable(ctor.newInstance());
} else if (Arrays.equals(MESSAGE_ONLY, parameterTypes)) {
checkMutable(ctor.newInstance("msg"));
} else if (Arrays.equals(CAUSE_ONLY, parameterTypes)) {
checkImutable(ctor.newInstance(new
RuntimeException("RE")));
} else if (Arrays.equals(BOTH, parameterTypes)) {
checkImutable(ctor.newInstance("msg",new
RuntimeException("RE")));
} else if (Arrays.equals(URI_REFERENCE_SPECIAL,
parameterTypes)) {
checkImutable(ctor.newInstance("msg",new
RuntimeException("RE"),new URIReference() {
@Override
public String getURI() {
return "URI";
}
@Override
public String getType() {
return "Type";
}
}));
}
}
}
private static void checkMutable(Throwable th) {
th.initCause(new RuntimeException());
}
private static void checkImutable(Throwable th) {
try {
th.initCause(new RuntimeException());
throw new RuntimeException("Exception expected");
} catch (IllegalStateException e) {
// Everything is fine
}
}
}
