Split ClassDescriptorResolver to be more expandable for future needs

I split up XMLClassDescriptorResolver as follows:

XMLClassDescriptorResolver will have the same interface as today and will know which strategy to use and how to instantiate the strategy.

ResolveStrategy interface that describes what information a strategy has to provide to XMLClassDescriptorResolver - it is more or less the same as cdr had been before, but now cdr has another purpose.

CastorXmlStrategy which implements ResolveStrategy and is the implementation of todays Castor strategy to resolve class descriptors. The strategy handles features like caching...

ResolveCommand interface that describes what a resolve command needs to provide.

All resolve steps of XMLClassDescriptorResolverImpl are now implementations of ResolveCommand.

So where's the patch ?

My path includes the following changes against HEAD:

org.exolab.castor.tools.MappingTool
replaced a reference to XMLClassDescriptorResolverImp with XMLClassDescriptorResolver

org.exolab.castor.xml.XMLConstants
add two new contants for XMLClassDescriptorResolver

org.exolab.castor.xml.util.XMLClassDescriptorResolverImpl
is now more or less a facade for the CastorXMLStrategy and in future for other resolving strategies yet to come

org.exolab.castor.xml.util.ResolverStrategy
interface of the resolve strategies

org.exolab.castor.xml.util.ResolverCommand
interface of resolve commands

org.exolab.castor.xml.util.resolvers.*
the resolve commands extracted from the old XMLClassDescriptorResolverImpl implementation

the patch - Ta Ta

I have reworked my path. Mainly I removed loads of interfaces that ensure the correct configuration and introduced properties instead. Have fun with it.

Hi Joachim,

wouldn't it be better to change ResolverCommand to define the following 2 methods:

public Map resolve(String className, Map p) throws ResolverException;
public Map resolve(Class clazz, Map p) throws ResolverException;

In addition I would create a abstract class AbstractClassNameResolver that implements:

public final Map resolve(Class clazz, Map p) throws ResolverException {
if (clazz == null)

return resolve(clazz.getName(), p);
}

This should remove quite some duplicated code.

In addition I wonder if the check for a classloader that happens at every resolve method of ResolverCommand's can also be extracted to a central place.

I'm also not a fan of inner classes or interfaces like DiscriptorCache. Any reasons why you hidded that interface?

Having said that I haven't looked at all details yet.

Hello Ralf,

my first implementation was a like what you suggest (ResolveCommand.resolve(String)) - and it failed a huge lot of tests... now I know why and I will, for sure, do some refactoring.

But before doing refactoring I want to know if:
*) the way of handing down configuration (in form of properties) is ok
*) the split of XMLClassDescriptorResolver / Strategy / Command is ok

As for inner interfaces: in JAXB I will need them, so I toyed around with them... and also I think it a nice way to say "this is what ResolveStrategy needs to be provided for it but no one else is probably intereseted in it - it is no interface of interest for Castor itself"

I'll take a closer look at the 2 points from above when separation of JDO modul got finished.

@inner interfaces/inner classes:

I never would use inner interfaces as I frequently find myself searching for them. The only exception for me is if one have been lazy enough to use such constructs in a public specification (e.g. in JAXB).

About the same applies for inner classes with the exception of simple value objects without additional responsibilities (e.g. Map.Entry).

You defined DescriptorCache interface in ResolverStrategy while its implementation is in XMLClassDescriptorResolverImpl. The interface is used at CastorXMLStrategy as intended but in addition it is used by XMLClassDescriptorResolverImpl which does not implement ResolverStrategy. Having said that I would throw away the interface in that case and move DescriptorCacheImpl out of XMLClassDescriptorResolverImpl.

@split of XMLClassDescriptorResolver / Strategy / Command:
very good

@handling of configuration:
As XMLClassDescriptorResolverImpl is responsible for instantiating ResolverStrategies as well as ResolverCommands we should pass a Configuration instance (the one I'm working at at another issue) from XMLClassDescriptorResolverImpl to each strategy and command. Each of these classes can hold a reference to this Configuration if it needs to access some of its properties.

I don't think CastorXMLResolver should offer get/setProperties methods nor is there a need to pass the Configuration to each Command.resolve() method call (at least as far as I understand the code ATM). I also would not put things like a DescriptorCache or MappingLoader instance into the Configuration. The keys of the properties in the Configuration should be defined as constants at one place (e.g. in an XMLConfiguration class). This omits typos and allows for easier search of places where they are used with a bit of more work to maintain them.

@inner interfaces / inner classes

During the implementation of this patch I started to like inner interfaces. Lets look onto ResolverStrategy.DescriptorCache:

• the name is ugly (maybe even a bit misleading for the current purpose) but here I took what had been there before...
• with this interface the ResolverStrategy states that it requires a place to put the descriptor found to and the outside world should provide it
  The other approach would be to have 'classic' interface, but I see the following disadvantages in it:
• we would create an interface, which is only used by very few classes... I would like to keep this 'more private'...
• for me it is the ResolverStrategy that wants to have a place to write its results to - so the interface is specific for this class

Beside of this arguments: I will remove the inner interface if it is required...

@ResolverStrategy.DescriptorCache

I will rename the interface to something like ResolverResults - any ideas?

@handling of configuration

The current configuration stuff is a bit fuzzy as I wanted to wait for your configuration stuff

I've overworked the patch a lot... the 'main' changes are:
*) introduced ResolverPackageCommand package are different then classes and with XMLClassDescriptorCacheImple.addPackage we have the need for pure package stuff
*) I did it!! Now only the class name is required to be handed down to strategy and commands... the class loaders had been beasts but I've tamed them
*) there are now abstract commands to ensure a maximum of NOT copied coded
maybe I've missed something in my list... but I think these are the high lights.

As for configuration: this needs to change as soon as the new configuration stuff is available.

Haven't had enough time to look at the new patch in more details but what I recognized are some warnings of ResolverException being not declared in ResolverCommands while it is declared to be thrown at one of the resolve methods. So I wonder if you intended it to be a checked or unchecked exception.

Corrected problem with not declared exception.

Final patch with some minor changes at Joachim's original code.