RE: dom loading in Barracuda - proposed change

> What it all boils down to, is when I create a custom DOMWriter and pass
> it back, is it supposed to leave output open or close it? As it
> currently stands, it looks like different pieces of Barracuda actually
> expect both options.

My inclination on this would be to say - let's keep the basic DOMWriter
functionality as it is: the DOMWriter writes to the stream and then closes
it when done. However, let's add a couple of methods to the interface:

setLeaveWriterOpen(boolean)
isLeaveWriterOpen()

I'm not set-in-stone on this, but it seems like the path of least
resistance: its probably the least likely to require changes by the end
users.

That's my .02...

Christian
----------------------------------------------
Christian Cryder
Internet Architect, ATMReports.com
Project Chair, BarracudaMVC - http://barracudamvc.org
----------------------------------------------
"Coffee? I could quit anytime, just not today"


> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of Shawn Wilson
> Sent: Monday, October 06, 2003 6:54 PM
> To: [email protected]
> Subject: Re: [Barracuda] dom loading in Barracuda - proposed change
>
>
> See my response below:
>
>  > c) Shawn's proposal is to clean up the DOMWriter class so that
> it neither
>  > opens nor closes the writer. I'm ok with this in principle,
> but I'm also
>  > sympathetic which Jakes objection that it does make it more
> cumbersome to
>  > use - in the majority of the cases, I think its most intuitive to
> just use
>  > the DOM writer the way we are: write to it and forget it.
>
> Actually, I'm okay with the methods the way they are. That is, you pass
> the DOMWriter an HttpServletResponse and it gets the writer from there,
> outputs to it, etc. What I'm proposing is that we either standardize
> that *all* DOMWriters either [a] always leave the output open or [b]
> always close it, -OR- [c] we add something like isLeaveWriterOpen() to
> the interface DOMWriter.
>
> There is no problem when DefaultDOMWriter is used as the underlying
> DOMWriter. The problem is when a ViewHandler passes back a custom
> DOMWriter implementation; there are no guidelines in the DOMWriter
> interface as to what is expected to happen.
>
> That being said, since we obviously need the capability for the output
> to be left open (as Christian pointed out with BlockIterateHandler),
> then that eliminates the possibility of option [b]. If we go option [c]
> then that doesn't really alleviate any work because everywhere a
> DOMWriter is used in Barracuda we would need to query whether or not
> output is being left open and then close it if necessary. This leaves me
> to believe that [a] is perhaps the best approach.
>
> What it all boils down to, is when I create a custom DOMWriter and pass
> it back, is it supposed to leave output open or close it? As it
> currently stands, it looks like different pieces of Barracuda actually
> expect both options.
>
> -shawn
>
> Christian Cryder wrote:
>
> > Ok, I'm _finally_ getting around to trying to catch up on my
> email. Sorry
> > for the delay. My comments follow...
> >
> > So Jake & Shawn, if I understand things correctly, there are basically 2
> > issues being discussed:
> >
> > 1. simplify the DOMLoading interface like this: (Jake's request)
> >
> >
> >>The following methods are now all that exist in DOMLoader....
> >>     public Document getDOM(String doc) throws IOException;
> >>     public Document getDOM(String doc, Locale locale) throws
> IOException;
> >>     public Document getDOM(Class clazz) throws IOException;
> >>     public Document getDOM(Class clazz, Locale locale) throws
> >>IOException;
> >
> > ...
> >
> >>Notice that the getDOM(String) methods now expect a file/URL path
> >
> > ...
> >
> >>Also notice that there are no methods taking ViewCapabilities.
> >
> >
> > I can't really think of any objections to doing this, because
> as you say -
> > its only going to implement those who have written their own DOMLoader
> > implementations, and that's probably not  very many people. My
> question here
> > is "are there any other changes beyond those I've just listed above" (I
> > don't think so, but I want to be sure).
> >
> >
> > 2. modify the DOMWriter class/implementations like this:
> (Shawn's request)
> >
> >
> >>My proposal is that we remove the "leaveWriterOpen" option from
> >>DefaultDOMWriter and provide eplanation in the comments of DOMWriter
> >>that implementing class MUST NOT close the output. It should be stated
> >>clearly that it is the responsibility of the class(es) using the
> >>DOMWriter to manually close any output streams.
> >
> >
> > Ok, regarding this change, here's some history...
> >
> > a) originally - the dom writer opened the stream, did the
> writing, and then
> > closed the stream (well, it didn't actually open it, but it did
> create the
> > Writer that wraps the output stream, set the headers,
> etc...since it opens
> > the Writer, I think it makes sense to close the writer as well)
> >
> > b) I ran into a situation where I needed to have the stream left open
> > because I wanted to be able to process very large DOMs in
> blocks, using the
> > BlockIterator. So the easiest way to handle this was to simply
> pass a flag
> > in saying "hey, leave that stream open! (because I'm going to
> call you again
> > in just a sec"
> >
> > c) Shawn's proposal is to clean up the DOMWriter class so that
> it neither
> > opens nor closes the writer. I'm ok with this in principle, but I'm also
> > sympathetic which Jakes objection that it does make it more
> cumbersome to
> > use - in the majority of the cases, I think its most intuitive
> to just use
> > the DOM writer the way we are: write to it and forget it.
> >
> > So I guess my big question here is this: what exactly does this
> change give
> > us over and above the current implementation?
> >
> > Whatever we decide, the bottom line is that we need to be able
> to write to
> > the DOM writer in 2 different ways - in the first (and by far the most
> > common), we simply write and forget about it; in the second (rare, but
> > definitely important), we are going to repeatedly use the
> writer to write
> > multiple doms to the same response stream.
> >
> > Ultimately, as long as we keep the functionality, I'm ok with Shawn's
> > approach, but I think I'd like to see more tangible value
> before doing it
> > (ie. we shouldn't do it just for the sake of doing it).
> >
> > So, to summarize...
> > #1 - +1
> > #2 - unsure until I see more data
> >
> > That help?
> >
> > Christian
> > ----------------------------------------------
> > Christian Cryder
> > Internet Architect, ATMReports.com
> > Project Chair, BarracudaMVC - http://barracudamvc.org
> > ----------------------------------------------
> > "Coffee? I could quit anytime, just not today"
> >
> >
> >
> >>-----Original Message-----
> >>From: [email protected]
> >>[mailto:[email protected]]On Behalf Of Jacob Kjome
> >>Sent: Wednesday, October 01, 2003 2:41 PM
> >>To: BarracudaMVC Users List
> >>Subject: [Barracuda] dom loading in Barracuda - proposed change
> >>
> >>
> >>
> >>Hi everyone,
> >>
> >>Since we now have a simple viable alternative to XMLC DOM Loading via
> >>Jivan, I thought I'd try it out.  I created a new JivanDOMFactory which
> >>seems to work nicely.  However, as I was looking at the DOMLoader and
> >>DefaultDOMLoader, I noticed some unnecessary complexity (partially my
> >>fault).  I'm wondering if anyone would object to a slight change to the
> >>interface.  I was thinking of getting rid of any method taking
> >>ViewCapabilities as a parameter and any non-getDOM() methods.
> This would
> >>simplify the interface significantly and keep more
> >>implementation-specific
> >>methods out of the Interface.  This would make other DOMLoader
> >>implementations much easier to create and maintain.
> >>
> >>What I'd like to do is point out the changes to the DOMLoader
> >>interface and
> >>where these changes affect existing code.  After evaluating that,
> >>I'd like
> >>to hear your point of view whether this change would cause any pain for
> >>existing applications.  So, here are the proposed changes...
> >>
> >>The following methods are now all that exist in DOMLoader....
> >>     public Document getDOM(String doc) throws IOException;
> >>     public Document getDOM(String doc, Locale locale) throws
> IOException;
> >>     public Document getDOM(Class clazz) throws IOException;
> >>     public Document getDOM(Class clazz, Locale locale) throws
> >>IOException;
> >>
> >>Notice that the getDOM(String) methods now expect a file/URL path to a
> >>document rather than being convenience methods to send in a fully
> >>qualified
> >>class name as they were before (and were only recently added by
> >>me).  This
> >>has replaced getDOMFromFile(String) that had been added to
> >>support dynamic
> >>loading of documents in Barracuda-1.2.5.  I don't think this
> should cause
> >>undue pain since it is more likely that people use the getDOM(Class)
> >>methods anyway.
> >>
> >>Also notice that there are no methods taking ViewCapabilities.
> >>The reason
> >>for this is that we might as well just pass in the Locale since
> >>that is all
> >>we use the ViewCapabilities for anyway...at least in dom loading; dom
> >>writing may be another issue.  The way this affects existing code in
> >>Barracuda is to change this:
> >>Document page =
> >>DefaultDOMLoader.getGlobalInstance().getDOM(clazz,
> >>vc.getViewCapabilities());
> >>to this:
> >>Document page =
> >>DefaultDOMLoader.getGlobalInstance().getDOM(clazz,
> >>vc.getViewCapabilities().getClientLocale());
> >>
> >>Not too tough, if you ask me.
> >>
> >>
> >>The other methods that have gone away from the DOMLoader
> interface are...
> >>
> >>     public void setDefaultDOMFactory(DOMFactory df);
> >>     public void registerDOMFactory(DOMFactory df, Class clazz);
> >>     public void deregisterDOMFactory(Class clazz);
> >>
> >>These seem to me to be more implementation-specific.  I've left modified
> >>versions of them in DefaultDOMLoader, but I think the DOMLoader can live
> >>with the getDOM() methods alone.  I changed these methods in
> >>DefaultDOMLoader to take a String instead of a Class to key particular
> >>DOMFactory's.  This makes it easier to support the keys as being either
> >>class names or document file paths which allows for registering
> >>individual
> >>files to be loaded by, say..., the JivanDOMFactory and individual class
> >>names by one of the XMLC dom factories.  Before, this was only
> >>possible for
> >>XMLC-based classes.
> >>
> >>
> >>Now, besides the previously described necessary change to
> >>existing code in
> >>order to be compatible with the new DOMLoader interface, I had
> to comment
> >>out all the dom loader stuff in DefaultApplicationAssembler.  I'm
> >>not sure
> >>how many people out there currently use the <dom-loader> and/or
> >><dom-loader-register> stuff in your application assembler files.
> >>I suspect
> >>not too many.  Of course, this functionality has already been
> >>superceded by
> >>the object repository assembler anyway.  So, the functionality is still
> >>there (in an even more flexible form), it just would no longer be
> >>supported
> >>in the DefaultApplicationAssembler.
> >>
> >>
> >>Basically, if people have been using the basics for dom loading, I don't
> >>see this change as affecting people much at all.   I hope that is the
> >>case.  Please let me know your opinion on what I've described above and
> >>tell me whether you think this change is acceptable or not.
> >>
> >>Jake
> >>
> >>_______________________________________________
> >>Barracuda mailing list
> >>[email protected]
> >>http://barracudamvc.org/lists/listinfo/barracuda
> >
> >
> > _______________________________________________
> > Barracuda mailing list
> > [email protected]
> > http://barracudamvc.org/lists/listinfo/barracuda
>
> _______________________________________________
> Barracuda mailing list
> [email protected]
> http://barracudamvc.org/lists/listinfo/barracuda