You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ss...@apache.org on 2016/11/23 18:40:33 UTC

svn commit: r1771025 - in /sling/trunk/contrib/extensions/contextaware-config/impl/src: main/java/org/apache/sling/caconfig/management/impl/ test/java/org/apache/sling/caconfig/management/impl/

Author: sseifert
Date: Wed Nov 23 18:40:33 2016
New Revision: 1771025

URL: http://svn.apache.org/viewvc?rev=1771025&view=rev
Log:
SLING-6137 fix value/effectiveValue/default detection in ValueInfoImpl

Removed:
    sling/trunk/contrib/extensions/contextaware-config/impl/src/test/java/org/apache/sling/caconfig/management/impl/ValueInfoImplTest.java
Modified:
    sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ConfigurationDataImpl.java
    sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ValueInfoImpl.java

Modified: sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ConfigurationDataImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ConfigurationDataImpl.java?rev=1771025&r1=1771024&r2=1771025&view=diff
==============================================================================
--- sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ConfigurationDataImpl.java (original)
+++ sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ConfigurationDataImpl.java Wed Nov 23 18:40:33 2016
@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Set;
 
 import org.apache.commons.collections.IteratorUtils;
+import org.apache.commons.lang3.ClassUtils;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceUtil;
 import org.apache.sling.api.resource.ValueMap;
@@ -47,6 +48,10 @@ final class ConfigurationDataImpl implem
     private final ConfigurationOverrideManager configurationOverrideManager;
     private final boolean configResourceCollection;
     
