Re: dom loading in Barracuda - proposed change

I'm okay with this. We should also add a blurb to the DOMWriter javadocs 
that the default should be to close the stream.

-shawn

Christian Cryder wrote:

>>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
> 
> 
> _______________________________________________
> Barracuda mailing list
> [email protected]
> http://barracudamvc.org/lists/listinfo/barracuda