You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Vidar S. Ramdal (JIRA)" <ji...@apache.org> on 2010/01/28 15:11:34 UTC

[jira] Created: (SLING-1336) Let SlingPostServlet return JSON

Let SlingPostServlet return JSON
--------------------------------

                 Key: SLING-1336
                 URL: https://issues.apache.org/jira/browse/SLING-1336
             Project: Sling
          Issue Type: New Feature
          Components: Servlets
    Affects Versions: Servlets Post 2.0.4
            Reporter: Vidar S. Ramdal
             Fix For: Servlets Post 2.0.6


>From http://markmail.org/thread/ic2752rha4iy2s2c

In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.

In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842582#action_12842582 ] 

Felix Meschberger commented on SLING-1336:
------------------------------------------

> Now, sling.servlets.post does not have a dependency to Engine. Do we want to introduce that? Or move RequestUtil to API? 

Right. Yes, moving RequestUtil to API might make sense, on the other hand, we should probably not add more real classes to the API module. Rather I was thinking of (yet another ?) commons module.

> This could probably be easily rewritten to use HtlmResponse's hashmap, and populating the JSONObject on send(). 

I would prefer that even though it might seem a bit more expensive. The main reason for me would be simplicity: You just have to overwrite a single method - the rendering method - and otherwise have the exact same implementation. For output JsonWriter could then be used instead of JSONObject.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12843457#action_12843457 ] 

Felix Meschberger commented on SLING-1336:
------------------------------------------

> I suggest we create a separate task for this (creating a new RequestUtil container bundle).

Agreed. Makes sense to me.

> I'm inclined to leaving this as it is for the moment, so that this issue can be resolved with
> changes to no other bundles than sling.servlet.post (new HtmlResponse.getProperties() method)

Agreed, too.



> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336-2.patch, SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842577#action_12842577 ] 

Vidar S. Ramdal commented on SLING-1336:
----------------------------------------

Thanks for the feedback:

> The IOException constructor on line 151 in JSONResponse has only been introduced in Java 6

Will fix.

>The MediaRangeList seems to duplicate work done in the Engine's RequestUtil.parseAcceptHeader method
     --> We might want to consolidate a number of request header parsing methods in some generic RequestUtil class ?

Ah, didn't realize there was such a method. Now, sling.servlets.post does not have a dependency to Engine. Do we want to introduce that? Or move RequestUtil to API?

> The JSONResponse class duplicates property storage in the JSONObject.
     --> Is there a reason for this ? 

The setProperty and getProperty methods are overridden from HtmlResponse. The JSONResponse methods store properties directly on the JSON object, without using the hashmap in HtmlResponse. Thus, there shouldn't any duplicate storage (but an unused HashMap in HtmlResponse).
This could probably be easily rewritten to use HtlmResponse's hashmap, and populating the JSONObject on send().

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Felix Meschberger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842573#action_12842573 ] 

Felix Meschberger commented on SLING-1336:
------------------------------------------

Thanks for preparing the patch. To me it basically looks good with the following remarks:

  * The IOException constructor on line 151 in JSONResponse has only been introduced in Java 6
     --> Easy fix is to to throw (IOException) new IOException(message).initCause(cause);

  * The MediaRangeList seems to duplicate work done in the Engine's RequestUtil.parseAcceptHeader method
     --> We might want to consolidate a number of request header parsing methods in some generic RequestUtil class ?

  * The JSONResponse class duplicates property storage in the JSONObject.
     --> Is there a reason for this ?

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vidar S. Ramdal updated SLING-1336:
-----------------------------------

    Attachment: SLING-1336.patch

Proposed implementation - please have a look.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: Creating a Sling Utilities Bundle (was: [jira] Updated: (SLING-1336) Let SlingPostServlet return JSON)

