Problems with REST API (Re: Is it intentional that REST interface requires basic authentication?)

[Ross Gardler <rg...@apache.org>]

On 27/02/2010 22:19, Ross Gardler wrote:
> On 27/02/2010 18:51, Scott Wilson wrote:
>>
>> On 27 Feb 2010, at 12:55, Ross Gardler wrote:
>>
>>> The REST documentation makes no mention of needing to authenticate. Is
>>> it intentional that some requests need to be authenticated, e..g.
>>> participants
>
> ...
>
>>
>> The only requests that require authentication are those that are
>> equivalent to admin interface functions for creating/updating/deleting
>> services/categories, and these are documented at
>
> ...
>
>> All other API requests only use the API key for authentication - if
>> Basic auth is being requested for these (such as participants), then
>> this is a configuration problem.
>
> This is using Wookie in dev mode (i.e. ant run) and thus there is no
> local config messing with things.
>
> Here's what appears to be happening:
>
> Performing a GET request for participants with a valid API Key is fine
>
> Performing a GET request for participants with an invalid API Key throws
> an exception (unauthorized) which results in a request for authentication.
>
> I've found other problems too, but have not fully debugged them, it may
> be my code.

No, it's not my code.

When doing a POST request via participants request().getPathInfo() in 
Controller returns null.

 From the Java api:

"public java.lang.String getPathInfo() Returns any extra path 
information associated with the URL the client sent when it made this 
request. The extra path information follows the servlet path but 
precedes the query string. This method returns null if there was no 
extra path information. "

In web.xml we have:

	<servlet>
		<description></description>
		<display-name>Participants</display-name>
		<servlet-name>ParticipantServlet</servlet-name>
		<servlet-class>
			org.apache.wookie.controller.ParticipantsController
		</servlet-class>
		<load-on-startup>2</load-on-startup>
	</servlet>	
	<servlet-mapping>
		<servlet-name>ParticipantServlet</servlet-name>
		<url-pattern>/participants</url-pattern>
	</servlet-mapping>

In other words we can't use getPathInfo() to retrieve the restName as is 
currently implemented in the Controller.

It is my initial evaluation that the whole REST API suffers from this 
problem. One solution is to use request.getContextPath() and 
request.getRequestURI() rather than getPathInfo().

However, this will break the default behaviour (to get an index of all 
resources) and quite possibly some other functionality. Why is it 
implemented this way in the first instance - is the code shared by the 
old non-REST interface?

Ross

Ross

Re: Problems with REST API (Re: Is it intentional that REST interface requires basic authentication?)

[Ross Gardler <rg...@apache.org>]

On 28/02/2010 11:17, Scott Wilson wrote:
> On 27 Feb 2010, at 23:35, Ross Gardler wrote:
>
>> On further investigation I have come to the conclusion that the
>> implementation of the various controllers is either horribly broken
>
> That's a bit harsh!

True.

I (as with others) can be overly harsh at times. I was clearly 
frustrated by the code at this point and should have left this email in 
my queue for an hour, re-read and sent after editing.

Thanks for giving the code some attention. I'll revisit this again soon 
to see if there was nay validity in my observations other than we need 
to get consistency in the implementation (which can come with gradual 
refactoring).

Ross

Re: Problems with REST API (Re: Is it intentional that REST interface requires basic authentication?)

[Scott Wilson <sc...@gmail.com>]

On 27 Feb 2010, at 23:35, Ross Gardler wrote:

> On further investigation I have come to the conclusion that the  
> implementation of the various controllers is either horribly broken

That's a bit harsh!

> or impossibly hard to follow due to bad naming conventions e.g. in  
> the WidgetController "restName" becomes "resource_id" which then  
> becomes "type".

Fixed.

> It looks to me like this code has been hacked and refactored a  
> number of times and never cleaned up to ensure consistency.

Yes, that's fair. Originally Wookie had a very unRESTlike API which  
has been moved towards consistency through several refactorings.

The abstract Controller base class represents a standard REST API,  
which each interface implements and/or overrides with its own behaviour.

> In short, the code needs rewriting and since I'm trying to write the  
> connector framework to use the REST API I guess I'll have to do it :-(

I'm happy to fix anything in the controllers - just flag it up with a  
test case against the API spec.

