On 04/02/2013 12:12, Jan Bernhardt wrote:
-----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.
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.
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.
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?
Ok, so we must match a non-functional REST need (providing stateless
APIs) with a functional need (user workflow is stateful by definition,
activate() and reactivate() maps to two completely different statuses).
Under these conditions, I am +1 to open an issue for that.
Regards.
[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/