Posted by Vidar Ramdal <vi...@idium.no>.
On Wed, Mar 10, 2010 at 7:55 AM, Felix Meschberger <fm...@gmail.com> wrote:
> Hi all,
>
> We currently have a multi-faceted issue: We have the Sling API bundle
> containing a number of utility classes (HtmlResponse, ResourceUtil). In
> addition there are two utility classes in the Engine bundle
> (RequestUtil, ResourceUtil) which are of general interest.
>
> The problem with the locations of these classes are (1) mixing
> implementations into an API bundle is problematic, (2) extending
> implementations means increasing the API version, (3) using the Engine
> bundle utilities creates a dependency on the Engine bundle.
>
> I think we should create a new Utilities bundle which will host these
> utility classes.
>
> The classes are simply copied and then further developed in the new
> utilities bundle while the old classes are simply deprecated and removed
> in future versions. This will keep the classes distinct and not create a
> dependency to the new util bundle in the API bundle.
>
> WDYT ?

In general, when a class/package/bundle is named something *Util, I
usually suspect that it is a catch-all for stuff that doesn't
naturally belong anywhere else. So perhaps we should find a better
name, and possibly divide it into multiple units.
In the case of RequestUtil, ideally, those methods would be moved to
SlingHttpServletRequest, but I guess there are reasons why they're not
already there.

That said, I'm all for moving util stuff out of Engine.

I think moving HtmlResponse will be hard, since it is used as
parameter for SlingPostOperations, so there will be huge backward
compatibility trouble. If we are to move it anyway, I'd really like to
rewrite it to an interface "PostResponse", which will be implemented
by a (new) HtmlResponse class and the newly-introduced JsonResponse.

-- 
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070

Re: Creating a Sling Utilities Bundle

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Wed, Mar 24, 2010 at 3:02 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...Ok, so the RequestLog is engine specific and should stay in the engine
> bundle. The question is if we should deprecate it and make it an
> internal interface or leave it like it is....

I'm for deprecating and making internal - we have the
RequestProgressTracker to log what happens with requests, that's
probably good enough for now.

-Bertrand

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Am 24.03.10 14:54, schrieb Felix Meschberger:

>> And maybe we should move the error handler into the API as well?
> 
> Uh, this smells like pandorrah's box ... lets reconsider this for a
> moment ;-)

:) Error handler is more an interface of the engine, so maybe we should
leave it like it is - usually people will not implement such a service.

>>> * RequestLog - this is probably Engine specific and
>>>       should stay here. But: do we need it really ??
>> I think this more an internal interface, so maybe we should just
>> deprecate it.
> 
> The intent of this interface was not really that it would be internal:
> bundles could implement this and register as a service and some
> configuration could instruct request-level log messages to be directed
> there ...
> 
> But probably, this is a bad idea  ... Maybe this has never been used ...
> Maybe it does not even work correctly ...
:)

Ok, so the RequestLog is engine specific and should stay in the engine
bundle. The question is if we should deprecate it and make it an
internal interface or leave it like it is.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 24.03.2010 14:03, Carsten Ziegeler wrote:
> Felix Meschberger  wrote
>>> a) copy the request and response util classes from engine to api so
>>> people have everything in a single place
>>> b) create the utilities bundle as Felix suggested
>>> c) leave everything as it is
>>>
>>> I'm personally in favour of a) :)
>>
>> I came to dislike mixing API (interfaces, exceptions) with
>> implementations (classes, utilities). Thus my preference would be b. But
>> (a) would be ok for me, too.
> Puuuh, great :) I think utility classes are more API than
> implementation, but maybe they are more an indicator that the API is
> incomplete :)
> Anyway, it would be great if we can continue with a)
> 
>>
>> We should also discuss about the rest of the o.a.s.engine contents:
>>
>> * EngineConstants
>>       sling.home/sling.home.url - maybe move to SlingConstants
>>           and have them provided by the SlingSettingsService ?
> Yes,
> 
>>       SLING_CURRENT_SERVLET_NAME - maybe make this an official
>>           request attribute while handling a sling request (and
>>           move the constant to SlingConstants) ?
>>           or add this to the SlingHttpServletRequest ?
> Let's move the constant
> 
>>       SLING_SERLVET_NAME - This is Sling Engine specific for
>>           initializing servlets with configuraiton.
>>           Shouldn't we move initialization of Servlets to the
>>           Servlet Resolver which also registers them with the
>>           ResourceResolver ?
> Yes, sounds good to me, however this is used in the
> engine.servlets package, class AbstractServiceReferenceConfig.
> Here we also have the ErrorHandler interface.
> I think the AbstractServiceReferenceConfig is more an internal
> class, so maybe we should just deprecate this as well.
> And maybe we should move the error handler into the API as well?

