This is an automated email from the ASF dual-hosted git repository.

martin_s pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/archiva-components.git

commit cc0652463d961c71983689e00947fdf1ffe12f06
Author: Martin Stockhammer <[email protected]>
AuthorDate: Thu Dec 12 18:16:39 2019 +0100

    Fixing event handling. Adding documentation.
---
 .../archiva/components/registry/Registry.java      | 58 +++++++++++----
 spring-registry/spring-registry-commons/pom.xml    | 20 +++---
 .../commons/CommonsConfigurationRegistry.java      | 18 ++++-
 .../commons/ConfigurationListenerDelegate.java     | 10 +++
 .../test/CommonsConfigurationRegistryTest.java     | 84 +++++++++++++++++++++-
 5 files changed, 161 insertions(+), 29 deletions(-)

diff --git 
a/spring-registry/spring-registry-api/src/main/java/org/apache/archiva/components/registry/Registry.java
 
b/spring-registry/spring-registry-api/src/main/java/org/apache/archiva/components/registry/Registry.java
index 9142bf0..b3eee81 100644
--- 
a/spring-registry/spring-registry-api/src/main/java/org/apache/archiva/components/registry/Registry.java
+++ 
b/spring-registry/spring-registry-api/src/main/java/org/apache/archiva/components/registry/Registry.java
@@ -25,9 +25,24 @@ import java.util.List;
 import java.util.Properties;
 
 /**
- * The Plexus registry is a single source of external configuration for Plexus 
components and applications.
+ * The Registry is a single source of external configuration.
  * It can be used by components to source configuration, knowing that it can 
be used from within applications
  * without the information being hard coded into the component.
+ *
+ * The configuration is stored hierarchically and each entry can be identified 
by a key.
+ * The standard notation for keys should be a dot separated string, where each 
dot represents a connection
+ * from parent to child node.
+ *
+ * The configuration can be combined from different sources. The sources are 
either defined during initialization
+ * of the concrete implementation or can be added by the 
<code>addConfigurationXXX()</code> methods.
+ *
+ * If you combine the configuration from different sources, you should modify 
the configuration from the
+ * sub configuration, which can be get by the {@link #getSection(String)} 
method. Because depending on the
+ * implementation the changes may not be persisted, if you change the combined 
tree.
+ *
+ * The standard implementation uses apache commons configuration 2 for storing 
the configuration.
+ * For implementation details see notes in 
<code>CommonsConfigurationRegistry</code>
+ *
  */
 public interface Registry
 {
@@ -117,7 +132,8 @@ public interface Registry
     void setBoolean( String key, boolean value );
 
     /**
-     * Load configuration from the given classloader resource.
+     * Add a configuration source to the combined configuration. Load the file 
from the
+     * given resource string.
      *
      * @param resource the location to load the configuration from
      * @throws RegistryException if a problem occurred reading the resource to 
add to the registry
@@ -126,17 +142,18 @@ public interface Registry
         throws RegistryException;
 
     /**
-     * Load configuration from the given classloader resource.
+     * Add a configuration source to the combined configuration and load 
configuration from the given classloader resource.
+     * The configuration source is added at the given prefix in the tree.
      *
      * @param resource the location to load the configuration from
-     * @param prefix   the location to add the configuration at in the registry
+     * @param prefix   the key prefix where the root node of the configuration 
is placed
      * @throws RegistryException if a problem occurred reading the resource to 
add to the registry
      */
     void addConfigurationFromResource( String resource, String prefix )
         throws RegistryException;
 
     /**
-     * Load configuration from the given file.
+     * Add a configuration source to the combined configuration and load it 
from the given file.
      *
      * @param file the location to load the configuration from
      * @throws RegistryException if a problem occurred reading the resource to 
add to the registry
@@ -145,15 +162,27 @@ public interface Registry
         throws RegistryException;
 
     /**
-     * Load configuration from the given file.
+     * Add the configuration source to the combined configuration and load 
from the given file.
      *
      * @param file   the location to load the configuration from
-     * @param prefix the location to add the configuration at in the registry
+     * @param prefix   the key prefix where the root node of the configuration 
is placed
      * @throws RegistryException if a problem occurred reading the resource to 
add to the registry
      */
     void addConfigurationFromFile( Path file, String prefix )
         throws RegistryException;
 
+
+    /**
+     * Add the configuration source to the combined configuration and load 
from the given file.
+     * Use the given name for referencing the configuration.
+     *
+     * @param file   the location to load the configuration from
+     * @param prefix   the key prefix where the root node of the configuration 
is placed
+     * @param name the name of the configuration
+     * @throws RegistryException if a problem occurred reading the resource to 
add to the registry
+     */
+    void addConfigurationFromFile( Path file, String name, String prefix ) 
throws RegistryException;
+
     /**
      * Determine if the registry contains any elements.
      *
@@ -216,6 +245,8 @@ public interface Registry
     /**
      * Get a subsection of the registry, identified by the given name. If it 
doesn't exist, <code>null</code> will be
      * returned.
+     * Subsections should be used to modify configurations. The name of the 
subsection depends on the implementation
+     * and the concrete initalization.
      *
      * @param name registry section name
      * @return the registry
@@ -224,6 +255,9 @@ public interface Registry
 
     /**
      * Save any changes to the registry since it was loaded.
+     * Be careful with the save method for combined configurations. It may be 
that changes are not written to
+     * disk or to a different file as expected. How and if changes from a 
combined configuration are written
+     * to disk depends on the implementation.
      *
      * @throws RegistryException             if there was a problem saving the 
registry
      * @throws UnsupportedOperationException if the registry is not writable
@@ -235,21 +269,21 @@ public interface Registry
      * Add a change listener. Note that settings this on the base registry 
will only detect 'invalidation' events, not
      * individual changes. You should retrieve the named sub-registry to 
listen for changes.
      *
-     * TODO: this isn't ideal, so maybe fix combined configuration to re-fire 
it's events to it's own listeners in the c-c implementation
-     *
      * @param listener the listener
      */
     void addChangeListener( RegistryListener listener );
 
     /**
-     * @param listener
-     * @return <code>true</code> if has been removed
+     * Remove the change listener, if it was registered before.
+     *
+     * @param listener the registered listener, that should be removed
+     * @return <code>true</code> if has been removed, otherwise 
<code>false</code>
      * @since 2.3
      */
     boolean removeChangeListener( RegistryListener listener );
 
     /**
-     * Get all the keys in this registry. Keys are only retrieved at a depth 
of 1.
+     * Get the first level keys in this registry.
      *
      * @return the set of keys
      */
diff --git a/spring-registry/spring-registry-commons/pom.xml 
b/spring-registry/spring-registry-commons/pom.xml
index 4993a74..96271ea 100644
--- a/spring-registry/spring-registry-commons/pom.xml
+++ b/spring-registry/spring-registry-commons/pom.xml
@@ -48,6 +48,14 @@
       <artifactId>commons-configuration2</artifactId>
       <version>${commons.configuration.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.springframework</groupId>
+      <artifactId>spring-beans</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>commons-beanutils</groupId>
+      <artifactId>commons-beanutils</artifactId>
+    </dependency>
 
     <dependency>
       <groupId>javax.annotation</groupId>
@@ -76,24 +84,12 @@
     </dependency>
 
     <dependency>
-      <groupId>org.springframework</groupId>
-      <artifactId>spring-beans</artifactId>
-      <scope>test</scope>
-    </dependency>
-
-    <dependency>
       <groupId>commons-collections</groupId>
       <artifactId>commons-collections</artifactId>
       <scope>test</scope>
     </dependency>
 
     <dependency>
-      <groupId>commons-beanutils</groupId>
-      <artifactId>commons-beanutils</artifactId>
-      <scope>test</scope>
-    </dependency>
-
-    <dependency>
       <groupId>javax.inject</groupId>
       <artifactId>javax.inject</artifactId>
     </dependency>
diff --git 
a/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/CommonsConfigurationRegistry.java
 
b/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/CommonsConfigurationRegistry.java
index c99d2be..44a93d8 100644
--- 
a/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/CommonsConfigurationRegistry.java
+++ 
b/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/CommonsConfigurationRegistry.java
@@ -509,7 +509,7 @@ public class CommonsConfigurationRegistry
      * @throws RegistryException if the configuration is not a 
<code>CombinedConfiguration</code>
      */
     @Override
-    public void addConfigurationFromFile( Path file, String prefix )
+    public void addConfigurationFromFile( Path file, String name, String 
prefix )
         throws RegistryException
     {
         CombinedConfiguration configuration = (CombinedConfiguration) 
this.configuration;
@@ -518,8 +518,11 @@ public class CommonsConfigurationRegistry
             try
             {
                 logger.debug( "Loading properties configuration from file: 
{}", file );
+                if (configuration.getConfigurationNames( ).contains( name )) {
+                    configuration.removeConfiguration( prefix );
+                }
                 Configurations configurations = new Configurations( );
-                configuration.addConfiguration( configurations.properties( 
file.toFile() ), prefix, prefix );
+                configuration.addConfiguration( configurations.properties( 
file.toFile() ), name, prefix );
             }
             catch ( ConfigurationException e )
             {
@@ -532,8 +535,11 @@ public class CommonsConfigurationRegistry
             try
             {
                 logger.debug( "Loading XML configuration from file: {}", file 
);
+                if (configuration.getConfigurationNames( ).contains( name )) {
+                    configuration.removeConfiguration( prefix );
+                }
                 Configurations configurations = new Configurations( );
-                configuration.addConfiguration( configurations.xml( 
file.toFile() ), prefix, prefix );
+                configuration.addConfiguration( configurations.xml( 
file.toFile() ), name, prefix );
             }
             catch ( ConfigurationException e )
             {
@@ -548,6 +554,12 @@ public class CommonsConfigurationRegistry
         }
     }
 
+    @Override
+    public void addConfigurationFromFile( Path file, String prefix ) throws 
RegistryException
+    {
+        addConfigurationFromFile( file, prefix, prefix );
+    }
+
 
     /**
      * This method tries to read a combined configuration definition either 
from the string given
diff --git 
a/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/ConfigurationListenerDelegate.java
 
b/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/ConfigurationListenerDelegate.java
index 4352b6c..79045c2 100644
--- 
a/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/ConfigurationListenerDelegate.java
+++ 
b/spring-registry/spring-registry-commons/src/main/java/org/apache/archiva/components/registry/commons/ConfigurationListenerDelegate.java
@@ -24,6 +24,8 @@ import 
org.apache.archiva.components.registry.RegistryListener;
 import org.apache.commons.configuration2.event.ConfigurationEvent;
 import org.apache.commons.configuration2.event.Event;
 import org.apache.commons.configuration2.event.EventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Commons configuration listener that delegates to the given registry 
listener.
@@ -31,6 +33,9 @@ import org.apache.commons.configuration2.event.EventListener;
 public class ConfigurationListenerDelegate
     implements EventListener
 {
+
+    private static final Logger log = LoggerFactory.getLogger( 
ConfigurationListenerDelegate.class );
+
     /**
      * Delegate listener.
      */
@@ -69,10 +74,15 @@ public class ConfigurationListenerDelegate
     @Override
     public void onEvent( Event event ) {
         // Do nothing
+        log.debug( "Event {}", event );
+        if (event instanceof ConfigurationEvent) {
+            onEvent( (ConfigurationEvent) event );
+        }
     }
 
     public void onEvent( ConfigurationEvent event )
     {
+        log.debug( "Configuration event {}", event );
         if ( event.isBeforeUpdate())
         {
             listener.beforeConfigurationChange( registry, 
event.getPropertyName( ), event.getPropertyValue( ) );
diff --git 
a/spring-registry/spring-registry-commons/src/test/java/org/apache/archiva/components/registry/test/CommonsConfigurationRegistryTest.java
 
b/spring-registry/spring-registry-commons/src/test/java/org/apache/archiva/components/registry/test/CommonsConfigurationRegistryTest.java
index c14a135..84b3488 100644
--- 
a/spring-registry/spring-registry-commons/src/test/java/org/apache/archiva/components/registry/test/CommonsConfigurationRegistryTest.java
+++ 
b/spring-registry/spring-registry-commons/src/test/java/org/apache/archiva/components/registry/test/CommonsConfigurationRegistryTest.java
@@ -31,6 +31,7 @@ import org.springframework.util.FileCopyUtils;
 
 import java.io.File;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -370,22 +371,101 @@ public class CommonsConfigurationRegistryTest
 
     }
 
+    @Test
+    public void testChangeListener() throws Exception
+    {
+        registry = getRegistry( "builder" );
+        MockChangeListener listener = new MockChangeListener( );
+        registry.addChangeListener( listener );
+
+        registry.setInt( "test.key", 100 );
+        registry.setBoolean( "test.boolean.first", false );
+        registry.setBoolean( "test.boolean.second", true );
+        registry.setString( "test.string", "test 1020" );
+
+        assertEquals( 4, listener.getBeforeEvents( ).size( ) );
+        assertEquals( 4, listener.getAfterEvents( ).size( ) );
+
+        assertEquals( registry, listener.getBeforeEvents( ).get( 0 
).getRegistry( ) );
+        assertEquals( "test.key", listener.getBeforeEvents( ).get( 0 
).getPropertyName( ) );
+        assertEquals( 100, listener.getBeforeEvents( ).get( 0 
).getPropertyValue( ) );
+        assertEquals( 100, listener.getAfterEvents( ).get( 0 
).getPropertyValue( ) );
+
+        assertEquals( registry, listener.getBeforeEvents( ).get( 1 
).getRegistry( ) );
+        assertEquals( "test.boolean.first", listener.getBeforeEvents( ).get( 1 
).getPropertyName( ) );
+        assertEquals( false, listener.getBeforeEvents( ).get( 1 
).getPropertyValue( ) );
+        assertEquals( false, listener.getAfterEvents( ).get( 1 
).getPropertyValue( ) );
+
+        assertEquals( registry, listener.getBeforeEvents( ).get( 2 
).getRegistry( ) );
+        assertEquals( "test.boolean.second", listener.getBeforeEvents( ).get( 
2 ).getPropertyName( ) );
+        assertEquals( true, listener.getBeforeEvents( ).get( 2 
).getPropertyValue( ) );
+        assertEquals( true, listener.getAfterEvents( ).get( 2 
).getPropertyValue( ) );
+
+        assertEquals( registry, listener.getBeforeEvents( ).get( 3 
).getRegistry( ) );
+        assertEquals( "test.string", listener.getBeforeEvents( ).get( 3 
).getPropertyName( ) );
+        assertEquals( "test 1020", listener.getBeforeEvents( ).get( 3 
).getPropertyValue( ) );
+        assertEquals( "test 1020", listener.getAfterEvents( ).get( 3 
).getPropertyValue( ) );
+
+    }
+
+
+    private static class ChangeEvent {
+        private Registry registry;
+        private String propertyName;
+        private Object propertyValue;
+
+        public ChangeEvent( Registry registry, String propertyName, Object 
propertyValue )
+        {
+            this.registry = registry;
+            this.propertyName = propertyName;
+            this.propertyValue = propertyValue;
+        }
+
+        public Object getPropertyValue( )
+        {
+            return propertyValue;
+        }
 
+        public Registry getRegistry( )
+        {
+            return registry;
+        }
+
+        public String getPropertyName( )
+        {
+            return propertyName;
+        }
+
+
+    }
 
 
     private static class MockChangeListener
         implements RegistryListener
     {
+        List<ChangeEvent> beforeEvents = new ArrayList<>( );
+        List<ChangeEvent> afterEvents = new ArrayList<>( );
+
         @Override
         public void beforeConfigurationChange( Registry registry, String 
propertyName, Object propertyValue )
         {
-            // no op
+            beforeEvents.add( new ChangeEvent( registry, propertyName, 
propertyValue ) );
         }
 
         @Override
         public void afterConfigurationChange( Registry registry, String 
propertyName, Object propertyValue )
         {
-            // no op
+            afterEvents.add( new ChangeEvent( registry, propertyName, 
propertyValue ) );
+        }
+
+        public List<ChangeEvent> getAfterEvents( )
+        {
+            return afterEvents;
+        }
+
+        public List<ChangeEvent> getBeforeEvents( )
+        {
+            return beforeEvents;
         }
     }
 }

Reply via email to