> Since I don't understand why it was implemented in this way to start  
> with I'd appreciate some careful reviews of my commits on this area.
>
> Ross
>
> On 27/02/2010 23:18, Ross Gardler wrote:
>> On 27/02/2010 22:19, Ross Gardler wrote:
>>> On 27/02/2010 18:51, Scott Wilson wrote:
>>>>
>>>> On 27 Feb 2010, at 12:55, Ross Gardler wrote:
>>>>
>>>>> The REST documentation makes no mention of needing to  
>>>>> authenticate. Is
>>>>> it intentional that some requests need to be authenticated, e..g.
>>>>> participants
>>>
>>> ...
>>>
>>>>
>>>> The only requests that require authentication are those that are
>>>> equivalent to admin interface functions for creating/updating/ 
>>>> deleting
>>>> services/categories, and these are documented at
>>>
>>> ...
>>>
>>>> All other API requests only use the API key for authentication - if
>>>> Basic auth is being requested for these (such as participants),  
>>>> then
>>>> this is a configuration problem.
>>>
>>> This is using Wookie in dev mode (i.e. ant run) and thus there is no
>>> local config messing with things.
>>>
>>> Here's what appears to be happening:
>>>
>>> Performing a GET request for participants with a valid API Key is  
>>> fine
>>>
>>> Performing a GET request for participants with an invalid API Key  
>>> throws
>>> an exception (unauthorized) which results in a request for
>>> authentication.
>>>
>>> I've found other problems too, but have not fully debugged them,  
>>> it may
>>> be my code.
>>
>> No, it's not my code.
>>
>> When doing a POST request via participants request().getPathInfo() in
>> Controller returns null.
>>
>> From the Java api:
>>
>> "public java.lang.String getPathInfo() Returns any extra path
>> information associated with the URL the client sent when it made this
>> request. The extra path information follows the servlet path but
>> precedes the query string. This method returns null if there was no
>> extra path information. "
>>
>> In web.xml we have:
>>
>> <servlet>
>> <description></description>
>> <display-name>Participants</display-name>
>> <servlet-name>ParticipantServlet</servlet-name>
>> <servlet-class>
>> org.apache.wookie.controller.ParticipantsController
>> </servlet-class>
>> <load-on-startup>2</load-on-startup>
>> </servlet>
>> <servlet-mapping>
>> <servlet-name>ParticipantServlet</servlet-name>
>> <url-pattern>/participants</url-pattern>
>> </servlet-mapping>
>>
>> In other words we can't use getPathInfo() to retrieve the restName  
>> as is
>> currently implemented in the Controller.
>>
>> It is my initial evaluation that the whole REST API suffers from this
>> problem. One solution is to use request.getContextPath() and
>> request.getRequestURI() rather than getPathInfo().
>>
>> However, this will break the default behaviour (to get an index of  
>> all
>> resources) and quite possibly some other functionality. Why is it
>> implemented this way in the first instance - is the code shared by  
>> the
>> old non-REST interface?
>>
>> Ross
>>
>> Ross
>

Re: Problems with REST API (Re: Is it intentional that REST interface requires basic authentication?)

[Ross Gardler <rg...@apache.org>]

On further investigation I have come to the conclusion that the 
implementation of the various controllers is either horribly broken or 
impossibly hard to follow due to bad naming conventions e.g. in the 
WidgetController "restName" becomes "resource_id" which then becomes "type".

It looks to me like this code has been hacked and refactored a number of 
times and never cleaned up to ensure consistency.

In short, the code needs rewriting and since I'm trying to write the 
connector framework to use the REST API I guess I'll have to do it :-(

Since I don't understand why it was implemented in this way to start 
with I'd appreciate some careful reviews of my commits on this area.

Ross

On 27/02/2010 23:18, Ross Gardler wrote:
> On 27/02/2010 22:19, Ross Gardler wrote:
>> On 27/02/2010 18:51, Scott Wilson wrote:
>>>
>>> On 27 Feb 2010, at 12:55, Ross Gardler wrote:
>>>
>>>> The REST documentation makes no mention of needing to authenticate. Is
>>>> it intentional that some requests need to be authenticated, e..g.
>>>> participants
>>
>> ...
>>
>>>
>>> The only requests that require authentication are those that are
>>> equivalent to admin interface functions for creating/updating/deleting
>>> services/categories, and these are documented at
>>
>> ...
>>
>>> All other API requests only use the API key for authentication - if
>>> Basic auth is being requested for these (such as participants), then
>>> this is a configuration problem.
>>
>> This is using Wookie in dev mode (i.e. ant run) and thus there is no
>> local config messing with things.
>>
>> Here's what appears to be happening:
>>
>> Performing a GET request for participants with a valid API Key is fine
>>
>> Performing a GET request for participants with an invalid API Key throws
>> an exception (unauthorized) which results in a request for
>> authentication.
>>
>> I've found other problems too, but have not fully debugged them, it may
>> be my code.
>
> No, it's not my code.
>
> When doing a POST request via participants request().getPathInfo() in
> Controller returns null.
>
>  From the Java api:
>
> "public java.lang.String getPathInfo() Returns any extra path
> information associated with the URL the client sent when it made this
> request. The extra path information follows the servlet path but
> precedes the query string. This method returns null if there was no
> extra path information. "
>
> In web.xml we have:
>
> <servlet>
> <description></description>
> <display-name>Participants</display-name>
> <servlet-name>ParticipantServlet</servlet-name>
> <servlet-class>
> org.apache.wookie.controller.ParticipantsController
> </servlet-class>
> <load-on-startup>2</load-on-startup>
> </servlet>
> <servlet-mapping>
> <servlet-name>ParticipantServlet</servlet-name>
> <url-pattern>/participants</url-pattern>
> </servlet-mapping>
>
> In other words we can't use getPathInfo() to retrieve the restName as is
> currently implemented in the Controller.
>
> It is my initial evaluation that the whole REST API suffers from this
> problem. One solution is to use request.getContextPath() and
> request.getRequestURI() rather than getPathInfo().
>
> However, this will break the default behaviour (to get an index of all
> resources) and quite possibly some other functionality. Why is it
> implemented this way in the first instance - is the code shared by the
> old non-REST interface?
>
> Ross
>
> Ross