We've implemented several RequestRewriters that aren't in Shindig, but they'd be trivial to refactor into ResponseRewriters as described here.
I'm unaware of anyone who's implemented a custom ImageRewriter (beyond BasicImageRewriter). What more-radical reorganization might you suggest? Any additional ideas beyond those suggested? At one point it seemed like a good idea to consolidate GadgetRewriter and Request||ResponseRewriter somehow, but the differences still seem notable enough as to disqualify that idea. --j On Tue, May 4, 2010 at 12:56 PM, Paul Lindner <[email protected]> wrote: > 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 <[email protected]> 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 > > >
