On Fri, Aug 27, 2010 at 11:45, Felix Meschberger <[email protected]> wrote: > Even though I said (and still think) that the SlingPostOperation > interface is defined to be able to inject new functionality into the > SlingPostServlet and probably should only be used by the > SlingPostServlet it might be possible to call SlingPostOperation > services from outside the SlingPostServlet.
Once you define an API, you always have to consider both sides of the story, so I think it is fair to assume that the interface could be used by someone else. It is at least a clear API. > The easiest thing would be to just register all predefined operations as > services, which would also be consumed by the SlingPostServlet itself. > Since the SlingPostServlet can register these services itself, there is > no issue for setting these (particularly for the ModifyOperation or the > ImportOperation). You mean the problematic constructors? Right, that's also a solution. > But, if we are at it: How about fixing another issue itching my back > every now and then: The HtmlResponse class is an extremely strange biest > -- and probably not correctly located in the Sling API bundle (from > today's perspective, at least). > > So here is my full proposal: > > 1. Create new response helper classes (replacing HtmlResponse): > AbstractHttpResponse -- help gather changes, status, etc. > and has abstract send method. > HtmlHttpResponse -- implements send method generating HTML > JsonResponse -- implements send method generating JSON I'd call it PostResponse instead of AbstractHttpResponse, and be it an interface (passing abstract* classes in APIs doesn't look nice, I think). > 2. Create a new PostOperation interface with a slightly different > API than today's SlingPostOperation: > > // the SlingPostProcess[] may be null > void run(SlingHttpServletRequest, AbstractHttpResponse, > SlingPostProcess[]) Why not simply add this new method to the existing SlingPostOperation class, and deprecate the old run() method? Then it is just one deprecated method instead of a deprecated class. > 3. Create an AbstractPostOperation analogous to the > AbstractSlingPostOperation > > 4. Refactor predefined operations of the Sling POST Servlet bundle > to the new interface and register them as services > > 5. For backwards compatibility, provide a service proxy for existing > SlingPostOperation services. The SlingPostOperation interface and > AbstractSlingPostOperation abstract class are deprecated. > > WDYT ? If we say that making the currently internal classes public is no option, +1 for that. For the "interesting" stuff, like NodeNameGenerator or MediaRangeList and co, I'd suggest to move them to public locations in commons-style-libraries, such as jackrabbit-jcr-commons or (I-don't-know-yet), to make them publicly available. Regards, Alex -- Alexander Klimetschek [email protected]
