Proxied classes need to be identified and receive special processing

Looking at those three issues, I'm worried that at some point someone will want to actually work with the proxy instead of the original... Is this a problem? Do we need to have switches to turn the above processing steps on and off (or just turn proxy identification off, at which point all classes will be treated the same)?

Using net.sf.cglib.proxy.Factory interface to find proxies sounds good.

I don't think we need to map proxies (3.) and therefore don't need to switch between proxy and original mode.

Now, I wouldn't mind if you assigned this to yourself .. .

Okay, I'll take a stab at it. Do we only need to worry about this on the XML side? Or are there JDO implications also? I think my biggest problem is going to be finding all the files this issue touches...

My first thought was that there are no implications on JDO but after after a while I realized that there are some. For this issue I'd suggest to focus on XML side only.

The best way to start with should be to write some testcases

• marshal proxy with mapping
• marshal object with relation to proxy with mapping
• marshal proxy with introspection
• marshal object with relation to proxy with introspection
• ...

Does that also affect classes generated with srcgen?

With the tests you should be able to reproduce the problem, debug where it happens and check if the fix works.

For an example of a CGLIB proxy, look at: src/org/castor/persist/proxy/SingleProxy.java

Ralf - Sounds like a plan to me. And thanks for the example on the Proxy, that was a big chunk I was missing.

Just to make sure I understand your conceptual test cases:

• Marshal an object that is a proxy for an object that is mapped
• Marshal an object that is mapped that holds a reference to an object that is a proxy
• Marshal an object that is a proxy for an object with no mapping
• Marshal an object with no mapping that holds a reference to an object that is a proxy
• (other variations of the above with combinations of introspection and mapping)

I have no idea the effects of this issue on source gen... Off the cuff I would guess this is above the level that src gen cares about (it creates the ClassDescriptor objects that we're trying to locate), but I don't know that code at all.

Okay, just a couple of comments ...

• I cannot think why the source generator should be affected.
• You could use a simple mapping that involves a JDO 1:1 relation (lazy-loaded) for your test cases.
• I'd be curious to learn as to why the JDO side is affected at all. Or are we trying to perists classes generated by Hibernate .. ?
• The test plan as agreed pon between the two of you sounds fine by me. If I were you, Stephen, I'd simple add a new bugXXXX package to src/bugs and start to write up the test cases, and simply try to execute them. At least that's how I start to go about fixing problems frequently.

Werner/Ralf - Having done some reading about the CGLIB proxies, I'm beginning to think the problems might end up being very proxy-implementation specific, i.e. if the proxy adds get methods (Paul's bug) or other items that might cause Castor troubles. Because of this, my current inclination is to write some test cases directly using CGLIB proxies rather than through an interface like Castor JDO or Hibernate. That way I can force the proxy to behave in certain ways and test specific implementations. If after an initial round of tests, you guys think something has been missed, I'm willing to work on pulling in specific proxy-using tools. I haven't quite gotten up to speed with the CTF, so my tests may end up being closer to straight JUnit, but I'll do my best to start working toward the framework.

Werner, my first thought also was: "why should someone ever use castor to persist hibernate objects?" but then I came to realize that CGLIB can also be used for for other reasons. For example, someone may use CGLIB to add some swing listener functionality to his POJO's for using them with swing or he may implement some of transaction lazy loading with CGLIB. Therefore I think we should also take care on CGLIB at JDO with a separate issue.

Werner and Ralf - so I think I've found two possible reasons for us to worry about (3 - Mapping Proxies) above... (Note: this is still much lower priority than 1 and 2 in the description)

1. Where I work we have some amount (large enough to bar a simple rewriting, but not huge) of legacy code hanging around that is very un-javabean-like, and in the past I had been writing wrappers for these classes in order to use them with Castor. It occurred to me this morning that another way to attack this problem would be to use proxies.
2. Proxies provide a way to get around the public/private field access debate that was going on a little while ago... The public API doesn't have set methods for fields, but a proxy can provide those, and as long as Castor is the only piece of code aware it is a proxy, the public API remains intact (maybe)

I'm not saying either one of these is the best way to handle their respective issue, but they are ways of handling the issues.

