You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-users@xalan.apache.org by Mark Weaver <ma...@npsl.co.uk> on 2003/01/22 10:35:54 UTC

RE: returning xml nodes from extensions functions

Hi David.

Well I realise this was back in October but (once again) I managed to run
out of time on it (sadly, the way that the company works is as follows: do
work on core stuff.  Find someone who is willing to pay money.  Immediately
drop everything for money.  Return to core stuff.  Repeat until bored). Over
Christmas (yay, holiday, good time to write some code :) I managed to get
this mainly completed.  Currently the review of the situation is as follows:

class XALAN_XPATH_EXPORT XalanDynamicBuilder
{
public:
	/**
	 * Get the builder objects used to build the tree.
	 */
	virtual ContentHandler*
	getContentHandler() = 0;

	virtual DTDHandler*
	getDTDHandler() = 0;

	virtual LexicalHandler*
	getLexicalHandler() = 0;

	/**
	 * Get an XObject for the nodeset or result tree fragment.  The XObject
adopts the tree.
	 */
	XObjectPtr
	getXObject();
};

With two derived classes; XalanDynamicNodeSetBuilder and
XalanDynamicResultTreeFragmentBuilder for building either one respectively.
As discussed, we have a GetAndReleaseCachedObject that takes care of these
two objects.  The base definition of the interface is in
XPathExecutionContext, with the implementations there returning 0.
Therefore, sounds largely complete.

However when trying to use these, I have run into a problem.  If you
remember, the original motivation for this framework was to allow easy
return of nodesets and result tree fragments from external functions.  So
the test function looks something like:

	virtual XObjectPtr
	execute(
			XPathExecutionContext&			executionContext,
			XalanNode*						context,
			const XObjectArgVectorType&		args,
			const Locator*					locator) const
	{
		XalanDynamicNodeSetBuilder *theBuilder = 0;
		executionContext.getObject(&theBuilder);

		ContentHandler *contentHandler = theBuilder->getContentHandler();
		const AttributesImpl emptyAttributes;
		contentHandler->startDocument();
		contentHandler->startElement(L"",L"test-node",L"",emptyAttributes);
		contentHandler->endDocument();

		XObjectPtr retVal = theBuilder->getXObject();
		executionContext.releaseObject(theBuilder);
	}

