Author: [email protected]
Date: Tue May 3 12:22:44 2011
New Revision: 1081
Log:
[AMDATUCASSANDRA-51] Fixed several issues as result of the review:
-1- The map key is now guaranteed unique by using URL encoding and space
separator
-2- The property map does not allow anymore values to be set other then String
and byte[]
-3- The getUser(String key, String value) now evaluates the properties instead
of the credentials for returning the proper
useradmincache/service/CachedDictionary.java
-4- Enhanced the unit test for all of the above and added an additional test
for testing usernames with non-alphanumeric characters
Modified:
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedDictionary.java
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedRole.java
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedUser.java
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/UserAdminCache.java
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminCacheTest.java
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminMock.java
Modified:
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedDictionary.java
==============================================================================
---
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedDictionary.java
(original)
+++
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedDictionary.java
Tue May 3 12:22:44 2011
@@ -13,62 +13,74 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.amdatu.core.useradmincache.service;
-
-import java.util.Dictionary;
-import java.util.Enumeration;
-
-/**
- * Implements a Dictionary cache. Note that user credentials can be changed
using
- * UserAdmin.getRole("username").getCredentials().put("key", "value") and Role
properties can be changed using
- * UserAdmin.getRole("username").getProperties().put("key", "value")
- * This effects our filtered role cache and user by credentials cache, which
need to be updated when put or remove
- * is invoked on this dictionary.
- *
- * @author ivol
- */
-@SuppressWarnings("rawtypes")
-public class CachedDictionary extends Dictionary {
- private Dictionary m_dictionary;
- private DictionaryChanged m_callback;
- private String m_scope;
-
- public CachedDictionary(Dictionary dictionary, DictionaryChanged callback,
String scope) {
- m_dictionary = dictionary;
- m_callback = callback;
- m_scope = scope;
- }
-
- public Enumeration elements() {
- return m_dictionary.elements();
- }
-
- public Object get(Object key) {
- return m_dictionary.get(key);
- }
-
- public boolean isEmpty() {
- return m_dictionary.isEmpty();
- }
-
- public Enumeration keys() {
- return m_dictionary.keys();
- }
-
- @SuppressWarnings("unchecked")
- public Object put(Object key, Object value) {
- Object result = m_dictionary.put(key, value);
- m_callback.onUpdated(m_scope);
- return result;
- }
-
- public Object remove(Object key) {
- Object result = m_dictionary.remove(key);
- m_callback.onUpdated(m_scope);
- return result;
- }
-
- public int size() {
- return m_dictionary.size();
- }
-}
+package org.amdatu.core.useradmincache.service;
+
+import java.util.Dictionary;
+import java.util.Enumeration;
+
+import org.osgi.service.log.LogService;
+
+/**
+ * Implements a Dictionary cache. Note that user credentials can be changed
using
+ * UserAdmin.getRole("username").getCredentials().put("key", "value") and Role
properties can be changed using
+ * UserAdmin.getRole("username").getProperties().put("key", "value")
+ * This effects our filtered role cache and user by credentials cache, which
need to be updated when put or remove
+ * is invoked on this dictionary.
+ *
+ * @author ivol
+ */
+@SuppressWarnings("rawtypes")
+public class CachedDictionary extends Dictionary {
+ private Dictionary m_dictionary;
+ private DictionaryChanged m_callback;
+ private String m_scope;
+ private UserAdminCache m_userAdminCache;
+
+ public CachedDictionary(UserAdminCache userAdminCache, Dictionary
dictionary, DictionaryChanged callback, String scope) {
+ m_dictionary = dictionary;
+ m_callback = callback;
+ m_scope = scope;
+ m_userAdminCache = userAdminCache;
+ }
+
+ public Enumeration elements() {
+ return m_dictionary.elements();
+ }
+
+ public Object get(Object key) {
+ return m_dictionary.get(key);
+ }
+
+ public boolean isEmpty() {
+ return m_dictionary.isEmpty();
+ }
+
+ public Enumeration keys() {
+ return m_dictionary.keys();
+ }
+
+ @SuppressWarnings("unchecked")
+ public Object put(Object key, Object value) {
+ // According to the spec, only String and byte[] are supported for the
value type, verify that!
+ if (value instanceof String || value instanceof byte[]) {
+ Object result = m_dictionary.put(key, value);
+ m_callback.onUpdated(m_scope);
+ return result;
+ }
+ else {
+ String msg = "The UserAdmin spec only allows property values of
type String and byte[]";
+ m_userAdminCache.getLogService().log(LogService.LOG_WARNING, msg);
+ return null;
+ }
+ }
+
+ public Object remove(Object key) {
+ Object result = m_dictionary.remove(key);
+ m_callback.onUpdated(m_scope);
+ return result;
+ }
+
+ public int size() {
+ return m_dictionary.size();
+ }
+}
Modified:
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedRole.java
==============================================================================
---
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedRole.java
(original)
+++
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedRole.java
Tue May 3 12:22:44 2011
@@ -15,9 +15,9 @@
*/
package org.amdatu.core.useradmincache.service;
-import java.util.Dictionary;
-
-import org.osgi.service.useradmin.Role;
+import java.util.Dictionary;
+
+import org.osgi.service.useradmin.Role;
/**
* Role cache implementation (case class for UserCache and GroupCache).
@@ -57,7 +57,7 @@
public Dictionary getProperties() {
if (m_properties == null) {
- m_properties = new CachedDictionary(m_role.getProperties(), this,
"properties");
+ m_properties = new CachedDictionary(m_userAdminCache,
m_role.getProperties(), this, "properties");
}
return m_properties;
}
Modified:
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedUser.java
==============================================================================
---
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedUser.java
(original)
+++
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/CachedUser.java
Tue May 3 12:22:44 2011
@@ -15,12 +15,12 @@
*/
package org.amdatu.core.useradmincache.service;
-import java.util.Dictionary;
-import java.util.Hashtable;
-import java.util.Map;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-
-import org.osgi.service.useradmin.User;
+import java.util.Dictionary;
+import java.util.Hashtable;
+import java.util.Map;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import org.osgi.service.useradmin.User;
/**
* User cache implementation.
@@ -45,7 +45,7 @@
public Dictionary getCredentials() {
if (m_credentials == null) {
- m_credentials = new CachedDictionary(m_user.getCredentials(),
this, "credentials");
+ m_credentials = new CachedDictionary(m_userAdminCache,
m_user.getCredentials(), this, "credentials");
}
return m_credentials;
}
@@ -92,9 +92,9 @@
public void onUpdated(String scope) {
super.onUpdated(scope);
- if ("credentials".equals(scope)) {
- // Invoked when the credentials of this user have been changed.
- m_userAdminCache.flushUserByCredential(m_user);
+ if ("properties".equals(scope)) {
+ // Invoked when the properties of this user have been changed.
+ m_userAdminCache.flushUserByProperty(m_user);
}
}
}
Modified:
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/UserAdminCache.java
==============================================================================
---
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/UserAdminCache.java
(original)
+++
trunk/amdatu-core/useradmincache/src/main/java/org/amdatu/core/useradmincache/service/UserAdminCache.java
Tue May 3 12:22:44 2011
@@ -15,21 +15,23 @@
*/
package org.amdatu.core.useradmincache.service;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Set;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.service.log.LogService;
-import org.osgi.service.useradmin.Authorization;
-import org.osgi.service.useradmin.Group;
-import org.osgi.service.useradmin.Role;
-import org.osgi.service.useradmin.User;
-import org.osgi.service.useradmin.UserAdmin;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.service.log.LogService;
+import org.osgi.service.useradmin.Authorization;
+import org.osgi.service.useradmin.Group;
+import org.osgi.service.useradmin.Role;
+import org.osgi.service.useradmin.User;
+import org.osgi.service.useradmin.UserAdmin;
/**
* Implementation of a very simple in-memory cache for UserAdmin entities
(Users, Groups and Roles).
@@ -56,8 +58,8 @@
private Map<String, Authorization> m_authorizationCache = new
HashMap<String, Authorization>();
// User by credentials cache
- private ReentrantReadWriteLock m_userByCredentialCacheLock = new
ReentrantReadWriteLock();
- private Map<String, User> m_userByCredentialsCache = new HashMap<String,
User>();
+ private ReentrantReadWriteLock m_userByPropertyCacheLock = new
ReentrantReadWriteLock();
+ private Map<String, User> m_userByPropertyCache = new HashMap<String,
User>();
public void start() throws InvalidSyntaxException {
m_logService.log(LogService.LOG_INFO, "UserAdmin cache started");
@@ -74,6 +76,10 @@
public void setLogService(LogService logService) {
m_logService = logService;
}
+
+ public LogService getLogService() {
+ return m_logService;
+ }
public Role createRole(String name, int type) {
Role newRole = m_userAdmin.createRole(name, type);
@@ -179,49 +185,60 @@
}
}
- public User getUser(String key, String value) {
- String mapKey = "k=" + key + "|v=" + value;
- m_userByCredentialCacheLock.readLock().lock();
+ public User getUser(String key, String value) {
+ // Use URL encoding/decoding such that we can use the space character
for
+ // separating key and value
+ String mapKey = escape(key) + " " + escape(value);
+ m_userByPropertyCacheLock.readLock().lock();
try {
- if (m_userByCredentialsCache.containsKey(mapKey)) {
- return m_userByCredentialsCache.get(mapKey);
+ if (m_userByPropertyCache.containsKey(mapKey)) {
+ return m_userByPropertyCache.get(mapKey);
}
- m_userByCredentialCacheLock.readLock().unlock();
- m_userByCredentialCacheLock.writeLock().lock();
+ m_userByPropertyCacheLock.readLock().unlock();
+ m_userByPropertyCacheLock.writeLock().lock();
try {
User user = m_userAdmin.getUser(key, value);
User cachedUser = null;
if (user != null) {
cachedUser = (User) createCachedRole(user);
- m_userByCredentialsCache.put(mapKey, cachedUser);
+ m_userByPropertyCache.put(mapKey, cachedUser);
}
return user;
} finally {
- m_userByCredentialCacheLock.writeLock().unlock();
- m_userByCredentialCacheLock.readLock().lock();
+ m_userByPropertyCacheLock.writeLock().unlock();
+ m_userByPropertyCacheLock.readLock().lock();
}
} finally {
- m_userByCredentialCacheLock.readLock().unlock();
+ m_userByPropertyCacheLock.readLock().unlock();
}
+ }
+
+ private String escape(String key) {
+ try {
+ return URLEncoder.encode(key, "UTF-8");
+ }
+ catch (UnsupportedEncodingException e) {
+ }
+ return null;
}
- public void flushUserByCredential(User user) {
- m_userByCredentialCacheLock.writeLock().lock();
+ public void flushUserByProperty(User user) {
+ m_userByPropertyCacheLock.writeLock().lock();
try {
- Set<String> keys = m_userByCredentialsCache.keySet();
+ Set<String> keys = m_userByPropertyCache.keySet();
if (keys != null) {
List<String> removeKeys = new ArrayList<String>();
for (String key : keys) {
- if (m_userByCredentialsCache.get(key).equals(user)) {
+ if (m_userByPropertyCache.get(key).equals(user)) {
removeKeys.add(key);
}
}
for (String removeKey : removeKeys) {
- m_userByCredentialsCache.remove(removeKey);
+ m_userByPropertyCache.remove(removeKey);
}
}
} finally {
- m_userByCredentialCacheLock.writeLock().unlock();
+ m_userByPropertyCacheLock.writeLock().unlock();
}
}
Modified:
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminCacheTest.java
==============================================================================
---
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminCacheTest.java
(original)
+++
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminCacheTest.java
Tue May 3 12:22:44 2011
@@ -16,20 +16,20 @@
package org.amdatu.useradmincache.test;
-import java.util.ArrayList;
-import java.util.List;
-
-import junit.framework.Assert;
-
-import org.amdatu.core.useradmincache.service.UserAdminCache;
-import org.junit.Test;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
-import org.osgi.service.log.LogService;
-import org.osgi.service.useradmin.Group;
-import org.osgi.service.useradmin.Role;
-import org.osgi.service.useradmin.User;
-import org.osgi.service.useradmin.UserAdmin;
+import java.util.ArrayList;
+import java.util.List;
+
+import junit.framework.Assert;
+
+import org.amdatu.core.useradmincache.service.UserAdminCache;
+import org.junit.Test;
+import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.log.LogService;
+import org.osgi.service.useradmin.Group;
+import org.osgi.service.useradmin.Role;
+import org.osgi.service.useradmin.User;
+import org.osgi.service.useradmin.UserAdmin;
@SuppressWarnings("unchecked")
public class UserAdminCacheTest {
@@ -45,7 +45,7 @@
for (int i=1; i<=TEST_SIZE; i++) {
String name = "User " + i;
User user = (User) userAdminCache.createRole(name, Role.USER);
- user.getProperties().put("test", i);
+ user.getProperties().put("test", i + ""); // we must ensure we
store the property as string
user.getCredentials().put("password", "secret " + i);
}
@@ -67,6 +67,62 @@
cachedUser.getProperties().put("test", "newvalue");
cachedUser = (User) userAdminCache.getRole("User 1");
Assert.assertTrue(cachedUser.getProperties().get("test").equals("newvalue"));
+ }
+
+ @Test
+ public void testNonalphanumeric() throws Exception {
+ // Create the UserAdminCache
+ UserAdminCache userAdminCache = createUserAdmin();
+
+ // Create user with many non alphanumeric characters
+ String nonAN = "~!@#$%^&*()_+`-=[]\\{}|';:\"/.,<>? ";
+ String name = "User " + nonAN;
+ User user = (User) userAdminCache.createRole(name, Role.USER);
+ user.getProperties().put("test " + nonAN, "value " + nonAN);
+ user.getCredentials().put("password", "secret " + nonAN);
+
+ User cachedUser = (User) userAdminCache.getRole(name);
+ Assert.assertTrue(cachedUser.getProperties().get("test " +
nonAN).equals("value " + nonAN));
+
Assert.assertTrue(cachedUser.getCredentials().get("password").equals("secret "
+ nonAN));
+ cachedUser = (User) userAdminCache.getUser("test " + nonAN, "value " +
nonAN);
+ Assert.assertTrue(cachedUser.getProperties().get("test " +
nonAN).equals("value " + nonAN));
+
Assert.assertTrue(cachedUser.getCredentials().get("password").equals("secret "
+ nonAN));
+ }
+
+ @Test
+ public void testMapKeyUniqueness() throws Exception {
+ // Create the UserAdminCache
+ UserAdminCache userAdminCache = createUserAdmin();
+
+ // Create test user
+ String name = "Test User";
+ User user = (User) userAdminCache.createRole(name, Role.USER);
+ user.getProperties().put("a", "b|v=c");
+
+ Assert.assertNotNull(userAdminCache.getUser("a", "b|v=c"));
+ Assert.assertNull(userAdminCache.getUser("a|v=b", "c"));
+
+ user.getProperties().put("a", "b");
+
+ Assert.assertNotNull(userAdminCache.getUser("a", "b"));
+ Assert.assertNull(userAdminCache.getUser("a b", ""));
+ Assert.assertNull(userAdminCache.getUser("", "a b"));
+ }
+
+ @Test
+ public void testWronglyTypedProperties() throws Exception {
+ // Create the UserAdminCache
+ UserAdminCache userAdminCache = createUserAdmin();
+
+ // Create test user
+ String name = "Test User";
+ User user = (User) userAdminCache.createRole(name, Role.USER);
+
+ // Now set properties of invalid types, according to the spec the
value must be
+ // either a string or a byte[]
+ Assert.assertNull(user.getProperties().put("key", 3)); // Integer
+ Assert.assertNull(user.getProperties().put("key", 3.0)); // double
+ Assert.assertNull(user.getProperties().put("key", user)); // Some
object
}
@Test
@@ -132,22 +188,26 @@
// Test user credentials
user.getCredentials().put(passwordKey, "secretkey");
- Assert.assertTrue(m_userAdmin.getUser(passwordKey,
"secretkey") != null);
+ Assert.assertNull(m_userAdmin.getUser(passwordKey,
"secretkey"));
Assert.assertTrue(((User)
m_userAdmin.getRole(userName)).getCredentials().get(passwordKey).equals("secretkey"));
Assert.assertTrue(user.getCredentials().get(passwordKey).equals("secretkey"));
user.getCredentials().put(passwordKey, "secretkey2");
- Assert.assertTrue(m_userAdmin.getUser(passwordKey,
"secretkey") == null);
- Assert.assertTrue(m_userAdmin.getUser(passwordKey,
"secretkey2") != null);
+ Assert.assertNull(m_userAdmin.getUser(passwordKey,
"secretkey"));
+ Assert.assertNull(m_userAdmin.getUser(passwordKey,
"secretkey2"));
Assert.assertTrue(((User)
m_userAdmin.getRole(userName)).getCredentials().get(passwordKey).equals("secretkey2"));
- // Test user properties
- m_userAdmin.getRole(userName).getProperties().put(propKey,
"testvalue");
+ // Test user properties
+ Assert.assertNull(m_userAdmin.getUser(propKey, "testvalue"));
+ m_userAdmin.getRole(userName).getProperties().put(propKey,
"testvalue");
+ Assert.assertNotNull(m_userAdmin.getUser(propKey,
"testvalue"));
Assert.assertTrue(m_userAdmin.getRole(userName).getProperties().get(propKey).equals("testvalue"));
String filter = "(" + propKey + "=" + "testvalue)";
Role[] filteredRoles = m_userAdmin.getRoles(filter);
Assert.assertTrue(filteredRoles.length == 1);
Assert.assertTrue(contains(filteredRoles,
m_userAdmin.getRole(userName)));
- m_userAdmin.getRole(userName).getProperties().put(propKey,
"testvalue2");
+ m_userAdmin.getRole(userName).getProperties().put(propKey,
"testvalue2");
+ Assert.assertNull(m_userAdmin.getUser(propKey, "testvalue"));
+ Assert.assertNotNull(m_userAdmin.getUser(propKey,
"testvalue2"));
filteredRoles = m_userAdmin.getRoles(filter);
Assert.assertTrue(filteredRoles.length == 0);
Assert.assertTrue(m_userAdmin.getRole(userName).getProperties().get(propKey).equals("testvalue2"));
@@ -217,10 +277,10 @@
for (int i=1; i<=TEST_SIZE; i++) {
String name = "User " + i;
User cachedUser = (User) userAdminCache.getRole(name);
-
Assert.assertTrue(cachedUser.getProperties().get("test").equals(i));
+ Assert.assertTrue(cachedUser.getProperties().get("test").equals(i
+ ""));
Assert.assertTrue(cachedUser.getCredentials().get("password").equals("secret "
+ i));
- cachedUser = (User) userAdminCache.getUser("password", "secret " +
i);
-
Assert.assertTrue(cachedUser.getProperties().get("test").equals(i));
+ cachedUser = (User) userAdminCache.getUser("test", i + "");
+ Assert.assertTrue(cachedUser.getProperties().get("test").equals(i
+ ""));
Assert.assertTrue(cachedUser.getCredentials().get("password").equals("secret "
+ i));
}
Assert.assertEquals(10, userAdminCache.getRoles(null).length);
Modified:
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminMock.java
==============================================================================
---
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminMock.java
(original)
+++
trunk/amdatu-core/useradmincache/src/test/java/org/amdatu/useradmincache/test/UserAdminMock.java
Tue May 3 12:22:44 2011
@@ -15,15 +15,15 @@
*/
package org.amdatu.useradmincache.test;
-import java.util.ArrayList;
-import java.util.List;
-
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.service.useradmin.Authorization;
-import org.osgi.service.useradmin.Group;
-import org.osgi.service.useradmin.Role;
-import org.osgi.service.useradmin.User;
-import org.osgi.service.useradmin.UserAdmin;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.service.useradmin.Authorization;
+import org.osgi.service.useradmin.Group;
+import org.osgi.service.useradmin.Role;
+import org.osgi.service.useradmin.User;
+import org.osgi.service.useradmin.UserAdmin;
public class UserAdminMock implements UserAdmin {
// The 'persistent storage' of roles
@@ -96,9 +96,9 @@
public synchronized User getUser(String key, String value) {
for (Role role : m_roles) {
// Very simple filter, only for unit testing. Filter: property
key=value
- if (role.getType() == Role.USER && ((User) role).getCredentials()
!= null
- && ((User) role).getCredentials().get(key) != null
- && ((User)
role).getCredentials().get(key).equals(value)) {
+ if (role.getType() == Role.USER && ((User) role).getProperties()
!= null
+ && ((User) role).getProperties().get(key) != null
+ && ((User)
role).getProperties().get(key).equals(value)) {
return (User) role;
}
}
_______________________________________________
Amdatu-commits mailing list
[email protected]
http://lists.amdatu.org/mailman/listinfo/amdatu-commits