Uh, this smells like pandorrah's box ... lets reconsider this for a
moment ;-)

> 
>>
>> * RequestLog - this is probably Engine specific and
>>       should stay here. But: do we need it really ??
> I think this more an internal interface, so maybe we should just
> deprecate it.

The intent of this interface was not really that it would be internal:
bundles could implement this and register as a service and some
configuration could instruct request-level log messages to be directed
there ...

But probably, this is a bad idea  ... Maybe this has never been used ...
Maybe it does not even work correctly ...

Regards
Felix

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Felix Meschberger  wrote
>> a) copy the request and response util classes from engine to api so
>> people have everything in a single place
>> b) create the utilities bundle as Felix suggested
>> c) leave everything as it is
>>
>> I'm personally in favour of a) :)
> 
> I came to dislike mixing API (interfaces, exceptions) with
> implementations (classes, utilities). Thus my preference would be b. But
> (a) would be ok for me, too.
Puuuh, great :) I think utility classes are more API than
implementation, but maybe they are more an indicator that the API is
incomplete :)
Anyway, it would be great if we can continue with a)

> 
> We should also discuss about the rest of the o.a.s.engine contents:
> 
> * EngineConstants
>       sling.home/sling.home.url - maybe move to SlingConstants
>           and have them provided by the SlingSettingsService ?
Yes,

>       SLING_CURRENT_SERVLET_NAME - maybe make this an official
>           request attribute while handling a sling request (and
>           move the constant to SlingConstants) ?
>           or add this to the SlingHttpServletRequest ?
Let's move the constant

>       SLING_SERLVET_NAME - This is Sling Engine specific for
>           initializing servlets with configuraiton.
>           Shouldn't we move initialization of Servlets to the
>           Servlet Resolver which also registers them with the
>           ResourceResolver ?
Yes, sounds good to me, however this is used in the
engine.servlets package, class AbstractServiceReferenceConfig.
Here we also have the ErrorHandler interface.
I think the AbstractServiceReferenceConfig is more an internal
class, so maybe we should just deprecate this as well.
And maybe we should move the error handler into the API as well?

> 
> * RequestLog - this is probably Engine specific and
>       should stay here. But: do we need it really ??
I think this more an internal interface, so maybe we should just
deprecate it.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 24.03.2010 12:04, Carsten Ziegeler wrote:
> I moved the service, but how do we proceed?
> 
> We can:
> a) copy the request and response util classes from engine to api so
> people have everything in a single place
> b) create the utilities bundle as Felix suggested
> c) leave everything as it is
> 
> I'm personally in favour of a) :)

I came to dislike mixing API (interfaces, exceptions) with
implementations (classes, utilities). Thus my preference would be b. But
(a) would be ok for me, too.

We should also discuss about the rest of the o.a.s.engine contents:

* EngineConstants
      sling.home/sling.home.url - maybe move to SlingConstants
          and have them provided by the SlingSettingsService ?
      SLING_CURRENT_SERVLET_NAME - maybe make this an official
          request attribute while handling a sling request (and
          move the constant to SlingConstants) ?
          or add this to the SlingHttpServletRequest ?
      SLING_SERLVET_NAME - This is Sling Engine specific for
          initializing servlets with configuraiton.
          Shouldn't we move initialization of Servlets to the
          Servlet Resolver which also registers them with the
          ResourceResolver ?

