[Commits] (bcm) bug 2833: stop embedding the User in the user form. instead, instantiate a new

commits at osafoundation.org commits at osafoundation.org
Tue Apr 12 14:58:26 PDT 2005


Commit by: bcm
Modified files:
server/core/src/org/osaf/cosmo/model/User.java 1.1 1.2
server/docs/TODO.txt 1.48 1.49
server/web/src/org/osaf/cosmo/ui/UserAction.java 1.3 1.4
server/web/web/WEB-INF/struts-config.xml 1.4 1.5
server/web/web/WEB-INF/validation.xml 1.1 1.2
server/web/web/WEB-INF/jsp/user/list.jsp 1.3 1.4
server/web/web/WEB-INF/jsp/user/view.jsp 1.2 1.3

Log message:
bug 2833: stop embedding the User in the user form. instead, instantiate a new
User in action's create method and fetch the User from the db in the update
method. this approach guarantees that we always have the full set of User data
available to be displayed on the user page (previously, the dummy User
embedded in the form did not contain create and modified dates)


Bugzilla links:
http://bugzilla.osafoundation.org/show_bug.cgi?id=2833

ViewCVS links:
http://cvs.osafoundation.org/index.cgi/server/core/src/org/osaf/cosmo/model/User.java.diff?r1=text&tr1=1.1&r2=text&tr2=1.2
http://cvs.osafoundation.org/index.cgi/server/docs/TODO.txt.diff?r1=text&tr1=1.48&r2=text&tr2=1.49
http://cvs.osafoundation.org/index.cgi/server/web/src/org/osaf/cosmo/ui/UserAction.java.diff?r1=text&tr1=1.3&r2=text&tr2=1.4
http://cvs.osafoundation.org/index.cgi/server/web/web/WEB-INF/struts-config.xml.diff?r1=text&tr1=1.4&r2=text&tr2=1.5
http://cvs.osafoundation.org/index.cgi/server/web/web/WEB-INF/validation.xml.diff?r1=text&tr1=1.1&r2=text&tr2=1.2
http://cvs.osafoundation.org/index.cgi/server/web/web/WEB-INF/jsp/user/list.jsp.diff?r1=text&tr1=1.3&r2=text&tr2=1.4
http://cvs.osafoundation.org/index.cgi/server/web/web/WEB-INF/jsp/user/view.jsp.diff?r1=text&tr1=1.2&r2=text&tr2=1.3

Index: server/core/src/org/osaf/cosmo/model/User.java
diff -u server/core/src/org/osaf/cosmo/model/User.java:1.1 server/core/src/org/osaf/cosmo/model/User.java:1.2
--- server/core/src/org/osaf/cosmo/model/User.java:1.1	Fri Apr  8 16:53:24 2005
+++ server/core/src/org/osaf/cosmo/model/User.java	Tue Apr 12 14:58:23 2005
@@ -115,6 +115,12 @@
 
     /**
      */
+    public void removeRole(Role role) {
+        roles.remove(role);
+    }
+
+    /**
+     */
     public void setRoles(Set roles) {
         this.roles = roles;
     }

Index: server/docs/TODO.txt
diff -u server/docs/TODO.txt:1.48 server/docs/TODO.txt:1.49
--- server/docs/TODO.txt:1.48	Mon Apr 11 17:13:28 2005
+++ server/docs/TODO.txt	Tue Apr 12 14:58:23 2005
@@ -7,8 +7,6 @@
  * don't pack hibernate config file in jar and place it somewhere a
    cosmo admin can find it to tweak the dialect
  * cache hibernate collections
- * re-implement user pages without the goofy form-based user object
-   that is screwing everything up.
 
 docs:
 

Index: server/web/web/WEB-INF/struts-config.xml
diff -u server/web/web/WEB-INF/struts-config.xml:1.4 server/web/web/WEB-INF/struts-config.xml:1.5
--- server/web/web/WEB-INF/struts-config.xml:1.4	Fri Apr  8 16:53:30 2005
+++ server/web/web/WEB-INF/struts-config.xml	Tue Apr 12 14:58:24 2005
@@ -9,10 +9,10 @@
   <form-beans>
     <form-bean name="userForm"
                type="org.apache.struts.validator.DynaValidatorForm">
-      <form-property name="user" type="org.osaf.cosmo.model.User"/>
+      <form-property name="username" type="java.lang.String"/>
+      <form-property name="email" type="java.lang.String"/>
       <form-property name="password" type="java.lang.String"/>
       <form-property name="confirm" type="java.lang.String"/>
-      <form-property name="username" type="java.lang.String"/>
       <form-property name="role" type="java.lang.String[]"/>
     </form-bean>
   </form-beans>

