<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>On Oct 29, 2012, at 21:31, Shane Bryzak &lt;<a href="mailto:sbryzak@redhat.com">sbryzak@redhat.com</a>&gt; wrote:</div><div><br></div><blockquote type="cite"><div>
  
    <meta content="text/html; charset=UTF-8" 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=UTF-8">
      <div>On Oct 29, 2012, at 17:20, Shane Bryzak &lt;<a moz-do-not-send="true" href="mailto:sbryzak@redhat.com">sbryzak@redhat.com</a>&gt;
        wrote:</div>
      <div><br>
      </div>
      <blockquote type="cite">
        <div>
          <meta content="text/html; charset=UTF-8" 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. &nbsp;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.&nbsp; 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>
          &nbsp;&nbsp;&nbsp; public static final QueryParameter FIRST_NAME = new
          QueryParameter() {};<br>
          &nbsp;&nbsp;&nbsp; public static final QueryParameter LAST_NAME = new
          QueryParameter() {};<br>
          &nbsp;&nbsp;&nbsp; ...<br>
          }<br>
          <br>
          Then the method signature for IdentityQuery.setParameter would
          look something like this:<br>
          <br>
          IdentityQuery&lt;T&gt; 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&lt;TYPE&gt; and have a void setValue(TYPE val), a
        &lt;TYPE&gt; 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.&nbsp;</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.&nbsp;</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">&lt;<a moz-do-not-send="true" href="mailto:psilva@redhat.com" target="_blank">psilva@redhat.com</a>&gt;</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>
                &nbsp; &nbsp; &nbsp; &nbsp; IdentityManager identityManager = // get the
                identity manager;<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; Query&lt;User&gt; query =
                identityManager.&lt;User&gt;createQuery();<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; query<br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .limit(100)<br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .offset(1)<br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .where()<br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .id("1")<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .property(User.FIRST_NAME,
                OPERATOR.equals, "John")<br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .property(User.EXPIRATION_DATE,
                OPERATOR.lessThan, new Date())<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .attribute("ssn", OPERATOR.equals,
                "SSN-123")<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .role(new SimpleRole("Payroll Officer"))<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; .group(new SimpleGroup("Superuser",
                "Superuser", null));<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; List&lt;User&gt; list = query.list();<br>
                <br>
                &nbsp; &nbsp; &nbsp; &nbsp; for (User user : list) {<br>
                &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; // handle users<br>
                &nbsp; &nbsp; &nbsp; &nbsp; }<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" &lt;<a moz-do-not-send="true" href="mailto:sbryzak@redhat.com">sbryzak@redhat.com</a>&gt;<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. &nbsp;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>
                    &nbsp; &nbsp; &nbsp;// &lt;snip other methods&gt;<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;&lt;T extends IdentityType&gt;
                    IdentityQuery&lt;T&gt; createQuery();<br>
                    }<br>
                    <br>
                    public interface IdentityQuery&lt;T extends
                    IdentityType&gt; {<br>
                    &nbsp; &nbsp; &nbsp;public enum Param {id, key, created, expired,
                    enabled, firstName,<br>
                    lastName, email, name, parent, memberOf};<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;public enum Operator { equals, notEquals,
                    greaterThan, lessThan };<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;IdentityQuery&lt;T&gt; reset();<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;IdentityQuery&lt;T&gt; setParameter(Param
                    param, Object value);<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;IdentityQuery&lt;T&gt; setParameter(Param
                    param, Operator operator,<br>
                    Object value);<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;IdentityQuery&lt;T&gt;
                    setAttributeParameter(String attributeName, Object<br>
                    value);<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;IdentityQuery&lt;T&gt;
                    setAttributeParameter(String attributeName,<br>
                    Operator operator, Object value);<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;IdentityQuery&lt;T&gt; setRange(Range range);<br>
                    <br>
                    &nbsp; &nbsp; &nbsp;List&lt;T&gt; getResultList();<br>
                    }<br>
                    <br>
                    This unified API basically replaces the 4 separate
                    existing interfaces<br>
                    we currently have; UserQuery, RoleQuery, GroupQuery
                    and<br>
                    MembershipQuery. &nbsp;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&lt;User&gt; users =
                    identityManager.&lt;User&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.firstName, "John")<br>
                    &nbsp; &nbsp; &nbsp;.getResultList();<br>
                    <br>
                    2) Find all expired users:<br>
                    <br>
                    List&lt;User&gt; users =
                    identityManager.&lt;User&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.expired, Operator.lessThan,
                    new Date())<br>
                    &nbsp; &nbsp; &nbsp;.getResultList();<br>
                    <br>
                    3) Find all users that are a member of the
                    "Superuser" group<br>
                    <br>
                    List&lt;User&gt; users =
                    identityManager.&lt;User&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.memberOf,
                    identityManager.getGroup("Superuser"))<br>
                    &nbsp; &nbsp; &nbsp;.getResultList();<br>
                    <br>
                    4) Find all sub-groups of the "Employees" group:<br>
                    <br>
                    List&lt;Group&gt; groups =
                    identityManager.&lt;Group&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.memberOf,
                    identityManager.getGroup("Employees"))<br>
                    &nbsp; &nbsp; &nbsp;.getResultList();<br>
                    <br>
                    5) Find all disabled roles:<br>
                    <br>
                    List&lt;Role&gt; roles =
                    identityManager.&lt;Role&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.enabled, false)<br>
                    &nbsp; &nbsp; &nbsp;.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&lt;IdentityType&gt; identities =
                    identityManager.&lt;IdentityType&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.memberOf,
                    identityManager.getGroup("Human<br>
                    Resources"))<br>
                    &nbsp; &nbsp; &nbsp;.setParameter(Param.memberOf,
                    identityManager.getRole("Payroll<br>
                    Officer"))<br>
                    &nbsp; &nbsp; &nbsp;.getResultList();<br>
                    <br>
                    7) Find all Users that have an attribute named
                    "Citizenship" with a<br>
                    value of "Greenland":<br>
                    <br>
                    List&lt;User&gt; users =
                    identityManager.&lt;User&gt;createQuery()<br>
                    &nbsp; &nbsp; &nbsp;.setAttributeParameter("Citizenship",
                    "Greenland")<br>
                    &nbsp; &nbsp; &nbsp;.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). &nbsp;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></body></html>