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