[security-dev] IdentityManager API review

[Bolesław Dawidowicz]

I think I'm in agreement with outcome of previous comments about API.

Debate around way of creating groups is more a matter of taste in the 
end. I'm more concern at this point about implementation and explicitly 
storing group ids as I know from experience that calculating this data 
can be performance killer in case of any more sophisticated query.


On 11/07/2012 09:14 PM, Jason Porter wrote:
> On Wed, Nov 7, 2012 at 1:11 PM, Shane Bryzak <sbryzak at redhat.com
> <mailto:sbryzak at redhat.com>> wrote:
>
>     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.
>
>
> +1
>
>     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?
>>
>
>
>     _______________________________________________
>     security-dev mailing list
>     security-dev at lists.jboss.org <mailto:security-dev at lists.jboss.org>
>     https://lists.jboss.org/mailman/listinfo/security-dev
>
>
>
>
> --
> Jason Porter
> http://lightguard-jp.blogspot.com
> http://twitter.com/lightguardjp
>
> Software Engineer
> Open Source Advocate
>
> PGP key id: 926CCFF5
> PGP key available at: keyserver.net <http://keyserver.net>, pgp.mit.edu
> <http://pgp.mit.edu>
>
>
> _______________________________________________
> security-dev mailing list
> security-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/security-dev
>