JBMDR-51; Annotation equals is completely broken

[alesj]

This AnnotationProxy::doEquals looks like it misses a lot of logic. ;-)

 private Object doEquals(Object proxy, Object obj)
 {
 if (obj == proxy)
 return Boolean.TRUE;
 if (obj == null)
 return Boolean.FALSE;

 Class[] intfs = proxy.getClass().getInterfaces();
 if (intfs[0].isAssignableFrom(obj.getClass()) == false)
 {
 return Boolean.FALSE;
 }
 try
 {
 Proxy.getInvocationHandler(obj);
 }
 catch (Exception ex)
 {
 return Boolean.FALSE;
 }
 return Boolean.TRUE;
 }

And that last attempt is wrong,
e.g. this is totally legit, but would also fail:

 String expr = "@"+Name.class.getName() + "(type=\"type\",subtype=\"subtype\")";
 Name n0 = (Name) AnnotationCreator.createAnnotation(expr, Name.class);
 Name n2 = new NameImpl("type", "subtype");
 assertEquals(n0, n2); // FAILURE -- n2 has no InvocationHandler

I'll fix the impl, but what to do with release, since we already did 2.0.0.GA?
2.0.1.GA?

[alesj]

"alesj" wrote:

 try
 {
 Proxy.getInvocationHandler(obj);
 }
 catch (Exception ex)
 {
 return Boolean.FALSE;
 }
 return Boolean.TRUE;
 }

I've changed this with

 for (String key : map.keySet())
 {
 Object value = getValue(key);
 Object other = getBeanInfo().getProperty(obj, key);
 if (Objects.isArray(value))
 {
 if (Objects.isArray(other) == false)
 return Boolean.FALSE;

 // TODO - how to generically compare arrays; primitives vs. objects
 if (Arrays.equals(...) == false)
 return Boolean.FALSE;
 }
 else if (JBossObject.equals(value, other) == false)
 return Boolean.FALSE;
 }

but see my TODO.

Do we have some generic code for arrays compare?
So that I don't re-invent the wheel. ;-)

[alesj]

"alesj" wrote:

Do we have some generic code for arrays compare?
So that I don't re-invent the wheel. ;-)

Too late, re-invented. :-)

 if (Objects.isArray(value))
 {
 if (Objects.isArray(other) == false)
 return Boolean.FALSE;

 int length = Array.getLength(value);
 if (length != Array.getLength(other))
 return Boolean.FALSE;

 for (int i = 0; i < length; i++)
 {
 if (JBossObject.equals(Array.get(value, i), Array.get(other, i)) == false)
 return Boolean.FALSE;
 }
 }

[starksm64]

Yes, a 2.0.1.GA is what we should do.

[alesj]

Yes, a 2.0.1.GA is what we should do.

OK, I need to port the changes to Branch_2_0.
Then I'll do the release, probably early Monday morning.

Performance wise this is quite different from what we had,
but on the other hand, what we had didn't work. :-)
And I don't really see how else we can do proper equals,
other than comparing all members - using jboss-reflect/BeanInfo.

[alesj]

"alesj" wrote:

Too late, re-invented. :-)

 if (Objects.isArray(value))
 {
 if (Objects.isArray(other) == false)
 return Boolean.FALSE;

 int length = Array.getLength(value);
 if (length != Array.getLength(other))
 return Boolean.FALSE;

 for (int i = 0; i < length; i++)
 {
 if (JBossObject.equals(Array.get(value, i), Array.get(other, i)) == false)
 return Boolean.FALSE;
 }
 }

As annotation can only take single dimension arrays as values this is fine.
But since I'm already fixing this:
- http://lists.jboss.org/pipermail/jboss-development/2008-October/012837.html
I'll add a generic (recursive) arrays equal to common-core.
Unless this is already done somewhere, and I just cannot find it. ;-)

[adrian.brock]

"alesj" wrote:

"alesj" wrote:

Too late, re-invented. :-)

 if (Objects.isArray(value))
 {
 if (Objects.isArray(other) == false)
 return Boolean.FALSE;

 int length = Array.getLength(value);
 if (length != Array.getLength(other))
 return Boolean.FALSE;

 for (int i = 0; i < length; i++)
 {
 if (JBossObject.equals(Array.get(value, i), Array.get(other, i)) == false)
 return Boolean.FALSE;
 }
 }

As annotation can only take single dimension arrays as values this is fine.
But since I'm already fixing this:
- http://lists.jboss.org/pipermail/jboss-development/2008-October/012837.html
I'll add a generic (recursive) arrays equal to common-core.
Unless this is already done somewhere, and I just cannot find it. ;-)

What's wrong with Arrays.equals(Object[], Object[])?

The square brackets probably don't show properly above? :-)

Also, javassist has an annotation implementation that does equals and hashcode
correctly (including default values).

[alesj]

What's wrong with Arrays.equals(Object[], Object[])?

I doubt that works on primitive arrays?

And BeanInfo::getProperty(Object, String) returns Object,
which can be anything Annotation member can be.

[alesj]

Also, javassist has an annotation implementation that does equals and hashcode correctly (including default values).

How do we enable that this gets used when creating annotations?

Since from MC/Kernel/AbstractAnnotationMetaData we use

ann = (Annotation)AnnotationCreator.createAnnotation(annString, cl);

where this at the end uses AnnotationProxy::createProxy.
No sign of checking if Javassist is present - to use its mechanism.