deltaspike-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marek Posolda <>
Subject Re: Early security IDM prototype feedback
Date Fri, 13 Jul 2012 10:02:23 GMT
Hi all,

some late feedback for IDM API from me.

1. On various API objects like Group, Role, GroupQuery etc. there are 
many dual methods with signatures similar to:

     GroupQuery setRelatedUser(User user);

     GroupQuery setRelatedUser(String user);

For the former one, it may be bettter to use signature like:

GroupQuery setRelatedUser(String userId);

to emphasize that expected value is Id of user. WDYT?

2. Similar case is for Role. Instead of:

     GroupQuery setRole(Role role);

     GroupQuery setRole(String role);

we can have for the second method signature like:

     GroupQuery setRole(String roleName);

3. On interface Group, I am not sure what's the purpose of methods:

     Collection<User> getUsers(User user);

     Collection<User> getUsers(String user);

Is it a mistake or am I missing something here? Instead of those, I 
think it could make sense to have method for returning all users of 
particular group with signature like:

Collection<User> getUsers();

Probably this method is not so usable because of losing information 
about role, but I think it makes sense for some usecases.

4. On Group interface, instead of method:

Map<Role, Set<User>> getMembershipsMap();

it may be good to have possibility to obtain reverse map as well. So 
having two methods instead like:

     Map<Role, Set<User>> getMembershipsMapByRole();

     Map<User, Set<Role>> getMembershipsMapByUser();

Same applies for interface User (and there is already TODO on User 
interface for this).

5. Method: IdentityStore.getGroup(String name); should be likely 
IdentityStore.getGroup(String id) I guess?

6. For Query objects, I am not sure why we have methods like:

List<User> executeQuery(UserQuery query);

and not simply:

List<User> executeQuery();

And usage of the API can simply looks like:

Collection<User> allJohnDoes = 

7. I like the approach proposed by Jason here

with the base Query superclass. But I think on the Query object should 
be method:

Collection<T> executeQuery();

instead of:

Collection<T> getResults(Query<T> query);

Also not sure if method "getSingleResult" makes sense here for the Query 

Also if we use this approach, the method for creating query on 
IdentityManager should be like:
+ <T extends IdmObject> Query<T> createQuery(Class<T> objectType);
- <T extends IdmObject> Query<T> createQuery();

I agree with Bolek for the IdmObject part. Adding IdmObject methods to 
IdentityManager will reduce total number of needed methods on 
IdentityManager, but it adds some unecessary complexity. I think that 
adding too much generics and abstractness to the API could make it 
harder to understand and use for normal users.


