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 <[email protected]> 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, <[email protected]> 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); >>>> >>>> >> >>