(Please ignore the not using GetAndReleasedCachedObject<T> for now).  The
problem here is that 'execute' is not passed a
StyleSheetExecutionContextDefault.  So the getObject call returns 0 (and
then crashes of course since it doesn't check ;).  On deeper examination, it
appears that we have:


const XObjectPtr
StylesheetExecutionContextDefault::extFunction(
			const XalanDOMString&			theNamespace,
			const XalanDOMString&			functionName,
			XalanNode*						context,
			const XObjectArgVectorType&		argVec,
			const Locator*					locator)
{
	return m_xpathExecutionContextDefault.extFunction(theNamespace,
functionName, context, argVec, locator);
}

As the cause of the problem.  i.e. StyleSheetExecutionContextDefault simply
uses a member XPathExecutionContextDefault to execute a bunch of its
functionality.  The effect of this, of course, is to hide the real
implementations of getObject from the external function.  I wondered if you
had any ideas on how best to solve this.  To be honest, given the number of
forwarding functions that exist, I'm wondering why
StyleSheetExecutionContextDefault is not simply derived from
XPathExecutionContextDefault rather than XPathExecutionContext.

Thanks,

Mark

> -----Original Message-----
> From: David N Bertoni/Cambridge/IBM [mailto:david_n_bertoni@us.ibm.com]
> Sent: 31 October 2002 18:00
> To: xalan-c-users@xml.apache.org
> Subject: RE: returning xml nodes from extensions functions
>
>
>
>
>
>
>
> Hi Mark,
>
> > > Hi Mark,
> > >
> > > Sorry, I've been incredibly busy the week or so, so I
> apologize for not
> > > responding sooner.
> >
> > No problem.  Hope things are calmer for you now.
>
> Yes, I'm on "vacation," so I'm writing code.  Wait, that's what I do when
> I'm _not_ on vacation...
>
> > >    1. I think two different classes, one which does the node-set, and
> > >    another which does RTFs would be better than one class with a
> > >    constructor parameter.  What do you think?
> >
> > Yes, that sounds like a better idea.
> >
> > >    2. We should use the SAX2 ContentHandler, LexicalHandler, and
> > > DTDHandler
> > >    interfaces, instead of Xalan's proprietary FormatterListener.
> >
> > I will change this over.  Presumably since we are building a
> XalanSourceTree
> > we will simply return a XalanSourceTreeContentHandler.
>
> You'll have to do it using the SAX2 intefaces, because
> XalanSourceTreeContentHandler is not available at that level in the
> architecture.  Also, it's not an abstract base class, so it's not good for
> use in interfaces.
>
> > >    3. We need an abstract base class that defines the interfaces, then
> we
> > >    can derive the implementation classes from them.  I'd like to get
> that
> > >    set first, so we know we've got the design right.
> >
> > The interface would be pretty much the same:
> >
> > class XalanDynamicBuilder {
> >            XalanSourceTreeContentHandler *getContentHandler()=0;
> >            XObjectPtr getXObject()=0;
> > };
> >
> > with maybe a 'reset' that allows the builder to be passed back to the
> > execution context.
>
> We should probably wrap it in the same style as the other "auto_ptr-like"
> classes already in the interfaces.  That way, the destruction/reset is
> automatic.  See things like XPathExecutionContext::
> GetAndReleaseCachedString, etc.  You may not like my naming style, so feel
> free to call it what you like.
>
> > >
> > > Actually, I think the document is owned by the fragment.  This is
> already
> > > done in the implementation of ResultTreeFrag in the XSLT subsystem.
> > >
> > > Ahh, I should have been clearer about this, and I apologize for not
> doing
> > > so.  This should not live in XPathExecutionContext.  The implemenation
> in
> > > XPathExecutionContext default should simply return 0 when
> someone wants
> to
> > > create one of these.  This is consistent -- there is functionality we
> just
> > > don't support when just using the XPath subsystem.  This does make
> sense,
> > > since XPath has no construction semantics, and so those using
> XPath for
> > > queries are not likely to need result tree fragments.
> >
> > Right, that makes things quite a bit simpler as the builder classes can
> work
> > with the StylesheetExecutionContext which already contains a bunch of
> > allocators that can be used.
> >
> > > I hope this helped!  ;-)  Now, I hope I won't disappear like that
> again...
> >
> > Definitely!  Let me know if i've got it right this time, and I'll go and
> > finish it off.
>
> Sounds good so far.  However, we may need to tweak it once it's built and
> in production.
>
> Yikes, code going into Xalan that wasn't wriiten by me!  That hasn't
> happened for a long time.  Can I cope, being the ultimate control-freak?
> ;-)
>
> > Thanks,
> >
> > Mark
>
> And thank you...
>
> Dave
>
>


RE: returning xml nodes from extensions functions

Posted by Mark Weaver <ma...@npsl.co.uk>.
> Hi Mark,
>
> It's good to hear back from you!
>
Yup, and sorry once again for the (vast) delays.  Hopefully I'll be able
to get this rounded off finally!

> I think you've uncovered a bug in the code.  There's no way an external
> function should get a reference to the XPathExecutionContextDefault
> instance inside of StylesheetExecutionContextDefault.
>
> It's always been a problem using this proxy object -- although it saves
> re-writing lots of code, it creates other problems.  The reason
> StylesheetExecutionContextDefault does not derive from
> XPathExecutionContextDefault is that it must also derive from
> StylesheetExecutionContext, so deriving from both would create
unnecessary
> multiple inheritence along with lots of potential ambiguity.
>
Sure, because StyleSheetExecutionContext also derives from
XPathExecutionContext.  I'm not terribly clear on why the split here; if
there are two different interfaces, then it would seem better to split them
up.  In my mind this is the cleanest model, as then taking this further you compose
interfaces of a small number of related functions (and provide default implementations
of those interfaces), allowing specific parts of the functionality to be easily
(re)implemented.

> I'm not sure what the best long-term solution is, but a short-term fix
is
> to move most of the functionality of
> XPathExecutionContextDefault::extFunction() into a static helper
function,
> and have StylesheetExecutionContextDefault::extFunction() call that.
Let
> me do that, and when I'm finished, I'll check it in a you can take a
look
> at the changes.  You'll have to update your local sources to the latest
> CVS, but you'll have to do that anyway, if we have any hope of merging
in
> your changes.
>
OK, I think I follow what you are saying.  If I get the chance to have a go
at this in the meantime, then I will just replicate the code as a shortcut to
being able to test the other stuff.  I agree that updating to the current CVS will
be necessary/useful as it has been such a long time since I started on this, so
I am of course happy to do this.

