Sorry I missed the earlier email discussion - I didn't subscribe to
commons-dev until this morning. Because of not being on the list I
hadn't realized Axiom was at the 1.0 RC stage, if I had realized this I
would have discussed my changes on the list prior to committing. Here's
my justifications for the changes -
I'd originally tried implementing data binding over Axiom using a
delegate approach, where I created an
org.apache.axiom.om.impl.llom.OMElementImpl subclass which basically
just acted as a placeholder. This would construct a regular
OMElementImpl on demand, using the data marshalled from a data bound
object. I had to make my placeholder a subclass of OMElementImpl because
the llom.* classes don't just rely on the interfaces, they actually
check in several places that the objects they're working with are
OMNodeImpl and/or OMElementImpl. Unfortunately this made my subclass
code very brittle - whenever anything changed in the OMElementImpl class
it generally broke my subclass's handling. It also meant that my Axis2
code was hardcoding the llom implementation and would not work if
another OM implementation were to be used instead. There's already been
some discussion of alternative implementations as a solution to the size
issue with Axiom, so it seemed very unsafe to rely on the particular
implementation currently in use.
Moving the general code into Axiom and creating the simple OMDataSource
interface allowed me to work around these issues in what I thought was a
very flexible and powerful way. This interface provided the hooks for
any source of XML data to be used transparently with Axiom. If a tree
actually needed to be constructed the interface allowed accessing the
data in the form of a XMLStreamReader, while otherwise the data could
just be written directly to output. Along with this change I also added
a way of constructing an OMElement which would represent an XML element
backed by an OMDataSource, and added an implementation of this class for
the llom implementation. These changes allowed me to confine the heavily
implementation-dependent code to the Axiom classes, while making the
Axis2 data binding code only dependent on the interface, which is how I
thought things should be structured (since Axiom developers probably
don't want to have to try building Axis2 every time they make a change).
I feel the simplification of the data binding implementation in Axis2
(and the decoupling of the Axis2 code from the internal details of the
Axiom implementation) justifies adding this code for the Axiom 1.0
release. It's certainly made the JiBX binding code much easier, and
should do the same for JAXB 2.0. I'd think it'd also be useful to other
developers working with Axiom, since it allows virtual XML blobs (not
text blobs) to be hung off the tree. This type of feature could be very
nice for working with documents that import other documents (such as
WSDL), as an example. I think the changes are also well-localized and
should not effect the stability of the existing Axiom code.
- Dennis
Sanjiva Weerawarana wrote:
On Fri, 2006-04-21 at 22:13 -0400, Glen Daniels wrote:
Dennis, -1's on code changes are, alas, vetos unless you can get the
dissenter to relent. I must say in this case, since your changes didn't
so much *change* anything as *add* something, I'd be fine with leaving
them in, and a cursory glance at the idea actually seemed good to me.
So I'm essentially -1 to the -1.... :)
Sanjiva, Chinthaka, do you really think these changes were destabilizing
for the mainline code path? And if not, why the -1 if Dennis needs this
stuff to make his data binding work?
Glen, AXIOM has been around for a long time. Dennis added a core
interface and a bunch of stuff without discussion after we had a 1.0 RC
out. That's why I -1'ed it.
We used to have such a thing before (remember OMBlob?) and removed it
because we found it was not necessary.
If this is to be added we need to have a thorough discussion to make
sure its really needed and not something that can be done with OMText as
we found out during the discussion.
Sanjiva.