* RequestLog - this is probably Engine specific and
      should stay here. But: do we need it really ??

Regards
Felix

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Felix Meschberger  wrote
> Hi,
> 
> On 24.03.2010 12:04, Carsten Ziegeler wrote:
>> I moved the service, but how do we proceed?
> 
> How about adding two methods to the SlingSettingsService ?
> 
>    * String getSlingHome();
>    * URL getSlingHome();
>      (I think Java 5 allows this)
> 
> or
> 
>    * Type getSlingHome(Class<Type> type);
> 
> where type is one of String, File, URL (this is probably too fancy ;-) )
> 
Yes, I think that's really too fancy :)

I would go for two methods either with the same name or different name.
I'll create an issue for this and implement it.

Carsten

> The goal is to reflect the sling.home framework property
> 
> Regards
> Felix
> 
>>
>> We can:
>> a) copy the request and response util classes from engine to api so
>> people have everything in a single place
>> b) create the utilities bundle as Felix suggested
>> c) leave everything as it is
>>
>> I'm personally in favour of a) :)
>>
>> Carsten
>>
>>
> 


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 24.03.2010 12:04, Carsten Ziegeler wrote:
> I moved the service, but how do we proceed?

How about adding two methods to the SlingSettingsService ?

   * String getSlingHome();
   * URL getSlingHome();
     (I think Java 5 allows this)

or

   * Type getSlingHome(Class<Type> type);

where type is one of String, File, URL (this is probably too fancy ;-) )

The goal is to reflect the sling.home framework property

Regards
Felix

> 
> We can:
> a) copy the request and response util classes from engine to api so
> people have everything in a single place
> b) create the utilities bundle as Felix suggested
> c) leave everything as it is
> 
> I'm personally in favour of a) :)
> 
> Carsten
> 
> 

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
I moved the service, but how do we proceed?

We can:
a) copy the request and response util classes from engine to api so
people have everything in a single place
b) create the utilities bundle as Felix suggested
c) leave everything as it is

I'm personally in favour of a) :)

Carsten


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Bertrand Delacretaz  wrote
> On Tue, Mar 23, 2010 at 8:41 AM, Carsten Ziegeler <cz...@apache.org> wrote:
>> ...As a first step I would like to move the SlingSettingsService interface
>> to the api bundle.
>> My proposal is to:
>> - Add the interface to the API in the o.a.s.api.services package
>> - Deprecate the interface in the enigne bundle and let it extend
>>  from the interface in the API
>> - Let the service impl in the engine bundle register itself for
>>  both interfaces.
>>
>> WDYT?
> 
> +1
> 
Ok, thanks, so I'll go ahead with the change.

Regards
Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Tue, Mar 23, 2010 at 8:41 AM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...As a first step I would like to move the SlingSettingsService interface
> to the api bundle.
> My proposal is to:
> - Add the interface to the API in the o.a.s.api.services package
> - Deprecate the interface in the enigne bundle and let it extend
>  from the interface in the API
> - Let the service impl in the engine bundle register itself for
>  both interfaces.
>
> WDYT?

+1

-Bertrand

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Carsten Ziegeler  wrote
> Ok, this conversion got stuck a little bit, but so far I think we more
> or less agree that having api/utility classes in the engine bundle is bad.
> 
> So what about taking small steps and solve this first?
> 
As a first step I would like to move the SlingSettingsService interface
to the api bundle.
My proposal is to:
- Add the interface to the API in the o.a.s.api.services package
- Deprecate the interface in the enigne bundle and let it extend
  from the interface in the API
- Let the service impl in the engine bundle register itself for
  both interfaces.

