Does lots of refactoring.

-> Removes most commented code (You can always check trunk)
-> Adds a lot of JavaDoc with NOTEs if there has been a bold design choice
-> Adds @Override annotations whenever a method gets overridden or implemented from an interface
-> Makes a lot of methods and classes private or package-local, because the scope can never be to small
-> Replaces some implementations with lambdas
-> Changes deprecated implementations to newer ones


git-svn-id: https://svn.libreccm.org/ccm/ccm_ng@4358 8810af33-2d31-482b-a856-94f89814c4df

ccm-cms/src/main/java/com/arsdigita/cms/ui/PartyAddForm.java

@@ -65,9 +65,6 @@ public abstract class PartyAddForm extends SimpleContainer
     private final static String CANCEL = "addCancel";
     private final static String SUBMIT_LABEL = "Add Members";
 
-    private final static String DQ_PARTY_ID = "partyId";
-    private final static String DQ_NAME = "name";
-
     private Widget m_search;
     private RequestLocal m_query;
 
@@ -76,8 +73,6 @@ public abstract class PartyAddForm extends SimpleContainer
 
     private Form m_form;
     private Hidden m_searchQuery;
-    private CheckboxGroup m_parties;
-    private Submit m_submit;
     private Submit m_cancel;
 
 
@@ -132,8 +127,9 @@ public abstract class PartyAddForm extends SimpleContainer
      *
      * @return The form
      */