Thanks again,

Mark


RE: returning xml nodes from extensions functions

Posted by David N Bertoni/Cambridge/IBM <da...@us.ibm.com>.



Hi Mark,

It's good to hear back from you!

I think you've uncovered a bug in the code.  There's no way an external
function should get a reference to the XPathExecutionContextDefault
instance inside of StylesheetExecutionContextDefault.

It's always been a problem using this proxy object -- although it saves
re-writing lots of code, it creates other problems.  The reason
StylesheetExecutionContextDefault does not derive from
XPathExecutionContextDefault is that it must also derive from
StylesheetExecutionContext, so deriving from both would create unnecessary
multiple inheritence along with lots of potential ambiguity.

I'm not sure what the best long-term solution is, but a short-term fix is
to move most of the functionality of
XPathExecutionContextDefault::extFunction() into a static helper function,
and have StylesheetExecutionContextDefault::extFunction() call that.  Let
me do that, and when I'm finished, I'll check it in a you can take a look
at the changes.  You'll have to update your local sources to the latest
CVS, but you'll have to do that anyway, if we have any hope of merging in
your changes.

Dave



                                                                                                                            
                      "Mark Weaver"                                                                                         
                      <mark@npsl.co.uk         To:      <xa...@xml.apache.org>                                      
                      >                        cc:      (bcc: David N Bertoni/Cambridge/IBM)                                
                                               Subject: RE: returning xml nodes from extensions functions                   
                      01/22/2003 01:35                                                                                      
                      AM                                                                                                    
                                                                                                                            



Hi David.

Well I realise this was back in October but (once again) I managed to run
out of time on it (sadly, the way that the company works is as follows: do
work on core stuff.  Find someone who is willing to pay money.  Immediately
drop everything for money.  Return to core stuff.  Repeat until bored).
Over
Christmas (yay, holiday, good time to write some code :) I managed to get
this mainly completed.  Currently the review of the situation is as
follows:

class XALAN_XPATH_EXPORT XalanDynamicBuilder
{
public:
             /**
              * Get the builder objects used to build the tree.
              */
             virtual ContentHandler*
             getContentHandler() = 0;

             virtual DTDHandler*
             getDTDHandler() = 0;

             virtual LexicalHandler*
             getLexicalHandler() = 0;

             /**
              * Get an XObject for the nodeset or result tree fragment.
The XObject
adopts the tree.
              */
             XObjectPtr
             getXObject();
};

With two derived classes; XalanDynamicNodeSetBuilder and
XalanDynamicResultTreeFragmentBuilder for building either one respectively.
As discussed, we have a GetAndReleaseCachedObject that takes care of these
two objects.  The base definition of the interface is in
XPathExecutionContext, with the implementations there returning 0.
Therefore, sounds largely complete.

However when trying to use these, I have run into a problem.  If you
remember, the original motivation for this framework was to allow easy
return of nodesets and result tree fragments from external functions.  So
the test function looks something like:

             virtual XObjectPtr
             execute(
                                     XPathExecutionContext&
             executionContext,
                                     XalanNode*
                                     context,
                                     const XObjectArgVectorType&
       args,
                                     const Locator*
                               locator) const
             {
                         XalanDynamicNodeSetBuilder *theBuilder = 0;
                         executionContext.getObject(&theBuilder);

                         ContentHandler *contentHandler =
theBuilder->getContentHandler();
                         const AttributesImpl emptyAttributes;
                         contentHandler->startDocument();

contentHandler->startElement(L"",L"test-node",L"",emptyAttributes);
                         contentHandler->endDocument();

                         XObjectPtr retVal = theBuilder->getXObject();
                         executionContext.releaseObject(theBuilder);
             }

