<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">I knew I shouldn't have clicked the
"Send" button so quickly. :) After 5 minutes of thinking about
this, there would be no reason to deprecate the Query API. JPA
supports both a query language and a criteria API, so there's no
reason why we couldn't do both. <br>
<br>
On 10/31/2012 08:52 AM, Shane Bryzak wrote:<br>
</div>
<blockquote cite="mid:50905A42.6050901@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">I've made some further minor changes
to the Query API, specifically in the area of memberships. By
introducing a new QueryParameter implementation called
MembershipParameter, we are now able to support queries that
look like this:<br>
<br>
List<User> users = this.<User>createQuery()<br>
.setParameter(User.MEMBER_OF.group("Head
Office").role("Admin"), true)<br>
.setParameter(User.MEMBER_OF.group("Springfield"), false)<br>
.setParameter(User.MEMBER_OF.role("Contractor"),
false)<br>
.getResultList();<br>
<br>
Translation of the above code: return all users that have the
"Admin" role in the "Head Office" group, but aren't a member of
the "Springfield" group and don't have the application role
"Contractor". <br>
<br>
This support for membership exclusion gives us a little bit more
flexibility in what we can return from the IdentityQuery. I
have to say though, I'm still not 100% satisfied with the Query
API - my opinion is that we should mark this feature as already
deprecated in the 3.0 release, and in 3.1 introduce a proper
query language (PLQL - PicketLink Query Language) where the
developer is able to provide free-form queries, e.g. being able
to run something like "SELECT User WHERE MEMBER_OF(Group("Head
Office"), Role("Office Administrator")) and enabled = true and
createdDate < :createdDate and not in (SELECT User WHERE NOT
MEMBER_OF("Admin"))". <br>
<br>
Maybe this feature would be going too far, so please let me know
if you think it is.<br>
<br>
<br>
On 10/30/2012 08:41 PM, Shane Bryzak wrote:<br>
</div>
<blockquote cite="mid:508FAEE4.3050202@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">I've managed to trim the
IdentityQuery API down to just this:<br>
<br>
public interface IdentityQuery<T extends IdentityType> {<br>
public enum Operator { equals, notEquals, greaterThan,
lessThan };<br>
IdentityQuery<T> setOffset(int offset);<br>
IdentityQuery<T> setLimit(int limit);<br>
IdentityQuery<T> setParameter(QueryParameter param,
Object value);<br>
IdentityQuery<T> setParameter(QueryParameter param,
Operator operator, Object value);<br>
List<T> getResultList();<br>
}<br>
<br>
While I did like the concept of Pedro's method refactor, I
think that sticking with an API that mimics that of JPA (which
most developers should be already familiar with) will be more
intuitive.<br>
<br>
To get rid of the setAttribute() methods, I had to do a little
bit of hackery by adding this to the IdentityType interface:<br>
<br>
public class AttributeParameter implements QueryParameter
{<br>
private String name;<br>
public AttributeParameter(String name) {<br>
this.name = name;<br>
}<br>
public String getName() {<br>
return name;<br>
}<br>
}<br>
public final class PARAM_ATTRIBUTE {<br>
public static AttributeParameter byName(String name) {<br>
return new AttributeParameter(name);<br>
}<br>
}<br>
<br>
This then allows us to simply use the setParameter() method to
set attribute values also, like so:<br>
<br>
List<User> users = im.<User>createQuery()<br>
.setParameter(User.PARAM_ENABLED, true)<br>
.setParameter(User.PARAM_ATTRIBUTE.byName("foo"), "bar")<br>
.getResultList();<br>
<br>
I'm still trying to decide if I like this or not. I guess it
somewhat simplifies usage as the developer can follow the same
pattern for setting all parameter values. What do you guys
think about it?<br>
<br>
<br>
On 10/30/2012 01:49 PM, Jason Porter wrote:<br>
</div>
<blockquote
cite="mid:61BF871B-C98B-4D3A-83E5-5DCD2EFD07EC@gmail.com"
type="cite">
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<div>On Oct 29, 2012, at 21:31, Shane Bryzak <<a
moz-do-not-send="true" href="mailto:sbryzak@redhat.com">sbryzak@redhat.com</a>>
wrote:</div>
<div><br>
</div>
<blockquote type="cite">
<div>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 10/30/2012 12:32 PM, Jason
Porter wrote:<br>
</div>
<blockquote
cite="mid:342B0DF8-E14E-4440-9863-F16670BD6FCE@gmail.com"
type="cite">
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<div>On Oct 29, 2012, at 17:20, Shane Bryzak <<a
moz-do-not-send="true"
href="mailto:sbryzak@redhat.com">sbryzak@redhat.com</a>>
wrote:</div>
<div><br>
</div>
<blockquote type="cite">
<div>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 10/30/2012 01:20 AM,
Jason Porter wrote:<br>
</div>
<blockquote
cite="mid:CAF9TksM6DKBaAsqr+HannN-vHptPCUFN4m-_=8MNCeOW6_CLwg@mail.gmail.com"
type="cite">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.<br>
</blockquote>
<br>
I agree, I wasn't a fan of using an enum either but
at the time I hashed this out I couldn't think of a
better alternative. How about instead of enums we
introduce a marker interface like this:<br>
<br>
public interface QueryParameter {<br>
<br>
}<br>
<br>
Then we can simply define supported parameters
directly on the core identity model interfaces, as
Pedro has suggested:<br>
<br>
public interface User extends IdentityType,
Serializable {<br>
public static final QueryParameter FIRST_NAME =
new QueryParameter() {};<br>
public static final QueryParameter LAST_NAME =
new QueryParameter() {};<br>
...<br>
}<br>
<br>
Then the method signature for
IdentityQuery.setParameter would look something like
this:<br>
<br>
IdentityQuery<T> setParameter(QueryParameter
param, Object value);<br>
<br>
This then makes the parameter set extensible (anyone
can create their own) and we can create an SPI (or
just make the IdentityStore implementations
extensible themselves) so that developers can
provide support for their own custom query
parameters (not to mention it makes it more future
proof if we want to add more parameter types later).<br>
</div>
</blockquote>
<div><br>
</div>
<div>Any reason not to do a generic interface or
abstract class? QueryParam<TYPE> and have a void
setValue(TYPE val), a <TYPE> getValue() and a
String getName() on that interface/class? That would
cut down on an argument. <br>
</div>
</blockquote>
<br>
I'm not quite getting this - could you give an example of
what the API would look like?<br>
</div>
</blockquote>
<div><br>
</div>
<div>I've thought about it a bit more and like your proposal
better for the enhanced readability. </div>
<br>
<blockquote type="cite">
<div>
<blockquote
cite="mid:342B0DF8-E14E-4440-9863-F16670BD6FCE@gmail.com"
type="cite">
<div><br>
</div>
<div>Though the final empty interfaces from the classes
does read nicely. </div>
<br>
<blockquote type="cite">
<div>
<blockquote
cite="mid:CAF9TksM6DKBaAsqr+HannN-vHptPCUFN4m-_=8MNCeOW6_CLwg@mail.gmail.com"
type="cite"><br>
<div class="gmail_quote">On Mon, Oct 29, 2012 at
7:10 AM, Pedro Igor Silva <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:psilva@redhat.com"
target="_blank">psilva@redhat.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex"> I liked your
proposal, it is simple and clear.<br>
<br>
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.<br>
<br>
IdentityManager identityManager = //
get the identity manager;<br>
<br>
Query<User> query =
identityManager.<User>createQuery();<br>
<br>
query<br>
.limit(100)<br>
.offset(1)<br>
.where()<br>
.id("1")<br>
<br>
.property(User.FIRST_NAME,
OPERATOR.equals, "John")<br>
.property(User.EXPIRATION_DATE,
OPERATOR.lessThan, new Date())<br>
<br>
.attribute("ssn",
OPERATOR.equals, "SSN-123")<br>
<br>
.role(new SimpleRole("Payroll
Officer"))<br>
<br>
.group(new
SimpleGroup("Superuser", "Superuser", null));<br>
<br>
List<User> list = query.list();<br>
<br>
for (User user : list) {<br>
// handle users<br>
}<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
Not sure if we need the query.reset() method
... Why not just discard the query instance
and build another one ?<br>
<br>
Regards.<br>
<span class="HOEnZb"><font color="#888888">Pedro
Igor<br>
</font></span>
<div class="HOEnZb">
<div class="h5"><br>
----- Original Message -----<br>
From: "Shane Bryzak" <<a
moz-do-not-send="true"
href="mailto:sbryzak@redhat.com">sbryzak@redhat.com</a>><br>
To: <a moz-do-not-send="true"
href="mailto:security-dev@lists.jboss.org">security-dev@lists.jboss.org</a><br>
Sent: Monday, October 29, 2012 6:38:14 AM<br>
Subject: [security-dev] IdentityManager
review - queries<br>
<br>
I've started reviewing the IdentityManager
interface to see where we can<br>
improve the API. The first area I'd like
to visit is the Query API, of<br>
which I've come to the conclusion that we
need to do some serious<br>
redesign - the current API is
non-intuitive, too verbose and not future<br>
proof.<br>
<br>
What I'd like to do is throw it all out
and start again, replacing it<br>
with a new cleaner API that looks
something like this:<br>
<br>
public interface IdentityManager {<br>
// <snip other methods><br>
<br>
<T extends IdentityType>
IdentityQuery<T> createQuery();<br>
}<br>
<br>
public interface IdentityQuery<T
extends IdentityType> {<br>
public enum Param {id, key, created,
expired, enabled, firstName,<br>
lastName, email, name, parent, memberOf};<br>
<br>
public enum Operator { equals,
notEquals, greaterThan, lessThan };<br>
<br>
IdentityQuery<T> reset();<br>
<br>
IdentityQuery<T>
setParameter(Param param, Object value);<br>
<br>
IdentityQuery<T>
setParameter(Param param, Operator
operator,<br>
Object value);<br>
<br>
IdentityQuery<T>
setAttributeParameter(String
attributeName, Object<br>
value);<br>
<br>
IdentityQuery<T>
setAttributeParameter(String
attributeName,<br>
Operator operator, Object value);<br>
<br>
IdentityQuery<T> setRange(Range
range);<br>
<br>
List<T> getResultList();<br>
}<br>
<br>
This unified API basically replaces the 4
separate existing interfaces<br>
we currently have; UserQuery, RoleQuery,
GroupQuery and<br>
MembershipQuery. I've put together a few
usage scenarios to show how it<br>
might work:<br>
<br>
1) Find users with first name 'John':<br>
<br>
List<User> users =
identityManager.<User>createQuery()<br>
.setParameter(Param.firstName,
"John")<br>
.getResultList();<br>
<br>
2) Find all expired users:<br>
<br>
List<User> users =
identityManager.<User>createQuery()<br>
.setParameter(Param.expired,
Operator.lessThan, new Date())<br>
.getResultList();<br>
<br>
3) Find all users that are a member of the
"Superuser" group<br>
<br>
List<User> users =
identityManager.<User>createQuery()<br>
.setParameter(Param.memberOf,
identityManager.getGroup("Superuser"))<br>
.getResultList();<br>
<br>
4) Find all sub-groups of the "Employees"
group:<br>
<br>
List<Group> groups =
identityManager.<Group>createQuery()<br>
.setParameter(Param.memberOf,
identityManager.getGroup("Employees"))<br>
.getResultList();<br>
<br>
5) Find all disabled roles:<br>
<br>
List<Role> roles =
identityManager.<Role>createQuery()<br>
.setParameter(Param.enabled, false)<br>
.getResultList();<br>
<br>
6) Find all Users, Groups and Roles that
have been granted the "Payroll<br>
Officer" role in the "Human Resources"
group:<br>
<br>
List<IdentityType> identities =
identityManager.<IdentityType>createQuery()<br>
.setParameter(Param.memberOf,
identityManager.getGroup("Human<br>
Resources"))<br>
.setParameter(Param.memberOf,
identityManager.getRole("Payroll<br>
Officer"))<br>
.getResultList();<br>
<br>
7) Find all Users that have an attribute
named "Citizenship" with a<br>
value of "Greenland":<br>
<br>
List<User> users =
identityManager.<User>createQuery()<br>
.setAttributeParameter("Citizenship",
"Greenland")<br>
.getResultList();<br>
<br>
I'm *pretty* certain that this API is at
least as capable as what we<br>
currently have, if not more so, and IMHO
provides a far simpler and more<br>
versatile design (being able to select
different IdentityTypes in a<br>
single query I think is a big plus). I'd
love to hear any feedback on<br>
whether you like it, hate it or can think
of any improvements to the<br>
design to make it better for our
developers. Also, please think<br>
especially about additional usage
scenarios and whether or not there are<br>
any particular use cases which might be
problematic for this API.<br>
<br>
<br>
Thanks!<br>
Shane<br>
_______________________________________________<br>
security-dev mailing list<br>
<a moz-do-not-send="true"
href="mailto:security-dev@lists.jboss.org">security-dev@lists.jboss.org</a><br>
<a moz-do-not-send="true"
href="https://lists.jboss.org/mailman/listinfo/security-dev"
target="_blank">https://lists.jboss.org/mailman/listinfo/security-dev</a><br>
_______________________________________________<br>
security-dev mailing list<br>
<a moz-do-not-send="true"
href="mailto:security-dev@lists.jboss.org">security-dev@lists.jboss.org</a><br>
<a moz-do-not-send="true"
href="https://lists.jboss.org/mailman/listinfo/security-dev"
target="_blank">https://lists.jboss.org/mailman/listinfo/security-dev</a><br>
</div>
</div>
</blockquote>
</div>
<br>
<br clear="all">
<div><br>
</div>
-- <br>
Jason Porter<br>
<a moz-do-not-send="true"
href="http://lightguard-jp.blogspot.com"
target="_blank">http://lightguard-jp.blogspot.com</a><br>
<a moz-do-not-send="true"
href="http://twitter.com/lightguardjp"
target="_blank">http://twitter.com/lightguardjp</a><br>
<br>
Software Engineer<br>
Open Source Advocate<br>
<br>
PGP key id: 926CCFF5<br>
PGP key available at: <a moz-do-not-send="true"
href="http://keyserver.net" target="_blank">keyserver.net</a>,
<a moz-do-not-send="true"
href="http://pgp.mit.edu" target="_blank">pgp.mit.edu</a><br>
</blockquote>
<br>
</div>
</blockquote>
</blockquote>
<br>
</div>
</blockquote>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
security-dev mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:security-dev@lists.jboss.org">security-dev@lists.jboss.org</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://lists.jboss.org/mailman/listinfo/security-dev">https://lists.jboss.org/mailman/listinfo/security-dev</a>
</pre>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
security-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:security-dev@lists.jboss.org">security-dev@lists.jboss.org</a>
<a class="moz-txt-link-freetext" href="https://lists.jboss.org/mailman/listinfo/security-dev">https://lists.jboss.org/mailman/listinfo/security-dev</a>
</pre>
</blockquote>
<br>
</body>
</html>