Index: server/web/web/WEB-INF/jsp/user/view.jsp
diff -u server/web/web/WEB-INF/jsp/user/view.jsp:1.2 server/web/web/WEB-INF/jsp/user/view.jsp:1.3
--- server/web/web/WEB-INF/jsp/user/view.jsp:1.2	Thu Apr  7 17:14:46 2005
+++ server/web/web/WEB-INF/jsp/user/view.jsp	Tue Apr 12 14:58:25 2005
@@ -28,9 +28,9 @@
         <b><fmt:message key="User.Form.Email"/></b>
       </td>
       <td class="md" align="left">
-        <html:text property="user.email" size="32" maxlength="32"
+        <html:text property="email" size="32" maxlength="32"
                    styleClass="md"/>
-        <cosmo:errmsg property="user.email"/>
+        <cosmo:errmsg property="email"/>
       </td>
     </tr>
     <tr>
@@ -84,5 +84,5 @@
       </td>
     </tr>
   </table>
-  <html:hidden property="user.username"/>
+  <html:hidden property="username"/>
 </html:form>

Index: server/web/web/WEB-INF/jsp/user/list.jsp
diff -u server/web/web/WEB-INF/jsp/user/list.jsp:1.3 server/web/web/WEB-INF/jsp/user/list.jsp:1.4
--- server/web/web/WEB-INF/jsp/user/list.jsp:1.3	Fri Apr  8 11:22:00 2005
+++ server/web/web/WEB-INF/jsp/user/list.jsp	Tue Apr 12 14:58:25 2005
@@ -67,9 +67,9 @@
         <b><fmt:message key="User.Form.Username"/></b>
       </td>
       <td class="md" align="left">
-        <html:text property="user.username" size="32" maxlength="32"
+        <html:text property="username" size="32" maxlength="32"
                    styleClass="md"/>
-        <cosmo:errmsg property="user.username"/>
+        <cosmo:errmsg property="username"/>
       </td>
     </tr>
     <tr>
@@ -77,9 +77,9 @@
         <b><fmt:message key="User.Form.Email"/></b>
       </td>
       <td class="md" align="left">
-        <html:text property="user.email" size="32" maxlength="32"
+        <html:text property="email" size="32" maxlength="32"
                    styleClass="md"/>
-        <cosmo:errmsg property="user.email"/>
+        <cosmo:errmsg property="email"/>
       </td>
     </tr>
     <tr>

Index: server/web/src/org/osaf/cosmo/ui/UserAction.java
diff -u server/web/src/org/osaf/cosmo/ui/UserAction.java:1.3 server/web/src/org/osaf/cosmo/ui/UserAction.java:1.4
--- server/web/src/org/osaf/cosmo/ui/UserAction.java:1.3	Mon Apr 11 17:10:06 2005
+++ server/web/src/org/osaf/cosmo/ui/UserAction.java	Tue Apr 12 14:58:24 2005
@@ -8,6 +8,7 @@
 import org.osaf.struts.OSAFStrutsConstants;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 
@@ -56,16 +57,15 @@
      */
     public static final String ATTR_ROLES = "Roles";
     /**
-     * The form attribute in which this action places the identified
-     * User and expects to find it when handling a form submission:
-     * <code>user</code>
+     * The form attribute in which this action expects to find the
+     * username: <code>username</code>
      */
-    public static final String FORMATTR_USER = "user";
+    public static final String FORMATTR_USERNAME = "username";
     /**
      * The form attribute in which this action expects to find the
-     * username: <code>user.username</code>
+     * username: <code>email</code>
      */