(Please ignore the not using GetAndReleasedCachedObject<T> for now).  The
problem here is that 'execute' is not passed a
StyleSheetExecutionContextDefault.  So the getObject call returns 0 (and
then crashes of course since it doesn't check ;).  On deeper examination,
it
appears that we have:


const XObjectPtr
StylesheetExecutionContextDefault::extFunction(
                                     const XalanDOMString&
             theNamespace,
                                     const XalanDOMString&
             functionName,
                                     XalanNode*
                                     context,
                                     const XObjectArgVectorType&
       argVec,
                                     const Locator*
                               locator)
{
             return
m_xpathExecutionContextDefault.extFunction(theNamespace,
functionName, context, argVec, locator);
}

As the cause of the problem.  i.e. StyleSheetExecutionContextDefault simply
uses a member XPathExecutionContextDefault to execute a bunch of its
functionality.  The effect of this, of course, is to hide the real
implementations of getObject from the external function.  I wondered if you
had any ideas on how best to solve this.  To be honest, given the number of
forwarding functions that exist, I'm wondering why
StyleSheetExecutionContextDefault is not simply derived from
XPathExecutionContextDefault rather than XPathExecutionContext.

Thanks,

Mark

> -----Original Message-----
> From: David N Bertoni/Cambridge/IBM [mailto:david_n_bertoni@us.ibm.com]
> Sent: 31 October 2002 18:00
> To: xalan-c-users@xml.apache.org
> Subject: RE: returning xml nodes from extensions functions
>
>
>
>
>
>
>
> Hi Mark,
>
> > > Hi Mark,
> > >
> > > Sorry, I've been incredibly busy the week or so, so I
> apologize for not
> > > responding sooner.
> >
> > No problem.  Hope things are calmer for you now.
>
> Yes, I'm on "vacation," so I'm writing code.  Wait, that's what I do when
> I'm _not_ on vacation...
>
> > >    1. I think two different classes, one which does the node-set, and
> > >    another which does RTFs would be better than one class with a
> > >    constructor parameter.  What do you think?
> >
> > Yes, that sounds like a better idea.
> >
> > >    2. We should use the SAX2 ContentHandler, LexicalHandler, and
> > > DTDHandler
> > >    interfaces, instead of Xalan's proprietary FormatterListener.
> >
> > I will change this over.  Presumably since we are building a
> XalanSourceTree
> > we will simply return a XalanSourceTreeContentHandler.
>
> You'll have to do it using the SAX2 intefaces, because
> XalanSourceTreeContentHandler is not available at that level in the
> architecture.  Also, it's not an abstract base class, so it's not good
for
> use in interfaces.
>
> > >    3. We need an abstract base class that defines the interfaces,
then
> we
> > >    can derive the implementation classes from them.  I'd like to get
> that
> > >    set first, so we know we've got the design right.
> >
> > The interface would be pretty much the same:
> >
> > class XalanDynamicBuilder {
> >            XalanSourceTreeContentHandler *getContentHandler()=0;
> >            XObjectPtr getXObject()=0;
> > };
> >
> > with maybe a 'reset' that allows the builder to be passed back to the
> > execution context.
>
> We should probably wrap it in the same style as the other "auto_ptr-like"
> classes already in the interfaces.  That way, the destruction/reset is
> automatic.  See things like XPathExecutionContext::
> GetAndReleaseCachedString, etc.  You may not like my naming style, so
feel
> free to call it what you like.
>
> > >
> > > Actually, I think the document is owned by the fragment.  This is
> already
> > > done in the implementation of ResultTreeFrag in the XSLT subsystem.
> > >
> > > Ahh, I should have been clearer about this, and I apologize for not
> doing
> > > so.  This should not live in XPathExecutionContext.  The
implemenation
> in
> > > XPathExecutionContext default should simply return 0 when
> someone wants
> to
> > > create one of these.  This is consistent -- there is functionality we
> just
> > > don't support when just using the XPath subsystem.  This does make
> sense,
> > > since XPath has no construction semantics, and so those using
> XPath for
> > > queries are not likely to need result tree fragments.
> >
> > Right, that makes things quite a bit simpler as the builder classes can
> work
> > with the StylesheetExecutionContext which already contains a bunch of
> > allocators that can be used.
> >
> > > I hope this helped!  ;-)  Now, I hope I won't disappear like that
> again...
> >
> > Definitely!  Let me know if i've got it right this time, and I'll go
and
> > finish it off.
>
> Sounds good so far.  However, we may need to tweak it once it's built and
> in production.
>
> Yikes, code going into Xalan that wasn't wriiten by me!  That hasn't
> happened for a long time.  Can I cope, being the ultimate control-freak?
> ;-)
>
> > Thanks,
> >
> > Mark
>
> And thank you...
>
> Dave
>
>