WDYT?

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Ok, this conversion got stuck a little bit, but so far I think we more
or less agree that having api/utility classes in the engine bundle is bad.

So what about taking small steps and solve this first?

Carsten


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Creating a Sling Utilities Bundle

Posted by Carsten Ziegeler <cz...@apache.org>.
Felix Meschberger  wrote
> Hi all,
> 
> We currently have a multi-faceted issue: We have the Sling API bundle
> containing a number of utility classes (HtmlResponse, ResourceUtil). In
> addition there are two utility classes in the Engine bundle
> (RequestUtil, ResourceUtil) which are of general interest.
> 
> The problem with the locations of these classes are (1) mixing
> implementations into an API bundle is problematic, (2) extending
> implementations means increasing the API version, (3) using the Engine
> bundle utilities creates a dependency on the Engine bundle.
> 
> I think we should create a new Utilities bundle which will host these
> utility classes.
> 
> The classes are simply copied and then further developed in the new
> utilities bundle while the old classes are simply deprecated and removed
> in future versions. This will keep the classes distinct and not create a
> dependency to the new util bundle in the API bundle.
> 
> WDYT ?

I don't have a definitiv answer.
Now I think that having a dependency on the engine bundle is bad and
maybe we could move all the public stuff from the engine bundle to
somewhere else - either the api bundle or a possible new utility bundle.

I'm not worried about too many bundles, but I'm a little bit worried
about too many dependencies or modules a developer has to know when
developing a sling application. having the utility code in the api
bundle has the advantage that everything a developer has to know and
needs is in a single place.
And in addition things like the ResourceUtil class (in the api) are
very useful and you can make good use of it all over the place. But on
the other hand we have a bug to add most of these methods to the
resource interface :)

So I'm definitly +1 on moving things out of the engine bundle.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Creating a Sling Utilities Bundle (was: [jira] Updated: (SLING-1336) Let SlingPostServlet return JSON)

Posted by Felix Meschberger <fm...@gmail.com>.
Hi all,

We currently have a multi-faceted issue: We have the Sling API bundle
containing a number of utility classes (HtmlResponse, ResourceUtil). In
addition there are two utility classes in the Engine bundle
(RequestUtil, ResourceUtil) which are of general interest.

The problem with the locations of these classes are (1) mixing
implementations into an API bundle is problematic, (2) extending
implementations means increasing the API version, (3) using the Engine
bundle utilities creates a dependency on the Engine bundle.

I think we should create a new Utilities bundle which will host these
utility classes.

The classes are simply copied and then further developed in the new
utilities bundle while the old classes are simply deprecated and removed
in future versions. This will keep the classes distinct and not create a
dependency to the new util bundle in the API bundle.

WDYT ?

Regards
Felix

[jira] Updated: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vidar S. Ramdal updated SLING-1336:
-----------------------------------

    Attachment: SLING-1336-2.patch

The issue with the IOException constructor is fixed in this revised patch. 
At first, implementing Felix's other suggestions seemed easy, but proved more complicated:

>> Now, sling.servlets.post does not have a dependency to Engine. Do we want to introduce that? Or move RequestUtil to API?

> Right. Yes, moving RequestUtil to API might make sense, on the other hand, we should probably not add more real classes to the API module. Rather I was thinking of (yet another ?) commons module.

I suggest we create a separate task for this. Once we have RequestUtil in a separate bundle, that sling.servlet.post can depend on, we'll remove MediaRangeList and use RequestUtil.parseAcceptHeader.

>> This could probably be easily rewritten to use HtmlResponse's hashmap, and populating the JSONObject on send().

> I would prefer that even though it might seem a bit more expensive. The main reason for me would be simplicity: You just have to overwrite a single method - the rendering method - and otherwise have the exact same implementation. For output JsonWriter could then be used instead of JSONObject. 