+    private Set<String> propertyNamesCache;
+    private ValueMap valuesCache;
+    private ValueMap effectiveValuesCache;
+    
     @SuppressWarnings("unchecked")
     public ConfigurationDataImpl(ConfigurationMetadata configMetadata,
             Resource resolvedConfigurationResource, Resource writebackConfigurationResource,
@@ -93,38 +98,48 @@ final class ConfigurationDataImpl implem
 
     @Override
     public Set<String> getPropertyNames() {
-        Set<String> propertyNames = new HashSet<>();
-        if (configMetadata != null) {
-            propertyNames.addAll(configMetadata.getPropertyMetadata().keySet());
-        }
-        if (resolvedConfigurationResource != null) {
-            propertyNames.addAll(ResourceUtil.getValueMap(resolvedConfigurationResource).keySet());
+        if (propertyNamesCache == null) {
+            propertyNamesCache = new HashSet<>();
+            if (configMetadata != null) {
+                propertyNamesCache.addAll(configMetadata.getPropertyMetadata().keySet());
+            }
+            if (resolvedConfigurationResource != null) {
+                propertyNamesCache.addAll(ResourceUtil.getValueMap(resolvedConfigurationResource).keySet());
+            }
         }
-        return propertyNames;
+        return propertyNamesCache;
     }
 
     @Override
     public ValueMap getValues() {
-        if (writebackConfigurationResource != null) {
-            return ResourceUtil.getValueMap(writebackConfigurationResource);
+        if (valuesCache == null) {
+            if (writebackConfigurationResource != null) {
+                valuesCache = ResourceUtil.getValueMap(writebackConfigurationResource);
+            }
+            else {
+                valuesCache = ValueMap.EMPTY;
+            }
         }
-        return ValueMap.EMPTY;
+        return valuesCache;
     }
 
     @Override
     public ValueMap getEffectiveValues() {
-        Map<String,Object> props = new HashMap<>();
-        if (configMetadata != null) {
-            for (PropertyMetadata<?> propertyMetadata : configMetadata.getPropertyMetadata().values()) {
-                if (propertyMetadata.getDefaultValue() != null) {
-                    props.put(propertyMetadata.getName(), propertyMetadata.getDefaultValue());
+        if (effectiveValuesCache == null) {
+            Map<String,Object> props = new HashMap<>();
+            if (configMetadata != null) {
+                for (PropertyMetadata<?> propertyMetadata : configMetadata.getPropertyMetadata().values()) {
+                    if (propertyMetadata.getDefaultValue() != null) {
+                        props.put(propertyMetadata.getName(), propertyMetadata.getDefaultValue());
+                    }
                 }
             }
+            if (resolvedConfigurationResource != null) {
+                props.putAll(ResourceUtil.getValueMap(resolvedConfigurationResource));
+            }
+            effectiveValuesCache = new ValueMapDecorator(props);
         }
-        if (resolvedConfigurationResource != null) {
-            props.putAll(ResourceUtil.getValueMap(resolvedConfigurationResource));
-        }
-        return new ValueMapDecorator(props);
+        return effectiveValuesCache;
     }
 
     @SuppressWarnings("unchecked")
@@ -132,15 +147,16 @@ final class ConfigurationDataImpl implem
     public ValueInfo<?> getValueInfo(String propertyName) {
         PropertyMetadata propertyMetadata = configMetadata != null ? configMetadata.getPropertyMetadata().get(propertyName) : null;
         Object value;
-        ValueMap properties = ResourceUtil.getValueMap(resolvedConfigurationResource);
+        Object effectiveValue;
         if (propertyMetadata != null) {
-            value = properties.get(propertyName, propertyMetadata.getType());
+            value = getValues().get(propertyName, propertyMetadata.getType());
+            effectiveValue = getEffectiveValues().get(propertyName, ClassUtils.primitiveToWrapper(propertyMetadata.getType()));
         }
         else {
-            value = properties.get(propertyName);
+            value = getValues().get(propertyName);
+            effectiveValue = getEffectiveValues().get(propertyName);
         }
-        return new ValueInfoImpl(propertyName, value,
-                propertyMetadata,
+        return new ValueInfoImpl(propertyName, value, effectiveValue, propertyMetadata,
                 resolvedConfigurationResource,
                 writebackConfigurationResource,
                 configurationResourceInheritanceChain,

Modified: sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ValueInfoImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ValueInfoImpl.java?rev=1771025&r1=1771024&r2=1771025&view=diff
==============================================================================
--- sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ValueInfoImpl.java (original)
+++ sling/trunk/contrib/extensions/contextaware-config/impl/src/main/java/org/apache/sling/caconfig/management/impl/ValueInfoImpl.java Wed Nov 23 18:40:33 2016
@@ -23,7 +23,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.caconfig.impl.override.ConfigurationOverrideManager;
@@ -34,6 +33,7 @@ final class ValueInfoImpl<T> implements
     
     private final String name;
     private final T value;
+    private final T effectiveValue;
     private final T defaultValue;
     private final PropertyMetadata<T> propertyMetadata;
     private final Resource resolvedConfigurationResource;
@@ -43,12 +43,13 @@ final class ValueInfoImpl<T> implements
     private final String configName;
     private final ConfigurationOverrideManager configurationOverrideManager;
     
-    public ValueInfoImpl(String name, T value, PropertyMetadata<T> propertyMetadata,
+    public ValueInfoImpl(String name, T value, T effectiveValue, PropertyMetadata<T> propertyMetadata,
             Resource resolvedConfigurationResource, Resource writebackConfigurationResource,
             List<Resource> configurationResourceInheritanceChain,
             Resource contextResource, String configName, ConfigurationOverrideManager configurationOverrideManager) {
         this.name = name;
         this.value = value;
+        this.effectiveValue = effectiveValue;
         this.defaultValue = propertyMetadata != null ? propertyMetadata.getDefaultValue() : null;
         this.propertyMetadata = propertyMetadata;
         this.resolvedConfigurationResource = resolvedConfigurationResource;
@@ -69,28 +70,19 @@ final class ValueInfoImpl<T> implements
         return propertyMetadata;
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public T getValue() {
-        if (writebackConfigurationResource == null) {
-            return null;
-        }
-        else if (propertyMetadata != null) {
-            return writebackConfigurationResource.getValueMap().get(name, propertyMetadata.getType());
-        }
-        else {
-            return (T)writebackConfigurationResource.getValueMap().get(name);
-        }
+        return value;
     }
 
     @Override
     public T getEffectiveValue() {
-        return ObjectUtils.defaultIfNull(value, defaultValue);
+        return effectiveValue;
     }
 
     @Override
     public String getConfigSourcePath() {
-        if (value != null && resolvedConfigurationResource != null) {
+        if (effectiveValue != null && resolvedConfigurationResource != null) {
             Resource resource = getResourceFromInheritanceChain();
             if (resource != null) {
                 return resource.getPath();
@@ -101,12 +93,20 @@ final class ValueInfoImpl<T> implements
 
     @Override
     public boolean isDefault() {
-        return value == null && defaultValue != null;
+        if (defaultValue == null) {
+            return false;
+        }
+        if (resolvedConfigurationResource == null) {
+            return true;
+        }
+        else {
+            return !resolvedConfigurationResource.getValueMap().containsKey(name);
+        }
     }
 
     @Override
     public boolean isInherited() {
-        if (isDefault() || value == null) {
+        if (isDefault() || effectiveValue == null) {
             return false;
         }
         else if (resolvedConfigurationResource == null) {
@@ -141,7 +141,7 @@ final class ValueInfoImpl<T> implements
             return null;
         }
         Resource resource = inheritanceChain.next();
-        Object valueFromResource = resource.getValueMap().get(name, value.getClass());
+        Object valueFromResource = resource.getValueMap().get(name, effectiveValue.getClass());
         if (valueFromResource != null) {
             return resource;
         }
@@ -157,7 +157,7 @@ final class ValueInfoImpl<T> implements
                     contextResource.getPath(), configName, Collections.<String,Object>emptyMap());
         if (overrideProperties != null) {
             return overrideProperties.containsKey(name)
-                    || (getValue() != null && value == null);
+                    || (getValue() != null && effectiveValue == null);
         }
         else {
             return false;