You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@syncope.apache.org by Colm O hEigeartaigh <co...@apache.org> on 2012/04/04 15:27:56 UTC
REST Controller consistency
Hi,
There is an inconsistency in the REST API as discussed on Syncope-42.
For example, both read and delete follow the same pattern for userIds
and usernames:
- /read/{userId}
- /read?username=X
Francesco suggested the following:
https://issues.apache.org/jira/browse/SYNCOPE-42?focusedCommentId=13245457&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13245457
However, the first approach does not seem to work, as he suspected.
However, if we move to the latter suggestion, this will cause
additional inconsistencies with other methods, such as
"user/reactivate/{userId}".
/read?userId={userId}
/read?username={username}
/delete?userId={userId}
/delete?username={username}
Any suggestions on the best way to proceed?
Colm.
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
Re: REST Controller consistency
Posted by Colm O hEigeartaigh <co...@apache.org>.
Hi Francesco,
> Basically, strict matching is used, but the caller (the integration test
> case) must explicitly appending ".json": in this way the latest part of
> matching string is stripped, but input is now correctly provided.
>
> WDYT?
Yes, this does the trick. I'll commit a fix based around your patch shortly.
Thanks,
Colm.
2012/4/14 Francesco Chicchiriccò <il...@apache.org>:
> On 13/04/2012 18:47, Colm O hEigeartaigh wrote:
>>
>> Hi Francesco,
>>
>> I've tried to go the reg-exp route, e.g.:
>>
>> @RequestMapping(method = RequestMethod.GET, value =
>> "/deleteByUsername/{username:.*}")
>> public UserTO delete(@PathVariable final String username)
>>
>> The username String gets picked up correctly
>> ("delete.by.username@apache.org"). However, I see another error in the
>> logs:
>>
>> SEVERE: Servlet.service() for servlet [syncope-core-rest] in context
>> with path [/syncope] threw exception [Could not resolve view with name
>> 'user/deleteByUsername/delete.by.username@apache' in servlet with name
>> 'syncope-core-rest'] with root cause
>> javax.servlet.ServletException: Could not resolve view with name
>> 'user/deleteByUsername/delete.by.username@apache' in servlet with name
>> 'syncope-core-rest'
>>
>> Any ideas?
>
>
> It seems that:
>
> * when the REST method is annotated with strict matching (i.e.
> "/deleteByUsername/{username}"), then Spring will strip off latest part of
> {username} (after last '.'); this will result in an incorrect input
> ("delete.by.username@apache")
>
> * when the REST method is annotated with extended matching (i.e.
> "/deleteByUsername/{username:.*}"), then input is correct, but Spring's
> ContentNegotiatingViewResolver will not be able to determine which kind of
> return format the caller is expecting (registered extensions are '.json' and
> '.xml', hence ".org" is not recognized)
>
> After some googling (which led to a nice set of issues on Spring's JIRA), I
> came to the conclusion that the following combination is (probably) the only
> one working.
>
> Index: core/src/test/java/org/syncope/core/rest/UserTestITCase.java
> ===================================================================
> --- core/src/test/java/org/syncope/core/rest/UserTestITCase.java
> (revisione 1326100)
> +++ core/src/test/java/org/syncope/core/rest/UserTestITCase.java (copia
> locale)
> @@ -778,8 +778,7 @@
> userTO = restTemplate.postForObject(BASE_URL + "user/create",
> userTO, UserTO.class);
>
> long id = userTO.getId();
> - userTO =
> - restTemplate.getForObject(BASE_URL + "user/delete?username=" +
> userTO.getUsername(), UserTO.class);
> + userTO = restTemplate.getForObject(BASE_URL +
> "user/deleteByUsername/{username}.json", UserTO.class,
> userTO.getUsername());
>
> assertNotNull(userTO);
> assertEquals(id, userTO.getId());
> Index:
> core/src/main/java/org/syncope/core/rest/controller/UserController.java
> ===================================================================
> --- core/src/main/java/org/syncope/core/rest/controller/UserController.java
> (revisione 1326100)
> +++ core/src/main/java/org/syncope/core/rest/controller/UserController.java
> (copia locale)
> @@ -440,8 +440,8 @@
> }
>
> @PreAuthorize("hasRole('USER_DELETE')")
> - @RequestMapping(method = RequestMethod.GET, value = "/delete")
> - public UserTO delete(@RequestParam("username") final String username)
> + @RequestMapping(method = RequestMethod.GET, value =
> "/deleteByUsername/{username}")
> + public UserTO delete(@PathVariable final String username)
> throws NotFoundException, WorkflowException,
> PropagationException, UnauthorizedRoleException {
> LOG.debug("User delete called with {}", username);
>
>
> Basically, strict matching is used, but the caller (the integration test
> case) must explicitly appending ".json": in this way the latest part of
> matching string is stripped, but input is now correctly provided.
>
> WDYT?
>
> Regards.
>
> --
> Francesco Chicchiriccò
>
> Apache Cocoon Committer and PMC Member
> http://people.apache.org/~ilgrosso/
>
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
Re: REST Controller consistency
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 13/04/2012 18:47, Colm O hEigeartaigh wrote:
> Hi Francesco,
>
> I've tried to go the reg-exp route, e.g.:
>
> @RequestMapping(method = RequestMethod.GET, value =
> "/deleteByUsername/{username:.*}")
> public UserTO delete(@PathVariable final String username)
>
> The username String gets picked up correctly
> ("delete.by.username@apache.org"). However, I see another error in the
> logs:
>
> SEVERE: Servlet.service() for servlet [syncope-core-rest] in context
> with path [/syncope] threw exception [Could not resolve view with name
> 'user/deleteByUsername/delete.by.username@apache' in servlet with name
> 'syncope-core-rest'] with root cause
> javax.servlet.ServletException: Could not resolve view with name
> 'user/deleteByUsername/delete.by.username@apache' in servlet with name
> 'syncope-core-rest'
>
> Any ideas?
It seems that:
* when the REST method is annotated with strict matching (i.e.
"/deleteByUsername/{username}"), then Spring will strip off latest part
of {username} (after last '.'); this will result in an incorrect input
("delete.by.username@apache")
* when the REST method is annotated with extended matching (i.e.
"/deleteByUsername/{username:.*}"), then input is correct, but Spring's
ContentNegotiatingViewResolver will not be able to determine which kind
of return format the caller is expecting (registered extensions are
'.json' and '.xml', hence ".org" is not recognized)
After some googling (which led to a nice set of issues on Spring's
JIRA), I came to the conclusion that the following combination is
(probably) the only one working.
Index: core/src/test/java/org/syncope/core/rest/UserTestITCase.java
===================================================================
--- core/src/test/java/org/syncope/core/rest/UserTestITCase.java
(revisione 1326100)
+++ core/src/test/java/org/syncope/core/rest/UserTestITCase.java
(copia locale)
@@ -778,8 +778,7 @@
userTO = restTemplate.postForObject(BASE_URL + "user/create",
userTO, UserTO.class);
long id = userTO.getId();
- userTO =
- restTemplate.getForObject(BASE_URL +
"user/delete?username=" + userTO.getUsername(), UserTO.class);
+ userTO = restTemplate.getForObject(BASE_URL +
"user/deleteByUsername/{username}.json", UserTO.class,
userTO.getUsername());
assertNotNull(userTO);
assertEquals(id, userTO.getId());
Index:
core/src/main/java/org/syncope/core/rest/controller/UserController.java
===================================================================
---
core/src/main/java/org/syncope/core/rest/controller/UserController.java
(revisione 1326100)
+++
core/src/main/java/org/syncope/core/rest/controller/UserController.java
(copia locale)
@@ -440,8 +440,8 @@
}
@PreAuthorize("hasRole('USER_DELETE')")
- @RequestMapping(method = RequestMethod.GET, value = "/delete")
- public UserTO delete(@RequestParam("username") final String username)
+ @RequestMapping(method = RequestMethod.GET, value =
"/deleteByUsername/{username}")
+ public UserTO delete(@PathVariable final String username)
throws NotFoundException, WorkflowException,
PropagationException, UnauthorizedRoleException {
LOG.debug("User delete called with {}", username);
Basically, strict matching is used, but the caller (the integration test
case) must explicitly appending ".json": in this way the latest part of
matching string is stripped, but input is now correctly provided.
WDYT?
Regards.
--
Francesco Chicchiriccò
Apache Cocoon Committer and PMC Member
http://people.apache.org/~ilgrosso/
Re: REST Controller consistency
Posted by Colm O hEigeartaigh <co...@apache.org>.
Hi Francesco,
I've tried to go the reg-exp route, e.g.:
@RequestMapping(method = RequestMethod.GET, value =
"/deleteByUsername/{username:.*}")
public UserTO delete(@PathVariable final String username)
The username String gets picked up correctly
("delete.by.username@apache.org"). However, I see another error in the
logs:
SEVERE: Servlet.service() for servlet [syncope-core-rest] in context
with path [/syncope] threw exception [Could not resolve view with name
'user/deleteByUsername/delete.by.username@apache' in servlet with name
'syncope-core-rest'] with root cause
javax.servlet.ServletException: Could not resolve view with name
'user/deleteByUsername/delete.by.username@apache' in servlet with name
'syncope-core-rest'
Any ideas?
Colm.
2012/4/6 Francesco Chicchiriccò <il...@apache.org>:
> On 05/04/2012 17:44, Colm O hEigeartaigh wrote:
>> Actually I've run into a problem when using
>> "/deleteByUsername/{username}", where "username" is an email-address.
>> Spring is truncating the @PathVariable and removing the ".org" at the
>> end of the email address. Any ideas? I tried setting
>> "useDefaultSuffixPattern" to false in the
>> DefaultAnnotationHandlerMapping object defined in restContext.xml, but
>> this seemed to cause other problems.
>
> Yes, because Syncope empowers ".json" and ".xml" Spring views.
>
> What do you think about the regexp solution proposed at
> http://stackoverflow.com/questions/3526523/spring-mvc-pathvariable-getting-truncated
> ?
>
> Cheers.
>
>> On Thu, Apr 5, 2012 at 3:24 PM, Colm O hEigeartaigh <co...@apache.org> wrote:
>>> Hi Francesco,
>>>
>>>> /read/{userId}
>>>> /readByUsername/{username}
>>>> /delete/{userId}
>>>> /deleteByUsername/{username}
>>> Makes sense - I'll follow your suggestion.
>>>
>>> Colm.
>>>
>>> 2012/4/4 Francesco Chicchiriccò <il...@apache.org>:
>>>> On 04/04/2012 15:27, Colm O hEigeartaigh wrote:
>>>>> Hi,
>>>>>
>>>>> There is an inconsistency in the REST API as discussed on Syncope-42.
>>>>> For example, both read and delete follow the same pattern for userIds
>>>>> and usernames:
>>>>>
>>>>> - /read/{userId}
>>>>> - /read?username=X
>>>>>
>>>>> Francesco suggested the following:
>>>>>
>>>>>
>>>>> https://issues.apache.org/jira/browse/SYNCOPE-42?focusedCommentId=13245457&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13245457
>>>>>
>>>>> However, the first approach does not seem to work, as he suspected.
>>>>> However, if we move to the latter suggestion, this will cause
>>>>> additional inconsistencies with other methods, such as
>>>>> "user/reactivate/{userId}".
>>>>>
>>>>> /read?userId={userId}
>>>>> /read?username={username}
>>>>> /delete?userId={userId}
>>>>> /delete?username={username}
>>>>>
>>>>> Any suggestions on the best way to proceed?
>>>>
>>>> Hi Colm,
>>>> moving everything to the "request parameter" way (i.e. the latter above) can
>>>> have some unexpected side effects on the other 16 REST controllers (besides
>>>> UserController).
>>>>
>>>> I mean, if we decide to change /read/{userId} to /read?userId={userId}, we
>>>> will end up by changing also /read/{notificationId} and so on.
>>>>
>>>> From the other side, keeping the situation as is for UserController leads to
>>>> an unacceptable inconsistency, as pointed out by you: hence my proposal is
>>>> to have:
>>>>
>>>> /read/{userId}
>>>> /readByUsername/{username}
>>>> /delete/{userId}
>>>> /deleteByUsername/{username}
>>>>
>>>>
>>>> We can do the same duplication for each REST method taking {userId} as
>>>> parameter.
>>>>
>>>> All this situation only affects UserController, since SyncopeUser is the
>>>> only entity having a primary key (Long id) and a field with UNIQUE NOT NULL
>>>> constraint (String username).
>>>>
>>>> Regards.
> --
> Francesco Chicchiriccò
>
> Apache Cocoon PMC and Apache Syncope PPMC Member
> http://people.apache.org/~ilgrosso/
>
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
Re: REST Controller consistency
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 05/04/2012 17:44, Colm O hEigeartaigh wrote:
> Actually I've run into a problem when using
> "/deleteByUsername/{username}", where "username" is an email-address.
> Spring is truncating the @PathVariable and removing the ".org" at the
> end of the email address. Any ideas? I tried setting
> "useDefaultSuffixPattern" to false in the
> DefaultAnnotationHandlerMapping object defined in restContext.xml, but
> this seemed to cause other problems.
Yes, because Syncope empowers ".json" and ".xml" Spring views.
What do you think about the regexp solution proposed at
http://stackoverflow.com/questions/3526523/spring-mvc-pathvariable-getting-truncated
?
Cheers.
> On Thu, Apr 5, 2012 at 3:24 PM, Colm O hEigeartaigh <co...@apache.org> wrote:
>> Hi Francesco,
>>
>>> /read/{userId}
>>> /readByUsername/{username}
>>> /delete/{userId}
>>> /deleteByUsername/{username}
>> Makes sense - I'll follow your suggestion.
>>
>> Colm.
>>
>> 2012/4/4 Francesco Chicchiriccò <il...@apache.org>:
>>> On 04/04/2012 15:27, Colm O hEigeartaigh wrote:
>>>> Hi,
>>>>
>>>> There is an inconsistency in the REST API as discussed on Syncope-42.
>>>> For example, both read and delete follow the same pattern for userIds
>>>> and usernames:
>>>>
>>>> - /read/{userId}
>>>> - /read?username=X
>>>>
>>>> Francesco suggested the following:
>>>>
>>>>
>>>> https://issues.apache.org/jira/browse/SYNCOPE-42?focusedCommentId=13245457&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13245457
>>>>
>>>> However, the first approach does not seem to work, as he suspected.
>>>> However, if we move to the latter suggestion, this will cause
>>>> additional inconsistencies with other methods, such as
>>>> "user/reactivate/{userId}".
>>>>
>>>> /read?userId={userId}
>>>> /read?username={username}
>>>> /delete?userId={userId}
>>>> /delete?username={username}
>>>>
>>>> Any suggestions on the best way to proceed?
>>>
>>> Hi Colm,
>>> moving everything to the "request parameter" way (i.e. the latter above) can
>>> have some unexpected side effects on the other 16 REST controllers (besides
>>> UserController).
>>>
>>> I mean, if we decide to change /read/{userId} to /read?userId={userId}, we
>>> will end up by changing also /read/{notificationId} and so on.
>>>
>>> From the other side, keeping the situation as is for UserController leads to
>>> an unacceptable inconsistency, as pointed out by you: hence my proposal is
>>> to have:
>>>
>>> /read/{userId}
>>> /readByUsername/{username}
>>> /delete/{userId}
>>> /deleteByUsername/{username}
>>>
>>>
>>> We can do the same duplication for each REST method taking {userId} as
>>> parameter.
>>>
>>> All this situation only affects UserController, since SyncopeUser is the
>>> only entity having a primary key (Long id) and a field with UNIQUE NOT NULL
>>> constraint (String username).
>>>
>>> Regards.
--
Francesco Chicchiriccò
Apache Cocoon PMC and Apache Syncope PPMC Member
http://people.apache.org/~ilgrosso/
Re: REST Controller consistency
Posted by Colm O hEigeartaigh <co...@apache.org>.
Actually I've run into a problem when using
"/deleteByUsername/{username}", where "username" is an email-address.
Spring is truncating the @PathVariable and removing the ".org" at the
end of the email address. Any ideas? I tried setting
"useDefaultSuffixPattern" to false in the
DefaultAnnotationHandlerMapping object defined in restContext.xml, but
this seemed to cause other problems.
Colm.
On Thu, Apr 5, 2012 at 3:24 PM, Colm O hEigeartaigh <co...@apache.org> wrote:
> Hi Francesco,
>
>> /read/{userId}
>> /readByUsername/{username}
>> /delete/{userId}
>> /deleteByUsername/{username}
>
> Makes sense - I'll follow your suggestion.
>
> Colm.
>
> 2012/4/4 Francesco Chicchiriccò <il...@apache.org>:
>> On 04/04/2012 15:27, Colm O hEigeartaigh wrote:
>>>
>>> Hi,
>>>
>>> There is an inconsistency in the REST API as discussed on Syncope-42.
>>> For example, both read and delete follow the same pattern for userIds
>>> and usernames:
>>>
>>> - /read/{userId}
>>> - /read?username=X
>>>
>>> Francesco suggested the following:
>>>
>>>
>>> https://issues.apache.org/jira/browse/SYNCOPE-42?focusedCommentId=13245457&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13245457
>>>
>>> However, the first approach does not seem to work, as he suspected.
>>> However, if we move to the latter suggestion, this will cause
>>> additional inconsistencies with other methods, such as
>>> "user/reactivate/{userId}".
>>>
>>> /read?userId={userId}
>>> /read?username={username}
>>> /delete?userId={userId}
>>> /delete?username={username}
>>>
>>> Any suggestions on the best way to proceed?
>>
>>
>> Hi Colm,
>> moving everything to the "request parameter" way (i.e. the latter above) can
>> have some unexpected side effects on the other 16 REST controllers (besides
>> UserController).
>>
>> I mean, if we decide to change /read/{userId} to /read?userId={userId}, we
>> will end up by changing also /read/{notificationId} and so on.
>>
>> From the other side, keeping the situation as is for UserController leads to
>> an unacceptable inconsistency, as pointed out by you: hence my proposal is
>> to have:
>>
>> /read/{userId}
>> /readByUsername/{username}
>> /delete/{userId}
>> /deleteByUsername/{username}
>>
>>
>> We can do the same duplication for each REST method taking {userId} as
>> parameter.
>>
>> All this situation only affects UserController, since SyncopeUser is the
>> only entity having a primary key (Long id) and a field with UNIQUE NOT NULL
>> constraint (String username).
>>
>> Regards.
>>
>> --
>> Francesco Chicchiriccò
>>
>> Apache Cocoon PMC and Apache Syncope PPMC Member
>> http://people.apache.org/~ilgrosso/
>>
>
>
>
> --
> Colm O hEigeartaigh
>
> Talend Community Coder
> http://coders.talend.com
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
Re: REST Controller consistency
Posted by Colm O hEigeartaigh <co...@apache.org>.
Hi Francesco,
> /read/{userId}
> /readByUsername/{username}
> /delete/{userId}
> /deleteByUsername/{username}
Makes sense - I'll follow your suggestion.
Colm.
2012/4/4 Francesco Chicchiriccò <il...@apache.org>:
> On 04/04/2012 15:27, Colm O hEigeartaigh wrote:
>>
>> Hi,
>>
>> There is an inconsistency in the REST API as discussed on Syncope-42.
>> For example, both read and delete follow the same pattern for userIds
>> and usernames:
>>
>> - /read/{userId}
>> - /read?username=X
>>
>> Francesco suggested the following:
>>
>>
>> https://issues.apache.org/jira/browse/SYNCOPE-42?focusedCommentId=13245457&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13245457
>>
>> However, the first approach does not seem to work, as he suspected.
>> However, if we move to the latter suggestion, this will cause
>> additional inconsistencies with other methods, such as
>> "user/reactivate/{userId}".
>>
>> /read?userId={userId}
>> /read?username={username}
>> /delete?userId={userId}
>> /delete?username={username}
>>
>> Any suggestions on the best way to proceed?
>
>
> Hi Colm,
> moving everything to the "request parameter" way (i.e. the latter above) can
> have some unexpected side effects on the other 16 REST controllers (besides
> UserController).
>
> I mean, if we decide to change /read/{userId} to /read?userId={userId}, we
> will end up by changing also /read/{notificationId} and so on.
>
> From the other side, keeping the situation as is for UserController leads to
> an unacceptable inconsistency, as pointed out by you: hence my proposal is
> to have:
>
> /read/{userId}
> /readByUsername/{username}
> /delete/{userId}
> /deleteByUsername/{username}
>
>
> We can do the same duplication for each REST method taking {userId} as
> parameter.
>
> All this situation only affects UserController, since SyncopeUser is the
> only entity having a primary key (Long id) and a field with UNIQUE NOT NULL
> constraint (String username).
>
> Regards.
>
> --
> Francesco Chicchiriccò
>
> Apache Cocoon PMC and Apache Syncope PPMC Member
> http://people.apache.org/~ilgrosso/
>
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
Re: REST Controller consistency
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 04/04/2012 15:27, Colm O hEigeartaigh wrote:
> Hi,
>
> There is an inconsistency in the REST API as discussed on Syncope-42.
> For example, both read and delete follow the same pattern for userIds
> and usernames:
>
> - /read/{userId}
> - /read?username=X
>
> Francesco suggested the following:
>
> https://issues.apache.org/jira/browse/SYNCOPE-42?focusedCommentId=13245457&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13245457
>
> However, the first approach does not seem to work, as he suspected.
> However, if we move to the latter suggestion, this will cause
> additional inconsistencies with other methods, such as
> "user/reactivate/{userId}".
>
> /read?userId={userId}
> /read?username={username}
> /delete?userId={userId}
> /delete?username={username}
>
> Any suggestions on the best way to proceed?
Hi Colm,
moving everything to the "request parameter" way (i.e. the latter above)
can have some unexpected side effects on the other 16 REST controllers
(besides UserController).
I mean, if we decide to change /read/{userId} to /read?userId={userId},
we will end up by changing also /read/{notificationId} and so on.
From the other side, keeping the situation as is for UserController
leads to an unacceptable inconsistency, as pointed out by you: hence my
proposal is to have:
/read/{userId}
/readByUsername/{username}
/delete/{userId}
/deleteByUsername/{username}
We can do the same duplication for each REST method taking {userId} as
parameter.
All this situation only affects UserController, since SyncopeUser is the
only entity having a primary key (Long id) and a field with UNIQUE NOT
NULL constraint (String username).
Regards.
--
Francesco Chicchiriccò
Apache Cocoon PMC and Apache Syncope PPMC Member
http://people.apache.org/~ilgrosso/