The problem is that HtmlResponse does not have a getProperties() method - only getProperty() methods to retrieve named properties. 
So that leaves me with two options: Either a) record keys of all properties that are changed in a set in JsonResponse, so we'll know which properties to get from HtmlResponse on send(). This kind of invalidates the simplicity argument :)
or b) implement a HtmlResponse.getProperties() method. Since HtmlResponse is in sling.api, this would mean that the dependency to API would have to be updated to 2.0.9-SNAPSHOT.

I'm inclined to leaving this as it is for the moment, so that this issue can be resolved with changes to no other bundles than sling.servlet.post. Which again means that we're not holding back a release of sling.servlet.post by introducing this feature.

WDYT?


> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336-2.patch, SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Reopened: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Bertrand Delacretaz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bertrand Delacretaz reopened SLING-1336:
----------------------------------------

      Assignee: Bertrand Delacretaz  (was: Vidar S. Ramdal)

Reopening as no accept header in the request causes NPEs.

I'll fix that and write a simple integration tests.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Bertrand Delacretaz
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336-2.patch, SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vidar S. Ramdal resolved SLING-1336.
------------------------------------

    Resolution: Fixed

Implemented in rev #921261

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336-2.patch, SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Jason Rose (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841945#action_12841945 ] 

Jason Rose commented on SLING-1336:
-----------------------------------

Has there been any development on this issue?  I'm very interested in allowing my slingpostoperations and posts in general to return JSON instead of HTML, since I am dealing with a SproutCore front end.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Reopened: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vidar S. Ramdal reopened SLING-1336:
------------------------------------


> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336-2.patch, SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vidar S. Ramdal updated SLING-1336:
-----------------------------------

    Assignee: Vidar S. Ramdal

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842272#action_12842272 ] 

Vidar S. Ramdal commented on SLING-1336:
----------------------------------------

I have come a long way on this - I'll do some testing, hopefully it can be committed early this week.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Bertrand Delacretaz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bertrand Delacretaz resolved SLING-1336.
----------------------------------------

    Resolution: Fixed

NPE fixed, integration test added.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>            Assignee: Bertrand Delacretaz
>             Fix For: Servlets Post 2.0.6
>
>         Attachments: SLING-1336-2.patch, SLING-1336.patch
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1336) Let SlingPostServlet return JSON

Posted by "Vidar S. Ramdal (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841947#action_12841947 ] 

Vidar S. Ramdal commented on SLING-1336:
----------------------------------------

Not yet, I think. I'm hoping to get some time to do it "soon". I might get some time this weekend though. If anyone (including you) feels like implementing this now, be my guest.

> Let SlingPostServlet return JSON
> --------------------------------
>
>                 Key: SLING-1336
>                 URL: https://issues.apache.org/jira/browse/SLING-1336
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>    Affects Versions: Servlets Post 2.0.4
>            Reporter: Vidar S. Ramdal
>             Fix For: Servlets Post 2.0.6
>
>
> From http://markmail.org/thread/ic2752rha4iy2s2c
> In many scenarios it would be nice to get JSON in response to a POST. You could write your own post servlet to do that, but then you will not benefit from the registered SlingPostOperations and SlingPostProcessors that are executed by the default SlingPostServlet.
> In the case of SlingPostOperation, an implementing class is passed a HtmlResponse object when run(). The HtmlResponse object is pretty agnostic to response format though, the only HTML related work is in the send() method. Thus, it should be possible ot replace/extend/abstract HtmlResponse with a more generic response object. The send() functionality is then moved to SlingPostServlet, which determines which format to return based on these criteria (in order):
> 1. The Accept HTTP header - if set to "application/json" return JSON, otherwise return HTML
> 2. Possibly also an :accept form field, with the same meaning as the HTTP header, in case it is proven that setting the HTTP header does not work in all browsers
> 3. The format of the posted data - if JSON is posted (see SLING-1172), return JSON, otherwise return HTML

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.