Good point.

Actually, considering that
- OMImplementation only exists in the 1.2.8 release and in the trunk,
- it is not yet complete (no getInstance method),
- it is currently only used in the test cases,
I think we can safely substitute it by an interface. Maybe we should
create that interface under a different name (e.g. OMMetaFactory) but
keep the method signatures, so that if somebody complains about the
fact that OMImplementation is disappearing in 1.2.9, we can put it
back (as an implementation of the new interface) later.

Andreas

On Mon, Feb 9, 2009 at 19:19, David Illsley <[email protected]> wrote:
> I agree with the analysis and like the proposed solution, but I think
> that OMImplementation not being an interface will cause problems in
> the OSGI case as it injects instances of a declared interface.
>
> We could add an additional interface that OMImplementation implements,
> or perhaps you know that OSGI sill support the abstract class?
> WDYT?
> David
>
> On Mon, Feb 9, 2009 at 9:50 AM, Andreas Veithen
> <[email protected]> wrote:
>> All,
>>
>> I had a look at David's OSGIification changes in Axiom. I think that
>> the underlying factory mechanism in Axiom should be simplified and
>> improved and this would also make the OSGi injection much simpler.
>> Indeed (independently of the OSGi question), the current
>> implementation of the factories in Axiom has a number of issues:
>>
>> 1) There are currently three independent factories (1 x OMFactory and
>> 2 x SOAPFactory) and their implementations are specified by three
>> system properties. I think that in general it would not make sense to
>> mix default factory implementations from different implementations of
>> the Axiom API (e.g. an OMFactory from the LLOM implementation and SOAP
>> factories from DOOM). Instead of having system properties for the
>> individual factories, it would make much more sense to have just one
>> system property specifying the concrete API implementation to use by
>> default.
>>
>> 2) As explained in the Javadoc [1], the current implementation of
>> OMAbstractFactory doesn't take into account the lifecycle requirements
>> for the factory implementations: while LLOM factories are stateless
>> (implying that the same instance can be used to build any number of
>> documents), with DOOM, a new factory instance must be created for each
>> document.
>>
>> 3) There is code that needs to work with different API implementations
>> chosen at runtime (where switching using system properties is not
>> possible). With the current mechanism (more precisely with Axiom
>> version prior to 1.2.8; see below), this requires to pass all three
>> factory instances to the code (at least if the code uses SOAP), which
>> is quite cumbersome.
>>
>> In order to overcome these issues (primarily the last one), in 1.2.8 I
>> introduced a new class called OMImplementation [2] which can be best
>> described as a meta factory: it is a factory for OMFactory and
>> SOAPFactory instances. My proposal would be to redesign the existing
>> factory mechanism around this class. This can be done in a backward
>> compatible way. For this we would
>>
>> * implement a static getInstance() method on OMImplementation that (in
>> a non OSGi environment) loads the implementation based on a system
>> property (with LLOM as default);
>> * change OMAbstractFactory to delegate to the default OMImplementation 
>> instance.
>>
>> The existing OMImplementation code already solves issue 3 (and is used
>> to implement reusable unit tests). The extensions described above
>> would obviously address issue 1. They also solve issue 2 because
>> OMImplementation is lifecycle-aware (i.e. the implementation may
>> choose to always return the same factory instance or a new one on
>> every invocation). Finally, this would simplify the OSGi issue because
>> there will be only a single component (instead of three) to inject.
>>
>> WDYT?
>>
>> Andreas
>>
>> [1] 
>> http://ws.apache.org/commons/axiom/apidocs/org/apache/axiom/om/OMAbstractFactory.html
>> [2] http://ws.apache.org/commons/axiom/apidocs/index.html
>>
>

Reply via email to