[security-dev] IdentityManager API review

[Shane Bryzak]

On a somewhat related note, we should probably fix the constructor for 
SimpleGroup also, which currently looks like this:

public SimpleGroup(String id, String name, Group parentGroup)

Having both id and parentGroup parameters is redundant, so I suggest 
removing the id parameter (and removing the id field altogether) and 
instead have the getId() method return a calculated id.


On 11/08/2012 06:08 AM, Shane Bryzak wrote:
> They're actually a fundamental part of the identity model (see [1]).  
> I have no real problem with the principle of removing the String 
> versions of createGroup() (we would also have to do the same to 
> createRole() for consistency) and in fact it would provide some 
> additional advantages.  For example, being able to set a Group to 
> being disabled at creation time, setting attribute values, etc.  My 
> only concern is from a coding "correctness" point of view, and I guess 
> is centered around the creation date being automatically set (or 
> potentially overridden) on the Group instance that's passed to 
> createGroup().  It's probably not an important concern though, and I'm 
> happy to concede on this one which would mean we end up with the 
> following methods (replacing all existing createGroup() and 
> createRole() methods):
>
> void createGroup(Group group);
>
> void createRole(Role role);
>
>
> [1] 
> https://github.com/picketlink/picketlink/blob/master/idm/api/src/main/java/org/picketlink/idm/model/IdentityType.java
>
> On 11/08/2012 05:37 AM, Jason Porter wrote:
>> Aren't those implementation details though?
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/security-dev/attachments/20121108/b11e0300/attachment.html