Re: dom loading in Barracuda - proposed change

At 09:16 AM 10/7/2003 -0600, you wrote:
>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

If you want to add the blurb and the methods Christian mentioned to 
DOMWriter, I'm ok with it.

+1

Jake

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