-    protected Form makeForm() {
+    private Form makeForm() {
         final CMSForm form = new CMSForm("AddParties") {
+                @Override
                 public final boolean isCancelled(final PageState state) {
                     return m_cancel.isSelected(state);
                 }
@@ -146,22 +142,20 @@ public abstract class PartyAddForm extends SimpleContainer
         form.add(m_searchQuery, ColumnPanel.FULL_WIDTH);
 
         Label l = new Label(
-                            "Check the box next to the name of the person(s) to assign to this role.");
+                new GlobalizedMessage("Check the box next to the name of the person(s) to assign to this role."));
         form.add(l, ColumnPanel.FULL_WIDTH);
 
         // Add the list of parties that can be added.
-        m_parties = new CheckboxGroup(PARTIES);
+        CheckboxGroup m_parties = new CheckboxGroup(PARTIES);
         m_parties.addValidationListener(new NotNullValidationListener());
         try {
-            m_parties.addPrintListener(new PrintListener() {
+            m_parties.addPrintListener(event -> {
-                    public void prepare(PrintEvent event) {
                 CheckboxGroup target = (CheckboxGroup) event.getTarget();
                 PageState state = event.getPageState();
                 // Ensures that the init listener gets fired before the
                 // print listeners.
                 FormData data = m_form.getFormData(state);
                 addParties(state, target);
-                    }
             });
         } catch (TooManyListenersException e) {
             UncheckedWrapperException.throwLoggedException(getClass(), "Too many listeners", e);
@@ -170,7 +164,7 @@ public abstract class PartyAddForm extends SimpleContainer
 
         // Submit and Cancel buttons.
         SimpleContainer s = new SimpleContainer();
-        m_submit = new Submit(SUBMIT, new GlobalizedMessage(SUBMIT_LABEL));
+        Submit m_submit = new Submit(SUBMIT, new GlobalizedMessage(SUBMIT_LABEL));
         s.add(m_submit);
         m_cancel = new Submit(CANCEL, new GlobalizedMessage("Cancel"));
         s.add(m_cancel);
@@ -219,7 +213,7 @@ public abstract class PartyAddForm extends SimpleContainer
      * @param target The option group
      * @pre ( state != null && target != null )
      */
-    protected void addParties(PageState state, OptionGroup target) {
+    private void addParties(PageState state, OptionGroup target) {
         @SuppressWarnings("unchecked")
         List<Party> parties = (List<Party>) m_query.get(state);
 
@@ -246,6 +240,7 @@ public abstract class PartyAddForm extends SimpleContainer
      *
      * @param event The form event
      */
+    @Override
     public void init(FormSectionEvent event) throws FormProcessException {
         PageState state = event.getPageState();
 
@@ -257,6 +252,7 @@ public abstract class PartyAddForm extends SimpleContainer
      *
      * @param event The form event
      */
+    @Override
     public abstract void process(FormSectionEvent event)
         throws FormProcessException;
 
@@ -267,6 +263,7 @@ public abstract class PartyAddForm extends SimpleContainer
      * @param state The page state
      * @param parent The parent DOM element
      */
+    @Override
     public void generateXML(PageState state, Element parent) {
 
         @SuppressWarnings("unchecked")

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/BaseRoleForm.java

@@ -45,7 +45,7 @@ import java.util.*;
  * @author Justin Ross <jross@redhat.com>
  * @version $Id: BaseRoleForm.java 287 2005-02-22 00:29:02Z sskracic $
  */
-public class BaseRoleForm extends BaseForm {
+class BaseRoleForm extends BaseForm {
 
     private static final Logger s_log = Logger.getLogger(BaseRoleForm.class);
 
@@ -53,15 +53,10 @@ public class BaseRoleForm extends BaseForm {
     final Description m_description;
     CheckboxGroup m_privileges;
 
-    private boolean m_useViewersGroup;
+    BaseRoleForm(final String key,
-
+                 final GlobalizedMessage message) {
-    public BaseRoleForm(final String key,
-                        final GlobalizedMessage message,
-                        final boolean useViewersGroup) {
         super(key, message);
 
-        m_useViewersGroup = useViewersGroup;
-
         m_name = new Name("label", 200, true);
         addField(gz("cms.ui.name"), m_name);
 

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/BaseRoleItemPane.java

@@ -57,18 +57,14 @@ class BaseRoleItemPane extends BaseItemPane {
         (BaseRoleItemPane.class);
 
     private final RoleRequestLocal m_role;
-    private final SingleSelectionModel m_model;
 
     private final MemberTable m_members;
     //private final AdminTable m_admins;
 
-    private final SimpleContainer m_detailPane;
+    BaseRoleItemPane(final SingleSelectionModel model,
-
-    public BaseRoleItemPane(final SingleSelectionModel model,
                      final RoleRequestLocal role,
                      final ActionLink editLink,
                      final ActionLink deleteLink) {
-        m_model = model;
         m_role = role;
 
         m_members = new MemberTable();
@@ -81,7 +77,7 @@ class BaseRoleItemPane extends BaseItemPane {
         final ActionLink adminAddLink = new ActionLink
             (new Label(gz("cms.ui.role.admin.add")));*/
 
-        m_detailPane = new SimpleContainer();
+        SimpleContainer m_detailPane = new SimpleContainer();
         add(m_detailPane);
         setDefault(m_detailPane);
 
@@ -94,11 +90,11 @@ class BaseRoleItemPane extends BaseItemPane {
         add(memberSearchForm);
 
         final RolePartyAddForm memberAddForm = new RolePartyAddForm
-            (m_model, memberSearchForm.getSearchWidget());
+            (model, memberSearchForm.getSearchWidget());
         add(memberAddForm);
 
-        final PartySearchForm adminSearchForm = new PartySearchForm();
+        //final PartySearchForm adminSearchForm = new PartySearchForm();
-        add(adminSearchForm);
+        //add(adminSearchForm);
 
         /*
         final RoleAdminAddForm adminAddForm = new RoleAdminAddForm
@@ -218,6 +214,7 @@ class BaseRoleItemPane extends BaseItemPane {
         }
 
         private class Listener extends TableActionAdapter {
+            @Override
             public final void cellSelected(final TableActionEvent e) {
                 final CdiUtil cdiUtil = CdiUtil.createCdiUtil();
                 final PageState state = e.getPageState();

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/MemberTableModelBuilder.java

@@ -55,6 +55,7 @@ class MemberTableModelBuilder extends AbstractTableModelBuilder {
         m_role = role;
     }
 
+    @Override
     public final TableModel makeModel(final Table table,
                                       final PageState state) {
         final Role role = m_role.getRole(state);
@@ -80,10 +81,12 @@ class MemberTableModelBuilder extends AbstractTableModelBuilder {
             iterator = m_parties.iterator();
         }
 
+        @Override
         public final int getColumnCount() {
             return 3;
         }
 
+        @Override
         public final boolean nextRow() {
             if (iterator.hasNext()) {
                 m_party = iterator.next();
@@ -93,23 +96,18 @@ class MemberTableModelBuilder extends AbstractTableModelBuilder {
             }
         }
 
+        @Override
         public final Object getKeyAt(final int column) {
             return m_party.getPartyId();
         }
 
+        @Override
         public final Object getElementAt(final int column) {
             switch (column) {
             case 0:
                 return m_party.getName();
             case 1:
-                //FIXME Party does not have a field for emails anymore.
-                final EmailAddress email = null;
-
-                if (email == null) {
                 return lz("cms.ui.none");
-                } else {
-                    return email.toString();
-                }
             case 2:
                 return lz("cms.ui.role.member.remove");
             default:
@@ -118,7 +116,7 @@ class MemberTableModelBuilder extends AbstractTableModelBuilder {
         }
     }
 
-    protected final static String lz(final String key) {
+    protected static String lz(final String key) {
         return (String) GlobalizationUtil.globalize(key).localize();
     }
 }

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/RoleAddForm.java

@@ -23,21 +23,19 @@ import com.arsdigita.bebop.PageState;
 import com.arsdigita.bebop.SingleSelectionModel;
 import com.arsdigita.bebop.event.FormProcessListener;
 import com.arsdigita.bebop.event.FormSectionEvent;
-import com.arsdigita.cms.CMS;
 import com.arsdigita.kernel.KernelConfig;
 import org.apache.log4j.Logger;
 import org.libreccm.cdi.utils.CdiUtil;
 import org.libreccm.configuration.ConfigurationManager;
 import org.libreccm.l10n.LocalizedString;
 import org.libreccm.security.*;
-import org.librecms.contentsection.ContentSection;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
 /**
- * For more detailed information see {@link com.arsdigita.bebop.Form}.
+ * Provides a {@link com.arsdigita.bebop.Form} for adding {@link Role roles}.
  *
  * @author <a href="mailto:yannick.buelter@yabue.de">Yannick Bülter</a>
  * @author Michael Pih
@@ -49,20 +47,25 @@ final class RoleAddForm extends BaseRoleForm {
     private static final Logger s_log = Logger.getLogger(RoleAddForm.class);
 
     private SingleSelectionModel m_model;
-    private final boolean m_useViewersGroup;
 
-    public RoleAddForm(SingleSelectionModel model, boolean useViewersGroup) {
+    RoleAddForm(SingleSelectionModel model) {
-        super("AddStaffRole", gz("cms.ui.role.add"), useViewersGroup);
+        super("AddStaffRole", gz("cms.ui.role.add"));
 
         m_model = model;
-        m_useViewersGroup = useViewersGroup;
 
         m_name.addValidationListener(new NameUniqueListener(null));
 
         addProcessListener(new ProcessListener());
     }
 
+    /**
+     * The {@link Role} gets saved to the database and permissions are granted as needed.
+     *
+     * NOTE: The part about granting and revoking privileges is mostly Copy & Paste from {@link RoleEditForm}.
+     * If you find any bugs or errors in this code, be sure to change it there accordingly.
+     */
     private class ProcessListener implements FormProcessListener {
+        @Override
         public final void process(final FormSectionEvent e)
                 throws FormProcessException {
             final PageState state = e.getPageState();
@@ -71,6 +74,7 @@ final class RoleAddForm extends BaseRoleForm {
             final PermissionManager permissionManager = cdiUtil.findBean(PermissionManager.class);
             final ConfigurationManager manager = cdiUtil.findBean(ConfigurationManager.class);
             final KernelConfig config = manager.findConfiguration(KernelConfig.class);
+            final RoleRepository roleRepository = cdiUtil.findBean(RoleRepository.class);
 
             final Role role = new Role();
 
@@ -80,22 +84,14 @@ final class RoleAddForm extends BaseRoleForm {
             localizedDescription.addValue(config.getDefaultLocale(), (String) m_description.getValue(state));
             role.setDescription(localizedDescription);
 
-            List<Permission> newPermissions = new ArrayList<>();
+            //We don't now if the permissions list is empty, so we have to save beforehand to not lose data.
+            roleRepository.save(role);
+
             String[] selectedPermissions = (String[]) m_privileges.getValue(state);
 
-            for (Permission p : role.getPermissions()) {
-                if (Arrays.stream(selectedPermissions).anyMatch(x -> x.equals(p.getGrantedPrivilege()))) {
-                    newPermissions.add(p);
-                } else {
-                    permissionManager.revokePrivilege(p.getGrantedPrivilege(), role);
-                }
-            }
-
             for (String s : selectedPermissions) {
-                if (newPermissions.stream().noneMatch(x -> x.getGrantedPrivilege().equals(s))) {
                 permissionManager.grantPrivilege(s, role);
             }
-            }
 
             m_model.setSelectedKey(state, Long.toString(role.getRoleId()));
         }

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/RoleAdminPane.java

@@ -47,7 +47,11 @@ import org.librecms.CmsConstants;
 import org.librecms.contentsection.ContentSection;
 
 /**
- * TODO Needs description
+ * Provides the logic to administer {@link Role roles}.
+ *
+ * NOTE: Prior, this class managed two {@link ListModelBuilder}.
+ * The reason being, that roles where differentiated between Viewer and Member groups.
+ * Since this is no longer the case, there exists only the {@link RoleListModelBuilder} now.
  *
  * @author <a href="mailto:yannick.buelter@yabue.de">Yannick Bülter</a>
  * @author Justin Ross &lt;jross@redhat.com&gt;
@@ -57,10 +61,7 @@ public class RoleAdminPane extends BaseAdminPane {
 
     private static final Logger s_log = Logger.getLogger(RoleAdminPane.class);
 
-    private static final CdiUtil cdiutil = CdiUtil.createCdiUtil();
-
     private final SingleSelectionModel m_model;
-    private final RoleRequestLocal m_role;
 
     private final List m_roles;
 
@@ -71,7 +72,7 @@ public class RoleAdminPane extends BaseAdminPane {
 
         m_model.addChangeListener(new SelectionListener());
 
-        m_role = new SelectionRequestLocal();
+        RoleRequestLocal m_role = new SelectionRequestLocal();
 
         m_roles = new List(new RoleListModelBuilder());
         m_roles.setSelectionModel(m_model);
@@ -83,7 +84,7 @@ public class RoleAdminPane extends BaseAdminPane {
         final RoleSection roleSection = new RoleSection();
         left.add(roleSection);
 
-        setEdit(gz("cms.ui.role.edit"), new RoleEditForm(m_role, false));
+        setEdit(gz("cms.ui.role.edit"), new RoleEditForm(m_role));
         setDelete(gz("cms.ui.role.delete"), new DeleteForm());
 
         setIntroPane(new Label(gz("cms.ui.role.intro")));
@@ -107,57 +108,14 @@ public class RoleAdminPane extends BaseAdminPane {
             group.addAction(new VisibilityComponent(link, CmsConstants.PRIVILEGE_ADMINISTER_ROLES),
                     ActionGroup.ADD);
 
-            final RoleAddForm form = new RoleAddForm(m_model, false);
+            final RoleAddForm form = new RoleAddForm(m_model);
             getBody().add(form);
             getBody().connect(link, form);
         }
     }
 
-    /*
-    private class StaffSection extends Section {
-        StaffSection() {
-            setHeading(gz("cms.ui.role.staff"));
-
-            final ActionGroup group = new ActionGroup();
-            setBody(group);
-
-            group.setSubject(m_staff);
-
-            final ActionLink link = new ActionLink
-                (new Label(gz("cms.ui.role.staff.add")));
-
-            group.addAction(new VisibilityComponent(link, SecurityConstants.STAFF_ADMIN),
-                            ActionGroup.ADD);
-
-            final RoleAddForm form = new RoleAddForm(m_model, false);
-            getBody().add(form);
-            getBody().connect(link, form);
-        }
-    }
-
-
-    private class ViewerSection extends Section {
-        ViewerSection() {
-            setHeading(gz("cms.ui.role.viewers"));
-
-            final ActionGroup group = new ActionGroup();
-            setBody(group);
-
-            group.setSubject(m_viewers);
-
-            final ActionLink link = new ActionLink
-                (new Label(gz("cms.ui.role.viewer.add")));
-
-            group.addAction(new VisibilityComponent(link, SecurityConstants.STAFF_ADMIN),
-                            ActionGroup.ADD);
-
-            final RoleAddForm form = new RoleAddForm(m_model, true);
-            getBody().add(form);
-            getBody().connect(link, form);
-        }
-    }*/
-
     private class SelectionListener implements ChangeListener {
+        @Override
         public final void stateChanged(final ChangeEvent e) {
             s_log.debug("Selection state changed; I may change " +
                         "the body's visible pane");
@@ -176,57 +134,27 @@ public class RoleAdminPane extends BaseAdminPane {
     }
 
     private class SelectionRequestLocal extends RoleRequestLocal {
+        @Override
         protected final Object initialValue(final PageState state) {
             final Long id = Long.parseLong(m_model.getSelectedKey(state).toString());
-            final RoleRepository roleRepository = cdiutil.findBean(RoleRepository.class);
+            final CdiUtil cdiUtil = CdiUtil.createCdiUtil();
+            final RoleRepository roleRepository = cdiUtil.findBean(RoleRepository.class);
 
             return roleRepository.findById(id);
         }
     }
 
-    /* TODO Removed since we don't split viewers and staff right now!
-    private static class StaffListModelBuilder extends LockableImpl
-            implements ListModelBuilder {
-        public StaffListModelBuilder() {
-            super();
-        }
-
-        public final ListModel makeModel(final List list,
-                                         final PageState state) {
-            final ContentSection section =
-                CMS.getContext().getContentSection();
-
-            //return new RoleListModel
-            //    (section.getStaffGroup().getOrderedRoles());
-            return new RoleListModel(new ArrayList<>());
-        }
-    }
-
-    private static class ViewerListModelBuilder extends LockableImpl
-            implements ListModelBuilder {
-        public final ListModel makeModel(final List list,
-                                         final PageState state) {
-            final ContentSection section =
-                CMS.getContext().getContentSection();
-
-            //return new RoleListModel
-            //    (section.getViewersGroup().getOrderedRoles());
-            return new RoleListModel(new ArrayList<>());
-        }
-    }
-
-*/
-
     /**
      * This builder provides a list model of the {@link Role roles} which correspond to the {@link ContentSection}
      * in this context.
      */
     private static class RoleListModelBuilder extends LockableImpl implements  ListModelBuilder {
 
-        public RoleListModelBuilder() {
+        RoleListModelBuilder() {
             super();
         }
 
+        @Override
         public final ListModel makeModel(final List list, final PageState state) {
             final ContentSection section = CMS.getContext().getContentSection();
 
@@ -244,11 +172,13 @@ public class RoleAdminPane extends BaseAdminPane {
             addSecurityListener(CmsConstants.PRIVILEGE_ADMINISTER_ROLES);
         }
 
+        @Override
         public final void process(final FormSectionEvent e)
                 throws FormProcessException {
             final PageState state = e.getPageState();
 
-            final RoleRepository roleRepository = cdiutil.findBean(RoleRepository.class);
+            final CdiUtil cdiUtil = CdiUtil.createCdiUtil();
+            final RoleRepository roleRepository = cdiUtil.findBean(RoleRepository.class);
             final Long id = Long.parseLong(m_model.getSelectedKey(state).toString());
             final Role role = roleRepository.findById(id);
 

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/RoleEditForm.java

@@ -35,7 +35,7 @@ import java.util.Arrays;
 import java.util.List;
 
 /**
- * For more detailed information see {@link com.arsdigita.bebop.Form}.
+ * Represents a {@link com.arsdigita.bebop.Form Form} to edit {@link Role roles}.
  *
  * @author <a href="mailto:yannick.buelter@yabue.de">Yannick Bülter</a>
  * @author Michael Pih
@@ -47,13 +47,11 @@ final class RoleEditForm extends BaseRoleForm {
     private static final Logger s_log = Logger.getLogger(RoleEditForm.class);
 
     private final RoleRequestLocal m_role;
-    //private final boolean m_useViewersGroup;
 
-    public RoleEditForm(RoleRequestLocal role, boolean useViewersGroup) {
+    RoleEditForm(RoleRequestLocal role) {
-        super("EditStaffRole", gz("cms.ui.role.edit"), useViewersGroup);
+        super("EditStaffRole", gz("cms.ui.role.edit"));
 
         m_role = role;
-        //m_useViewersGroup = useViewersGroup;
 
         m_name.addValidationListener(new NameUniqueListener(m_role));
 
@@ -61,7 +59,11 @@ final class RoleEditForm extends BaseRoleForm {
         addProcessListener(new ProcessListener());
     }
 
+    /**
+     * Sets the initial values of a {@link Role} which were received from the database.
+     */
     private class InitListener implements FormInitListener {
+        @Override
         public final void init(final FormSectionEvent e) {
             final PageState state = e.getPageState();
             final Role role = m_role.getRole(state);
@@ -69,8 +71,6 @@ final class RoleEditForm extends BaseRoleForm {
             m_name.setValue(state, role.getName());
             m_description.setValue(state, role.getDescription());
 
-            //final String[] privileges = RoleFactory.getRolePrivileges
-             //   (CMS.getContext().getContentSection(), role);
             final String[] permissions = role.getPermissions().stream().
                     map(Permission::getGrantedPrivilege).toArray(String[]::new);
 
@@ -78,7 +78,15 @@ final class RoleEditForm extends BaseRoleForm {
         }
     }
 
+    /**
+     * Updates a role and it's permissions. It uses the {@link PermissionManager} to grant and revoke permissions
+     * as needed.
+     *
+     * NOTE: The part about granting and revoking privileges is mostly identical to {@link RoleAddForm}.
+     * If you find any bugs or errors in this code, be sure to change it there accordingly.
+     */
     private class ProcessListener implements FormProcessListener {
+        @Override
         public final void process(final FormSectionEvent e)
                 throws FormProcessException {
             final PageState state = e.getPageState();
@@ -90,11 +98,15 @@ final class RoleEditForm extends BaseRoleForm {
             final PermissionManager permissionManager = cdiUtil.findBean(PermissionManager.class);
             final ConfigurationManager manager = cdiUtil.findBean(ConfigurationManager.class);
             final KernelConfig config = manager.findConfiguration(KernelConfig.class);
+            final RoleRepository roleRepository = cdiUtil.findBean(RoleRepository.class);
 
             LocalizedString localizedDescription = role.getDescription();
             localizedDescription.addValue(config.getDefaultLocale(), (String) m_description.getValue(state));
             role.setDescription(localizedDescription);
 
+            //We don't now if the permissions list is empty, so we have to save beforehand to not lose data.
+            roleRepository.save(role);
+
             List<Permission> newPermissions = new ArrayList<>();
             String[] selectedPermissions = (String[]) m_privileges.getValue(state);
 

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/RoleListModel.java

@@ -42,20 +42,23 @@ class RoleListModel implements ListModel {
 
     private Role currentRole;
 
-    public RoleListModel(final Collection<Role> roles) {
+    RoleListModel(final Collection<Role> roles) {
         m_roles = roles;
         iterator = roles.iterator();
     }
 
+    @Override
     public final boolean next() {
         currentRole = iterator.next();
         return currentRole != null;
     }
 
+    @Override
     public final Object getElement() {
         return currentRole.getName();
     }
 
+    @Override
     public final String getKey() {
         return Long.toString(currentRole.getRoleId());
     }

ccm-cms/src/main/java/com/arsdigita/cms/ui/role/RoleRequestLocal.java

@@ -29,9 +29,9 @@ import org.libreccm.security.Role;
  * @author <a href="mailto:yannick.buelter@yabue.de">Yannick Bülter</a>
  * @version $Id: RoleRequestLocal.java 287 2005-02-22 00:29:02Z sskracic $
  */
-public abstract class RoleRequestLocal extends RequestLocal {
+abstract class RoleRequestLocal extends RequestLocal {
 
-    public final Role getRole(final PageState state) {
+    final Role getRole(final PageState state) {
         final Role role = (Role) get(state);
 
         Assert.exists(role, "Role role");