On 17.01.2013 14:43, Fabio Martelli wrote:
> Il giorno 17/gen/2013, alle ore 14.03, Jan Bernhardt ha scritto:
>
>> Hi Syncoper,
>>
>> The following changes are proposed regarding  AuthenticationController:
>>
>> === Renaming Service ===
>> Rename AuthenticationController to EntitlementService(Impl), since 
>> containing methods have little to nothing to do with authentication. It is 
>> only about Entitlements...
> Why not AuthorizationController or AccessController? I'd prefer the first one.
> May be this controller will be improved to add access controller features int 
> the next future (please, take a look at the roadmap).
I agree with the names Fabio proposed. If it is planned to put more
Authorization stuff in there then it makes sense to not have Entitlement
in the name.
Like Andrei proposed we should either rename all controllers to Service
or none. If we rename them partly people will be confused.

>> === Changing response Type ===
>> listEntitlements() returns a List<String> whereas getEntitlements() returns 
>> a Set<String>.
>>
>> From my point of view both methods should return a SET since entitlements 
>> have no order and cannot exists or be assigned more than once.
> Agree
I also agree with Set.
>
>> Due to some JAX-B / JAX-RS limitations, it is not possible to return a 
>> collection with primitive data types in java. A wrapper class is required, 
>> e.g. Set<EntitlementTO>.
>>
>> EntitlementTO can be modeled in one of two ways: 
>>
>> Option 1:
>> <EntitlementTOs>
>>   <EntitlementTO>ROLE_ADMIN</EntitlementTO>
>>   <EntitlementTO>ROLE_SUPERUSER</EntitlementTO>
>> </EntitlementTOs>
>>
>> Option 2:
>> <EntitlementTOs>
>>   <EntitlementTO>
>>      <name>ROLE_ADMIN</name>
>>  </EntitlementTO>
>>   <EntitlementTO>
>>      <name> ROLE_SUPERUSER </name>
>>  </EntitlementTO>
>> </EntitlementTOs>
>>
>> Option 1 matches more or less current marshaling. I personally would prefer 
>> Option 2 because this would give us the opportunity to easily extend 
>> EntitlementTO at a later point (if needed) without becoming incompatible 
>> with previous version.
> Agree with you for the reason given above.
Not sure about this one as I do not know how probable it is that we have
more attributes in Entitlement.

Btw. I would rather name the xml element Entitlement than EntitlementTO
as the fact that it is a transfer object is not important on the xml level.

Christian

>
>> WDYT?
>>
>> All of these changes will not apply to current Spring Controller/Services, 
>> but only future CXF REST Service. So AuthenticationController will not be 
>> renamed now, and responsetype of AuthenticationController will not change. 
>> Changes only apply for new Service Interface and (Proxy) Implementation.
> Good.
>
> Regards,
> F.
>

Reply via email to