You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jetspeed-dev@portals.apache.org by rw...@apache.org on 2006/01/07 06:54:59 UTC
svn commit: r366682 - in /portals/jetspeed-2/trunk/components:
page-manager/src/java/org/apache/jetspeed/om/page/
page-manager/src/java/org/apache/jetspeed/om/page/impl/
page-manager/src/java/org/apache/jetspeed/page/document/impl/
page-manager/src/tes...
Author: rwatler
Date: Fri Jan 6 21:54:52 2006
New Revision: 366682
URL: http://svn.apache.org/viewcvs?rev=366682&view=rev
Log:
correct DB PageManager Security Constraints implementation bugs: NPE, JS2-456, and JS2-458
Modified:
portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java
portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java
portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java
portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java
portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java
portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java
portals/jetspeed-2/trunk/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java
Modified: portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java (original)
+++ portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/SecurityConstraintImpl.java Fri Jan 6 21:54:52 2006
@@ -76,6 +76,7 @@
*/
public String getUsersAsString()
{
+ // get from users list if not immediately available
if ((users == null) && (usersList != null) && !usersList.isEmpty())
{
users = formatCSVList(usersList);
@@ -90,6 +91,7 @@
*/
public void setUsersAsString(String users)
{
+ // set and propagate to users list setting
this.users = users;
usersList = parseCSVList(users);
}
@@ -101,6 +103,7 @@
*/
public String getRolesAsString()
{
+ // get from roles list if not immediately available
if ((roles == null) && (rolesList != null) && !rolesList.isEmpty())
{
roles = formatCSVList(rolesList);
@@ -115,6 +118,7 @@
*/
public void setRolesAsString(String roles)
{
+ // set and propagate to roles list setting
this.roles = roles;
rolesList = parseCSVList(roles);
}
@@ -126,6 +130,7 @@
*/
public String getGroupsAsString()
{
+ // get from groups list if not immediately available
if ((groups == null) && (groupsList != null) && !groupsList.isEmpty())
{
groups = formatCSVList(groupsList);
@@ -140,6 +145,7 @@
*/
public void setGroupsAsString(String groups)
{
+ // set and propagate to groups list setting
this.groups = groups;
groupsList = parseCSVList(groups);
}
@@ -147,10 +153,11 @@
/**
* getPermissionsAsString
*
- * @return permmissions CSV list
+ * @return permissions CSV list
*/
public String getPermissionsAsString()
{
+ // get from permissions list if not immediately available
if ((permissions == null) && (permissionsList != null) && !permissionsList.isEmpty())
{
permissions = formatCSVList(permissionsList);
@@ -161,10 +168,11 @@
/**
* setPermissionsAsString
*
- * @param permissions permmissions CSV list
+ * @param permissions permissions CSV list
*/
public void setPermissionsAsString(String permissions)
{
+ // set and propagate to permissions list setting
this.permissions = permissions;
permissionsList = parseCSVList(permissions);
}
@@ -192,8 +200,9 @@
*/
public void setUsers(List users)
{
- this.users = formatCSVList(users);
+ // set and clear potentially stale string representation
usersList = users;
+ this.users = null;
}
/**
@@ -219,8 +228,9 @@
*/
public void setRoles(List roles)
{
- this.roles = formatCSVList(roles);
+ // set and clear potentially stale string representation
rolesList = roles;
+ this.roles = null;
}
/**
@@ -246,8 +256,9 @@
*/
public void setGroups(List groups)
{
- this.groups = formatCSVList(groups);
+ // set and clear potentially stale string representation
groupsList = groups;
+ this.groups = null;
}
/**
@@ -260,9 +271,6 @@
*/
public List getPermissions()
{
- if (permissionsList != null && !permissionsList.isEmpty()) {
- this.permissions = this.getPermissionsAsString();
- }
return permissionsList;
}
@@ -276,8 +284,9 @@
*/
public void setPermissions(List permissions)
{
- this.permissions = formatCSVList(permissions);
+ // set and clear potentially stale string representation
permissionsList = permissions;
+ this.permissions = null;
}
/**
@@ -296,13 +305,15 @@
*/
public boolean principalsMatch(List userPrincipals, List rolePrincipals, List groupPrincipals, boolean allowDefault)
{
- return ((allowDefault && (getUsersAsString() == null) && (getRolesAsString() == null) && (getGroupsAsString() == null)) ||
- ((users != null) && (userPrincipals != null) &&
- (users.equals(WILD_CHAR) || containsAny(usersList, userPrincipals))) ||
- ((roles != null) && (rolePrincipals != null) &&
- (roles.equals(WILD_CHAR) || containsAny(rolesList, rolePrincipals))) ||
- ((groups != null) && (groupPrincipals != null) &&
- (groups.equals(WILD_CHAR) || containsAny(groupsList, groupPrincipals))));
+ // test match using users, roles, and groups list members
+ // since these are the master representation in this impl
+ return ((allowDefault && (usersList == null) && (rolesList == null) && (groupsList == null)) ||
+ ((usersList != null) && (userPrincipals != null) &&
+ (containsAny(usersList, userPrincipals) || usersList.contains(WILD_CHAR))) ||
+ ((rolesList != null) && (rolePrincipals != null) &&
+ (containsAny(rolesList, rolePrincipals) || rolesList.contains(WILD_CHAR))) ||
+ ((groupsList != null) && (groupPrincipals != null) &&
+ (containsAny(groupsList, groupPrincipals) || groupsList.contains(WILD_CHAR))));
}
/**
@@ -318,8 +329,10 @@
*/
public boolean actionMatch(String action)
{
- return ((permissions != null) &&
- (permissions.equals(WILD_CHAR) || permissionsList.contains(action)));
+ // test match using permissions list member since
+ // this is the master representation in this impl
+ return ((permissionsList != null) &&
+ (permissionsList.contains(action) || permissionsList.contains(WILD_CHAR)));
}
/**
@@ -335,7 +348,7 @@
*/
public static List parseCSVList(String csv)
{
- if ((csv != null) && !csv.equals(WILD_CHAR))
+ if (csv != null)
{
List csvList = new ArrayList(4);
if (csv.indexOf(',') != -1)
Modified: portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java (original)
+++ portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/om/page/impl/SecurityConstraintsImpl.java Fri Jan 6 21:54:52 2006
@@ -375,6 +375,9 @@
*/
public boolean isEmpty()
{
- return ((owner == null) && getSecurityConstraints().isEmpty() && getSecurityConstraintsRefs().isEmpty());
+ // test only persistent members for any specified constraints
+ return ((owner == null) &&
+ ((constraints == null) || constraints.isEmpty()) &&
+ ((constraintsRefs == null) || constraintsRefs.isEmpty()));
}
}
Modified: portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java (original)
+++ portals/jetspeed-2/trunk/components/page-manager/src/java/org/apache/jetspeed/page/document/impl/NodeSetImpl.java Fri Jan 6 21:54:52 2006
@@ -130,11 +130,11 @@
*/
public Iterator iterator()
{
- if (nodes != null)
+ if (nodes == null)
{
- return nodes.values().iterator();
+ nodes = new TreeMap(comparator);
}
- return null;
+ return nodes.values().iterator();
}
/* (non-Javadoc)
@@ -142,17 +142,13 @@
*/
public NodeSet subset(String type)
{
- NodeSetImpl subset = null;
- Iterator nodeItr = nodes.values().iterator();
+ NodeSetImpl subset = new NodeSetImpl(comparator);
+ Iterator nodeItr = iterator();
while (nodeItr.hasNext())
{
Node node = (Node) nodeItr.next();
if (node.getType().equals(type))
{
- if (subset == null)
- {
- subset = new NodeSetImpl(comparator);
- }
subset.add(node);
}
}
@@ -165,17 +161,13 @@
public NodeSet inclusiveSubset(String regex)
{
Pattern pattern = getCachedPattern(regex);
- NodeSetImpl subset = null;
- Iterator nodeItr = nodes.values().iterator();
+ NodeSetImpl subset = new NodeSetImpl(comparator);
+ Iterator nodeItr = iterator();
while (nodeItr.hasNext())
{
Node node = (Node) nodeItr.next();
if (pattern.matcher(node.getName()).matches())
{
- if (subset == null)
- {
- subset = new NodeSetImpl(comparator);
- }
subset.add(node);
}
}
@@ -188,17 +180,13 @@
public NodeSet exclusiveSubset(String regex)
{
Pattern pattern = getCachedPattern(regex);
- NodeSetImpl subset = null;
- Iterator nodeItr = nodes.values().iterator();
+ NodeSetImpl subset = new NodeSetImpl(comparator);
+ Iterator nodeItr = iterator();
while (nodeItr.hasNext())
{
Node node = (Node) nodeItr.next();
if (!pattern.matcher(node.getName()).matches())
{
- if (subset == null)
- {
- subset = new NodeSetImpl(comparator);
- }
subset.add(node);
}
}
Modified: portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java (original)
+++ portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/PageManagerTestShared.java Fri Jan 6 21:54:52 2006
@@ -132,7 +132,7 @@
*/
static List makeListFromCSV(String csv)
{
- if (!csv.equals("*"))
+ if (csv != null)
{
List csvList = new ArrayList();
if (csv.indexOf(',') != -1)
Modified: portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java (original)
+++ portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestCastorXmlPageManager.java Fri Jan 6 21:54:52 2006
@@ -917,7 +917,9 @@
assertTrue(constraint.getUsers() != null);
assertTrue(constraint.getUsers().size() == 2);
assertTrue(Shared.makeCSVFromList(constraint.getUsers()).equals("user10,user11"));
- assertTrue(constraint.getRoles() == null);
+ assertTrue(constraint.getRoles() != null);
+ assertTrue(constraint.getRoles().size() == 1);
+ assertTrue(Shared.makeCSVFromList(constraint.getRoles()).equals("*"));
assertTrue(constraint.getPermissions() != null);
assertTrue(constraint.getPermissions().size() == 2);
assertTrue(Shared.makeCSVFromList(constraint.getPermissions()).equals("edit,view"));
Modified: portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java (original)
+++ portals/jetspeed-2/trunk/components/page-manager/src/test/org/apache/jetspeed/page/TestDatabasePageManager.java Fri Jan 6 21:54:52 2006
@@ -496,7 +496,8 @@
assertEquals(3, folder.getAll().subset(Page.DOCUMENT_TYPE).size());
assertNotNull(folder.getAll().inclusiveSubset(".*other.*"));
assertEquals(2, folder.getAll().inclusiveSubset(".*other.*").size());
- assertNull(folder.getAll().inclusiveSubset("nomatch"));
+ assertNotNull(folder.getAll().inclusiveSubset("nomatch"));
+ assertEquals(0, folder.getAll().inclusiveSubset("nomatch").size());
assertNotNull(folder.getAll().exclusiveSubset(".*-page.psml"));
assertEquals(3, folder.getAll().exclusiveSubset(".*-page.psml").size());
}
@@ -522,6 +523,7 @@
assertEquals("public-view", ((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getName());
assertNotNull(((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints());
assertEquals(1, ((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints().size());
+ assertEquals("*", Shared.makeCSVFromList(((SecurityConstraint)((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints().get(0)).getUsers()));
assertEquals("view", Shared.makeCSVFromList(((SecurityConstraint)((SecurityConstraintsDef)check.getSecurityConstraintsDefs().get(1)).getSecurityConstraints().get(0)).getPermissions()));
assertNotNull(check.getGlobalSecurityConstraintsRefs());
assertEquals(2, check.getGlobalSecurityConstraintsRefs().size());
@@ -682,7 +684,7 @@
assertEquals(2, check.getSecurityConstraints().getSecurityConstraints().size());
assertEquals("user,admin", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getUsers()));
assertEquals("manager", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getRoles()));
- assertNull(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getGroups());
+ assertEquals("*", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(0)).getGroups()));
assertEquals("edit", Shared.makeCSVFromList(((SecurityConstraint)check.getSecurityConstraints().getSecurityConstraints().get(1)).getPermissions()));
assertNotNull(check.getDocumentOrder());
assertEquals(2, check.getDocumentOrder().size());
Modified: portals/jetspeed-2/trunk/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java
URL: http://svn.apache.org/viewcvs/portals/jetspeed-2/trunk/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java?rev=366682&r1=366681&r2=366682&view=diff
==============================================================================
--- portals/jetspeed-2/trunk/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java (original)
+++ portals/jetspeed-2/trunk/components/portal-site/src/java/org/apache/jetspeed/om/folder/proxy/FolderProxy.java Fri Jan 6 21:54:52 2006
@@ -749,8 +749,6 @@
// create and save proxies for concrete children
NodeSet children = folder.getAll();
Iterator childrenIter = children.iterator();
- if (childrenIter == null)
- continue;
while (childrenIter.hasNext())
{
Node child = (Node)childrenIter.next();
---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@portals.apache.org
For additional commands, e-mail: jetspeed-dev-help@portals.apache.org