This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 5376d45 Improve the reusability of UserDatabase code
5376d45 is described below
commit 5376d45258b8bf496951a99eab435517d2ed70f0
Author: remm <[email protected]>
AuthorDate: Tue Aug 17 14:56:24 2021 +0200
Improve the reusability of UserDatabase code
Add intermediate concrete implementation classes. Add hooks to allow
partail database updates on save. Will likely make alternate
implementations more realistic.
---
java/org/apache/catalina/UserDatabase.java | 27 ++++
.../users/{MemoryGroup.java => GenericGroup.java} | 60 ++++----
.../users/{MemoryRole.java => GenericRole.java} | 44 +++---
.../users/{MemoryUser.java => GenericUser.java} | 126 ++++++++--------
java/org/apache/catalina/users/MemoryGroup.java | 116 +--------------
java/org/apache/catalina/users/MemoryRole.java | 36 +----
java/org/apache/catalina/users/MemoryUser.java | 158 +--------------------
webapps/docs/changelog.xml | 5 +
8 files changed, 138 insertions(+), 434 deletions(-)
diff --git a/java/org/apache/catalina/UserDatabase.java
b/java/org/apache/catalina/UserDatabase.java
index 8bac810..9242170 100644
--- a/java/org/apache/catalina/UserDatabase.java
+++ b/java/org/apache/catalina/UserDatabase.java
@@ -156,6 +156,33 @@ public interface UserDatabase {
/**
+ * Signal the specified {@link Group} from this user database has been
+ * modified.
+ *
+ * @param group The group that has been modified
+ */
+ public default void modifiedGroup(Group group) {}
+
+
+ /**
+ * Signal the specified {@link Role} from this user database has been
+ * modified.
+ *
+ * @param role The role that has been modified
+ */
+ public default void modifiedRole(Role role) {}
+
+
+ /**
+ * Signal the specified {@link User} from this user database has been
+ * modified.
+ *
+ * @param user The user that has been modified
+ */
+ public default void modifiedUser(User user) {}
+
+
+ /**
* Save any updated information to the persistent storage location for this
* user database.
*
diff --git a/java/org/apache/catalina/users/MemoryGroup.java
b/java/org/apache/catalina/users/GenericGroup.java
similarity index 73%
copy from java/org/apache/catalina/users/MemoryGroup.java
copy to java/org/apache/catalina/users/GenericGroup.java
index aad4180..d59e57d 100644
--- a/java/org/apache/catalina/users/MemoryGroup.java
+++ b/java/org/apache/catalina/users/GenericGroup.java
@@ -25,17 +25,15 @@ import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.catalina.Role;
import org.apache.catalina.User;
import org.apache.catalina.UserDatabase;
-import org.apache.tomcat.util.buf.StringUtils;
/**
- * <p>Concrete implementation of {@link org.apache.catalina.Group} for the
- * {@link MemoryUserDatabase} implementation of {@link UserDatabase}.</p>
+ * <p>Concrete implementation of {@link org.apache.catalina.Group} for a
+ * {@link UserDatabase}.</p>
*
* @author Craig R. McClanahan
- * @since 4.1
*/
-public class MemoryGroup extends AbstractGroup {
+public class GenericGroup<UD extends UserDatabase> extends AbstractGroup {
// ----------------------------------------------------------- Constructors
@@ -43,19 +41,23 @@ public class MemoryGroup extends AbstractGroup {
/**
* Package-private constructor used by the factory method in
- * {@link MemoryUserDatabase}.
+ * {@link UserDatabase}.
*
- * @param database The {@link MemoryUserDatabase} that owns this group
+ * @param database The {@link UserDatabase} that owns this group
* @param groupname Group name of this group
* @param description Description of this group
+ * @param roles The roles of this group
*/
- MemoryGroup(MemoryUserDatabase database,
- String groupname, String description) {
+ GenericGroup(UD database,
+ String groupname, String description, List<Role> roles) {
super();
this.database = database;
- setGroupname(groupname);
- setDescription(description);
+ this.groupname = groupname;
+ this.description = description;
+ if (roles != null) {
+ this.roles.addAll(roles);
+ }
}
@@ -64,9 +66,9 @@ public class MemoryGroup extends AbstractGroup {
/**
- * The {@link MemoryUserDatabase} that owns this group.
+ * The {@link UserDatabase} that owns this group.
*/
- protected final MemoryUserDatabase database;
+ protected final UD database;
/**
@@ -123,7 +125,9 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void addRole(Role role) {
- roles.addIfAbsent(role);
+ if (roles.addIfAbsent(role)) {
+ database.modifiedGroup(this);
+ }
}
@@ -145,7 +149,9 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void removeRole(Role role) {
- roles.remove(role);
+ if (roles.remove(role)) {
+ database.modifiedGroup(this);
+ }
}
@@ -154,27 +160,11 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void removeRoles() {
- roles.clear();
+ if (!roles.isEmpty()) {
+ roles.clear();
+ database.modifiedGroup(this);
+ }
}
- /**
- * <p>Return a String representation of this group in XML format.</p>
- */
- @Override
- public String toString() {
- StringBuilder sb = new StringBuilder("<group groupname=\"");
- sb.append(groupname);
- sb.append("\"");
- if (description != null) {
- sb.append(" description=\"");
- sb.append(description);
- sb.append("\"");
- }
- sb.append(" roles=\"");
- StringUtils.join(roles, ',', Role::getRolename, sb);
- sb.append("\"");
- sb.append("/>");
- return sb.toString();
- }
}
diff --git a/java/org/apache/catalina/users/MemoryRole.java
b/java/org/apache/catalina/users/GenericRole.java
similarity index 63%
copy from java/org/apache/catalina/users/MemoryRole.java
copy to java/org/apache/catalina/users/GenericRole.java
index b8f4970..c6b0eba 100644
--- a/java/org/apache/catalina/users/MemoryRole.java
+++ b/java/org/apache/catalina/users/GenericRole.java
@@ -21,13 +21,12 @@ import org.apache.catalina.UserDatabase;
/**
- * <p>Concrete implementation of {@link org.apache.catalina.Role} for the
- * {@link MemoryUserDatabase} implementation of {@link UserDatabase}.</p>
+ * <p>Concrete implementation of {@link org.apache.catalina.Role} for a
+ * {@link UserDatabase}.</p>
*
* @author Craig R. McClanahan
- * @since 4.1
*/
-public class MemoryRole extends AbstractRole {
+public class GenericRole<UD extends UserDatabase> extends AbstractRole {
// ----------------------------------------------------------- Constructors
@@ -35,19 +34,19 @@ public class MemoryRole extends AbstractRole {
/**
* Package-private constructor used by the factory method in
- * {@link MemoryUserDatabase}.
+ * {@link UserDatabase}.
*
- * @param database The {@link MemoryUserDatabase} that owns this role
+ * @param database The {@link UserDatabase} that owns this role
* @param rolename Role name of this role
* @param description Description of this role
*/
- MemoryRole(MemoryUserDatabase database,
+ GenericRole(UD database,
String rolename, String description) {
super();
this.database = database;
- setRolename(rolename);
- setDescription(description);
+ this.rolename = rolename;
+ this.description = description;
}
@@ -56,9 +55,9 @@ public class MemoryRole extends AbstractRole {
/**
- * The {@link MemoryUserDatabase} that owns this role.
+ * The {@link UserDatabase} that owns this role.
*/
- protected final MemoryUserDatabase database;
+ protected final UserDatabase database;
// ------------------------------------------------------------- Properties
@@ -73,24 +72,17 @@ public class MemoryRole extends AbstractRole {
}
- // --------------------------------------------------------- Public Methods
+ @Override
+ public void setDescription(String description) {
+ database.modifiedRole(this);
+ super.setDescription(description);
+ }
- /**
- * <p>Return a String representation of this role in XML format.</p>
- */
@Override
- public String toString() {
- StringBuilder sb = new StringBuilder("<role rolename=\"");
- sb.append(rolename);
- sb.append("\"");
- if (description != null) {
- sb.append(" description=\"");
- sb.append(description);
- sb.append("\"");
- }
- sb.append("/>");
- return sb.toString();
+ public void setRolename(String rolename) {
+ database.modifiedRole(this);
+ super.setRolename(rolename);
}
diff --git a/java/org/apache/catalina/users/MemoryUser.java
b/java/org/apache/catalina/users/GenericUser.java
similarity index 62%
copy from java/org/apache/catalina/users/MemoryUser.java
copy to java/org/apache/catalina/users/GenericUser.java
index 007dd56..77b70b4 100644
--- a/java/org/apache/catalina/users/MemoryUser.java
+++ b/java/org/apache/catalina/users/GenericUser.java
@@ -18,22 +18,20 @@ package org.apache.catalina.users;
import java.util.Iterator;
+import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.catalina.Group;
import org.apache.catalina.Role;
import org.apache.catalina.UserDatabase;
-import org.apache.tomcat.util.buf.StringUtils;
-import org.apache.tomcat.util.security.Escape;
/**
- * <p>Concrete implementation of {@link org.apache.catalina.User} for the
- * {@link MemoryUserDatabase} implementation of {@link UserDatabase}.</p>
+ * <p>Concrete implementation of {@link org.apache.catalina.User} for a
+ * {@link UserDatabase}.</p>
*
* @author Craig R. McClanahan
- * @since 4.1
*/
-public class MemoryUser extends AbstractUser {
+public class GenericUser<UD extends UserDatabase> extends AbstractUser {
// ----------------------------------------------------------- Constructors
@@ -41,21 +39,30 @@ public class MemoryUser extends AbstractUser {
/**
* Package-private constructor used by the factory method in
- * {@link MemoryUserDatabase}.
+ * {@link UserDatabase}.
*
- * @param database The {@link MemoryUserDatabase} that owns this user
+ * @param database The {@link UserDatabase} that owns this user
* @param username Logon username of the new user
* @param password Logon password of the new user
* @param fullName Full name of the new user
+ * @param groups The groups of this user
+ * @param roles The roles of this user
*/
- MemoryUser(MemoryUserDatabase database, String username,
- String password, String fullName) {
+ GenericUser(UD database, String username,
+ String password, String fullName, List<Group> groups,
+ List<Role> roles) {
super();
this.database = database;
- setUsername(username);
- setPassword(password);
- setFullName(fullName);
+ this.username = username;
+ this.password = password;
+ this.fullName = fullName;
+ if (groups != null) {
+ this.groups.addAll(groups);
+ }
+ if (roles != null) {
+ this.roles.addAll(roles);
+ }
}
@@ -64,9 +71,9 @@ public class MemoryUser extends AbstractUser {
/**
- * The {@link MemoryUserDatabase} that owns this user.
+ * The {@link UserDatabase} that owns this user.
*/
- protected final MemoryUserDatabase database;
+ protected final UD database;
/**
@@ -121,7 +128,9 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void addGroup(Group group) {
- groups.addIfAbsent(group);
+ if (groups.addIfAbsent(group)) {
+ database.modifiedUser(this);
+ }
}
@@ -132,7 +141,9 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void addRole(Role role) {
- roles.addIfAbsent(role);
+ if (roles.addIfAbsent(role)) {
+ database.modifiedUser(this);
+ }
}
@@ -167,7 +178,9 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeGroup(Group group) {
- groups.remove(group);
+ if (groups.remove(group)) {
+ database.modifiedUser(this);
+ }
}
@@ -176,7 +189,10 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeGroups() {
- groups.clear();
+ if (!groups.isEmpty()) {
+ groups.clear();
+ database.modifiedUser(this);
+ }
}
@@ -187,7 +203,9 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeRole(Role role) {
- roles.remove(role);
+ if (roles.remove(role)) {
+ database.modifiedUser(this);
+ }
}
@@ -196,62 +214,32 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeRoles() {
+ if (!roles.isEmpty()) {
+ database.modifiedUser(this);
+ }
roles.clear();
}
- /**
- * <p>Return a String representation of this user in XML format.</p>
- *
- * <p><strong>IMPLEMENTATION NOTE</strong> - For backwards compatibility,
- * the reader that processes this entry will accept either
- * <code>username</code> or <code>name</code> for the username
- * property.</p>
- * @return the XML representation
- */
- public String toXml() {
-
- StringBuilder sb = new StringBuilder("<user username=\"");
- sb.append(Escape.xml(username));
- sb.append("\" password=\"");
- sb.append(Escape.xml(password));
- sb.append("\"");
- if (fullName != null) {
- sb.append(" fullName=\"");
- sb.append(Escape.xml(fullName));
- sb.append("\"");
- }
- sb.append(" groups=\"");
- StringUtils.join(groups, ',', (x) -> Escape.xml(x.getGroupname()), sb);
- sb.append("\"");
- sb.append(" roles=\"");
- StringUtils.join(roles, ',', (x) -> Escape.xml(x.getRolename()), sb);
- sb.append("\"");
- sb.append("/>");
- return sb.toString();
+ @Override
+ public void setFullName(String fullName) {
+ database.modifiedUser(this);
+ super.setFullName(fullName);
}
- /**
- * <p>Return a String representation of this user.</p>
- */
@Override
- public String toString() {
-
- StringBuilder sb = new StringBuilder("User username=\"");
- sb.append(Escape.xml(username));
- sb.append("\"");
- if (fullName != null) {
- sb.append(", fullName=\"");
- sb.append(Escape.xml(fullName));
- sb.append("\"");
- }
- sb.append(", groups=\"");
- StringUtils.join(groups, ',', (x) -> Escape.xml(x.getGroupname()), sb);
- sb.append("\"");
- sb.append(", roles=\"");
- StringUtils.join(roles, ',', (x) -> Escape.xml(x.getRolename()), sb);
- sb.append("\"");
- return sb.toString();
+ public void setPassword(String password) {
+ database.modifiedUser(this);
+ super.setPassword(password);
}
+
+
+ @Override
+ public void setUsername(String username) {
+ database.modifiedUser(this);
+ // Note: changing the user name is a problem ...
+ super.setUsername(username);
+ }
+
}
diff --git a/java/org/apache/catalina/users/MemoryGroup.java
b/java/org/apache/catalina/users/MemoryGroup.java
index aad4180..f1008ff 100644
--- a/java/org/apache/catalina/users/MemoryGroup.java
+++ b/java/org/apache/catalina/users/MemoryGroup.java
@@ -17,13 +17,7 @@
package org.apache.catalina.users;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-import java.util.concurrent.CopyOnWriteArrayList;
-
import org.apache.catalina.Role;
-import org.apache.catalina.User;
import org.apache.catalina.UserDatabase;
import org.apache.tomcat.util.buf.StringUtils;
@@ -35,10 +29,7 @@ import org.apache.tomcat.util.buf.StringUtils;
* @author Craig R. McClanahan
* @since 4.1
*/
-public class MemoryGroup extends AbstractGroup {
-
-
- // ----------------------------------------------------------- Constructors
+public class MemoryGroup extends GenericGroup<MemoryUserDatabase> {
/**
@@ -51,110 +42,7 @@ public class MemoryGroup extends AbstractGroup {
*/
MemoryGroup(MemoryUserDatabase database,
String groupname, String description) {
-
- super();
- this.database = database;
- setGroupname(groupname);
- setDescription(description);
-
- }
-
-
- // ----------------------------------------------------- Instance Variables
-
-
- /**
- * The {@link MemoryUserDatabase} that owns this group.
- */
- protected final MemoryUserDatabase database;
-
-
- /**
- * The set of {@link Role}s associated with this group.
- */
- protected final CopyOnWriteArrayList<Role> roles = new
CopyOnWriteArrayList<>();
-
-
- // ------------------------------------------------------------- Properties
-
-
- /**
- * Return the set of {@link Role}s assigned specifically to this group.
- */
- @Override
- public Iterator<Role> getRoles() {
- return roles.iterator();
- }
-
-
- /**
- * Return the {@link UserDatabase} within which this Group is defined.
- */
- @Override
- public UserDatabase getUserDatabase() {
- return this.database;
- }
-
-
- /**
- * Return the set of {@link org.apache.catalina.User}s that are members of
this group.
- */
- @Override
- public Iterator<User> getUsers() {
- List<User> results = new ArrayList<>();
- Iterator<User> users = database.getUsers();
- while (users.hasNext()) {
- User user = users.next();
- if (user.isInGroup(this)) {
- results.add(user);
- }
- }
- return results.iterator();
- }
-
-
- // --------------------------------------------------------- Public Methods
-
-
- /**
- * Add a new {@link Role} to those assigned specifically to this group.
- *
- * @param role The new role
- */
- @Override
- public void addRole(Role role) {
- roles.addIfAbsent(role);
- }
-
-
- /**
- * Is this group specifically assigned the specified {@link Role}?
- *
- * @param role The role to check
- */
- @Override
- public boolean isInRole(Role role) {
- return roles.contains(role);
- }
-
-
- /**
- * Remove a {@link Role} from those assigned to this group.
- *
- * @param role The old role
- */
- @Override
- public void removeRole(Role role) {
- roles.remove(role);
- }
-
-
- /**
- * Remove all {@link Role}s from those assigned to this group.
- */
- @Override
- public void removeRoles() {
- roles.clear();
+ super(database, groupname, description, null);
}
diff --git a/java/org/apache/catalina/users/MemoryRole.java
b/java/org/apache/catalina/users/MemoryRole.java
index b8f4970..10f6d22 100644
--- a/java/org/apache/catalina/users/MemoryRole.java
+++ b/java/org/apache/catalina/users/MemoryRole.java
@@ -27,10 +27,7 @@ import org.apache.catalina.UserDatabase;
* @author Craig R. McClanahan
* @since 4.1
*/
-public class MemoryRole extends AbstractRole {
-
-
- // ----------------------------------------------------------- Constructors
+public class MemoryRole extends GenericRole<MemoryUserDatabase> {
/**
@@ -43,39 +40,10 @@ public class MemoryRole extends AbstractRole {
*/
MemoryRole(MemoryUserDatabase database,
String rolename, String description) {
-
- super();
- this.database = database;
- setRolename(rolename);
- setDescription(description);
-
- }
-
-
- // ----------------------------------------------------- Instance Variables
-
-
- /**
- * The {@link MemoryUserDatabase} that owns this role.
- */
- protected final MemoryUserDatabase database;
-
-
- // ------------------------------------------------------------- Properties
-
-
- /**
- * Return the {@link UserDatabase} within which this role is defined.
- */
- @Override
- public UserDatabase getUserDatabase() {
- return this.database;
+ super(database, rolename, description);
}
- // --------------------------------------------------------- Public Methods
-
-
/**
* <p>Return a String representation of this role in XML format.</p>
*/
diff --git a/java/org/apache/catalina/users/MemoryUser.java
b/java/org/apache/catalina/users/MemoryUser.java
index 007dd56..f271fb2 100644
--- a/java/org/apache/catalina/users/MemoryUser.java
+++ b/java/org/apache/catalina/users/MemoryUser.java
@@ -17,11 +17,6 @@
package org.apache.catalina.users;
-import java.util.Iterator;
-import java.util.concurrent.CopyOnWriteArrayList;
-
-import org.apache.catalina.Group;
-import org.apache.catalina.Role;
import org.apache.catalina.UserDatabase;
import org.apache.tomcat.util.buf.StringUtils;
import org.apache.tomcat.util.security.Escape;
@@ -33,10 +28,7 @@ import org.apache.tomcat.util.security.Escape;
* @author Craig R. McClanahan
* @since 4.1
*/
-public class MemoryUser extends AbstractUser {
-
-
- // ----------------------------------------------------------- Constructors
+public class MemoryUser extends GenericUser<MemoryUserDatabase> {
/**
@@ -50,153 +42,7 @@ public class MemoryUser extends AbstractUser {
*/
MemoryUser(MemoryUserDatabase database, String username,
String password, String fullName) {
-
- super();
- this.database = database;
- setUsername(username);
- setPassword(password);
- setFullName(fullName);
-
- }
-
-
- // ----------------------------------------------------- Instance Variables
-
-
- /**
- * The {@link MemoryUserDatabase} that owns this user.
- */
- protected final MemoryUserDatabase database;
-
-
- /**
- * The set of {@link Group}s that this user is a member of.
- */
- protected final CopyOnWriteArrayList<Group> groups = new
CopyOnWriteArrayList<>();
-
-
- /**
- * The set of {@link Role}s associated with this user.
- */
- protected final CopyOnWriteArrayList<Role> roles = new
CopyOnWriteArrayList<>();
-
-
- // ------------------------------------------------------------- Properties
-
-
- /**
- * Return the set of {@link Group}s to which this user belongs.
- */
- @Override
- public Iterator<Group> getGroups() {
- return groups.iterator();
- }
-
-
- /**
- * Return the set of {@link Role}s assigned specifically to this user.
- */
- @Override
- public Iterator<Role> getRoles() {
- return roles.iterator();
- }
-
-
- /**
- * Return the {@link UserDatabase} within which this User is defined.
- */
- @Override
- public UserDatabase getUserDatabase() {
- return this.database;
- }
-
-
- // --------------------------------------------------------- Public Methods
-
-
- /**
- * Add a new {@link Group} to those this user belongs to.
- *
- * @param group The new group
- */
- @Override
- public void addGroup(Group group) {
- groups.addIfAbsent(group);
- }
-
-
- /**
- * Add a new {@link Role} to those assigned specifically to this user.
- *
- * @param role The new role
- */
- @Override
- public void addRole(Role role) {
- roles.addIfAbsent(role);
- }
-
-
- /**
- * Is this user in the specified group?
- *
- * @param group The group to check
- */
- @Override
- public boolean isInGroup(Group group) {
- return groups.contains(group);
- }
-
-
- /**
- * Is this user specifically assigned the specified {@link Role}? This
- * method does <strong>NOT</strong> check for roles inherited based on
- * {@link Group} membership.
- *
- * @param role The role to check
- */
- @Override
- public boolean isInRole(Role role) {
- return roles.contains(role);
- }
-
-
- /**
- * Remove a {@link Group} from those this user belongs to.
- *
- * @param group The old group
- */
- @Override
- public void removeGroup(Group group) {
- groups.remove(group);
- }
-
-
- /**
- * Remove all {@link Group}s from those this user belongs to.
- */
- @Override
- public void removeGroups() {
- groups.clear();
- }
-
-
- /**
- * Remove a {@link Role} from those assigned to this user.
- *
- * @param role The old role
- */
- @Override
- public void removeRole(Role role) {
- roles.remove(role);
- }
-
-
- /**
- * Remove all {@link Role}s from those assigned to this user.
- */
- @Override
- public void removeRoles() {
- roles.clear();
+ super(database, username, password, fullName, null, null);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index eee49bb..3008837 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -127,6 +127,11 @@
<code>false</code>. Fixed via pull request <pr>438</pr> provided by
Robert Rodewald. (markt)
</fix>
+ <update>
+ Improve the reusability of the <code>UserDatabase</code> by adding
+ intermediate concrete implementation classes and allowing to do
+ partial database updates on <code>save</code>. (remm)
+ </update>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
