On Thu, Sep 4, 2008 at 6:02 PM, John Hjelmstad <[EMAIL PROTECTED]> wrote:
> 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.
Why don't you pass the parse tree itself? Why the superfluous wrapper
objects? You're just going to call x.getParseTree anyway, so why not get rid
of that and just pass the object you're dealing with?
interface Rewriter {
void rewrite(HttpResponse source, ParseTree tree);
void rewrite(Gadget gadget, ParseTree tree);
}
and
interface RewriterRegistry {
String rewrite(HttpResponse response);
String rewrite(Gadget gadget);
}
where rewrite can be:
String rewrite(HttpResponse response) {
ParseTree tree = parse(response.getResponseAsString(),
response.getContentType());
for (Rewriter rewriter : rewriters) {
rewriter.rewrite(response, tree);
}
return tree.toString();
}
String rewrite(Gadget gadget) {
ParseTree tree = parse(gadget.getContent(), "text/html");
for (Rewriter rewriter : rewriters) {
rewriter.rewrite(gadget, tree);
}
return tree.toString();
}
>
> 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.
> >>
> >>
> >
>
