> -----Original Message----- > From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] > Sent: Freitag, 1. Februar 2013 17:45 > To: dev@syncope.apache.org > Subject: Re: [DISCUSS] User Service > > On 01/02/2013 17:32, Jan Bernhardt wrote: > > Hi Syncoper, > > > > I'm almost done with new REST API Service Interface documentation [1]. > Last peace missing is only User Service. Please take time to review changes to > new Service Interfaces and let's have a discussion on this mailing-list. I > think > we already discussed all general changes, but if you find something missing, > just continue this discussion. > > > > I have a couple of proposals for changes in User Service Interface, which I > would like to get your feedback for. After we all agree with User Interface I > will update Interface and Documentation accordingly. > > > > 1. User Controller contains 5 methods focusing on workflow activities. My > proposal would be to move these methods to Workflow Service interface. > We could leave business code in UserController class for 1.1.0 release and > only change User and Workflow Service Interfaces for now, so that this > change will only be visible to new CXF REST API. > > The WorkflowController is a general interface for managing and querying the > workflow definitions (that can vary depending on the standard or custom > workflow adapters you have on your own project). > > 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. > > > 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? > > > 3. There are two methods (and status types) to activate an user account. > ACTIVATE and REACTIVATE. From my point of view REACTIVATE is redundant, > since it shouldn't matter if user was active previously or not. All that > matters > is that user should be active afterwards. Therefore my proposal would be to > remove REACTIVATE status type and also reactivate operation method. > > Activate and reactivate are differentiated at UserWorkflowAdapter level > because they are in fact two distinct operations. This difference is then > naturally reflected into the REST interface. One strength of a REST is its "STATELESS" design [1], which brings many advantages IMHO. So I would still recommend to refactor our current REST implementation. Of course we don't need to do this for 1.1.0 release. But do you agree that this would be a valuable improvement and that I should create a JIRA issue for that? Best regards. Jan [1] http://en.wikipedia.org/wiki/Representational_state_transfer#Constraints > > Regards. > > > [1] > > https://cwiki.apache.org/confluence/display/SYNCOPE/REST+API+upgrade > > -- > Francesco Chicchiriccò > > ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member > http://people.apache.org/~ilgrosso/