You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by ad...@apache.org on 2010/10/16 19:24:16 UTC

svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Author: adc
Date: Sat Oct 16 17:24:16 2010
New Revision: 1023335

URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
Log:
SHIRO-18 better constructors

Modified:
    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
==============================================================================
--- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
+++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
@@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
 public class CrowdRealm extends AuthorizingRealm {
 
     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
-    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
+    private final SecurityServerClient crowdClient;
     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
 
     /**
-     * Override the default mechanism of obtaining a Crowd client from a
-     * <code>SecurityServerClientFactory</code>.
+     * Initialize the Shiro Crowd realm with an instance of
+     * {@link SecurityServerClient} generated by the factory
+     * {@link SecurityServerClientFactory}.  The method
+     * {@link SecurityServerClient#authenticate} is called by this
+     * constructor.
      *
-     * @param crowdClient the crowd client to be used by the realm.
-     * @see SecurityServerClientFactory
+     * @throws InvalidAuthorizationTokenException
+     *                         if the credentials in the <code>crowd.properties</code> are not correct
+     * @throws RemoteException if the client cannot reach the Crowd server
      */
-    public void setCrowdClient(SecurityServerClient crowdClient) {
+    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
+        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
+        crowdClient.authenticate();
+    }
+
+    /**
+     * Initialize the Shiro Crowd realm with an instance of
+     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
+     * is assumed to be called by the creator of this realm.
+     *
+     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
+     */
+    public CrowdRealm(SecurityServerClient crowdClient) {
+        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
+
         this.crowdClient = crowdClient;
     }
 

Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
==============================================================================
--- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
+++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
@@ -22,7 +22,6 @@ import java.util.Arrays;
 import java.util.EnumSet;
 
 import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
-import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
 import org.junit.Test;
 import static org.easymock.EasyMock.*;
 import static org.junit.Assert.*;
@@ -41,14 +40,12 @@ public class CrowdRealmTest {
     @Test
     public void testAuthentication() throws Exception {
 
-        CrowdRealm realm = new CrowdRealm();
-        realm.setName("NutHouse");
-
         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
         replay(client);
 
-        realm.setCrowdClient(client);
+        CrowdRealm realm = new CrowdRealm(client);
+        realm.setName("NutHouse");
 
         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
 
@@ -69,15 +66,13 @@ public class CrowdRealmTest {
     @Test
     public void testDefaultRoles() throws Exception {
 
-        CrowdRealm realm = new CrowdRealm();
-        realm.setName("NutHouse");
-
         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
         replay(client);
 
-        realm.setCrowdClient(client);
+        CrowdRealm realm = new CrowdRealm(client);
+        realm.setName("NutHouse");
 
         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
@@ -92,16 +87,14 @@ public class CrowdRealmTest {
     @Test
     public void testRoleMemberships() throws Exception {
 
-        CrowdRealm realm = new CrowdRealm();
-        realm.setName("NutHouse");
-        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
-
         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
         replay(client);
 
-        realm.setCrowdClient(client);
+        CrowdRealm realm = new CrowdRealm(client);
+        realm.setName("NutHouse");
+        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
 
         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
@@ -117,16 +110,14 @@ public class CrowdRealmTest {
     @Test
     public void testGroupMemberships() throws Exception {
 
-        CrowdRealm realm = new CrowdRealm();
-        realm.setName("NutHouse");
-        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
-
         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
         replay(client);
 
-        realm.setCrowdClient(client);
+        CrowdRealm realm = new CrowdRealm(client);
+        realm.setName("NutHouse");
+        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
 
         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
@@ -140,17 +131,15 @@ public class CrowdRealmTest {
     @Test
     public void testAll() throws Exception {
 
-        CrowdRealm realm = new CrowdRealm();
-        realm.setName("NutHouse");
-        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
-
         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
         replay(client);
 
-        realm.setCrowdClient(client);
+        CrowdRealm realm = new CrowdRealm(client);
+        realm.setName("NutHouse");
+        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
 
         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
@@ -164,16 +153,13 @@ public class CrowdRealmTest {
         assertTrue(authorizationInfo.getRoles().contains("naughty"));
     }
 
+    @Test
     public void testIntegration() throws Exception {
+
         CrowdRealm realm = new CrowdRealm();
         realm.setName("NutHouse");
         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
 
-        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
-        client.authenticate();
-
-        realm.setCrowdClient(client);
-
         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
 
         assertNotNull(authenticationInfo);



Re: svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
Done.

On Oct 16, 2010, at 11:12 AM, Les Hazlewood wrote:

> One other note:
> 
> Because of IoC and INI configuration, it would probably be a good idea
> to have a getter/setter for the crowdClient instance as well.   Our
> INI at least doesn't (currently) support the notion of
> constructor-based injection.
> 
> Cheers,
> 
> Les
> 
> On Sat, Oct 16, 2010 at 11:07 AM, Les Hazlewood <lh...@apache.org> wrote:
>> Cool stuff!
>> 
>> One request though: that the public getter/setter for 'roleSources'
>> does not expose or accept a concrete collection class and instead just
>> assumes a  java.util.Set - this way it can be more-easily configured
>> in IoC environments.
>> 
>> My .02,
>> 
>> Les
>> 
>> On Sat, Oct 16, 2010 at 10:24 AM,  <ad...@apache.org> wrote:
>>> Author: adc
>>> Date: Sat Oct 16 17:24:16 2010
>>> New Revision: 1023335
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
>>> Log:
>>> SHIRO-18 better constructors
>>> 
>>> Modified:
>>>    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>>    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>> 
>>> Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>>> ==============================================================================
>>> --- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
>>> +++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
>>> @@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
>>>  public class CrowdRealm extends AuthorizingRealm {
>>> 
>>>     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
>>> -    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>>> +    private final SecurityServerClient crowdClient;
>>>     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
>>> 
>>>     /**
>>> -     * Override the default mechanism of obtaining a Crowd client from a
>>> -     * <code>SecurityServerClientFactory</code>.
>>> +     * Initialize the Shiro Crowd realm with an instance of
>>> +     * {@link SecurityServerClient} generated by the factory
>>> +     * {@link SecurityServerClientFactory}.  The method
>>> +     * {@link SecurityServerClient#authenticate} is called by this
>>> +     * constructor.
>>>      *
>>> -     * @param crowdClient the crowd client to be used by the realm.
>>> -     * @see SecurityServerClientFactory
>>> +     * @throws InvalidAuthorizationTokenException
>>> +     *                         if the credentials in the <code>crowd.properties</code> are not correct
>>> +     * @throws RemoteException if the client cannot reach the Crowd server
>>>      */
>>> -    public void setCrowdClient(SecurityServerClient crowdClient) {
>>> +    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
>>> +        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>>> +        crowdClient.authenticate();
>>> +    }
>>> +
>>> +    /**
>>> +     * Initialize the Shiro Crowd realm with an instance of
>>> +     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
>>> +     * is assumed to be called by the creator of this realm.
>>> +     *
>>> +     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
>>> +     */
>>> +    public CrowdRealm(SecurityServerClient crowdClient) {
>>> +        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
>>> +
>>>         this.crowdClient = crowdClient;
>>>     }
>>> 
>>> 
>>> Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>>> ==============================================================================
>>> --- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
>>> +++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
>>> @@ -22,7 +22,6 @@ import java.util.Arrays;
>>>  import java.util.EnumSet;
>>> 
>>>  import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
>>> -import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
>>>  import org.junit.Test;
>>>  import static org.easymock.EasyMock.*;
>>>  import static org.junit.Assert.*;
>>> @@ -41,14 +40,12 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testAuthentication() throws Exception {
>>> 
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         replay(client);
>>> 
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> 
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>> 
>>> @@ -69,15 +66,13 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testDefaultRoles() throws Exception {
>>> 
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>         replay(client);
>>> 
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> 
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -92,16 +87,14 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testRoleMemberships() throws Exception {
>>> 
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>         replay(client);
>>> 
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>> 
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -117,16 +110,14 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testGroupMemberships() throws Exception {
>>> 
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>>         replay(client);
>>> 
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>> 
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -140,17 +131,15 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testAll() throws Exception {
>>> 
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>>         replay(client);
>>> 
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>> 
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -164,16 +153,13 @@ public class CrowdRealmTest {
>>>         assertTrue(authorizationInfo.getRoles().contains("naughty"));
>>>     }
>>> 
>>> +    @Test
>>>     public void testIntegration() throws Exception {
>>> +
>>>         CrowdRealm realm = new CrowdRealm();
>>>         realm.setName("NutHouse");
>>>         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>> 
>>> -        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
>>> -        client.authenticate();
>>> -
>>> -        realm.setCrowdClient(client);
>>> -
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>> 
>>>         assertNotNull(authenticationInfo);
>>> 
>>> 


Re: svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Posted by Les Hazlewood <lh...@apache.org>.
One other note:

Because of IoC and INI configuration, it would probably be a good idea
to have a getter/setter for the crowdClient instance as well.   Our
INI at least doesn't (currently) support the notion of
constructor-based injection.

Cheers,

Les

On Sat, Oct 16, 2010 at 11:07 AM, Les Hazlewood <lh...@apache.org> wrote:
> Cool stuff!
>
> One request though: that the public getter/setter for 'roleSources'
> does not expose or accept a concrete collection class and instead just
> assumes a  java.util.Set - this way it can be more-easily configured
> in IoC environments.
>
> My .02,
>
> Les
>
> On Sat, Oct 16, 2010 at 10:24 AM,  <ad...@apache.org> wrote:
>> Author: adc
>> Date: Sat Oct 16 17:24:16 2010
>> New Revision: 1023335
>>
>> URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
>> Log:
>> SHIRO-18 better constructors
>>
>> Modified:
>>    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>
>> Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>> ==============================================================================
>> --- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
>> +++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
>> @@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
>>  public class CrowdRealm extends AuthorizingRealm {
>>
>>     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
>> -    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>> +    private final SecurityServerClient crowdClient;
>>     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
>>
>>     /**
>> -     * Override the default mechanism of obtaining a Crowd client from a
>> -     * <code>SecurityServerClientFactory</code>.
>> +     * Initialize the Shiro Crowd realm with an instance of
>> +     * {@link SecurityServerClient} generated by the factory
>> +     * {@link SecurityServerClientFactory}.  The method
>> +     * {@link SecurityServerClient#authenticate} is called by this
>> +     * constructor.
>>      *
>> -     * @param crowdClient the crowd client to be used by the realm.
>> -     * @see SecurityServerClientFactory
>> +     * @throws InvalidAuthorizationTokenException
>> +     *                         if the credentials in the <code>crowd.properties</code> are not correct
>> +     * @throws RemoteException if the client cannot reach the Crowd server
>>      */
>> -    public void setCrowdClient(SecurityServerClient crowdClient) {
>> +    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
>> +        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>> +        crowdClient.authenticate();
>> +    }
>> +
>> +    /**
>> +     * Initialize the Shiro Crowd realm with an instance of
>> +     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
>> +     * is assumed to be called by the creator of this realm.
>> +     *
>> +     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
>> +     */
>> +    public CrowdRealm(SecurityServerClient crowdClient) {
>> +        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
>> +
>>         this.crowdClient = crowdClient;
>>     }
>>
>>
>> Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>> ==============================================================================
>> --- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
>> +++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
>> @@ -22,7 +22,6 @@ import java.util.Arrays;
>>  import java.util.EnumSet;
>>
>>  import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
>> -import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
>>  import org.junit.Test;
>>  import static org.easymock.EasyMock.*;
>>  import static org.junit.Assert.*;
>> @@ -41,14 +40,12 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testAuthentication() throws Exception {
>>
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         replay(client);
>>
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>>
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>
>> @@ -69,15 +66,13 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testDefaultRoles() throws Exception {
>>
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>         replay(client);
>>
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>>
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -92,16 +87,14 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testRoleMemberships() throws Exception {
>>
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>         replay(client);
>>
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -117,16 +110,14 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testGroupMemberships() throws Exception {
>>
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>         replay(client);
>>
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -140,17 +131,15 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testAll() throws Exception {
>>
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>         replay(client);
>>
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -164,16 +153,13 @@ public class CrowdRealmTest {
>>         assertTrue(authorizationInfo.getRoles().contains("naughty"));
>>     }
>>
>> +    @Test
>>     public void testIntegration() throws Exception {
>> +
>>         CrowdRealm realm = new CrowdRealm();
>>         realm.setName("NutHouse");
>>         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>
>> -        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
>> -        client.authenticate();
>> -
>> -        realm.setCrowdClient(client);
>> -
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>
>>         assertNotNull(authenticationInfo);
>>
>>

Re: svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
EnumSet extends Set.  Since this is a set of enums then I thought it would be nice to expose that.  I see it more like being a SortedSet not a TreeSet.

Regards,
Alan

On Oct 16, 2010, at 1:28 PM, Les Hazlewood wrote:

> Even if we weren't talking about IoC, why would you want to impose a
> concrete class on API end-users?  There's nothing in the
> implementation that requires an EnumSet be used (just a Set - or even
> a Collection actually), so I recommend having the getter/setter use a
> Set instead.  The internal attribute could be initialized via an
> EnumSet however, i.e.
> 
> private Set<RoleSource> roleSources =
> EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
> 
> Cheers,
> 
> Les
> 
> On Sat, Oct 16, 2010 at 1:02 PM, Alan D. Cabrera <li...@toolazydogs.com> wrote:
>> Normally I would agree but I was thinking that since this is an enum set it might be better to return an EnumSet<>.   IoC environments, e.g. Spring, should still be able to inject since it has the same interface Set.
>> 
>> Not married to the idea and happy to change it to Set<>.  Please advise.
>> 
>> 
>> Regards,
>> Alan
>> 
>> 
>> On Oct 16, 2010, at 11:07 AM, Les Hazlewood wrote:
>> 
>>> Cool stuff!
>>> 
>>> One request though: that the public getter/setter for 'roleSources'
>>> does not expose or accept a concrete collection class and instead just
>>> assumes a  java.util.Set - this way it can be more-easily configured
>>> in IoC environments.
>>> 
>>> My .02,
>>> 
>>> Les
>>> 
>>> On Sat, Oct 16, 2010 at 10:24 AM,  <ad...@apache.org> wrote:
>>>> Author: adc
>>>> Date: Sat Oct 16 17:24:16 2010
>>>> New Revision: 1023335
>>>> 
>>>> URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
>>>> Log:
>>>> SHIRO-18 better constructors
>>>> 
>>>> Modified:
>>>>    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>>>    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>>> 
>>>> Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>>>> ==============================================================================
>>>> --- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
>>>> +++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
>>>> @@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
>>>>  public class CrowdRealm extends AuthorizingRealm {
>>>> 
>>>>     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
>>>> -    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>>>> +    private final SecurityServerClient crowdClient;
>>>>     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
>>>> 
>>>>     /**
>>>> -     * Override the default mechanism of obtaining a Crowd client from a
>>>> -     * <code>SecurityServerClientFactory</code>.
>>>> +     * Initialize the Shiro Crowd realm with an instance of
>>>> +     * {@link SecurityServerClient} generated by the factory
>>>> +     * {@link SecurityServerClientFactory}.  The method
>>>> +     * {@link SecurityServerClient#authenticate} is called by this
>>>> +     * constructor.
>>>>      *
>>>> -     * @param crowdClient the crowd client to be used by the realm.
>>>> -     * @see SecurityServerClientFactory
>>>> +     * @throws InvalidAuthorizationTokenException
>>>> +     *                         if the credentials in the <code>crowd.properties</code> are not correct
>>>> +     * @throws RemoteException if the client cannot reach the Crowd server
>>>>      */
>>>> -    public void setCrowdClient(SecurityServerClient crowdClient) {
>>>> +    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
>>>> +        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>>>> +        crowdClient.authenticate();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * Initialize the Shiro Crowd realm with an instance of
>>>> +     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
>>>> +     * is assumed to be called by the creator of this realm.
>>>> +     *
>>>> +     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
>>>> +     */
>>>> +    public CrowdRealm(SecurityServerClient crowdClient) {
>>>> +        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
>>>> +
>>>>         this.crowdClient = crowdClient;
>>>>     }
>>>> 
>>>> 
>>>> Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>>>> ==============================================================================
>>>> --- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
>>>> +++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
>>>> @@ -22,7 +22,6 @@ import java.util.Arrays;
>>>>  import java.util.EnumSet;
>>>> 
>>>>  import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
>>>> -import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
>>>>  import org.junit.Test;
>>>>  import static org.easymock.EasyMock.*;
>>>>  import static org.junit.Assert.*;
>>>> @@ -41,14 +40,12 @@ public class CrowdRealmTest {
>>>>     @Test
>>>>     public void testAuthentication() throws Exception {
>>>> 
>>>> -        CrowdRealm realm = new CrowdRealm();
>>>> -        realm.setName("NutHouse");
>>>> -
>>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>>         replay(client);
>>>> 
>>>> -        realm.setCrowdClient(client);
>>>> +        CrowdRealm realm = new CrowdRealm(client);
>>>> +        realm.setName("NutHouse");
>>>> 
>>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>> 
>>>> @@ -69,15 +66,13 @@ public class CrowdRealmTest {
>>>>     @Test
>>>>     public void testDefaultRoles() throws Exception {
>>>> 
>>>> -        CrowdRealm realm = new CrowdRealm();
>>>> -        realm.setName("NutHouse");
>>>> -
>>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>>         replay(client);
>>>> 
>>>> -        realm.setCrowdClient(client);
>>>> +        CrowdRealm realm = new CrowdRealm(client);
>>>> +        realm.setName("NutHouse");
>>>> 
>>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>>> @@ -92,16 +87,14 @@ public class CrowdRealmTest {
>>>>     @Test
>>>>     public void testRoleMemberships() throws Exception {
>>>> 
>>>> -        CrowdRealm realm = new CrowdRealm();
>>>> -        realm.setName("NutHouse");
>>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>>> -
>>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>>         replay(client);
>>>> 
>>>> -        realm.setCrowdClient(client);
>>>> +        CrowdRealm realm = new CrowdRealm(client);
>>>> +        realm.setName("NutHouse");
>>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>>> 
>>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>>> @@ -117,16 +110,14 @@ public class CrowdRealmTest {
>>>>     @Test
>>>>     public void testGroupMemberships() throws Exception {
>>>> 
>>>> -        CrowdRealm realm = new CrowdRealm();
>>>> -        realm.setName("NutHouse");
>>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>>> -
>>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>>>         replay(client);
>>>> 
>>>> -        realm.setCrowdClient(client);
>>>> +        CrowdRealm realm = new CrowdRealm(client);
>>>> +        realm.setName("NutHouse");
>>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>>> 
>>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>>> @@ -140,17 +131,15 @@ public class CrowdRealmTest {
>>>>     @Test
>>>>     public void testAll() throws Exception {
>>>> 
>>>> -        CrowdRealm realm = new CrowdRealm();
>>>> -        realm.setName("NutHouse");
>>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>>> -
>>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>>>         replay(client);
>>>> 
>>>> -        realm.setCrowdClient(client);
>>>> +        CrowdRealm realm = new CrowdRealm(client);
>>>> +        realm.setName("NutHouse");
>>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>>> 
>>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>>> @@ -164,16 +153,13 @@ public class CrowdRealmTest {
>>>>         assertTrue(authorizationInfo.getRoles().contains("naughty"));
>>>>     }
>>>> 
>>>> +    @Test
>>>>     public void testIntegration() throws Exception {
>>>> +
>>>>         CrowdRealm realm = new CrowdRealm();
>>>>         realm.setName("NutHouse");
>>>>         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>>> 
>>>> -        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
>>>> -        client.authenticate();
>>>> -
>>>> -        realm.setCrowdClient(client);
>>>> -
>>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>> 
>>>>         assertNotNull(authenticationInfo);
>>>> 
>>>> 
>> 
>> 


Re: svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Posted by Les Hazlewood <lh...@apache.org>.
Even if we weren't talking about IoC, why would you want to impose a
concrete class on API end-users?  There's nothing in the
implementation that requires an EnumSet be used (just a Set - or even
a Collection actually), so I recommend having the getter/setter use a
Set instead.  The internal attribute could be initialized via an
EnumSet however, i.e.

private Set<RoleSource> roleSources =
EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);

Cheers,

Les

On Sat, Oct 16, 2010 at 1:02 PM, Alan D. Cabrera <li...@toolazydogs.com> wrote:
> Normally I would agree but I was thinking that since this is an enum set it might be better to return an EnumSet<>.   IoC environments, e.g. Spring, should still be able to inject since it has the same interface Set.
>
> Not married to the idea and happy to change it to Set<>.  Please advise.
>
>
> Regards,
> Alan
>
>
> On Oct 16, 2010, at 11:07 AM, Les Hazlewood wrote:
>
>> Cool stuff!
>>
>> One request though: that the public getter/setter for 'roleSources'
>> does not expose or accept a concrete collection class and instead just
>> assumes a  java.util.Set - this way it can be more-easily configured
>> in IoC environments.
>>
>> My .02,
>>
>> Les
>>
>> On Sat, Oct 16, 2010 at 10:24 AM,  <ad...@apache.org> wrote:
>>> Author: adc
>>> Date: Sat Oct 16 17:24:16 2010
>>> New Revision: 1023335
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
>>> Log:
>>> SHIRO-18 better constructors
>>>
>>> Modified:
>>>    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>>    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>>
>>> Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>>> ==============================================================================
>>> --- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
>>> +++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
>>> @@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
>>>  public class CrowdRealm extends AuthorizingRealm {
>>>
>>>     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
>>> -    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>>> +    private final SecurityServerClient crowdClient;
>>>     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
>>>
>>>     /**
>>> -     * Override the default mechanism of obtaining a Crowd client from a
>>> -     * <code>SecurityServerClientFactory</code>.
>>> +     * Initialize the Shiro Crowd realm with an instance of
>>> +     * {@link SecurityServerClient} generated by the factory
>>> +     * {@link SecurityServerClientFactory}.  The method
>>> +     * {@link SecurityServerClient#authenticate} is called by this
>>> +     * constructor.
>>>      *
>>> -     * @param crowdClient the crowd client to be used by the realm.
>>> -     * @see SecurityServerClientFactory
>>> +     * @throws InvalidAuthorizationTokenException
>>> +     *                         if the credentials in the <code>crowd.properties</code> are not correct
>>> +     * @throws RemoteException if the client cannot reach the Crowd server
>>>      */
>>> -    public void setCrowdClient(SecurityServerClient crowdClient) {
>>> +    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
>>> +        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>>> +        crowdClient.authenticate();
>>> +    }
>>> +
>>> +    /**
>>> +     * Initialize the Shiro Crowd realm with an instance of
>>> +     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
>>> +     * is assumed to be called by the creator of this realm.
>>> +     *
>>> +     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
>>> +     */
>>> +    public CrowdRealm(SecurityServerClient crowdClient) {
>>> +        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
>>> +
>>>         this.crowdClient = crowdClient;
>>>     }
>>>
>>>
>>> Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>>> ==============================================================================
>>> --- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
>>> +++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
>>> @@ -22,7 +22,6 @@ import java.util.Arrays;
>>>  import java.util.EnumSet;
>>>
>>>  import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
>>> -import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
>>>  import org.junit.Test;
>>>  import static org.easymock.EasyMock.*;
>>>  import static org.junit.Assert.*;
>>> @@ -41,14 +40,12 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testAuthentication() throws Exception {
>>>
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         replay(client);
>>>
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>>
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>
>>> @@ -69,15 +66,13 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testDefaultRoles() throws Exception {
>>>
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>         replay(client);
>>>
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>>
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -92,16 +87,14 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testRoleMemberships() throws Exception {
>>>
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>         replay(client);
>>>
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>>>
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -117,16 +110,14 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testGroupMemberships() throws Exception {
>>>
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>>         replay(client);
>>>
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>>>
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -140,17 +131,15 @@ public class CrowdRealmTest {
>>>     @Test
>>>     public void testAll() throws Exception {
>>>
>>> -        CrowdRealm realm = new CrowdRealm();
>>> -        realm.setName("NutHouse");
>>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>> -
>>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>>         replay(client);
>>>
>>> -        realm.setCrowdClient(client);
>>> +        CrowdRealm realm = new CrowdRealm(client);
>>> +        realm.setName("NutHouse");
>>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>>
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>>> @@ -164,16 +153,13 @@ public class CrowdRealmTest {
>>>         assertTrue(authorizationInfo.getRoles().contains("naughty"));
>>>     }
>>>
>>> +    @Test
>>>     public void testIntegration() throws Exception {
>>> +
>>>         CrowdRealm realm = new CrowdRealm();
>>>         realm.setName("NutHouse");
>>>         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>>>
>>> -        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
>>> -        client.authenticate();
>>> -
>>> -        realm.setCrowdClient(client);
>>> -
>>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>>
>>>         assertNotNull(authenticationInfo);
>>>
>>>
>
>

Re: svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Posted by "Alan D. Cabrera" <li...@toolazydogs.com>.
Normally I would agree but I was thinking that since this is an enum set it might be better to return an EnumSet<>.   IoC environments, e.g. Spring, should still be able to inject since it has the same interface Set.

Not married to the idea and happy to change it to Set<>.  Please advise.


Regards,
Alan


On Oct 16, 2010, at 11:07 AM, Les Hazlewood wrote:

> Cool stuff!
> 
> One request though: that the public getter/setter for 'roleSources'
> does not expose or accept a concrete collection class and instead just
> assumes a  java.util.Set - this way it can be more-easily configured
> in IoC environments.
> 
> My .02,
> 
> Les
> 
> On Sat, Oct 16, 2010 at 10:24 AM,  <ad...@apache.org> wrote:
>> Author: adc
>> Date: Sat Oct 16 17:24:16 2010
>> New Revision: 1023335
>> 
>> URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
>> Log:
>> SHIRO-18 better constructors
>> 
>> Modified:
>>    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>>    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>> 
>> Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>> ==============================================================================
>> --- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
>> +++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
>> @@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
>>  public class CrowdRealm extends AuthorizingRealm {
>> 
>>     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
>> -    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>> +    private final SecurityServerClient crowdClient;
>>     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
>> 
>>     /**
>> -     * Override the default mechanism of obtaining a Crowd client from a
>> -     * <code>SecurityServerClientFactory</code>.
>> +     * Initialize the Shiro Crowd realm with an instance of
>> +     * {@link SecurityServerClient} generated by the factory
>> +     * {@link SecurityServerClientFactory}.  The method
>> +     * {@link SecurityServerClient#authenticate} is called by this
>> +     * constructor.
>>      *
>> -     * @param crowdClient the crowd client to be used by the realm.
>> -     * @see SecurityServerClientFactory
>> +     * @throws InvalidAuthorizationTokenException
>> +     *                         if the credentials in the <code>crowd.properties</code> are not correct
>> +     * @throws RemoteException if the client cannot reach the Crowd server
>>      */
>> -    public void setCrowdClient(SecurityServerClient crowdClient) {
>> +    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
>> +        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
>> +        crowdClient.authenticate();
>> +    }
>> +
>> +    /**
>> +     * Initialize the Shiro Crowd realm with an instance of
>> +     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
>> +     * is assumed to be called by the creator of this realm.
>> +     *
>> +     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
>> +     */
>> +    public CrowdRealm(SecurityServerClient crowdClient) {
>> +        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
>> +
>>         this.crowdClient = crowdClient;
>>     }
>> 
>> 
>> Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
>> ==============================================================================
>> --- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
>> +++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
>> @@ -22,7 +22,6 @@ import java.util.Arrays;
>>  import java.util.EnumSet;
>> 
>>  import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
>> -import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
>>  import org.junit.Test;
>>  import static org.easymock.EasyMock.*;
>>  import static org.junit.Assert.*;
>> @@ -41,14 +40,12 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testAuthentication() throws Exception {
>> 
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         replay(client);
>> 
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> 
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>> 
>> @@ -69,15 +66,13 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testDefaultRoles() throws Exception {
>> 
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>         replay(client);
>> 
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> 
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -92,16 +87,14 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testRoleMemberships() throws Exception {
>> 
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>         replay(client);
>> 
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>> 
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -117,16 +110,14 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testGroupMemberships() throws Exception {
>> 
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>         replay(client);
>> 
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>> 
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -140,17 +131,15 @@ public class CrowdRealmTest {
>>     @Test
>>     public void testAll() throws Exception {
>> 
>> -        CrowdRealm realm = new CrowdRealm();
>> -        realm.setName("NutHouse");
>> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>> -
>>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>>         replay(client);
>> 
>> -        realm.setCrowdClient(client);
>> +        CrowdRealm realm = new CrowdRealm(client);
>> +        realm.setName("NutHouse");
>> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>> 
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
>> @@ -164,16 +153,13 @@ public class CrowdRealmTest {
>>         assertTrue(authorizationInfo.getRoles().contains("naughty"));
>>     }
>> 
>> +    @Test
>>     public void testIntegration() throws Exception {
>> +
>>         CrowdRealm realm = new CrowdRealm();
>>         realm.setName("NutHouse");
>>         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>> 
>> -        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
>> -        client.authenticate();
>> -
>> -        realm.setCrowdClient(client);
>> -
>>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>> 
>>         assertNotNull(authenticationInfo);
>> 
>> 


Re: svn commit: r1023335 - in /shiro/sandbox/crowd/src: main/java/org/apache/shiro/realm/crowd/CrowdRealm.java test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java

Posted by Les Hazlewood <lh...@apache.org>.
Cool stuff!

One request though: that the public getter/setter for 'roleSources'
does not expose or accept a concrete collection class and instead just
assumes a  java.util.Set - this way it can be more-easily configured
in IoC environments.

My .02,

Les

On Sat, Oct 16, 2010 at 10:24 AM,  <ad...@apache.org> wrote:
> Author: adc
> Date: Sat Oct 16 17:24:16 2010
> New Revision: 1023335
>
> URL: http://svn.apache.org/viewvc?rev=1023335&view=rev
> Log:
> SHIRO-18 better constructors
>
> Modified:
>    shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
>    shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
>
> Modified: shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java
> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java?rev=1023335&r1=1023334&r2=1023335&view=diff
> ==============================================================================
> --- shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java (original)
> +++ shiro/sandbox/crowd/src/main/java/org/apache/shiro/realm/crowd/CrowdRealm.java Sat Oct 16 17:24:16 2010
> @@ -59,17 +59,35 @@ import org.apache.shiro.subject.Principa
>  public class CrowdRealm extends AuthorizingRealm {
>
>     private static final Logger LOG = LoggerFactory.getLogger(CrowdRealm.class);
> -    private SecurityServerClient crowdClient = SecurityServerClientFactory.getSecurityServerClient();
> +    private final SecurityServerClient crowdClient;
>     private EnumSet<RoleSource> roleSources = EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES);
>
>     /**
> -     * Override the default mechanism of obtaining a Crowd client from a
> -     * <code>SecurityServerClientFactory</code>.
> +     * Initialize the Shiro Crowd realm with an instance of
> +     * {@link SecurityServerClient} generated by the factory
> +     * {@link SecurityServerClientFactory}.  The method
> +     * {@link SecurityServerClient#authenticate} is called by this
> +     * constructor.
>      *
> -     * @param crowdClient the crowd client to be used by the realm.
> -     * @see SecurityServerClientFactory
> +     * @throws InvalidAuthorizationTokenException
> +     *                         if the credentials in the <code>crowd.properties</code> are not correct
> +     * @throws RemoteException if the client cannot reach the Crowd server
>      */
> -    public void setCrowdClient(SecurityServerClient crowdClient) {
> +    public CrowdRealm() throws InvalidAuthorizationTokenException, RemoteException {
> +        crowdClient = SecurityServerClientFactory.getSecurityServerClient();
> +        crowdClient.authenticate();
> +    }
> +
> +    /**
> +     * Initialize the Shiro Crowd realm with an instance of
> +     * {@link SecurityServerClient}.  The method {@link SecurityServerClient#authenticate}
> +     * is assumed to be called by the creator of this realm.
> +     *
> +     * @param crowdClient an instance of {@link SecurityServerClient} to be used when communicating with the Crowd server
> +     */
> +    public CrowdRealm(SecurityServerClient crowdClient) {
> +        if (crowdClient == null) throw new IllegalArgumentException("Crowd client cannot be null");
> +
>         this.crowdClient = crowdClient;
>     }
>
>
> Modified: shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java
> URL: http://svn.apache.org/viewvc/shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java?rev=1023335&r1=1023334&r2=1023335&view=diff
> ==============================================================================
> --- shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java (original)
> +++ shiro/sandbox/crowd/src/test/java/org/apache/shiro/realm/crowd/CrowdRealmTest.java Sat Oct 16 17:24:16 2010
> @@ -22,7 +22,6 @@ import java.util.Arrays;
>  import java.util.EnumSet;
>
>  import com.atlassian.crowd.integration.service.soap.client.SecurityServerClient;
> -import com.atlassian.crowd.integration.service.soap.client.SecurityServerClientFactory;
>  import org.junit.Test;
>  import static org.easymock.EasyMock.*;
>  import static org.junit.Assert.*;
> @@ -41,14 +40,12 @@ public class CrowdRealmTest {
>     @Test
>     public void testAuthentication() throws Exception {
>
> -        CrowdRealm realm = new CrowdRealm();
> -        realm.setName("NutHouse");
> -
>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>         replay(client);
>
> -        realm.setCrowdClient(client);
> +        CrowdRealm realm = new CrowdRealm(client);
> +        realm.setName("NutHouse");
>
>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>
> @@ -69,15 +66,13 @@ public class CrowdRealmTest {
>     @Test
>     public void testDefaultRoles() throws Exception {
>
> -        CrowdRealm realm = new CrowdRealm();
> -        realm.setName("NutHouse");
> -
>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>         replay(client);
>
> -        realm.setCrowdClient(client);
> +        CrowdRealm realm = new CrowdRealm(client);
> +        realm.setName("NutHouse");
>
>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
> @@ -92,16 +87,14 @@ public class CrowdRealmTest {
>     @Test
>     public void testRoleMemberships() throws Exception {
>
> -        CrowdRealm realm = new CrowdRealm();
> -        realm.setName("NutHouse");
> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
> -
>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>         replay(client);
>
> -        realm.setCrowdClient(client);
> +        CrowdRealm realm = new CrowdRealm(client);
> +        realm.setName("NutHouse");
> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_ROLES));
>
>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
> @@ -117,16 +110,14 @@ public class CrowdRealmTest {
>     @Test
>     public void testGroupMemberships() throws Exception {
>
> -        CrowdRealm realm = new CrowdRealm();
> -        realm.setName("NutHouse");
> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
> -
>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>         replay(client);
>
> -        realm.setCrowdClient(client);
> +        CrowdRealm realm = new CrowdRealm(client);
> +        realm.setName("NutHouse");
> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS));
>
>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
> @@ -140,17 +131,15 @@ public class CrowdRealmTest {
>     @Test
>     public void testAll() throws Exception {
>
> -        CrowdRealm realm = new CrowdRealm();
> -        realm.setName("NutHouse");
> -        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
> -
>         SecurityServerClient client = createStrictMock(SecurityServerClient.class);
>         expect(client.authenticatePrincipalSimple("yoko", "barbie")).andReturn("UNUSED");
>         expect(client.findRoleMemberships("yoko")).andReturn(new String[]{"big_sister", "table_setter", "dog_walker"});
>         expect(client.findGroupMemberships("yoko")).andReturn(new String[]{"girls", "naughty"});
>         replay(client);
>
> -        realm.setCrowdClient(client);
> +        CrowdRealm realm = new CrowdRealm(client);
> +        realm.setName("NutHouse");
> +        realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>
>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>         AuthorizationInfo authorizationInfo = realm.doGetAuthorizationInfo(authenticationInfo.getPrincipals());
> @@ -164,16 +153,13 @@ public class CrowdRealmTest {
>         assertTrue(authorizationInfo.getRoles().contains("naughty"));
>     }
>
> +    @Test
>     public void testIntegration() throws Exception {
> +
>         CrowdRealm realm = new CrowdRealm();
>         realm.setName("NutHouse");
>         realm.setRoleSources(EnumSet.of(RoleSource.ROLES_FROM_CROWD_GROUPS, RoleSource.ROLES_FROM_CROWD_ROLES));
>
> -        SecurityServerClient client = SecurityServerClientFactory.getSecurityServerClient();
> -        client.authenticate();
> -
> -        realm.setCrowdClient(client);
> -
>         AuthenticationInfo authenticationInfo = realm.doGetAuthenticationInfo(new UsernamePasswordToken("yoko", "barbie"));
>
>         assertNotNull(authenticationInfo);
>
>