I very much like the DSL you have going there. Removing the Range class is
also a good idea, imo. However, I'm at -1 for the use of enums. You can't
extend them, and you can't anticipate everything a user would add to a
class. Sure we could do it in the simple idm idea where we provide the
structure, but as soon as they advance past that then they're forced to use
a different API.
On Mon, Oct 29, 2012 at 7:10 AM, Pedro Igor Silva <psilva(a)redhat.com> wrote:
I liked your proposal, it is simple and clear.
Considering what you did I thought in another solution. At first glance,
it seems more odd. But I think it can be more intuitive for users. For
example, instead of having the Param.memberOf, users may use
query.role(Role) or query.group(Group) directly.
IdentityManager identityManager = // get the identity manager;
Query<User> query = identityManager.<User>createQuery();
query
.limit(100)
.offset(1)
.where()
.id("1")
.property(User.FIRST_NAME, OPERATOR.equals, "John")
.property(User.EXPIRATION_DATE, OPERATOR.lessThan, new
Date())
.attribute("ssn", OPERATOR.equals, "SSN-123")
.role(new SimpleRole("Payroll Officer"))
.group(new SimpleGroup("Superuser", "Superuser",
null));
List<User> list = query.list();
for (User user : list) {
// handle users
}
I think we can avoid having a Range class/interface by using some methods
on the Query interface (eg. limit and offset above). Th Query interface can
be used only to configure how the query should be executed, global
configuration, etc.
The Where interface can be useful to provide some specific validation
depending of the IdentityType and how the restrictions are configured. For
example, users do not have a parent relationship.
The property names (firstName, email, etc) can be a Enum for each
IdentityType type (user,group and role). Each IdentityType type should
specify which properties are searchable + the common properties defined by
the IdentityType interface.
Not sure if we need the query.reset() method ... Why not just discard the
query instance and build another one ?
Regards.
Pedro Igor
----- Original Message -----
From: "Shane Bryzak" <sbryzak(a)redhat.com>
To: security-dev(a)lists.jboss.org
Sent: Monday, October 29, 2012 6:38:14 AM
Subject: [security-dev] IdentityManager review - queries
I've started reviewing the IdentityManager interface to see where we can
improve the API. The first area I'd like to visit is the Query API, of
which I've come to the conclusion that we need to do some serious
redesign - the current API is non-intuitive, too verbose and not future
proof.
What I'd like to do is throw it all out and start again, replacing it
with a new cleaner API that looks something like this:
public interface IdentityManager {
// <snip other methods>
<T extends IdentityType> IdentityQuery<T> createQuery();
}
public interface IdentityQuery<T extends IdentityType> {
public enum Param {id, key, created, expired, enabled, firstName,
lastName, email, name, parent, memberOf};
public enum Operator { equals, notEquals, greaterThan, lessThan };
IdentityQuery<T> reset();
IdentityQuery<T> setParameter(Param param, Object value);
IdentityQuery<T> setParameter(Param param, Operator operator,
Object value);
IdentityQuery<T> setAttributeParameter(String attributeName, Object
value);
IdentityQuery<T> setAttributeParameter(String attributeName,
Operator operator, Object value);
IdentityQuery<T> setRange(Range range);
List<T> getResultList();
}
This unified API basically replaces the 4 separate existing interfaces
we currently have; UserQuery, RoleQuery, GroupQuery and
MembershipQuery. I've put together a few usage scenarios to show how it
might work:
1) Find users with first name 'John':
List<User> users = identityManager.<User>createQuery()
.setParameter(Param.firstName, "John")
.getResultList();
2) Find all expired users:
List<User> users = identityManager.<User>createQuery()
.setParameter(Param.expired, Operator.lessThan, new Date())
.getResultList();
3) Find all users that are a member of the "Superuser" group
List<User> users = identityManager.<User>createQuery()
.setParameter(Param.memberOf, identityManager.getGroup("Superuser"))
.getResultList();
4) Find all sub-groups of the "Employees" group:
List<Group> groups = identityManager.<Group>createQuery()
.setParameter(Param.memberOf, identityManager.getGroup("Employees"))
.getResultList();
5) Find all disabled roles:
List<Role> roles = identityManager.<Role>createQuery()
.setParameter(Param.enabled, false)
.getResultList();
6) Find all Users, Groups and Roles that have been granted the "Payroll
Officer" role in the "Human Resources" group:
List<IdentityType> identities = identityManager.<IdentityType>createQuery()
.setParameter(Param.memberOf, identityManager.getGroup("Human
Resources"))
.setParameter(Param.memberOf, identityManager.getRole("Payroll
Officer"))
.getResultList();
7) Find all Users that have an attribute named "Citizenship" with a
value of "Greenland":
List<User> users = identityManager.<User>createQuery()
.setAttributeParameter("Citizenship", "Greenland")
.getResultList();
I'm *pretty* certain that this API is at least as capable as what we
currently have, if not more so, and IMHO provides a far simpler and more
versatile design (being able to select different IdentityTypes in a
single query I think is a big plus). I'd love to hear any feedback on
whether you like it, hate it or can think of any improvements to the
design to make it better for our developers. Also, please think
especially about additional usage scenarios and whether or not there are
any particular use cases which might be problematic for this API.
Thanks!
Shane
_______________________________________________
security-dev mailing list
security-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/security-dev
_______________________________________________
security-dev mailing list
security-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/security-dev
Software Engineer
Open Source Advocate
PGP key id: 926CCFF5
PGP key available at: