Skip to content

Commit

Permalink
Merge pull request #518 from elandau/bugfix/typo
Browse files Browse the repository at this point in the history
Improve initializing and reseting listeners in the static bridge
  • Loading branch information
elandau authored Sep 22, 2017
2 parents 0d9c079 + 49c28f4 commit bd9f00a
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
package com.netflix.archaius.bridge;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;

import javax.inject.Inject;
import javax.inject.Singleton;

import org.apache.commons.configuration.AbstractConfiguration;
import org.apache.commons.configuration.Configuration;
import org.apache.commons.configuration.event.ConfigurationEvent;
import org.apache.commons.configuration.event.ConfigurationListener;

import com.netflix.config.AggregatedConfiguration;
Expand All @@ -11,28 +23,19 @@
import com.netflix.config.DynamicPropertySupport;
import com.netflix.config.PropertyListener;

import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedQueue;

import javax.inject.Inject;
import javax.inject.Singleton;

/**
* @see StaticArchaiusBridgeModule
*/
@Singleton
public class StaticAbstractConfiguration extends AbstractConfiguration implements AggregatedConfiguration, DynamicPropertySupport {

private static volatile AbstractConfigurationBridge delegate;
private static ConcurrentLinkedQueue<PropertyListener> pendingListeners = new ConcurrentLinkedQueue<>();

private static final StaticAbstractConfiguration INSTANCE = new StaticAbstractConfiguration();

@Inject
public static void initialize(DeploymentContext context, AbstractConfigurationBridge config) {
public synchronized static void initialize(DeploymentContext context, AbstractConfigurationBridge config) {
reset();
delegate = config;

AbstractConfiguration actualConfig = ConfigurationManager.getConfigInstance();
Expand All @@ -44,21 +47,66 @@ public static void initialize(DeploymentContext context, AbstractConfigurationBr
}

DynamicPropertyFactory.initWithConfigurationSource((AbstractConfiguration)INSTANCE);

PropertyListener listener;
while (null != (listener = pendingListeners.poll())) {
delegate.addConfigurationListener(listener);
}

// Bridge change notification from the new delegate to any listeners registered on
// this static class. Notifications will be removed if the StaticAbstractConfiguration
// is reset and will reattached to a new bridge should initialize be called again.
config.addConfigurationListener(INSTANCE.forwardingConfigurationListener);
config.addConfigurationListener(INSTANCE.forwardingPropertyListener);
}

public static AbstractConfiguration getInstance() {
return INSTANCE;
}

public static void reset() {
public synchronized static void reset() {
if (delegate != null) {
delegate.removeConfigurationListener(INSTANCE.forwardingConfigurationListener);
}
delegate = null;
}

private final PropertyListener forwardingPropertyListener;
private final ConfigurationListener forwardingConfigurationListener;
private final CopyOnWriteArrayList<PropertyListener> propertyListeners = new CopyOnWriteArrayList<>();

public StaticAbstractConfiguration() {
this.forwardingPropertyListener = new PropertyListener() {
@Override
public void configSourceLoaded(Object source) {
propertyListeners.forEach(listener -> listener.configSourceLoaded(source));
}

@Override
public void addProperty(Object source, String name, Object value, boolean beforeUpdate) {
propertyListeners.forEach(listener -> listener.addProperty(source, name, value, beforeUpdate));
}

@Override
public void setProperty(Object source, String name, Object value, boolean beforeUpdate) {
propertyListeners.forEach(listener -> listener.setProperty(source, name, value, beforeUpdate));
}

@Override
public void clearProperty(Object source, String name, Object value, boolean beforeUpdate) {
propertyListeners.forEach(listener -> listener.clearProperty(source, name, value, beforeUpdate));
}

@Override
public void clear(Object source, boolean beforeUpdate) {
propertyListeners.forEach(listener -> listener.clear(source, beforeUpdate));
}

};

this.forwardingConfigurationListener = new ConfigurationListener() {
@Override
public void configurationChanged(ConfigurationEvent event) {
StaticAbstractConfiguration.this.fireEvent(event.getType(), event.getPropertyName(), event.getPropertyValue(), event.isBeforeUpdate());
}
};
}

@Override
public boolean isEmpty() {
if (delegate == null) {
Expand Down Expand Up @@ -168,25 +216,14 @@ protected void clearPropertyDirect(String key) {
delegate.clearProperty(key);
}

@Override
public void addConfigurationListener(PropertyListener expandedPropertyListener) {
if (delegate == null) {
pendingListeners.add(expandedPropertyListener);
}
else {
delegate.addConfigurationListener(expandedPropertyListener);
}
}

public void addConfigurationListener(ConfigurationListener l) {
delegate.addConfigurationListener(l);
}

public boolean removeConfigurationListener(ConfigurationListener l) {
return delegate.removeConfigurationListener(l);
}

public Collection<ConfigurationListener> getConfigurationListeners() {
return getConfigurationListeners();
}
List<ConfigurationListener> listeners = new ArrayList<>(super.getConfigurationListeners());
Optional.ofNullable(delegate).ifPresent(d -> listeners.addAll(d.getConfigurationListeners()));
return listeners;
}

@Override
public void addConfigurationListener(PropertyListener expandedPropertyListener) {
propertyListeners.add(expandedPropertyListener);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package com.netflix.archaius.bridge;

import org.apache.commons.configuration.event.ConfigurationEvent;
import org.apache.commons.configuration.event.ConfigurationListener;
import org.junit.Test;
import org.mockito.Mockito;

import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.netflix.archaius.api.config.SettableConfig;
import com.netflix.archaius.api.inject.RuntimeLayer;
import com.netflix.archaius.guice.ArchaiusModule;
import com.netflix.config.ConfigurationManager;

import javax.inject.Inject;

public class StaticBridgeAddConfigurationTest {
private static ConfigurationListener listener = Mockito.mock(ConfigurationListener.class);

public static class Foo {
public static void addListenerBeforeBridgeInitialization() {
ConfigurationManager.getConfigInstance().addConfigurationListener(listener);
}
}

@Inject
@RuntimeLayer
SettableConfig settableConfig;

@Test
public void listenerAddedToStaticBeforeStaticInjection() {
Guice.createInjector(new ArchaiusModule(), new StaticArchaiusBridgeModule(), new AbstractModule() {
@Override
protected void configure() {
Foo.addListenerBeforeBridgeInitialization();
this.requestInjection(StaticBridgeAddConfigurationTest.this);
}
});

// Verify that the listener is called
Mockito.verify(listener, Mockito.never()).configurationChanged(Mockito.isA(ConfigurationEvent.class));
settableConfig.setProperty("foo", "bar");
Mockito.verify(listener, Mockito.times(2)).configurationChanged(Mockito.isA(ConfigurationEvent.class));

// Listener no longer invoked after reset
StaticArchaiusBridgeModule.resetStaticBridges();
settableConfig.setProperty("bar", "baz");
Mockito.verify(listener, Mockito.times(2)).configurationChanged(Mockito.isA(ConfigurationEvent.class));

}
}

0 comments on commit bd9f00a

Please sign in to comment.