Hi, On 27.08.2010 12:47, Alexander Klimetschek wrote: > 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.
Well, the API is defined to be implemented by extensions to the SlingPostServlet, which is declared to be the consumer of the API. So there is -- currently -- no second side of the story ;-) > >> 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). Makes sense to me. > >> 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. The problem is with existing implementations: If we extend the interface we break existing implementations of the interface. We could do though: * Add a new interface SlingPostOperation2 extends SlingPostOperation * AbstractSlingPostOperation implements SlingPostOperation2 Thus extensions of the AbstractSlingPostOperation are SlingPostOperation2 for free. Plain implementations of SlingPostOperation still have to be supported with a proxy. > >> 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. jackrabbit-jcr-commons is an uncontrollable biest .. ;-) So I'd rather not put stuff there, esp. not Sling specific or Web App specific stuff. Lets for the moment keep it here and see where it goes. Regards Felix > > Regards, > Alex >
