How many people have implemented custom rewriters? I'm guessing it's a small number, in which case I'd suggest a more radical reorganization of the code.
In any case these are all good ideas. On Fri, Apr 30, 2010 at 5:13 PM, John Hjelmstad <fa...@google.com> wrote: > Hi all, > > I'm looking into the rewriting pipeline, specifically for HTTP responses, > with a few goals in mind: > 1. Consolidate ImageRewriter with RequestRewriter, so it's not just a > one-off rewriter injection. > - And to simplify the rewriter interfaces overall, to allow for uniform > rewriter profiling code etc. > 2. Expand RequestRewriters' capability to modify HTTP response headers. > 3. Maintain - and expand - our ability to cache the result of rewriting > passes intelligently. Especially important for ImageRewriter, the output of > which is presently cached for notable performance gains. > > To this end, I've created the following CL as a starting point. It builds > upon the CL I recently submitted adding the ability to manipulate > MutableContent by byte-array: http://codereview.appspot.com/1032042/show > > Downstream of this CL, I'm proposing to change the rewriter interface > roughly as follows: > > OLD > // Return true if content modified. > boolean RequestRewriter.rewrite(HttpRequest, HttpResponse, MutableContent); > > NEW > // Return value represents caching behavior. > // Naive strategy: return boolean. True = HttpRequest is a sufficient key > to > cache the rewrite. Works for simple cases, but compound rewriter cases may > be more difficult. > // More sophisticated implementation: return a RewriteCacheKey object, > containing a normalized set of all inputs to the rewrite behavior > (essentially a compound key). > // In turn, these objects would be used by a RewriterCache manager > responsible for managing these states. > ??? ResponseRewriter.rewrite(HttpRequest, HttpResponseBuilder); > > Note that I suggest a new interface: ResponseRewriter (a more accurate name > for RequestRewriter anyway), so we can support both Request and > ResponseRewriter implementations during transition. > > Alternate approaches: > I also considered simply splitting out the HTTP header-manipulation methods > from HttpResponseBuilder into an interface: HttpResponseHeaders. > RequestRewriter's API would be modified simply as: > boolean RequestRewriter.rewrite(HttpRequest, HttpResponseHeaders, > MutableContent); > > While simpler, this had the downsides: > * Still involves a signature change. > * Doesn't account for better caching behavior. > * Involves 2 objects rather than 1 to manipulate for output. > > > At present, I'd like to go the conservative route: > 1. HttpResponseBuilder as MutableContent (@see CL) > 2. Introduce ResponseRewriter interface, returning RewriteCacheKey with > simple flags: HttpRequest-is-sufficient and rewrite-is-idempotent. > 3. Transition ImageRewriter to ResponseRewriter. > 4. Transition RequestRewriters to ResponseRewriter interface. > > > Thoughts? Input welcome. > > Cheers, > John >