Dne 9.7.2012 09:32, Boleslaw Dawidowicz napsal(a):
> Sorry for late reply - didn't have much time to look last week. Few comments:
> 1) I like the approach you took with Query and would like to improve it in similar direction.
> 2) I'm not convinced that everything should extend IdmObject. The only benefit is to
reduce number of methods however in some cases it can constrain design.
> 3) Methods like:
> <T extends IdmObject>  T get(Class<T>  objectType, String objectIdentifier);
> <T extends IdmObject>  void remove(T objectInstance);
> <T extends IdmObject>  T createBasic(Class<T>  idmType, String identifier);
> are I think good and suitable for SPI like IdentityStore. However I don't think they
are very user friendly for API that users will consume - like IdentityManger.
> Bolek
> On Jul 2, 2012, at 6:53 PM, Jason Porter wrote:
>> Due to Apache's blanket attachment scrub policy, you can find the same info
>> at my github fork:
>> On Mon, Jul 2, 2012 at 12:20 PM, Jason Porter<>wrote:
>>> Attached is my diff from what I had pulled down as of Wednesday or so.
>>> Feel free to discuss, dissect, etc
>>> On Sun, Jul 1, 2012 at 12:49 PM, Jason Porter<>wrote:
>>>> I have some stuff I did on the plane from Summit. I'm going to go back
>>>> over it tomorrow and see if I included everything I wanted. I'll send the
>>>> diff to the list.
>>>> Sent from my iPhone
>>>> On Jul 1, 2012, at 9:43, Boleslaw Dawidowicz<
>>>>>  wrote:
>>>>> I think it all comes down (again) to use cases [0] which we think this
>>>> API should address. Current ones are mainly around typical web application
>>>> development and proposed API prototype was focusing on easy usage. My main
>>>> fear is to go end up with too abstract and generic design that won't be
>>>> really appealing to most of developers. If you look at use cases addressed
>>>> by JSR 351 [1] it is all about dealing with attributes and scenarios closer
>>>> to healthcare - and this so far results in quite different API design. API
>>>> which I believe will be less appealing to typical web developer who doesn't
>>>> care about such use cases.
>>>>> More inline below
>>>>> [0]
>>>>> [1]
>>>>> On Jun 29, 2012, at 4:56 PM, Jason Porter wrote:
>>>>>> == Security IDM prototype feedback ==
>>>>>> * Fold PermissionQuery into PermissionManager to give the developers
>>>>>> similar model as JPA.
>>>>>> * Because a Group is an IdentityType, I wonder if it would be better
>>>>>> just have one set of methods and pass in the IdentityType sub
>>>> interface you
>>>>>> would like to receive back
>>>>> I guess thats a bit matter of taste. We could prepare quick prototype
>>>> and compare which one looks more useful to majority of folks here.
>>>>>> * I feel the methods on UserQuery are restricting how the IDM can
>>>> used.
>>>>>> We can't always guarantee that there will be a first name, last name,
>>>>>> email, etc. However, being completely open and free form like what
>>>> done
>>>>>> in the PicketLink IDM is too much. Perhaps we can find some sort
>>>> balance
>>>>>> or meta model similar to JPA?
>>>>>> * Maybe we could have a base no method interface for things and fold
>>>> all
>>>>>> the Query and Manager objects into one (similar to above about JPA)
>>>>>> * We probably don't want to go down the route of create a query
>>>> language,
>>>>>> but we almost have a DSL with a fluent API here. Maybe we should
>>>>>> about about JPA the names out a bit more and actually create a fluent
>>>> API
>>>>>> for the Query objects
>>>>> Would you have time to prepare some simple prototype like this to
>>>> discuss? I'm not sure how much generic design are you proposing in fact.
>>>> Just some bare code snippets would do to back your suggestions with some
>>>> example usage.
>>>>> UserQuery is not something I'm perfectly satisfied with however it is
>>>> still an attempt to have something similar to JPA Criteria Query - which
>>>> very intuitive and easy to use IMO. I would love to see more feedback from
>>>> others on how to improve this part in fact.
>>>>>> * I believe we should remove some of the exception constructors and
>>>> make a
>>>>>> message required (this would probably be good to have in all of
>>>>>> DeltaSpike). Adding a cause is great, but being able to create an
>>>> exception
>>>>>> without a message is less helpful for everyone.
>>>>> Good point. I think this part is simply not really polished yet.
>>>>>> * Is there a point for having addObject and addObjects type methods?
>>>> Would
>>>>>> one that's simply a varag be enough? We could check the type that
>>>> comes in
>>>>>> if thy send us a collection, or simply have varags and collection
>>>>>> addObjects methods.
>>>>> Sounds good to me. Again this is probably more matter of taste and
>>>> consistency with other APIs in the project. Maybe there should be some
>>>> general project wide design guideline for choices like this.
>>>>>> --
>>>>>> Jason Porter
>>>>>> Software Engineer
>>>>>> Open Source Advocate
>>>>>> Author of Seam Catch - Next Generation Java Exception Handling
>>>>>> PGP key id: 926CCFF5
>>>>>> PGP key available at:,
>>> --
>>> Jason Porter
>>> Software Engineer
>>> Open Source Advocate
>>> Author of Seam Catch - Next Generation Java Exception Handling
>>> PGP key id: 926CCFF5
>>> PGP key available at:,
>> -- 
>> Jason Porter
>> Software Engineer
>> Open Source Advocate
>> Author of Seam Catch - Next Generation Java Exception Handling
>> PGP key id: 926CCFF5
>> PGP key available at:,

View raw message