I should be clear, by the way, in saying that I understand the value of an immutable HttpResponse, what with all the work that's gone into HttpResponseBuilder making that so. Basically, I want a common intermediary representation of content (as you mention) which has the following properties: 1. It can be shared across multiple rewrites. Today that's not possible due to the ContentRewriter interface returning a new HttpResponse object, which internally would require a re-parse for each rewriter in the HTML case. Obviously, that's really bad for performance. 2. Generation of its parse tree can be determined by the rewriter/caller. Perhaps this requirement is a non-starter or not required - I'm just imagining cases in which mime type text/text (or whatever) is actually HTML.
Thoughts? --John On Thu, Sep 4, 2008 at 5:49 PM, John Hjelmstad <[EMAIL PROTECTED]> wrote: > I don't get it. In the Gadget case, it's always HTML content. That's why > MutableHtmlContent could IMO be a base class of Gadget - an intermediary > representation of a request being processed, a mutable concept description > as I hear it - while being *potentially* a member of HttpResponse. Rewriters > modify HttpResponse right now as it is, so it's effectively mutable even if > the object isn't. I don't really have religion on how that modification is > structured, to be honest with you -- the ContentRewriter interface dealing > with HttpResponse manipulation could be ContentRewriter.rewrite(HttpRequest > request[Context], HttpResponse originalResponse, MutableHtmlContent > mutableContent) for all I care. It just seems to make sense to me for the > HttpResponse object itself to know whether or not its contents *can* be > represented as [Mutable] HTML due to its ownership of its own MIME type > metadata. > Underlying much of this is the bet-hedging I'm doing by maintaining the > ability to modify either String or Parse Tree representation. I expect and > hope that in practice, only the parse tree is ever really used. But > consider: > > <__MSG_nodeName__>content</__MSG_nodeName__> > > This can really only be manipulated as a String. Yes, this has to do with > hangman substitution, which has a host of ugliness and other issues with it, > but at present A) it's our localization solution, and B) hangman > substitution applies to the entire contents of a document, not just Strings > in parsed context, per the spec (which says nothing about it). > > I'm currently passing in Gadget to the Gadget rewriting API for another > reason, notably JsLibraries, which are another form of mutable Gadget > content. You and I have, unless I'm totally misunderstanding what we've > said, had discussions about JsLibraries really just being another type of > node in the tree. That's great for a future update of the Gadget object - > perhaps with the Js Library code listening to relevant nodes in the parse > tree - but isn't available today. Today, the parse tree is only what's > derived from <Content> in the gadget. > > On Thu, Sep 4, 2008 at 5:37 PM, Kevin Brown (JIRA) <[EMAIL PROTECTED]>wrote: > >> >> [ >> https://issues.apache.org/jira/browse/SHINDIG-571?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12628527#action_12628527] >> >> Kevin Brown commented on SHINDIG-571: >> ------------------------------------- >> >> It's not HTML content. It's only HTML content a small percentage of the >> time. >> >> Why can't rewriters just be implemented to take a mutable parse tree and >> the appropriate context (i.e. the original objects, probably mapped to a >> common interface)? >> >> HttpResponse MUST NOT be made mutable -- including the addition of mutable >> fields. This is critical. >> >> We just worked through a whole lot of stuff to make it immutable (again). >> This is incredibly important since HttpResponse is the primary object in >> caches. Once again, this is cramming behavior into HttpResponse that just >> doesn't belong there. >> >> The interface should be changed to just take both the original object and >> the mutable thing being operated on. Cramming it into some other object is >> nothing but a lot of bugs waiting to happen (i.e. Law of Demeter violation). >> >> > Create shared intermediary representation of mutable HTML content for >> Gadget and HttpResponse to use. >> > >> ----------------------------------------------------------------------------------------------------- >> > >> > Key: SHINDIG-571 >> > URL: https://issues.apache.org/jira/browse/SHINDIG-571 >> > Project: Shindig >> > Issue Type: Sub-task >> > Components: Common Components (Java) >> > Reporter: John Hjelmstad >> > >> > While it still appears to make sense to have two Content Rewriter APIs, >> one for HTTP responses (makeRequest et al) and one for Gadget rewriting, it >> remains true that most rewriters will treat each relatively the same: obtain >> contents, manipulate, output. >> > As much code as possible should be shared for each rewriter API. This is >> facilitated by a shared notion of mutable HTML content - the type that's >> actually rewritten. >> > Thus I'd like to create a new class, MutableHtmlContent, which has the >> following methods: >> > MutableHtmlContent.setContent(String) >> > String MutableHtmlContent.getContent() >> > ParsedHtmlNode MutableHtmlContent.getParseTree() >> > ...in other words, the content manipulation methods from Gadget pulled >> out into a context-agnostic object that simply maintains a String and its >> parse-tree equivalent. >> > This code will be shared between Gadget and HttpResponse. At present, >> this arrangement seems convenient: >> > 1. Gadget extends MutableHtmlContent. It could simply contain a >> MutableHtmlContent object as well, but doing so would simply have it passing >> through same-named methods. This can be changed later if conditions dictate. >> > 2. Augment HttpResponse with a new method: >> HttpResponse.getMutableHtmlContent(), which returns a MutableHtmlContent if >> the MIME type is appropriate. A "force" option could be provided here as >> well. >> > Given these changes, rewriters can easily be implemented in terms of >> helper methods that consume and manipulate MutableHtmlContent in the >> majority case. >> >> -- >> This message is automatically generated by JIRA. >> - >> You can reply to this email to add a comment to the issue online. >> >> >
