On Wed, Nov 7, 2012 at 1:11 PM, Shane Bryzak <span dir="ltr"><<a href="mailto:sbryzak@redhat.com" target="_blank">sbryzak@redhat.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>On a somewhat related note, we should
probably fix the constructor for SimpleGroup also, which currently
looks like this:<br>
<br>
public SimpleGroup(String id, String name, Group parentGroup)<br>
<br>
Having both id and parentGroup parameters is redundant, so I
suggest removing the id parameter (and removing the id field
altogether) and instead have the getId() method return a
calculated id.<div class="im"></div></div></div></blockquote><div><br></div><div>+1</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">
<div><div class="im">On 11/08/2012 06:08 AM, Shane Bryzak wrote:<br>
</div></div><div class="im">
<blockquote type="cite">
<div>They're actually a fundamental part
of the identity model (see [1]). I have no real problem with
the principle of removing the String versions of createGroup()
(we would also have to do the same to createRole() for
consistency) and in fact it would provide some additional
advantages. For example, being able to set a Group to being
disabled at creation time, setting attribute values, etc. My
only concern is from a coding "correctness" point of view, and I
guess is centered around the creation date being automatically
set (or potentially overridden) on the Group instance that's
passed to createGroup(). It's probably not an important concern
though, and I'm happy to concede on this one which would mean we
end up with the following methods (replacing all existing
createGroup() and createRole() methods):<br>
<br>
void createGroup(Group group);<br>
<br>
void createRole(Role role);<br>
<br>
<br>
[1]
<a href="https://github.com/picketlink/picketlink/blob/master/idm/api/src/main/java/org/picketlink/idm/model/IdentityType.java" target="_blank">https://github.com/picketlink/picketlink/blob/master/idm/api/src/main/java/org/picketlink/idm/model/IdentityType.java</a><br>
<br>
On 11/08/2012 05:37 AM, Jason Porter wrote:<br>
</div>
<blockquote type="cite">Aren't those implementation details though?</blockquote>
<br>
</blockquote>
<br>
</div></div>
<br>_______________________________________________<br>
security-dev mailing list<br>
<a href="mailto:security-dev@lists.jboss.org">security-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/security-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/security-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Jason Porter<br><a href="http://lightguard-jp.blogspot.com" target="_blank">http://lightguard-jp.blogspot.com</a><br><a 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 href="http://keyserver.net" target="_blank">keyserver.net</a>, <a href="http://pgp.mit.edu" target="_blank">pgp.mit.edu</a><br>
</div>