One other idea for the username / userId based methods.
Why not simply use:
/users/{userIdorName}
So basically you could do
get /users/100 or get /users/chris@mydomain
We could handle both using
@GET
@Path("{userId}")
UserTO read(@PathParam("userId") String userId);
This is convenient for any kind of client and quite efficient at the
same time.
Christian
On 04.02.2013 14:00, Christian Schneider wrote:
> On 04.02.2013 12:41, Francesco Chicchiriccò wrote:
>> On 04/02/2013 12:12, Jan Bernhardt wrote:
>>>>
>>>> The workflow-related methods in UserController (and RoleController BTW)
>>>> are there because they are involved in user (and role) lifecycle
>>>> management.
>>>>
>>>> Given these things, I wouldn't make any movement.
>>> OK, it still doesn't feel right to me, but of course we still can
>>> make refactoring's at a later time, when we all feel a greater need
>>> to do so.
>> It is not matter of feeling, it is just incorrect: UserController and
>> RoleController make usage of workflow adapters to perform user and
>> role operations while the WorkflowController manipulates the workflow
>> definition, used to initialize and configure the workflow adapters.
> I agree with you that WorkflowController and the workflow methods in
> UserController do not belong together. On the other hand I think it may
> be a good idea to separate them from the UserService. So would it make
> sense to have a UserWorkflowService? This is supported by the fact that
> we already have a separate ApprovalRestClient in the client module.
>
>>>>> 2. There are quite some many methods capable of handling username and
>>>> userId likewise (suspendByUsername, deleteByUsername). As far as I can
>>>> see console does not use any of them (*byUsername). From my point of
>>>> view, these methods are redundant (except one), since you can always
>>>> get
>>>> user id by readUser(String username) operation. My proposal would be to
>>>> not support these methods in new interface any longer.
>>>>
>>>> The username-based methods were introduced for making more user-
>>>> friendly the invocation of user methods, even from command line
>>>> using tools
>>>> like curl; the admin console was built before that, and hence it's
>>>> using the
>>>> former id-based methods.
>>>>
>>>> I agree that username-based are redundant, but they were introduced to
>>>> increase the ease of use, not the efficiency.
>>> IMHO I would not add so many convenience functions to core interface,
>>> just for tools like curl. Any adaptor of syncope should rather use
>>> IDs than (temporary) names. I think these kind of convenience
>>> functions would belong to a rich syncope client [SYNCOPE-150], rather
>>> than core interface.
>>>
>>> One problem with username is, that it makes sub-resource access more
>>> difficult or increases potential path collisions with other resources.
>>>
>>> Of course we can keep these methods as they currently are, but I
>>> would still like to remove them.
>>>
>>> Who else has an opinion on this matter?
>> Adding some bits to this: SYNCOPE-42 and SYNCOPE-91 introduced in
>> 1.0.0-incubating such methods that were absent before.
>>
>> Anyway, I am +-0 to remove such methods in 1.2.0.
> Perhaps these convenience methods show that it might be better to have
> the user name as a key in the rest service instead of the id.
> I am not really sure about this though. When talking with Jan about it
> he argued that a user name can change over time.
>
> On the other hand I think the user id we currently have is an internal
> thing of our database and may better be hidden to the outside.
>
> One other thing we could consider is using a redirect from the user
> based rest resource to the id based resource. So when you do a get on
> users/readByUsername/{username} we could redirect to users/{username}.
>
> For the status changing methods like users/activate/{userid} we could
> instead use: users/{userId}/status.
> You could send put with body active to users/{userId}/status to make a
> user active. Or you could get users/{userId}/status to retrieve the
> status of a user.
>
> I think it makes sense to think about these things to make the API as
> good as possible. On the other hand I think these considerations should
> not block the 1.1.0 release.
>
> Best regards
>
> Christian
>
--
Christian Schneider
http://www.liquid-reality.de
Open Source Architect
http://www.talend.com
