shiro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lhazlew...@apache.org
Subject 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
Date Sat, 04 May 2013 22:17:38 GMT
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;
+    }
+}



Mime
View raw message