diff --git a/src/org/exist/config/Configuration.java b/src/org/exist/config/Configuration.java index 6925baa8142..facf81b2df6 100644 --- a/src/org/exist/config/Configuration.java +++ b/src/org/exist/config/Configuration.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.exist.security.PermissionDeniedException; @@ -166,7 +167,14 @@ public interface Configuration { */ public void save(DBBroker broker) throws PermissionDeniedException, ConfigurationException; - public boolean equals(Object obj, String uniqField); + /** + * Determines equality based on a property value of the configuration + * + * @param obj The Configured instance + * @param property The name of the property to use for comparison, or + * if empty, the {@link ConfigurationImpl#ID} is used. + */ + public boolean equals(Object obj, Optional property); /** * Free up memory allocated for cache. diff --git a/src/org/exist/config/ConfigurationImpl.java b/src/org/exist/config/ConfigurationImpl.java index 6ce489ec9a6..3574ae95947 100644 --- a/src/org/exist/config/ConfigurationImpl.java +++ b/src/org/exist/config/ConfigurationImpl.java @@ -22,13 +22,9 @@ package org.exist.config; import java.lang.ref.WeakReference; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; +import jdk.nashorn.internal.runtime.regexp.joni.Config; import org.exist.dom.persistent.DocumentImpl; import org.exist.security.PermissionDeniedException; import org.exist.storage.DBBroker; @@ -461,33 +457,24 @@ public void save(final DBBroker broker) throws PermissionDeniedException, Config } @Override - public boolean equals(Object obj) { - if (obj instanceof ConfigurationImpl) { - final ConfigurationImpl conf = (ConfigurationImpl)obj; - if (!(getName().equals(conf.getName()))) - {return false;} - final String id = getProperty(Configuration.ID); - if (id == null) { - return false; - } - if (id.equals(conf.getProperty(Configuration.ID))) - {return true;} - } - return false; + public boolean equals(final Object obj) { + return equals(obj, Optional.empty()); } - public boolean equals(Object obj, String uniqField) { + @Override + public boolean equals(final Object obj, final Optional property) { if (obj instanceof ConfigurationImpl) { final ConfigurationImpl conf = (ConfigurationImpl)obj; - if (!(getName().equals(conf.getName()))) - {return false;} - final String uniq = getProperty( uniqField); - if (uniq == null) { + if (!(getName().equals(conf.getName()))) { return false; } - if (uniq.equals(conf.getProperty(uniqField))) - {return true;} + + final String name = property.orElse(Configuration.ID); + final Optional value = Optional.ofNullable(getProperty(name)); + + return value.map(v -> v.equals(conf.getProperty(name))).orElse(false); } + return false; } } diff --git a/src/org/exist/config/Configurator.java b/src/org/exist/config/Configurator.java index 11ace5450a6..e4edbe0f3ee 100644 --- a/src/org/exist/config/Configurator.java +++ b/src/org/exist/config/Configurator.java @@ -36,19 +36,17 @@ import java.lang.reflect.Method; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.NoSuchElementException; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.exist.Database; @@ -71,6 +69,7 @@ import org.exist.storage.txn.TransactionManager; import org.exist.storage.txn.Txn; import org.exist.util.MimeType; +import org.exist.util.function.ConsumerE; import org.exist.util.serializer.SAXSerializer; import org.exist.xmldb.FullXmldbURI; import org.exist.xmldb.XmldbURI; @@ -94,7 +93,6 @@ public class Configurator { private final static String EOL = System.getProperty("line.separator", "\n"); protected static ConcurrentMap hotConfigs = new ConcurrentHashMap(); - //TODO should be replaced with a naturally ordered List, we need to maintain the order of XML elements based on the order of class members!!! protected static AFields getConfigurationAnnotatedFields(Class clazz) { final AFields fields = new AFields(); for (final Field field : clazz.getDeclaredFields()) { @@ -198,6 +196,26 @@ public static Method searchForAddMethod(final Class clazz, final String prope return null; } + public static Method searchForInsertMethod(final Class clazz, final String property) { + try { + final String methodName = ("insert" + property).toLowerCase(); + for (final Method method : clazz.getMethods()) { + if (method.getName().toLowerCase().equals(methodName) + && method.getParameterTypes().length == 2 + && int.class.getName().equals(method.getParameterTypes()[0].getName()) + && String.class.getName().equals(method.getParameterTypes()[1].getName())) { + return method; + } + } + } catch (final SecurityException se) { + LOG.error(se.getMessage(), se); + } catch (final NoClassDefFoundError ncdfe) { + LOG.error(ncdfe.getMessage(), ncdfe); + } + + return null; + } + public static Configuration configure(final Configurable instance, final Configuration configuration) { if (configuration == null) { return null; @@ -387,21 +405,21 @@ private static Configuration configureByCurrent(final Configurable instance, fin field = element.getField(); final Class fieldType = field.getType(); - + if (List.class == fieldType) { //List final String confName = element.getAnnotation().value(); field.setAccessible(true); List list = (List) field.get(instance); - String referenceBy; - - final List confs; + + final Optional referenceBy; + List confs; if (field.isAnnotationPresent(ConfigurationReferenceBy.class)) { confs = configuration.getConfigurations(confName); - referenceBy = field.getAnnotation(ConfigurationReferenceBy.class).value(); + referenceBy = Optional.ofNullable(field.getAnnotation(ConfigurationReferenceBy.class).value()); } else { confs = configuration.getConfigurations(confName); - referenceBy = null; + referenceBy = Optional.empty(); } if (list == null) { @@ -411,43 +429,47 @@ private static Configuration configureByCurrent(final Configurable instance, fin if (confs != null) { //remove & update - for (final Iterator iterator = list.iterator(); iterator.hasNext();) { - final Object obj = iterator.next(); + + final Map removed = new HashMap<>(); //referencedBy -> index + + for (int i = 0; i < list.size(); i++) { + final Object obj = list.get(i); Configuration current_conf = null; if (!(obj instanceof Configurable)) { - iterator.remove(); + list.remove(i); //TODO Surely we should log a problem here or throw an exception? continue; } else if (obj instanceof Reference) { - if (referenceBy == null) { + if (!referenceBy.isPresent()) { LOG.error("illegal design '"+configuration.getName()+"' ["+field+"]"); - iterator.remove(); + list.remove(i); continue; - } - - final String name = ((Reference)obj).getName(); - - //Lookup for new configuration, update if found - boolean found = false; - for (final Iterator i = confs.iterator(); i.hasNext();) { - final Configuration conf = i.next(); - - final String uniq = conf.getProperty( referenceBy ); + } else { - if (uniq != null && uniq.equals(name)) { - i.remove(); - found = true; - break; + final String name = ((Reference) obj).getName(); + + //Lookup for new configuration, update if found + final List applicableConfs = filter(confs, conf -> + Optional.ofNullable(conf.getPropertyBoolean(referenceBy.get())) + .map(value -> !value.equals(name)) + .orElse(true)); + + if(applicableConfs.size() == confs.size()) { + LOG.debug("Configuration was removed, will attempt to replace object [" + obj + "]."); + final Field referee = getFieldRecursive(Optional.of(list.get(i).getClass()), referenceBy.get()); + if(referee != null) { + referee.setAccessible(true); + removed.put((String) referee.get(list.remove(i)), i); + } else { + LOG.error("Could not lookup referenced field: " + referenceBy.get() + " against: " + list.get(i).getClass().getName()); + list.remove(i); + } + } else { + confs = applicableConfs; } } - - if (!found) { - LOG.debug("Configuration was removed, removing the object [" + obj + "]."); - //XXX: remove by method call - iterator.remove(); - } } else { current_conf = ((Configurable) obj).getConfiguration(); @@ -459,73 +481,66 @@ private static Configuration configureByCurrent(final Configurable instance, fin continue; } - LOG.debug("Unconfigured instance [" + obj + "], removing the object."); - //XXX: remove by method call - iterator.remove(); + LOG.debug("Unconfigured instance [" + obj + "], will attempt to replace object..."); + + final Field referee = getFieldRecursive(Optional.of(list.get(i).getClass()), referenceBy.get()); + if(referee != null) { + referee.setAccessible(true); + removed.put((String) referee.get(list.remove(i)), i); + } else { + LOG.error("Could not lookup referenced field: " + referenceBy.get() + " against: " + list.get(i).getClass().getName()); + list.remove(i); + } continue; } - + //Lookup for new configuration, update if found - boolean found = false; - for (final Iterator i = confs.iterator(); i.hasNext();) { - final Configuration conf = i.next(); - if (referenceBy != null && current_conf.equals(conf, referenceBy)) { - i.remove(); - found = true; - break; - - } else if (referenceBy == null && current_conf.equals(conf)) { - current_conf.checkForUpdates(conf.getElement()); - i.remove(); - found = true; - break; - } - } - - if (!found) { - LOG.debug("Configuration was removed, removing the object [" + obj + "]."); - //XXX: remove by method call - iterator.remove(); + final Configuration final_current_conf = current_conf; + final List applicableConfs = filter(confs, conf -> !final_current_conf.equals(conf, referenceBy)); + + if(applicableConfs.size() == confs.size()) { + LOG.debug("Configuration was removed, will attempt to replace [" + obj + "]."); + removed.put(((Configurable)list.remove(i)).getConfiguration().getProperty(referenceBy.get()), i); + } else { + confs = applicableConfs; } } //create for (final Configuration conf : confs) { - - if (referenceBy != null) { - final String value = conf.getProperty(referenceBy); - if (value != null) { - Method method = searchForAddMethod(instance.getClass(), confName); - if (method != null) { + if (referenceBy.isPresent()) { + final String value = conf.getProperty(referenceBy.get()); + if(value != null) { + final Optional> updateFn = updateListFn(instance, confName, removed, value); + + if(!updateFn.isPresent()) { + LOG.error("Could not insert configured object"); + } else { try { - method.invoke(instance, value); + updateFn.get().accept(value); continue; - - } catch (final Exception e) { - LOG.warn("Could not execute method on class " + instance.getClass().getName() + " for configuration '" + conf.getName() + "' referenceBy '" + referenceBy + "' for value '" + value + "'", e); - method = null; + } catch(final ReflectiveOperationException e) { + LOG.warn("Could not update " + instance.getClass().getName() + " for configuration '" + conf.getName() + "' referenceBy '" + referenceBy.get() + "' for value '" + value + "'", e); } } } - } else { final Type genericType = field.getGenericType(); if (genericType != null) { - if ("java.util.List".equals(genericType.toString())) { - final String value = conf.getValue(); if (value != null) { - Method method = searchForAddMethod(instance.getClass(), confName); - if (method != null) { + final Optional> updateFn = updateListFn(instance, confName, removed, value); + + if(!updateFn.isPresent()) { + LOG.error("Could not insert configured object"); + } else { try { - method.invoke(instance, value); + updateFn.get().accept(value); continue; - - } catch (final Exception e) { - LOG.debug("Found method " + method.getName() + " on " + instance.getClass().getName() + ", however invoke failed with: " + e.getMessage(), e); - method = null; + } catch (final ReflectiveOperationException e) { + LOG.warn("Could not update " + instance.getClass().getName() + " for configuration '" + conf.getName() + "' for value '" + value + "'", e); } } } @@ -599,6 +614,33 @@ private static Configuration configureByCurrent(final Configurable instance, fin return configuration; } + /** + * If the value was previously removed, we can attempt + * to reinsert it at the same index so as to keep the + * ordering consistent. Otherwise... we just add it. + */ + private static Optional> updateListFn(final Configurable instance, final String confName, final Map removed, final String value) { + return Optional.ofNullable(removed.get(value)).>>map(removedIdx -> Optional.ofNullable( + searchForInsertMethod(instance.getClass(), confName)) + .map(insertMethod -> v -> insertMethod.invoke(instance, removedIdx, v)) + ).orElse(Optional.ofNullable( + searchForAddMethod(instance.getClass(), confName)) + .map(addMethod -> v -> addMethod.invoke(instance, v))); + } + + private static Field getFieldRecursive(final Optional maybeClazz, final String name) { + return maybeClazz.map(clazz -> + Stream.of(clazz.getDeclaredFields()) + .filter(field -> field.getName().equals(name)) + .findFirst() + .orElse(getFieldRecursive(Optional.ofNullable(clazz.getSuperclass()), name))) + .orElse(null); + } + + private static List filter(final List configurations, Predicate predicate) { + return configurations.stream().filter(predicate).collect(Collectors.toList()); + } + /** * @return The Configurable or null */ diff --git a/src/org/exist/security/AbstractAccount.java b/src/org/exist/security/AbstractAccount.java index 5412ca33a68..78436d293a6 100644 --- a/src/org/exist/security/AbstractAccount.java +++ b/src/org/exist/security/AbstractAccount.java @@ -92,7 +92,7 @@ public Group addGroup(final String name) throws PermissionDeniedException { return addGroup(group); } - //this method used by Configurator + //this method is used by Configurator protected final Group addGroup(final Configuration conf) throws PermissionDeniedException { if (conf == null) { return null; diff --git a/src/org/exist/security/internal/AccountImpl.java b/src/org/exist/security/internal/AccountImpl.java index d6d679643e3..cbe9833eabb 100644 --- a/src/org/exist/security/internal/AccountImpl.java +++ b/src/org/exist/security/internal/AccountImpl.java @@ -38,6 +38,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.Optional; import java.util.Properties; import org.exist.security.Credential; @@ -312,4 +313,33 @@ private synchronized String getProperty(final String propertyName) { return loadedSecurityProperties.getProperty(propertyName); } } + + //this method is used by Configurator + public final Group insertGroup(final int index, final String name) throws PermissionDeniedException { + //if we cant find the group in our own realm, try other realms + final Group group = Optional.ofNullable(getRealm().getGroup(name)) + .orElse(getRealm().getSecurityManager().getGroup(name)); + + return insertGroup(index, group); + } + + private Group insertGroup(final int index, final Group group) throws PermissionDeniedException { + + if(group == null){ + return null; + } + + final Account user = getDatabase().getSubject(); + group.assertCanModifyGroup(user); + + if(!groups.contains(group)) { + groups.add(index, group); + + if(SecurityManager.DBA_GROUP.equals(group.getName())) { + hasDbaRole = true; + } + } + + return group; + } }