mapping of super interfaces is ignored

Hi,

I have added a test case to reproduce the bug.
I thought that I could change the code a bit such that it work. But it seems a bit more complicated.
Why? Dozer has method for determining the mappings between super classes and a method to determine mappings between interfaces. Apart from that this latter method contains a bug as it doesn't check the interfaces recursively, there doesn't exists a method that determine a mixture, that is the mapping between an interface and class.
I don't think this is very uncommon.
Example: My domain objects must have an interface to hide functionality but that can be used by tools as dozer and hibernate. It makes them so called proxy safe in this way.
My dot's are just simple pojo's that don't need an interface.
But ofcourse I need to define a mapping between my domain object and dto object.
This is easier and more robust when using the domain interface. Example: mapping from Member to MemberDto and Taxer to TaxerDto.
Why more robust: I know need to use the implementation class to define the mapping, which means I have to use a lotttt of hints to tell dozer that it should MemberDef when it encounters a field with type Member :(... This is very fragil and easy forgotten.
Always when I change something, one or more mapping tests fail which is often unnessary. That's what I mean by robust.

I think the method MappingProcessor.checkForSuperTypeMapping and ClassMappings.findInterfaceMappings should be mixed.
Implementation idea: make one Class array for all source super classes and interfaces and one for all destination super classes and interfaces. Then just iterate over all combinations, and you will find all possible mappings..
I also noticed that this latter method: findInterfaceMappings, doesn't use caching like the former method checkForSuper does.

Please let me know how the Dozer team things about supporting this? and when?

Cheers,
Ed

Please let me k

I was really getting completely nuts, crazy, frustrated of this bug.
Why?... everytime I changed something on the right side, something on the left side didn't work anymore...
I didn't want to solve this bug and tried to work around it in my configuration.... but ... no luck....

Like mentioned before, The configuration was very fragil because of this bug, so I had to solve it, as it really is a showstopper.
It seems to work fine for now and I hope that dozer will keep working fine for some time now as one week of 12 hours a day bug fixing Dozer is a bit enough for now. I already have such a delay :(
Some of the dozer test fail (with the latest update of the Calendar funcationality just fixed by Dimitri), but I think they were already failing as mentioned in the bug tracker.
My configuration seems much more robust and the performance is fine.

The code changes are added as attachment. I have added one file that contains new versions of the changed methods in the MappingProcessor.
Also an improved test case is added.
I think it also solves some other reported interface bugs.

Anyway, in words: like mentioned below, I mixed the calculation super class/interface mapping together. That's the only way to retrieve any mixed interface <-> concrete class mapping. Which I don't think is a very uncommon situation.
I replaced the method MappingProcessor.checkForSuperTypeMapping (see attachment) by a new one and that it's the only method that is called in the map(classMap, ...) method, so the other method call ClassMappings.findInterfaceMappings has been removed. In fact, I think this whole method could be removed as it buggy as explained below.

This new method will call a new method called MappingProcessor.determineSuperClassMappings to determine the collection of mappings and will cache this collection of mapping (like already toke place). The method determineSuperClassMappings will call some helper (recursive) methods to determine any possible mapping between any of the source and target class/interface. Have a look at the testcase it contains a source object with a complext graph of super classes and interfaces.
The idea: on one side you have a source graph that exists of a one source class with his associated super classes and all his interfaces and on the other side the target class as you mapping target with his super classes and interfaces. You visit all source classes/interfaces and all belonging target classes/intefaces (outer join/permutations) and wil look if there is any mapping between the encoutnered combination. The encountered mappings are put in a list in such an order that the super classes/interfaces are mapped first. Even with quite a lot of interfaces and super classes, it seems to perform well (and believe me, I have some complex/necessary class graphs).

I am looking forward to the reaction of the Dozer team and how they will integrate this/solve this bug for the comming release?
BTW: I have put the functionality in just a few recursive methods now, such that the change is rather isolated and the performance high which is a priority for Dozer ofcourse. But you when integrating this you might want to consider delegating this functionality in some special classes that do the job. I think you just should be carefull not to create too many temporarily objects during the iteration. That is the reason that recursive is very fast in this case, it just consists of simple method calls. I am sorry Dimitry I know you don't like them ;).. and I am currious how you will implement this without recursion ;)...

-- Ed

Changed methods in MappingProcessor

Improved test case

Hi Ed,

Can you attach the superInterfaceMapping.xml file that is required by the unit test...or if it's somewhere I am not seeing, just let me know? Thanks

testcase mapping

Hi,

Yep, forgot to attach it, sorry.

I just attached the mapping file. Just put it in the test/resource folder of dozer and it will work.

(I did put all my bug test cases in the mapping.bug package)

-- Ed

I tried to apply the patch and it resulted in 4 unit test failures of existing regression tests. All of the failing tests are proxied object tests which runs mapping use cases for cglib proxied data objects. Any idea which code in the patch broke existing functionality?

ProxiedGranularDozerBeanMapperTest
ProxiedInheritanceAbstractClassMappingTest
ProxiedInheritanceMappingTest

btw, in the meantime I added this test case to our collection of identified failing use cases.

btw, in the meantime I added this test case to our collection of identified failing use cases.

btw, in the meantime I added this test case to our collection of identified failing use cases.

Yep,

I did. Looked at:
-- testPrimitiveArrayToList_UsingHint
The test class fails because you use both the old fashion proxy and new proxy functionality in dozer, which isn't working very nice of course. Like mentioned below you should remove the method getRealClassName and use the new proxy functionality.

What happens now:
When entering the map method, the correct class map is found, but never used as the same mapping is already done in the method processSuperTypeMapping. Just comment out the following line and you will see that this test will run correctly:
mappedParentFields = processSuperTypeMapping(superClasses, srcObj, destObj, mapId);

The super class mapping found is however a general mapping as it doesn't find the mapping that is identified through the map-Id, such that the int values are mapped to Integers and not to String values as indicated by the hint....

What goes wrong? No unproxy takes place such that the first super class of the source object is the class PrimitiveArrayObj.
Because this unproxy doesn't take place the method checkForSuperTypeMapping will find a mapping between the super class PrimitiveArrayObj and PrimitiveArrayObjPrime.
If the unproxy takes place correctly the first super class would be BaseTestObject such that only real super class mappings (and interfaces) will be found as it should. As such that the mapping PrimitiveArrayObj will not be found.

So the solution: correctly implement the unproxy method (and remove the realClass and getRealSuperclass in MappingUtils as they are useless anyway).

Have a look a the proxy bug, it contains al the details why the current proxy functionality doesn't work.

-- Ed

The patch broke existing tests and feature request added for custom proxy resolver support

https://sourceforge.net/tracker/index.php?func=detail&aid=2624921&group_id=133517&atid=727371

The patch broke existing tests and feature request added for custom proxy resolver support

https://sourceforge.net/tracker/index.php?func=detail&aid=2624921&group_id=133517&atid=727371

Hi Matt,

I noticed that you closed the interface bug
https://sourceforge.net/tracker2/?func=detail&atid=727368&aid=2556896&group_id=133517
and tend to solve it through
https://sourceforge.net/tracker2/index.php?func=detail&aid=2624921&group_id=1335
17&atid=727371.

I don't really understand this:
1) Solving the RFC:
https://sourceforge.net/tracker2/index.php?func=detail&aid=2624921&group_id=1335
17&atid=727371 won't solve the interface bug:
https://sourceforge.net/tracker2/?func=detail&atid=727368&aid=2556896&group_id=1
33517, so why did you delete this bug?

2) Yes, if the proxy RFC will be implemented you can solve the interface bug
more easy, so there is a dependency. But that doesn't mean that the interface
bug is solved when the proxy RFC is implemented :(....

I would leave the interface bug open en make e dependency to the RFC.

-- Ed

I had already added your unit test for this to our regression suite as a known failing use case that should be fixed. So it is accounted for as a defect. Lets get the proxy resolver feature request implemented and then readdress this one with a clear ticket as the patch implementation code would most likely change.

The large amount of verbiage and back/forth in this ticket would make it inefficient/confusing to work once the prereq's are in place. Also, the attached patch broke existing tests, so dont want that to add to the confusion. When working tickets with pages and pages of history/details it is difficult and inefficient to try and make sense out of it to determine what in the world was the original issue.

So I think it will be most efficient to open a new clear and concise ticket for this bug once the proxy resolver Feature Request is implemented.
https://sourceforge.net/tracker/index.php?func=detail&aid=2624921&group_id=133517&atid=727371

Going forward I will put these types to some other status than deleted so that you can update. I didn't realize that deleted tickets dont allow updates for people that are not on the project team. I am able to update deleted tickets obviously

not yet fixed

Fixed by a patch submitted.
Test case has been incorporated into Dozer test suite.