<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>On Oct 29, 2012, at 17:20, 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 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.&nbsp;</div><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></body></html>