You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by lh...@apache.org on 2013/05/05 00:17:38 UTC
svn commit: r1479202 - in /shiro/branches/1.2.x/core/src:
main/java/org/apache/shiro/realm/SimpleAccountRealm.java
main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java
test/java/org/apache/shiro/realm/text/TextConfigurationRealmTest.java
Author: lhazlewood
Date: Sat May 4 22:17:38 2013
New Revision: 1479202
URL: http://svn.apache.org/r1479202
Log:
SHIRO-379: patch applied but with improvements (using concurrent read/write locks)
Added:
shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/realm/text/TextConfigurationRealmTest.java
Modified:
shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/SimpleAccountRealm.java
shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java
Modified: shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/SimpleAccountRealm.java
URL: http://svn.apache.org/viewvc/shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/SimpleAccountRealm.java?rev=1479202&r1=1479201&r2=1479202&view=diff
==============================================================================
--- shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/SimpleAccountRealm.java (original)
+++ shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/SimpleAccountRealm.java Sat May 4 22:17:38 2013
@@ -28,6 +28,9 @@ import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
* A simple implementation of the {@link Realm Realm} interface that
@@ -43,13 +46,16 @@ import java.util.Set;
public class SimpleAccountRealm extends AuthorizingRealm {
//TODO - complete JavaDoc
-
protected final Map<String, SimpleAccount> users; //username-to-SimpleAccount
protected final Map<String, SimpleRole> roles; //roleName-to-SimpleRole
+ protected final ReadWriteLock USERS_LOCK;
+ protected final ReadWriteLock ROLES_LOCK;
public SimpleAccountRealm() {
this.users = new LinkedHashMap<String, SimpleAccount>();
this.roles = new LinkedHashMap<String, SimpleRole>();
+ USERS_LOCK = new ReentrantReadWriteLock();
+ ROLES_LOCK = new ReentrantReadWriteLock();
//SimpleAccountRealms are memory-only realms - no need for an additional cache mechanism since we're
//already as memory-efficient as we can be:
setCachingEnabled(false);
@@ -61,7 +67,12 @@ public class SimpleAccountRealm extends
}
protected SimpleAccount getUser(String username) {
- return this.users.get(username);
+ USERS_LOCK.readLock().lock();
+ try {
+ return this.users.get(username);
+ } finally {
+ USERS_LOCK.readLock().unlock();
+ }
}
public boolean accountExists(String username) {
@@ -88,11 +99,21 @@ public class SimpleAccountRealm extends
protected void add(SimpleAccount account) {
String username = getUsername(account);
- this.users.put(username, account);
+ USERS_LOCK.writeLock().lock();
+ try {
+ this.users.put(username, account);
+ } finally {
+ USERS_LOCK.writeLock().unlock();
+ }
}
protected SimpleRole getRole(String rolename) {
- return roles.get(rolename);
+ ROLES_LOCK.readLock().lock();
+ try {
+ return roles.get(rolename);
+ } finally {
+ ROLES_LOCK.readLock().unlock();
+ }
}
public boolean roleExists(String name) {
@@ -104,7 +125,12 @@ public class SimpleAccountRealm extends
}
protected void add(SimpleRole role) {
- roles.put(role.getName(), role);
+ ROLES_LOCK.writeLock().lock();
+ try {
+ roles.put(role.getName(), role);
+ } finally {
+ ROLES_LOCK.writeLock().unlock();
+ }
}
protected static Set<String> toSet(String delimited, String delimiter) {
@@ -144,6 +170,12 @@ public class SimpleAccountRealm extends
}
protected AuthorizationInfo doGetAuthorizationInfo(PrincipalCollection principals) {
- return this.users.get(getUsername(principals));
+ String username = getUsername(principals);
+ USERS_LOCK.readLock().lock();
+ try {
+ return this.users.get(username);
+ } finally {
+ USERS_LOCK.readLock().unlock();
+ }
}
}
\ No newline at end of file
Modified: shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java
URL: http://svn.apache.org/viewvc/shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java?rev=1479202&r1=1479201&r2=1479202&view=diff
==============================================================================
--- shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java (original)
+++ shiro/branches/1.2.x/core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java Sat May 4 22:17:38 2013
@@ -47,8 +47,8 @@ public class TextConfigurationRealm exte
//TODO - complete JavaDoc
- private String userDefinitions;
- private String roleDefinitions;
+ private volatile String userDefinitions;
+ private volatile String roleDefinitions;
public TextConfigurationRealm() {
super();
@@ -143,7 +143,6 @@ public class TextConfigurationRealm exte
if (roleDefs == null || roleDefs.isEmpty()) {
return;
}
-
for (String rolename : roleDefs.keySet()) {
String value = roleDefs.get(rolename);
@@ -159,7 +158,6 @@ public class TextConfigurationRealm exte
}
protected void processUserDefinitions() throws ParseException {
-
String userDefinitions = getUserDefinitions();
if (userDefinitions == null) {
return;
@@ -174,7 +172,6 @@ public class TextConfigurationRealm exte
if (userDefs == null || userDefs.isEmpty()) {
return;
}
-
for (String username : userDefs.keySet()) {
String value = userDefs.get(username);
Added: shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/realm/text/TextConfigurationRealmTest.java
URL: http://svn.apache.org/viewvc/shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/realm/text/TextConfigurationRealmTest.java?rev=1479202&view=auto
==============================================================================
--- shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/realm/text/TextConfigurationRealmTest.java (added)
+++ shiro/branches/1.2.x/core/src/test/java/org/apache/shiro/realm/text/TextConfigurationRealmTest.java Sat May 4 22:17:38 2013
@@ -0,0 +1,264 @@
+package org.apache.shiro.realm.text;
+
+import org.apache.shiro.authz.AuthorizationException;
+import org.apache.shiro.subject.PrincipalCollection;
+import org.apache.shiro.subject.SimplePrincipalCollection;
+import org.junit.Test;
+
+import java.text.ParseException;
+import java.util.Arrays;
+
+import static org.junit.Assert.*;
+
+public class TextConfigurationRealmTest {
+
+ private TestRealm realm;
+
+ private void setRoles() {
+ StringBuilder roleDefinitions = new StringBuilder()
+ .append("role1 = role1_permission1\n")
+ .append("role2 = role2_persission1, role2_permission2\n");
+ realm.setRoleDefinitions(roleDefinitions.toString());
+ }
+
+ private void setUsers() {
+ StringBuilder userDefinitions = new StringBuilder();
+ for (int i = 1; i < 3; i++) {
+ userDefinitions.append(String.format("user%1$d = user%1$d_password, role1, role2%n", i));
+ }
+ realm.setUserDefinitions(userDefinitions.toString());
+ }
+
+ private void setUpForReadConfigurationTest() {
+ realm = new TestRealm() {
+ /*
+ * Demonstrates that a lock can't be obtained on the realm by a read thread until after
+ * the lock is released.
+ */
+ public void test(Thread runnable) throws InterruptedException {
+ // Obtain the realm's locks
+ USERS_LOCK.writeLock().lock();
+ try {
+ ROLES_LOCK.writeLock().lock();
+ try {
+ // Any read threads attempting to obtain the realms lock will block
+ runnable.start();
+ Thread.sleep(500);
+ // Process role and user definitions
+ realm.onInit();
+
+ } finally {
+ ROLES_LOCK.writeLock().unlock();
+ }
+ } finally {
+ USERS_LOCK.writeLock().unlock();
+ }
+ }
+ };
+ setRoles();
+ setUsers();
+ }
+
+ /*
+ * Executes a test that attempts to read to read from a realm before it is loaded.
+ */
+ private void executeTest(Runnable runnable) throws InterruptedException {
+ TestThread testThread = new TestThread(runnable);
+ Thread testTask = new Thread(testThread);
+ realm.test(testTask);
+ testTask.join(500);
+ // Check whether any assertion error was thrown by the read thread
+ testThread.test();
+ }
+
+ /*
+ * Tests that roles and account can't be tested while the realm is being loaded.
+ */
+ @Test
+ public void testRoleAndUserAccount() throws InterruptedException {
+ setUpForReadConfigurationTest();
+ executeTest(new Runnable() {
+ public void run() {
+ assertTrue("role not found when it was expected", realm.roleExists("role1"));
+ assertTrue("user not found when it was expected", realm.accountExists("user1"));
+ }
+ });
+ }
+
+ /*
+ * Tests that roles can't be read while the realm is being loaded.
+ */
+ @Test
+ public void testHasRole() throws InterruptedException {
+ setUpForReadConfigurationTest();
+ executeTest(new Runnable() {
+ public void run() {
+ PrincipalCollection principalCollection = new SimplePrincipalCollection("user1", "realm1");
+ assertTrue("principal doesn't have role when it should",
+ realm.hasRole(principalCollection, "role2"));
+ assertTrue("principal doesn't have all roles when it should",
+ realm.hasAllRoles(principalCollection, Arrays.asList(new String[]{"role1", "role2"})));
+ }
+ });
+ }
+
+ /*
+ * Tests that roles can't be checked while the realm is being loaded.
+ */
+ @Test
+ public void testCheckRole() throws InterruptedException {
+ setUpForReadConfigurationTest();
+ executeTest(new Runnable() {
+ public void run() {
+ PrincipalCollection principalCollection = new SimplePrincipalCollection("user1", "realm1");
+ try {
+ realm.checkRoles(principalCollection, new String[]{"role1", "role2"});
+ } catch (AuthorizationException ae) {
+ fail("principal doesn't have all roles when it should");
+ }
+ }
+ });
+ }
+
+ /*
+ * Tests that a principal's permissions can't be checked while the realm is being loaded.
+ */
+ @Test
+ public void testCheckPermission() throws InterruptedException {
+ setUpForReadConfigurationTest();
+ executeTest(new Runnable() {
+ public void run() {
+ PrincipalCollection principalCollection = new SimplePrincipalCollection("user1", "realm1");
+ try {
+ realm.checkPermission(principalCollection, "role1_permission1");
+ realm.checkPermissions(principalCollection, new String[]{"role1_permission1", "role2_permission2"});
+ } catch (AuthorizationException ae) {
+ fail("principal doesn't have permission when it should");
+ }
+ }
+ });
+ }
+
+ /*
+ * Tests that a principal's permissions can't be checked while the realm is being loaded.
+ */
+ @Test
+ public void testIsPermitted() throws InterruptedException {
+ setUpForReadConfigurationTest();
+ executeTest(new Runnable() {
+ public void run() {
+ PrincipalCollection principalCollection = new SimplePrincipalCollection("user1", "realm1");
+ assertTrue("permission not permitted when it should be", realm.isPermitted(principalCollection, "role1_permission1"));
+ assertTrue("permission not permitted when it should be",
+ realm.isPermittedAll(principalCollection, new String[]{"role1_permission1", "role2_permission2"}));
+ }
+ });
+ }
+
+ /*
+ * Test that role definitions cannot be updated when a read thread holds the realm's lock.
+ */
+ @Test
+ public void testProcessRoleDefinitions() throws InterruptedException {
+ realm = new TestRealm() {
+ public void test(Thread runnable) throws InterruptedException {
+ // While the realm's lock is held by this thread role definitions cannot be processed
+ // Obtain the realm's locks
+ ROLES_LOCK.writeLock().lock();
+ try {
+ runnable.start();
+ Thread.sleep(500);
+ // No role until lock is released and role definitions are processed
+ assertFalse("role exists when it shouldn't", realm.roleExists("role1"));
+ } finally {
+ ROLES_LOCK.writeLock().unlock();
+ }
+ }
+ };
+ // A thread to process new role definitions
+ TestThread testThread = new TestThread(new Runnable() {
+ public void run() {
+ try {
+ realm.processRoleDefinitions();
+ } catch (ParseException e) {
+ fail("Unable to parse role definitions");
+ }
+ }
+ });
+ setRoles();
+ Thread testTask = new Thread(testThread);
+ realm.test(testTask);
+ testTask.join(500);
+ assertTrue("role doesn't exist when it should", realm.roleExists("role1"));
+ testThread.test();
+ }
+
+ /*
+ * Test that user definitions cannot be updated when a read thread holds the realm's lock.
+ */
+ @Test
+ public void testProcessUserDefinitions() throws InterruptedException {
+ realm = new TestRealm() {
+ public void test(Thread runnable) throws InterruptedException {
+ // While the realm's lock is held by this thread user definitions cannot be processed
+ // Obtain the realm's locks
+ USERS_LOCK.writeLock().lock();
+ try {
+ runnable.start();
+ Thread.sleep(500);
+ // No account until lock is released and user definitions are processed
+ assertFalse("account exists when it shouldn't", realm.accountExists("user1"));
+ } finally {
+ USERS_LOCK.writeLock().unlock();
+ }
+ }
+ };
+ TestThread testThread = new TestThread(new Runnable() {
+ public void run() {
+ try {
+ realm.processUserDefinitions();
+ } catch (ParseException e) {
+ fail("Unable to parse user definitions");
+ }
+ }
+ });
+ setUsers();
+ Thread testTask = new Thread(testThread);
+ realm.test(testTask);
+ testTask.join(500);
+ assertTrue("account doesn't exist when it should", realm.accountExists("user1"));
+ testThread.test();
+ }
+
+ /*
+ * A Class that captures a thread's assertion error.
+ */
+ private class TestThread implements Runnable {
+ private Runnable test;
+ private volatile AssertionError ae;
+
+ public TestThread(Runnable test) {
+ this.test = test;
+ }
+
+ public void run() {
+ try {
+ test.run();
+ } catch (AssertionError ae) {
+ this.ae = ae;
+ }
+ }
+
+ public void test() {
+ if (ae != null)
+ throw ae;
+ }
+ }
+
+ /*
+ * Provides an additional method that has access to the realm's lock for mutual exclusion.
+ */
+ private abstract class TestRealm extends TextConfigurationRealm {
+ abstract public void test(Thread runnable) throws InterruptedException;
+ }
+}