-    public static final String FORMATTR_USERNAME = "user.username";
+    public static final String FORMATTR_EMAIL = "email";
     /**
      * The form attribute in which this action expects to find the
      * user's password when handling a form submission:
@@ -103,23 +103,20 @@
         throws Exception {
         DynaActionForm userForm = (DynaActionForm) form;
 
+        // the User may have previously been set by a request
+        // attribute. if not, the username should be available as a
+        // request parameter.
         User user = (User) request.getAttribute(ATTR_USER);
         if (user == null) {
             String username = request.getParameter(PARAM_USERNAME);
-            if (username != null) {
-                if (log.isDebugEnabled()) {
-                    log.debug("viewing user " + username);
-                }
-                user = mgr.getUser(username);
-                populateUserForm(userForm, user);
+            if (log.isDebugEnabled()) {
+                log.debug("viewing user " + username);
             }
-        }
-        if (user == null) {
-            user = (User) userForm.get(FORMATTR_USER);
+            user = mgr.getUser(username);
+            populateForm(userForm, user);
         }
 
         request.setAttribute(ATTR_USER, user);
-        userForm.set(FORMATTR_USER, user);
         request.setAttribute(ATTR_ROLES, getSortedRoleNames());
 
         addTitleParam(request, user.getUsername());
@@ -135,7 +132,9 @@
                                 HttpServletRequest request,
                                 HttpServletResponse response)
         throws Exception {
-        User formUser = populateUser((DynaActionForm) form);
+        DynaActionForm userForm = (DynaActionForm) form;
+        User formUser = new User();
+        populateUser(formUser, userForm);
 
         try {
             if (log.isDebugEnabled()) {
@@ -164,12 +163,14 @@
         DynaActionForm userForm = (DynaActionForm) form;
 
         if (isCancelled(request)) {
-            resetUserForm(userForm);
+            resetForm(userForm);
             return mapping.findForward(OSAFStrutsConstants.FWD_CANCEL);
         }
 
         try {
-            User formUser = populateUser(userForm);
+            User formUser =
+                mgr.getUser((String) userForm.get(FORMATTR_USERNAME));
+            populateUser(formUser, userForm);
             if (log.isDebugEnabled()) {
                 log.debug("updating user " + formUser.getUsername());
             }
@@ -195,12 +196,14 @@
         throws Exception {
         String username = request.getParameter(PARAM_USERNAME);
 
-        if (log.isDebugEnabled()) {
-            log.debug("removing user " + username);
-        }
-        mgr.removeUser(username);
+        if (username != null) {
+            if (log.isDebugEnabled()) {
+                log.debug("removing user " + username);
+            }
+            mgr.removeUser(username);
 
-        saveConfirmationMessage(request, MSG_CONFIRM_REMOVE);
+            saveConfirmationMessage(request, MSG_CONFIRM_REMOVE);
+        }
 
         return mapping.findForward(OSAFStrutsConstants.FWD_SUCCESS);
     }
@@ -258,35 +261,36 @@
         return mapRolesToRolenames((Role[]) roles.toArray(new Role[0]));
     }
 
-    private User populateUser(DynaActionForm form) {
-        User user = (User) form.get(FORMATTR_USER);
+    private void populateUser(User user, DynaActionForm form) {
+        user.setUsername((String) form.get(FORMATTR_USERNAME));
+        user.setEmail((String) form.get(FORMATTR_EMAIL));
         user.setPassword((String) form.get(FORMATTR_PASSWORD));
         String[] roles = (String[]) form.get(FORMATTR_ROLE);
+        HashMap idx = new HashMap();
         for (int i=0; i<roles.length; i++) {
             Role role = mgr.getRole(roles[i]);
+            idx.put(role.getName(), role);
             user.addRole(role);
         }
-
-        // XXX: here's an ugly hack to work around the fact that the
-        // form does not contain the user's creation date. it's
-        // probably cleaner to just put the username and email fields
-        // into the form directly and stop having the form pass around
-        // a User, since the current approach makes us think the
-        // form's User is capable of being used directly for an
-        // update, and as this method shows, it's clearly not.
-        User storedUser = mgr.getUser(user.getUsername());
-        user.setDateCreated(storedUser.getDateCreated());
-
-        return user;
+        for (Iterator i=user.getRoles().iterator(); i.hasNext();) {
+            Role role = (Role) i.next();
+            if (idx.get(role.getName()) == null) {
+                i.remove();
+            }
+        }
     }
 
-    private void populateUserForm(DynaActionForm form, User user) {
+    private void populateForm(DynaActionForm form, User user) {
+        form.set(FORMATTR_USERNAME, user.getUsername());
+        form.set(FORMATTR_EMAIL, user.getEmail());
+        // never set password in the form
         Role[] roles = (Role[]) user.getRoles().toArray(new Role[0]);
         form.set(FORMATTR_ROLE, mapRolesToRolenames(roles));
     }
 
-    private void resetUserForm(DynaActionForm form) {
-        form.set(FORMATTR_USER, new User());
+    private void resetForm(DynaActionForm form) {
+        form.set(FORMATTR_USERNAME, null);
+        form.set(FORMATTR_EMAIL, null);
         form.set(FORMATTR_PASSWORD, null);
         form.set(FORMATTR_ROLE, new String[0]);
     }

Index: server/web/web/WEB-INF/validation.xml
diff -u server/web/web/WEB-INF/validation.xml:1.1 server/web/web/WEB-INF/validation.xml:1.2
--- server/web/web/WEB-INF/validation.xml:1.1	Wed Apr  6 20:15:03 2005
+++ server/web/web/WEB-INF/validation.xml	Tue Apr 12 14:58:24 2005
@@ -13,7 +13,7 @@
    </global>
    <formset>
      <form name="userForm">
-       <field property="user.username"
+       <field property="username"
               depends="required,mask,minlength,maxlength">
          <msg name="required" key="Form.FieldRequired"/>
          <msg name="mask" key="User.Form.UsernameInvalid"/>
@@ -36,7 +36,7 @@
            <var-value>32</var-value>
          </var>
        </field>
-       <field property="user.email"
+       <field property="email"
               depends="required,email,maxlength">
          <msg name="required" key="Form.FieldRequired"/>
          <msg name="email" key="Form.EmailInvalid"/>



More information about the Commits mailing list