CCM NG: Bug fixing for Configuration Admin UI

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

ccm-core/src/main/java/com/arsdigita/ui/admin/AdminServlet.java

@@ -30,6 +30,7 @@ import com.arsdigita.templating.Templating;
 import com.arsdigita.ui.SiteBanner;
 import com.arsdigita.ui.UserBanner;
 import com.arsdigita.ui.admin.categories.CategoriesTab;
+import com.arsdigita.ui.admin.configuration.ConfigurationTab;
 import com.arsdigita.web.BaseApplicationServlet;
 import com.arsdigita.web.LoginSignal;
 import com.arsdigita.xml.Document;
@@ -131,7 +132,7 @@ public class AdminServlet
         tabbedPane.addTab(
                 new Label(new GlobalizedMessage("ui.admin.tab.configuration.title",
                                                 BUNDLE_NAME)),
-                new RegistryAdminTab());
+                new ConfigurationTab());
 
         tabbedPane.addTab(
                 new Label(new GlobalizedMessage("ui.admin.tab.workflows.title",

ccm-core/src/main/java/com/arsdigita/ui/admin/RegistryAdminTab.java

@@ -1,39 +0,0 @@
-/*
- * Copyright (C) 2016 LibreCCM Foundation.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
- * MA 02110-1301  USA
- */
-package com.arsdigita.ui.admin;
-
-import com.arsdigita.bebop.Label;
-import com.arsdigita.toolbox.ui.LayoutPanel;
-
-/**
- *
- * @author <a href="mailto:jens.pelzetter@googlemail.com">Jens Pelzetter</a>
- */
-public class RegistryAdminTab extends LayoutPanel {
-
-    public RegistryAdminTab() {
-        super();
-
-        setLeft(new Label("Registry Admin Tab Left"));
-
-        setBody(new Label("Registry Admin Tab Body"));
-
-    }
-
-}

ccm-core/src/main/java/com/arsdigita/ui/admin/configuration/ConfigurationsTable.java

@@ -163,7 +163,12 @@ public class ConfigurationsTable extends Table {
                     final ConfigurationInfo info = confManager
                         .getConfigurationInfo(c);
 //                    return c.getName().startsWith(filterTerm);
-                    return info.getTitle(negoiatedLocale).startsWith(filterTerm);
+                    if (filterTerm == null || filterTerm.isEmpty()) {
+                        return true;
+                    } else {
+                        return info.getTitle(negoiatedLocale).startsWith(
+                            filterTerm);
+                    }
                 })
                 .collect(Collectors.toCollection(ArrayList::new));
             configurations.sort((c1, c2) -> {

ccm-core/src/main/java/org/libreccm/configuration/ConfigurationInfo.java

@@ -18,8 +18,12 @@
  */
 package org.libreccm.configuration;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
 import java.util.Collections;
 import java.util.Locale;
+import java.util.MissingResourceException;
 import java.util.NavigableMap;
 import java.util.Objects;
 import java.util.ResourceBundle;
@@ -32,6 +36,9 @@ import java.util.TreeMap;
  */
 public final class ConfigurationInfo {
 
+    private final static Logger LOGGER = LogManager.getLogger(
+        ConfigurationInfo.class);
+
     /**
      * The fully qualified name of the configuration.
      */
@@ -81,7 +88,15 @@ public final class ConfigurationInfo {
     }
 
     public ResourceBundle getDescriptionBundle(final Locale locale) {
-        return ResourceBundle.getBundle(descBundle);
+        try {
+            return ResourceBundle.getBundle(descBundle);
+        } catch (MissingResourceException ex) {
+            LOGGER.warn(
+                "Failed to find ResourceBundle for base name '{}' and "
+                    + "locale '{}'.",
+                descBundle, locale);
+            return null;
+        }
     }
 
     public String getTitleKey() {
@@ -101,11 +116,41 @@ public final class ConfigurationInfo {
     }
 
     public String getTitle(final Locale locale) {
-        return getDescriptionBundle(locale).getString(titleKey);
+        final ResourceBundle bundle = getDescriptionBundle(locale);
+
+        if (bundle == null) {
+            return name;
+        } else {
+            try {
+                return bundle.getString(titleKey);
+            } catch (MissingResourceException ex) {
+                LOGGER.warn("Can't find resource for bundle '{}', "
+                                + "key '{}' and locale '{}'.",
+                            descBundle,
+                            titleKey,
+                            locale);
+                return name;
+            }
+        }
     }
 
     public String getDescription(final Locale locale) {
-        return getDescriptionBundle(locale).getString(descKey);
+        final ResourceBundle bundle = getDescriptionBundle(locale);
+
+        if (bundle == null) {
+            return "";
+        } else {
+            try {
+                return bundle.getString(descKey);
+            } catch (MissingResourceException ex) {
+                LOGGER.warn("Can't find resource for bundle '{}', "
+                                + "key '{}' and locale '{}'.",
+                            descBundle,
+                            descKey,
+                            locale);
+                return "";
+            }
+        }
     }
 
     public NavigableMap<String, SettingInfo> getSettings() {
@@ -159,11 +204,11 @@ public final class ConfigurationInfo {
     @Override
     public String toString() {
         return String.format("%s{ "
-                                     + "name = \"%s\", "
+                                 + "name = \"%s\", "
-                                     + "descBundle = \"%s\", "
+                                 + "descBundle = \"%s\", "
-                                     + "titleKey = \"%s\", "
+                                 + "titleKey = \"%s\", "
-                                     + "descKey = \"%s\""
+                                 + "descKey = \"%s\""
-                                     + " }",
+                                 + " }",
                              super.toString(),
                              name,
                              descBundle,

ccm-core/src/main/java/org/libreccm/configuration/ConfigurationManager.java

@@ -44,7 +44,7 @@ import java.util.TreeSet;
 public class ConfigurationManager {
 
     private static final Logger LOGGER = LogManager.getLogger(
-            ConfigurationManager.class);
+        ConfigurationManager.class);
 
     @Inject
     private SettingManager settingManager;
@@ -54,43 +54,43 @@ public class ConfigurationManager {
 
     /**
      * Finds all configuration classes listed by the installed modules.
-     * 
+     *
-     * @return A sorted set containing all configuration classes. 
+     * @return A sorted set containing all configuration classes.
-     * 
+     *
-     * @see Module#configurations() 
+     * @see Module#configurations()
      */
     public SortedSet<Class<?>> findAllConfigurations() {
-         final ServiceLoader<CcmModule> modules = ServiceLoader.load(
+        final ServiceLoader<CcmModule> modules = ServiceLoader.load(
             CcmModule.class);
-         
-         final SortedSet<Class<?>> configurations = new TreeSet<>((c1, c2) -> {
-             return c1.getName().compareTo(c2.getName());
-         });
-         
-         for(CcmModule module : modules) {
-             final Module annotation = module.getClass().getAnnotation(
-                 Module.class);
-             
-             if (annotation == null) {
-                 continue;
-             }
-             
-             Arrays.stream(annotation.configurations()).forEach(c -> {
-                 configurations.add(c);
-             });
-         }
 
-         return configurations;
+        final SortedSet<Class<?>> configurations = new TreeSet<>((c1, c2) -> {
+            return c1.getName().compareTo(c2.getName());
+        });
+
+        for (CcmModule module : modules) {
+            final Module annotation = module.getClass().getAnnotation(
+                Module.class);
+
+            if (annotation == null) {
+                continue;
+            }
+
+            Arrays.stream(annotation.configurations()).forEach(c -> {
+                configurations.add(c);
+            });
+        }
+
+        return configurations;
     }
-    
+
     /**
      * Load all settings of the provided configuration class.
      *
-     * @param <T> Type of the configuration class.
+     * @param <T>       Type of the configuration class.
      * @param confClass The configuration class.
      *
      * @return An instance of the configuration class with all settings set to
-     * the values stored in the registry.
+     *         the values stored in the registry.
      */
     public <T> T findConfiguration(final Class<T> confClass) {
         if (confClass == null) {
@@ -99,9 +99,9 @@ public class ConfigurationManager {
 
         if (confClass.getAnnotation(Configuration.class) == null) {
             throw new IllegalArgumentException(String.format(
-                    "Provided class \"%s\" is not annotated with \"%s\".",
+                "Provided class \"%s\" is not annotated with \"%s\".",
-                    confClass.getName(),
+                confClass.getName(),
-                    Configuration.class.getName()));
+                Configuration.class.getName()));
         }
 
         final String confName = confClass.getName();
@@ -114,11 +114,13 @@ public class ConfigurationManager {
      * registry.
      *
      * @param configuration The configuration to save. The class of the provided
-     * object must be annotation with {@link Configuration}.
+     *                      object must be annotation with
+     *                      {@link Configuration}.
      *
      * @throws IllegalArgumentException If the {@code configuration} parameter
-     * is {@code null} or if the class of the provided object is not annotation
+     *                                  is {@code null} or if the class of the
-     * with {@link Configuration}.
+     *                                  provided object is not annotation with
+     *                                  {@link Configuration}.
      */
     public void saveConfiguration(final Object configuration) {
         if (configuration == null) {
@@ -127,10 +129,10 @@ public class ConfigurationManager {
 
         if (configuration.getClass().getAnnotation(Configuration.class) == null) {
             throw new IllegalArgumentException(String.format(
-                    "The class \"%s\" of the provided object is not annotated "
+                "The class \"%s\" of the provided object is not annotated "
-                            + "with \"%s\".",
+                    + "with \"%s\".",
-                    configuration.getClass().getName(),
+                configuration.getClass().getName(),
-                    Configuration.class.getName()));
+                Configuration.class.getName()));
         }
 
         LOGGER.debug(String.format("Saving configuration \"%s\"...",
@@ -141,10 +143,10 @@ public class ConfigurationManager {
 
             if (field.getAnnotation(Setting.class) == null) {
                 LOGGER.debug(String.format(
-                        "Field \"%s\" of class \"%s\" is not "
+                    "Field \"%s\" of class \"%s\" is not "
-                                + "a setting. Ignoring it.",
+                        + "a setting. Ignoring it.",
-                        configuration.getClass().getName(),
+                    configuration.getClass().getName(),
-                        field.getName()));
+                    field.getName()));
                 continue;
             }
 
@@ -155,16 +157,16 @@ public class ConfigurationManager {
                                 field.get(configuration));
             } catch (IllegalAccessException ex) {
                 LOGGER.error(String.format(
-                        "Failed to write setting value for setting \"%s\" "
+                    "Failed to write setting value for setting \"%s\" "
-                                + "of configuration \"%s\"",
+                        + "of configuration \"%s\"",
-                        getSettingName(field),
+                    getSettingName(field),
-                        configuration.getClass().getName()),
+                    configuration.getClass().getName()),
                              ex);
                 throw new IllegalStateException(String.format(
-                        "Failed to write setting value for setting \"%s\" "
+                    "Failed to write setting value for setting \"%s\" "
-                                + "of configuration \"%s\"",
+                        + "of configuration \"%s\"",
-                        getSettingName(field),
+                    getSettingName(field),
-                        configuration.getClass().getName()),
+                    configuration.getClass().getName()),
                                                 ex);
             }
         }
@@ -176,10 +178,10 @@ public class ConfigurationManager {
      * @param configuration The configuration for which the info is generated.
      *
      * @return a {@link ConfigurationInfo} instance describing the provided
-     * configuration.
+     *         configuration.
      */
     public ConfigurationInfo getConfigurationInfo(
-            final Class<?> configuration) {
+        final Class<?> configuration) {
 
         if (configuration == null) {
             throw new IllegalArgumentException("Configuration can't be null");
@@ -187,23 +189,23 @@ public class ConfigurationManager {
 
         if (configuration.getAnnotation(Configuration.class) == null) {
             throw new IllegalArgumentException(String.format(
-                    "The class \"%s\" of the provided object is not annotated "
+                "The class \"%s\" of the provided object is not annotated "
-                            + "with \"%s\".",
+                    + "with \"%s\".",
-                    configuration.getClass().getName(),
+                configuration.getClass().getName(),
-                    Configuration.class.getName()));
+                Configuration.class.getName()));
         }
 
         final Configuration annotation = configuration.getAnnotation(
-                Configuration.class);
+            Configuration.class);
 
         final ConfigurationInfo confInfo = new ConfigurationInfo();
-        confInfo.setName(configuration.getClass().getName());
+        confInfo.setName(configuration.getName());
         if (annotation.descBundle() == null
-                    || annotation.descBundle().isEmpty()) {
+                || annotation.descBundle().isEmpty()) {
-            confInfo.setDescBundle(String.join("",
+            confInfo.setDescBundle(
-                                               configuration.getClass()
+                String.join("",
-                                               .getName(),
+                            configuration.getName(),
-                                               "Description"));
+                            "Description"));
         } else {
             confInfo.setDescBundle(annotation.descBundle());
         }
@@ -223,7 +225,7 @@ public class ConfigurationManager {
             field.setAccessible(true);
             if (field.getAnnotation(Setting.class) != null) {
                 confInfo.addSetting(settingManager.getSettingInfo(
-                        configuration, field.getName()));
+                    configuration, field.getName()));
             }
         }
 
@@ -239,11 +241,11 @@ public class ConfigurationManager {
      * @param field The setting field.
      *
      * @return The name of the field or if the {@link Setting} annotation of the
-     * field has a name value, the value of that field.
+     *         field has a name value, the value of that field.
      */
     String getSettingName(final Field field) {
         LOGGER.debug(String.format("Trying to get setting name from field: "
-                                           + "\"%s\"",
+                                       + "\"%s\"",
                                    field.getName()));
         final Setting annotation = field.getAnnotation(Setting.class);
 
@@ -257,13 +259,13 @@ public class ConfigurationManager {
     /**
      * Create a setting instance of a specific value type.
      *
-     * @param <T> Type variable.
+     * @param <T>       Type variable.
      * @param valueType The type of the value of the setting to create.
      *
      * @return An setting instance of the provided value type.
      *
      * @throws IllegalArgumentException If there is not setting type for the
-     * provided value type.
+     *                                  provided value type.
      */
 //    @SuppressWarnings("unchecked")
 //    <T> AbstractSetting<T> createSettingForValueType(
@@ -294,11 +296,11 @@ public class ConfigurationManager {
     /**
      * Sets a value on a setting in the registry.
      *
-     * @param <T> The value type of the setting.
+     * @param <T>           The value type of the setting.
      * @param configuration The configuration to which the settings belongs.
-     * @param settingName The name of the setting.
+     * @param settingName   The name of the setting.
-     * @param valueType The type of the value of the setting.
+     * @param valueType     The type of the value of the setting.
-     * @param value The value to set.
+     * @param value         The value to set.
      */
     @Transactional(Transactional.TxType.REQUIRED)
     private <T> void setSettingValue(final Object configuration,
@@ -312,28 +314,28 @@ public class ConfigurationManager {
                                                                 valueType);
         if (setting == null) {
             LOGGER.debug(String.format(
-                    "Setting \"%s#%s\" does not yet exist in "
+                "Setting \"%s#%s\" does not yet exist in "
-                            + "database. Creating new setting.",
+                    + "database. Creating new setting.",
-                    confClassName,
+                confClassName,
-                    settingName));
+                settingName));
             setting = settingConverter.createSettingForValueType(valueType);
         }
         setting.setConfigurationClass(confClassName);
         setting.setName(settingName);
 
         LOGGER.debug(String.format(
-                "New value of setting \"%s#%s\" is: \"%s\"",
+            "New value of setting \"%s#%s\" is: \"%s\"",
-                confClassName,
+            confClassName,
-                settingName,
+            settingName,
-                value.toString()));
+            value.toString()));
         @SuppressWarnings("unchecked")
         final T settingValue = (T) value;
         setting.setValue(settingValue);
         LOGGER.debug(String.format(
-                "Value of setting \"%s#%s\" is now: \"%s\"",
+            "Value of setting \"%s#%s\" is now: \"%s\"",
-                confClassName,
+            confClassName,
-                settingName,
+            settingName,
-                setting.getValue().toString()
+            setting.getValue().toString()
         ));
         LOGGER.debug("Saving changed setting to DB...");
         settingManager.saveSetting(setting);
@@ -342,11 +344,11 @@ public class ConfigurationManager {
     /**
      * Helper method for loading a configuration from the database.
      *
-     * @param <T> The type of the configuration.
+     * @param <T>       The type of the configuration.
      * @param confClass The configuration class.
      *
      * @return An instance of the configuration class with all setting fields
-     * set to the values stored in the registry.
+     *         set to the values stored in the registry.
      */
     <T> T findConfiguration(final String confName, final Class<T> confClass) {
         final T conf;
@@ -355,8 +357,8 @@ public class ConfigurationManager {
             conf = confClass.newInstance();
         } catch (InstantiationException | IllegalAccessException ex) {
             LOGGER.warn(String.format(
-                    "Failed to instantiate configuration \"%s\".",
+                "Failed to instantiate configuration \"%s\".",
-                    confClass.getName()),
+                confClass.getName()),
                         ex);
             return null;
         }
@@ -372,7 +374,7 @@ public class ConfigurationManager {
 
             final Class<?> settingType = field.getType();
             final AbstractSetting<?> setting = settingManager.findSetting(
-                    confName, settingName, settingType);
+                confName, settingName, settingType);
             if (setting != null) {
                 try {
                     LOGGER.debug("Setting \"{}#{}\" found. Value: {}",
@@ -382,10 +384,10 @@ public class ConfigurationManager {
                     field.set(conf, setting.getValue());
                 } catch (IllegalAccessException ex) {
                     LOGGER.warn(
-                            "Failed to set value of configuration class \"{}\". "
+                        "Failed to set value of configuration class \"{}\". "
                             + "Ignoring.",
-                            confClass.getName(),
+                        confClass.getName(),
-                            ex);
+                        ex);
                 }
             }
         }

ccm-core/src/main/java/org/libreccm/configuration/SettingInfo.java

@@ -18,8 +18,11 @@
  */
 package org.libreccm.configuration;
 
-import org.libreccm.configuration.*;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
 import java.util.Locale;
+import java.util.MissingResourceException;
 import java.util.Objects;
 import java.util.ResourceBundle;
 
@@ -31,6 +34,8 @@ import java.util.ResourceBundle;
  */
 public final class SettingInfo {
 
+    private static final Logger LOGGER = LogManager.getLogger(SettingInfo.class);
+
     /**
      * The fully qualified name of the setting.
      */
@@ -104,7 +109,14 @@ public final class SettingInfo {
     }
 
     public ResourceBundle getDescriptionBundle(final Locale locale) {
-        return ResourceBundle.getBundle(descBundle, locale);
+        try {
+            return ResourceBundle.getBundle(descBundle, locale);
+        } catch (MissingResourceException ex) {
+            LOGGER.warn("Failed to find bundle for base name '{}'"
+                            + " and locale '{}'.",
+                        descBundle, locale);
+            return null;
+        }
     }
 
     public String getLabelKey() {
@@ -114,9 +126,24 @@ public final class SettingInfo {
     void setLabelKey(final String labelKey) {
         this.labelKey = labelKey;
     }
-    
+
     public String getLabel(final Locale locale) {
-        return getDescriptionBundle(locale).getString(labelKey);
+        final ResourceBundle bundle = getDescriptionBundle(locale);
+
+        if (bundle == null) {
+            return name;
+        } else {
+            try {
+                return bundle.getString(labelKey);
+            } catch (MissingResourceException ex) {
+                LOGGER.warn("Can't find resource for bundle '{}', "
+                                + "key '{}' and locale '{}'.",
+                            descBundle,
+                            labelKey,
+                            locale);
+                return name;
+            }
+        }
     }
 
     public String getDescKey() {
@@ -128,7 +155,22 @@ public final class SettingInfo {
     }
 
     public String getDescription(final Locale locale) {
-        return getDescriptionBundle(locale).getString(descKey);
+        final ResourceBundle bundle = getDescriptionBundle(locale);
+
+        if (bundle == null) {
+            return "";
+        } else {
+            try {
+                return bundle.getString(descKey);
+            } catch (MissingResourceException ex) {
+                LOGGER.warn("Can't find resource for bundle '{}', "
+                                + "key '{}' and locale '{}'.",
+                            descBundle,
+                            descKey,
+                            locale);
+                return "";
+            }
+        }
     }
 
     @Override

ccm-core/src/main/java/org/libreccm/configuration/SettingManager.java

@@ -32,6 +32,7 @@ import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.util.Strings;
 
 import java.util.ArrayList;
+import java.util.Objects;
 
 /**
  *
@@ -130,7 +131,10 @@ public class SettingManager {
                         ex);
             return null;
         }
-
+        
+        //Make the field accessible even if it has a private modifier
+        field.setAccessible(true);
+            
         if (field.getAnnotation(Setting.class) == null) {
             return null;
         }
@@ -148,7 +152,7 @@ public class SettingManager {
 
         try {
             final Object conf = configuration.newInstance();
-            settingInfo.setDefaultValue(field.get(conf).toString());
+            settingInfo.setDefaultValue(Objects.toString(field.get(conf)));
         } catch (InstantiationException | IllegalAccessException ex) {
             LOGGER.warn(String.format("Failed to create instance of \"%s\" to "
                                           + "get default values.",

ccm-core/src/main/java/org/libreccm/modules/CcmIntegrator.java

@@ -56,6 +56,13 @@ public class CcmIntegrator implements Integrator {
     private static final Logger LOGGER = LogManager.getLogger(
         CcmIntegrator.class);
 
+    /**
+     * Name of the property which is used to retrieve the data source in use
+     * from Hibernate.
+     */
+    private static final String DATASOURCE_PROPERTY
+                                    = "hibernate.connection.datasource";
+
     /**
      * Service loader containing all modules. Initialised by the
      * {@link #integrate(Configuration, SessionFactoryImplementor, SessionFactoryServiceRegistry)}
@@ -91,10 +98,13 @@ public class CcmIntegrator implements Integrator {
             final List<TreeNode> tree = treeManager.generateTree(modules);
             final List<TreeNode> orderedNodes = treeManager.orderModules(tree);
 
-            //Get DataSource and Connection from the sessionFactory of 
+//            //Get DataSource and Connection from the sessionFactory of 
-            //Hibernate.
+//            //Hibernate.
             final DataSource dataSource = (DataSource) sessionFactory.
-                getProperties().get("javax.persistence.jtaDataSource");
+                getProperties().get(DATASOURCE_PROPERTY);
+            if (dataSource == null) {
+                throw new IllegalStateException("No data source available.");
+            }
             connection = dataSource.getConnection();
 
             //Migrate tables and sequences which don't belong to a module 
@@ -312,7 +322,7 @@ public class CcmIntegrator implements Integrator {
 
             //Get JDBC connection
             final DataSource dataSource = (DataSource) sessionFactory
-                .getProperties().get("javax.persistence.jtaDataSource");
+                .getProperties().get(DATASOURCE_PROPERTY);
             connection = dataSource.getConnection();
             System.out.println("checking modules...");
             LOGGER.info("Checking modules...");