diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java index 4fc632abc..b85970ea7 100644 --- a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyFactory.java @@ -20,7 +20,6 @@ * Factory of PropertyContainer objects. * * @see PropertyContainer - * @author elandau */ public interface PropertyFactory { /** diff --git a/archaius2-core/src/main/java/com/netflix/archaius/property/DefaultPropertyContainer.java b/archaius2-core/src/main/java/com/netflix/archaius/property/DefaultPropertyContainer.java index 73d3146ea..a36c3cd4d 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/property/DefaultPropertyContainer.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/property/DefaultPropertyContainer.java @@ -26,6 +26,7 @@ import java.math.BigDecimal; import java.math.BigInteger; +import java.util.Iterator; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -42,14 +43,11 @@ * * Once created a PropertyContainer property cannot be removed. However, listeners may be * added and removed. - * - * @author elandau - * */ public class DefaultPropertyContainer implements PropertyContainer { private final Logger LOG = LoggerFactory.getLogger(DefaultPropertyContainer.class); - enum Type { + enum CacheType { INTEGER (int.class, Integer.class), BYTE (byte.class, Byte.class), SHORT (short.class, Short.class), @@ -64,12 +62,12 @@ enum Type { private final Class[] types; - Type(Class ... type) { + CacheType(Class ... type) { types = type; } - static Type fromClass(Class clazz) { - for (Type type : values()) { + static CacheType fromClass(Class clazz) { + for (CacheType type : values()) { for (Class cls : type.types) { if (cls.equals(clazz)) { return type; @@ -116,45 +114,23 @@ public DefaultPropertyContainer(String key, Config config, AtomicInteger version this.masterVersion = version; } - abstract class CachedProperty implements Property { + class PropertyWithDefault implements Property { + private CachedProperty delegate; + private T defaultValue; + public PropertyWithDefault(CachedProperty delegate, T defaultValue) { + this.defaultValue = defaultValue; + this.delegate = delegate; + } + @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((defaultValue == null) ? 0 : defaultValue.hashCode()); - result = prime * result + type; - return result; + public T get() { + T value = delegate.get(); + return value != null ? value : defaultValue; } @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - CachedProperty other = (CachedProperty) obj; - if (defaultValue == null) { - if (other.defaultValue != null) - return false; - } else if (!defaultValue.equals(other.defaultValue)) - return false; - if (type != other.type) - return false; - return true; - } - private final AtomicStampedReference cache = new AtomicStampedReference<>(null, -1); - private final int type; - private final T defaultValue; - - CachedProperty(Type type, T defaultValue) { - this.type = type.ordinal(); - this.defaultValue = defaultValue; - } - - public void addListener(final PropertyListener listener) { + public void addListener(PropertyListener listener) { listeners.add(listener, new ListenerUpdater() { private AtomicReference last = new AtomicReference(null); @@ -165,35 +141,63 @@ public void update() { try { value = get(); - } - catch (Exception e) { + } catch (Exception e) { listener.onParseError(e); return; } - if (prev != value) { - if (last.compareAndSet(prev, value)) { - listener.onChange(value); - } + if (prev != value && last.compareAndSet(prev, value)) { + listener.onChange(value != null ? value : defaultValue); } } }); } - + + @Override + public void removeListener(PropertyListener listener) { + listeners.remove(listener); + } + @Override public String getKey() { - return DefaultPropertyContainer.this.key; + return delegate.getKey(); } + } + + abstract class CachedProperty { - public void removeListener(PropertyListener listener) { - listeners.remove(listener); + @Override + public int hashCode() { + return type; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + CachedProperty other = (CachedProperty) obj; + return type != other.type; + } + private final AtomicStampedReference cache = new AtomicStampedReference<>(null, -1); + private final int type; + + CachedProperty(int type) { + this.type = type; } + public String getKey() { + return DefaultPropertyContainer.this.key; + } + /** * Fetch the latest version of the property. If not up to date then resolve to the latest * value, inline. * - * TODO: Make resolving property value an offline task + * TODO: Make resolving property value an background task * * @return */ @@ -213,29 +217,22 @@ public T get() { if (cache.compareAndSet(currentValue, newValue, cacheVersion, latestVersion)) { // Slight race condition here but not important enough to warrent locking lastUpdateTimeInMillis = System.currentTimeMillis(); - return firstNonNull(newValue, defaultValue); + return newValue; } } - return firstNonNull(cache.getReference(), defaultValue); + return cache.getReference(); } public long getLastUpdateTime(TimeUnit units) { return units.convert(lastUpdateTimeInMillis, TimeUnit.MILLISECONDS); } - private T firstNonNull(T first, T second) { - return first == null ? second : first; - } /** * Resolve to the most recent value * @return * @throws Exception */ protected abstract T resolveCurrent() throws Exception; - - private DefaultPropertyContainer getOuterType() { - return DefaultPropertyContainer.this; - } } /** @@ -245,118 +242,153 @@ private DefaultPropertyContainer getOuterType() { * @return */ @SuppressWarnings("unchecked") - private CachedProperty add(final CachedProperty newProperty) { - // TODO(nikos): This while() looks like it's redundant - // since we are only calling add() after a get(). - while (!cache.add(newProperty)) { - for (CachedProperty property : cache) { - if (property.equals(newProperty)) { - return (CachedProperty) property; + private Property add(final int type, final Function> creator, final T defaultValue) { + do { + Iterator> iter = cache.iterator(); + while (iter.hasNext()) { + CachedProperty element = iter.next(); + if (element.type == type) { + return new PropertyWithDefault((CachedProperty)element, defaultValue); } } - } - - return newProperty; + + // TODO(nikos): This while() looks like it's redundant + // since we are only calling add() after a get(). + CachedProperty cachedProperty = creator.apply(type); + if (cache.addIfAbsent(cachedProperty)) { + return new PropertyWithDefault(cachedProperty, defaultValue); + } + } while (true); } @Override public Property asString(final String defaultValue) { - return add(new CachedProperty(Type.STRING, defaultValue) { - @Override - protected String resolveCurrent() throws Exception { - return config.getString(key, null); - } - }); + return add( + CacheType.STRING.ordinal(), + type -> new CachedProperty(type) { + @Override + protected String resolveCurrent() throws Exception { + return config.getString(key, null); + } + }, + defaultValue); } @Override public Property asInteger(final Integer defaultValue) { - return add(new CachedProperty(Type.INTEGER, defaultValue) { - @Override - protected Integer resolveCurrent() throws Exception { - return config.getInteger(key, null); - } - }); - } + return add( + CacheType.INTEGER.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Integer resolveCurrent() throws Exception { + return config.getInteger(key, null); + } + }, + defaultValue); + } @Override public Property asLong(final Long defaultValue) { - return add(new CachedProperty(Type.LONG, defaultValue) { - @Override - protected Long resolveCurrent() throws Exception { - return config.getLong(key, null); - } - }); + return add( + CacheType.LONG.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Long resolveCurrent() throws Exception { + return config.getLong(key, null); + } + }, + defaultValue); } @Override public Property asDouble(final Double defaultValue) { - return add(new CachedProperty(Type.DOUBLE, defaultValue) { - @Override - protected Double resolveCurrent() throws Exception { - return config.getDouble(key, null); - } - }); + return add( + CacheType.DOUBLE.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Double resolveCurrent() throws Exception { + return config.getDouble(key, null); + } + }, + defaultValue); } @Override public Property asFloat(final Float defaultValue) { - return add(new CachedProperty(Type.FLOAT, defaultValue) { - @Override - protected Float resolveCurrent() throws Exception { - return config.getFloat(key, null); - } - }); + return add( + CacheType.FLOAT.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Float resolveCurrent() throws Exception { + return config.getFloat(key, null); + } + }, + defaultValue); } @Override public Property asShort(final Short defaultValue) { - return add(new CachedProperty(Type.SHORT, defaultValue) { - @Override - protected Short resolveCurrent() throws Exception { - return config.getShort(key, null); - } - }); + return add( + CacheType.SHORT.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Short resolveCurrent() throws Exception { + return config.getShort(key, null); + } + }, + defaultValue); } @Override public Property asByte(final Byte defaultValue) { - return add(new CachedProperty(Type.BYTE, defaultValue) { - @Override - protected Byte resolveCurrent() throws Exception { - return config.getByte(key, defaultValue); - } - }); + return add( + CacheType.BYTE.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Byte resolveCurrent() throws Exception { + return config.getByte(key, null); + } + }, + defaultValue); } @Override public Property asBigDecimal(final BigDecimal defaultValue) { - return add(new CachedProperty(Type.BIG_DECIMAL, defaultValue) { - @Override - protected BigDecimal resolveCurrent() throws Exception { - return config.getBigDecimal(key, defaultValue); - } - }); + return add( + CacheType.BIG_DECIMAL.ordinal(), + type -> new CachedProperty(type) { + @Override + protected BigDecimal resolveCurrent() throws Exception { + return config.getBigDecimal(key, null); + } + }, + defaultValue); } @Override public Property asBoolean(final Boolean defaultValue) { - return add(new CachedProperty(Type.BOOLEAN, defaultValue) { - @Override - protected Boolean resolveCurrent() throws Exception { - return config.getBoolean(key, defaultValue); - } - }); + return add( + CacheType.BOOLEAN.ordinal(), + type -> new CachedProperty(type) { + @Override + protected Boolean resolveCurrent() throws Exception { + return config.getBoolean(key, null); + } + }, + defaultValue); } @Override public Property asBigInteger(final BigInteger defaultValue) { - return add(new CachedProperty(Type.BIG_INTEGER, defaultValue) { - @Override - protected BigInteger resolveCurrent() throws Exception { - return config.getBigInteger(key, defaultValue); - } - }); + return add( + CacheType.BIG_INTEGER.ordinal(), + type -> new CachedProperty(type) { + @Override + protected BigInteger resolveCurrent() throws Exception { + return config.getBigInteger(key, null); + } + }, + defaultValue); } /** @@ -365,7 +397,7 @@ protected BigInteger resolveCurrent() throws Exception { @SuppressWarnings("unchecked") @Override public Property asType(final Class type, final T defaultValue) { - switch (Type.fromClass(type)) { + switch (CacheType.fromClass(type)) { case INTEGER: return (Property) asInteger((Integer)defaultValue); case BYTE: @@ -386,25 +418,29 @@ public Property asType(final Class type, final T defaultValue) { return (Property) asBigDecimal((BigDecimal)defaultValue); case BIG_INTEGER: return (Property) asBigInteger((BigInteger)defaultValue); - default: { - CachedProperty prop = add(new CachedProperty(Type.CUSTOM, defaultValue) { + default: + return add( + CacheType.CUSTOM.ordinal(), + t -> new CachedProperty(t) { @Override protected T resolveCurrent() throws Exception { - return config.get(type, key, defaultValue); + return config.get(type, key, null); } - }); - return prop; - } + }, + defaultValue); } } @Override public Property asType(Function type, String defaultValue) { - return add(new CachedProperty(Type.CUSTOM, null) { - @Override - protected T resolveCurrent() throws Exception { - return type.apply(config.getString(key, defaultValue)); - } - }); + return add( + CacheType.CUSTOM.ordinal(), + t -> new CachedProperty(t) { + @Override + protected T resolveCurrent() throws Exception { + return type.apply(config.getString(key, defaultValue)); + } + }, + type.apply(defaultValue)); } } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/property/ListenerManager.java b/archaius2-core/src/main/java/com/netflix/archaius/property/ListenerManager.java index 4ba095364..da5c006ec 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/property/ListenerManager.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/property/ListenerManager.java @@ -11,8 +11,6 @@ * an optimization so it is not necessary to iterate through all property * containers when the listeners need to be invoked since the expectation * is to have far less listeners than property containers. - * - * @author elandau */ public class ListenerManager { public static interface ListenerUpdater { diff --git a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java index 2ef9a1be7..b3a6dd1a6 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java @@ -15,13 +15,6 @@ */ package com.netflix.archaius.property; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.util.concurrent.atomic.AtomicInteger; - -import org.junit.Assert; -import org.junit.Test; - import com.netflix.archaius.DefaultPropertyFactory; import com.netflix.archaius.api.Property; import com.netflix.archaius.api.PropertyFactory; @@ -30,6 +23,13 @@ import com.netflix.archaius.api.exceptions.ConfigException; import com.netflix.archaius.config.DefaultSettableConfig; +import org.junit.Assert; +import org.junit.Test; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.concurrent.atomic.AtomicInteger; + public class PropertyTest { static class MyService { private Property value; @@ -209,4 +209,13 @@ public void onParseError(Throwable error) { config.setProperty("foo", "${goo}"); Assert.assertEquals(456, current.intValue()); } + + @Test + public void testDifferentDefaults() { + SettableConfig config = new DefaultSettableConfig(); + + DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + Assert.assertFalse(factory.getProperty("foo").asBoolean(false).get()); + Assert.assertTrue(factory.getProperty("foo").asBoolean(true).get()); + } }