If there is interest in working with these two issues, I would propose a change to the mapping file. In the <class> element, there should be a use-proxy attribute (default false), and in the <field> element there should be something that specifies that a proxy must be used to access that field (don't know exactly how this one would take shape yet). But the general idea would be if use-proxy="true", Castor would defer "no method found" errors on proxy-accessed-fields until it actually has an object, at which point if that object doesn't provide the correct methods, then Castor can throw the exceptions (basically move the loadMapping exception to marshall).

I still need to write the test cases, but my initial feeling is that if use-proxy="false" (I think this is our current approach) and Castor detects a proxy, all operations will be performed on the super-class rather than the proxy (thus making both of the cases above infeasible).

I suggest to focus on points 1 and 2 of your description of the issue. What you described at your last comment may get a bit difficult and may need some more discussion. It may propably best to create a new issue on that so we can handle and talk about it seperate.

Stephen, assuming that you won't be able to provide us with a fully working and tested patch before COB tonight .. , can you please move this to 1.0?

Well, if people would stop requesting documentation from me, maybe I could work on it! j/k

Stephen, as you seemed to have worked on this to some degree .. , is there anything you could attach to this issue (even if untested) ?

And what I'd really appreciate if a quick pointer to some code area that would make a good starting point to look at ... with regards to the issue at hand, of course .. .

Just another short comment, as I have amended the JIRA notification scheme in use for Castor, instructing Jira to forward comments posted on an issue to the original reporter as well.

I have an issue whereby I try to marshall into XML an object that has been loaded via HIbernate, and this object has a parent child relationship with another (on other words both point at eachother).

In the Castor mapping this is circumvented by using a reference on one of the fields, and this works fine if the objects in question have not been loaded via Hibernate, but for the Hibernate loaded objects the marshal method never returns, I assume it has gone into an infininte loop as it is trying to marsahll unmapped proxy classes instead of the mapped real ones (it really seems to ignore the mapping file big time in such cases, stepping through in debug shows it trying to marshall all sorts of low level stuff like Hibernate sesssions and JDBC driver classes, as well as the few properties that I want to marshall).

Until this fix is done is there a work around that can be used, perhaps along the lines of not using introspection but using get methods, which end up invoking proxy code that eventually populates the instance with the real objects after going back to the database for a lazy load ?

Paul, I don't think there's a workaround (as discussed above). But if you wanted, you could help me with the required patch to have this enabled. I wonder what's the best way to get my hands on a small sample that enabled me to replay the problems discussed ? And that's without using Hibernate, if possible. Having said that, I think that using a 1:1 relation with Castor JDO with lazy loading enabled and then trying to marshall this should be sufficient.

Would you be in a position to provide me with your (simplified) code that I could modify to switch towards using Castor JDO rather than Hibernate ?

Sorry, I'm late in responding. For some reason i didn't have this as a watched item. The code that calls on hibernate is pretty simple, it simply loads an instance of a class from storage then tries to marshal it. It's setting up hibernate and the mapping files etc that is the big job.

It's not clear to me what the issue(s) are at this point in time as i'm a little vague on the previous comment. Is the issue that there are 2 objects pointing to each other? Or is the issue the original one, that castor is using introspection on the proxy class and is trying to unmarshal objects that the proxy has added?

This is a sticky problem. It's not clear that it is specific to CGLIB. For example, when defining a collection as a list in java and the mapping file, hibernate will (at times) return its implementation of a list (org.hibernate.collection.persistentbag), which has public getters that may cause problems. I will attempt to reproduce this again with some code in the next week and get back to you.

Paul, we (Ralf and I) have been looking at this last weekend in some detail, and I guess we have gained some substantial understanding as to why things happen as they currently happen. We are still not a 100% sure hot to go about a fix. I have to admit, though, that we are currently only investigating how to improve the integration of CGLIB-generated classes, as that's what both Hibernate and Castor return when loading objects referenced in a 1:1 relation lazy-loaded.

The problem generally occurs when a framework like Hibernate (and Castor JDO) return their own classes (even collection type implementations in the context of lazy loading, i.e. late materialization.

I will provide some more updates later this week.

Re Paul grillo's question:

Say you have an object called Parent, which has a collection containing a set of Child objects. On each child object there is a property called Parent.

You use Hibernate to map this (Parent class has a one-to-many mapping with Child, Child has a many-to-one mapping with Parent.

When you map these two classes in Castor, you have to specify the reference attribute on the Parent object property of Child, otherwise Castor will go into an infinite loop trying to marshall Parent which marshalls Child which marshalls Parent which marshalls Child etc etc.

Now if you load a Parent and its set of Child objects via Hibernate, by default the collection contains a Set of CGLIB generated proxies for the children, which a class name which is something like Child$$Enhanced by CGLIB$$. Castor doesn't recognise this as a mapped class, even though Child is mapped, because it's dealing with the proxy not the real class.

This being so it ends up introspecting the proxy class, rather than following the mapping for the real class. It is therefore unaware that the Parent object on the proxied Child class is a reference, so it goes into an infinite loop, as if you had left off the reference attribute for the child.

I think the above is just another example of the main problem described on this Jira entry. If necessary I can provide an Eclipse project that will demonstrate the above via a Junit test case, however it would be quite a large post, due to the inclusion of Hibernate, CGLIB, Castor and Spring jar files amongst others.

Would this be helpful or can we get on without this ?

By the way the workaround for Hibernate is to make all one-to-many relationships to be lazy="false", and all many-to-one relationships to be set to outer-join="true". This is not ideal as lazy is preferred so as not to dredge out a large portion of the domain model on a single query, but it's something we can live with just about.

Thanks for the offer (and the very thorough explanations), but I have got a fully working JUnit test case that uses Castor JDO for a (CGLIB generated) proxy class to be generated.

I've made a quick fix for this issue.

Attached is a exported test case eclipse project showing the fix. In the root there is a patch for the org.exolab.castor.xml.Marshaller.

Patch is made based on the http://svn.codehaus.org/castor/castor/trunk rev: 6121 source

------

What it does is:

• Check to see if the current Object is Proxied (Proxy.isProxyClass(Object))
• Lookup the interfaces on the object, return the one you find in the mapping file.
• If no match is found, return the Proxy.class like usual.

Needs to set verify-constructable="false" on the mapped interface class for it to work

Thanks, Askal. I have had a look at your patch, and it looks like your patch resolves any problems related to the use of proxied classes with unmarshalling. I guess over the next few days, I will have to get the same working for unmarshalling.

A reminder for myself: what to do about the additional run-time dependency ? Is this actually acceptable ?

Another quick fix for the unmarshaling part of it.

Attached is a exported test case eclipse project showing the fix for both marshaling and unmarshaling.

In the root there is a patch for the

• org.exolab.castor.xml.Marshaller.
• org.exolab.castor.util.DefaultObjectFactory

And the added Class

• org.exolab.castor.util.DynamicInstance

Patch is made based on the http://svn.codehaus.org/castor/castor/trunk rev: 6121 sourc

The DefaultObjectFactory patch makes it possible to unmarshal any interface using the DynamicInstance class.

The DynamicInstance class returns a Proxied object based on the given Interface.
It holds the get/set values in a map.

This class basicly only supports getMethod/setMethod value interfaces, but could easly be extended to handle mapping matching methods.
ie:

• put/get
• add/get

Havn't looked to much into this beeing used with mapping options like set-method, get-method..

Thank you, Askal, for keeping me informed about the progres you are making, otherwise I might have had a look myself .. . Anyhow, would it be possible for you (in fiture) to attach a unified patch ?

Unified patch, incl. the sample test case provided relative to src/bugs.

My name is Aslak, not Askal, but.. :o)

Yea, sorry about that.. Will do next time.

The patch you added is missing the new test case.
Use the test project from castor-testcase-proxy_marsh_unmarsh.jar insted..

Sorry for the typo, my fault. And the test vcase from the second JAR is actually included, but named TestCastorBug2.

nevermind, bit to quick.. didn't see it there

Just a question on this issue.
I believe this issue relates to the problem I am having with Castor, but maybe there is a simpler solution I can implement prior to this fix being released.

I am using Hibernate. Since Hibernate is using cglib, the actualy class (or child/related) passed into the marshall function is not getting correctly picked up and therefore not mapped, since the class name is similar but not exact (I forget the actual name format ).

I don't have this problem with Hibernate 2, which I am still using (and would rather not)

It seems like all this would take is a modificaiton to the name of the class that is being matched prior to marshalling. so instead of mathing contact, match contact*.

I there is not an easier fix for myself, do you have a timeframe when this might be fixed.

Thanks

DM

Testcase and patch for review.

Could anyone please test if the patch realy fixes the problem with the proxied objects from hibernate. Having said that you have to set:

org.exolab.castor.xml.proxyInterface=net.sf.cglib.proxy.Factory

in your custom castor.properties file to enable this feature. In addition it need to be noted that it will have a valuable impact on marshaller performance when it is enabled.

Ralf, looks good to me. Will test this with another submission by a Castor user.

Excerpt of comments for Richard Gundersen on dev mailing list:

It works!!!

I had to make one small change: rather than the proxy interface being net.sf.cglib.proxy.Factory, my Hibernate one was org.hibernate.proxy.HibernateProxy in castor.properties. So I just added it and it seems to work perfectly

I guess the Castor download could come with this property already set with a list of known interfaces, and then people can override the property with their own if need be perhaps?

So, just a minor suggestion Ralf - could you make that property accept multiple values (if you think it's necessary of course).

When a name of a proxy interface is specified, I'm comparing names of all interfaces of the object to be marshalled with it. In case of a match the descriptor will be search with the objects superclass instead of the class itself. As this will happen for every object I expect a valueable impact on performance at marshalling.

If we allow a list of proxy interfaces this will encrease the time needed even a bit more. On request I'll add the list of interfaces support but I'm not sure if that is neccessary so I left it off initialy.

According to the performance impact I also prefered the deault mode to not detect proxies as this will not be needed by everone.

An alternative of the string compare of interface names would be, to load the interfaces in marshaller constructor and test the object to be marshalled with instanceof against the interface. While the drawback of that is that the interface(s) need to be loaded it may perform a bit better.

Please let me know your thoughts.

Personally, I would not worry to much about the performance impact, as usually the time to 'reflect upon' the proxy classes is way faster than actually lazy loading an entity from a database. Yes, there's an impact, but in the context of this issue I think one can almost ignore it. Unless I am overlooking something, and there's an overhead even for non-proxy objects.

If we search for proxy interfaces this search will be done for every object marshalled be it a proxy or not.

According to some tests the penulty is around 1-5% for the string compare implemented at the patch. A compare with 'instanceof' may be slightly faster but causes much more problems at classloading of interfaces. If we decide to support multiple proxy interfaces the best option seam to be a HashSet where we call contains with name of every interface an object has. I expect the contains to be a bit slower then the string compare.

Final patch including test case relative to src/bugs.

Before I open another bug, i'd like to comment here for those that are listening.

This patch handled the original problem reported so that Castor will use the un-proxied class when marshalling.

Now that Castor has this information (when configured properly, as I have), there is an additional problem.

If a class is mapped in castor as a reference, with a key,

org.exolab.castor.xml.Marshaller makes a call to getObjectID and passes the incoming Object. The Class Name of that Object is used to look up the class descriptor and field descriptor to get the mapping to fetch the object ID (key).

this fails for objects that have been proxied, as the classname is not known to Castor Marshaller and thus the Class Descriptor cannot be found.

Has anybody run into this? Does this make sense? Shall i Open a JIRA?

I have unsuccessfully tried for a couple of hours to build Castor1.2 from source file today to try patching this area so as to ensure that the "unproxied" class is sent.

I will continue to do so.

Paul, it may be possible that we missed to care for the proxies at some areas. Can you please raise a new issue and attach a minimal test that allows me to reproduce things. Having said that a test case that requires hibernate is not an option to me.

I have unsuccessfully tried for a couple of hours to build Castor1.2 from source file today to try patching this area so as to ensure that the "unproxied" class is sent.

Paul, unfortunately, the 1.2 source distribution do miss one or two modules, something that has been reported immediately after the release and has been fixed in SVN.

Name of attachment 'patch.c1342.20080827.txt' leads users to use it as latest patch but it is a very old one from 2006. I intend to remove this attachment.

Removed old attachment 'patch.c1342.20080827.txt'.