That seems reasonable to me: one place for all "new" metadata-driven APIs.
This is consistent w/ refactoring I'd like to do btw ProxyHandler and ProxyServlet (making Handler an abstract logic handler rather than consuming and processing HttpServletX objects), so I'll do that in a patch I'll add to this review. A brief implementation note, sidebar: once the APIs support streaming HTTP fetch, the metadata/data URI proxy handler will be forced into buffered-read mode. Distinctly not a really big concern (data URI isn't appropriate for large objects anyway); just an awareness item. Stay tuned for an updated CL. I'll respond re: Gagan's comments as well. --j On Mon, Jul 26, 2010 at 1:34 PM, Ziv Horesh <[email protected]> wrote: > Code looks nice but I would argue that this data should be served by the > new JSON interface - GadgetHandler and not ProxyServlet. > I think it will be clearer to the user to access one end point for JSON > interface, and another for data/html. > I plan on extending the GadgetHandler to provide Proxy url, so I think it > would be simpler (from API point of view) to extend it to what you are > doing. > > -Ziv > > > On Fri, Jul 23, 2010 at 5:59 PM, <[email protected]> wrote: > >> Reviewers: dev-remailer_shindig.apache.org, >> >> Description: >> Adds the ability to request, via &output=js, proxied content in dataUri >> form. >> >> In the presence of this parameter, a JSON object is emitted containing a >> 'dataUri' field w/ the URI contents, as well as any metadata that is >> associated w/ the HttpResponse (allowing augmentation in various cases >> such as image proxying, where height/width/etc. may be set). >> >> This isn't particularly configurable as yet (ie. max size of response), >> but seems to work just fine. A test page (added in this CL) demonstrates >> this. We may also consider adding JSONP and CORS support. >> >> Please review this at http://codereview.appspot.com/1696056/show >> >> Affected files: >> content/container/datauri_proxy.html >> >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java >> >> >> java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java >> >> >> >
