Changes requested by Carsten’s Code Review or PR 15
diff --git a/pom.xml b/pom.xml
index 744001f..970114c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -63,32 +63,6 @@
</execution>
</executions>
</plugin>
-<!--
- <plugin>
- <groupId>org.apache.felix</groupId>
- <artifactId>maven-bundle-plugin</artifactId>
- <extensions>true</extensions>
- <configuration>
- <instructions>
- <Import-Package>
- javax.jcr;resolution:=optional,
- *
- </Import-Package>
- <!- - Check if that is still needed - ->
- <Export-Package>
- org.apache.sling.resourceresolver.impl.mapping
- </Export-Package>
- <Provide-Capability>
- osgi.service;objectClass=javax.servlet.Servlet,
- osgi.service;objectClass=org.apache.sling.api.resource.ResourceResolverFactory,
- osgi.service;objectClass=org.apache.sling.api.resource.observation.ResourceChangeListener,
- osgi.service;objectClass=org.apache.sling.api.resource.runtime.RuntimeService,
- osgi.service;objectClass=org.apache.sling.spi.resource.provider.ResourceProvider
- </Provide-Capability>
- </instructions>
- </configuration>
- </plugin>
--->
<plugin>
<groupId>biz.aQute.bnd</groupId>
<artifactId>bnd-maven-plugin</artifactId>
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
index 2b42531..c419f1e 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
@@ -109,18 +109,9 @@
@Reference
EventAdmin eventAdmin;
- /** Event admin. */
+ /** String Interpolation Provider. */
@Reference
StringInterpolationProvider stringInterpolationProvider;
-// public void bindStringInterpolationProviderImpl(StringInterpolationProviderImpl stringInterpolationProviderImpl) {
-// logger.info("Bind StringInterpolationProviderImpl: '{}'", stringInterpolationProviderImpl);
-// this.stringInterpolationProviderImpl = stringInterpolationProviderImpl;
-// }
-//
-// public void unbindStringInterpolationProviderImpl(StringInterpolationProviderImpl stringInterpolationProviderImpl) {
-// this.stringInterpolationProviderImpl = null;
-// }
-
/** Service User Mapper */
@Reference
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/Interpolator.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/Interpolator.java
index 47878bb..afa5147 100755
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/Interpolator.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/Interpolator.java
@@ -34,6 +34,7 @@
public static final String START = "$[";
+ public static final char NAME_SEPARATOR = ':';
public static final char DIRECTIVES_SEPARATOR = ';';
public static final char DIRECTIVES_VALUE_SEPARATOR = '=';
@@ -100,13 +101,15 @@
continue;
}
- String key = result.substring(start + START.length(), index - 1);
- int sep = key.indexOf(':');
+ final String key = result.substring(start + START.length(), index - 1);
+ final int sep = key.indexOf(NAME_SEPARATOR);
if (sep == -1) {
- LOGGER.trace("No ':' found, set it to: '{}'", key);
+ // invalid key
+ start = index;
+ continue;
}
- final String type = key.substring(0, sep < 0 ? 0 : sep);
+ final String type = key.substring(0, sep);
final String postfix = key.substring(sep + 1);
LOGGER.trace("Type: '{}', postfix: '{}'", type, postfix);
@@ -116,7 +119,7 @@
if (dirPos == -1) {
name = postfix;
directives = Collections.emptyMap();
- LOGGER.trace("No Default");
+ LOGGER.trace("No Directives");
} else {
name = postfix.substring(0, dirPos);
directives = new HashMap<>();
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
index fa4eaf8..cdc5bcc 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
@@ -142,7 +142,7 @@
private boolean updateBloomFilterFile = false;
- private StringInterpolationProvider stringInterpolationProvider;
+ private final StringInterpolationProvider stringInterpolationProvider;
@SuppressWarnings({ "unchecked" })
public MapEntries(final MapConfigurationProvider factory, final BundleContext bundleContext, final EventAdmin eventAdmin, final StringInterpolationProvider stringInterpolationProvider)
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderConfiguration.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderConfiguration.java
index 3d2f81a..56fc598 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderConfiguration.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderConfiguration.java
@@ -23,15 +23,15 @@
@ObjectClassDefinition(
name = "Apache Sling String Interpolation Provider",
- description = "Configures the String Interpolation Provider and the location of its key/value pairs"
+ description = "Configures the String Interpolation Provider key/value pairs"
)
public @interface StringInterpolationProviderConfiguration {
@AttributeDefinition(
name = "Placeholder Values",
description = "A list of key / value pairs separated by a equal (=) sign. " +
- "The key is not permitted to contain a '=' sign and the first occurrence of '=' " +
- "separates the key from the value. If no '=' is found the entire key / value pair " +
- "is dropped.")
+ "The key is not permitted to contain a '=' sign as the first occurrence of '=' " +
+ "separates the key from the value. If no '=' is found the entry " +
+ "is ignored")
String[] placeHolderKeyValuePairs() default {"phv.default.host.name=localhost"};
}
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImpl.java
index 87a8ebb..1c91a8d 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImpl.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImpl.java
@@ -31,7 +31,7 @@
import java.util.Map;
@Designate(ocd = StringInterpolationProviderConfiguration.class)
-@Component(name = "org.apache.sling.resourceresolver.impl.mapping.StringInterpolationProviderImpl")
+@Component
public class StringInterpolationProviderImpl
implements StringInterpolationProvider
{
@@ -39,9 +39,10 @@
private static final String TYPE_PROP = "prop";
+ private static final String TYPE_CONFIG = "config";
+
private static final String DIRECTIVE_DEFAULT = "default";
- /** Logger. */
private final Logger logger = LoggerFactory.getLogger(this.getClass());
private Map<String, String> placeholderEntries = new HashMap<>();
@@ -57,8 +58,7 @@
this.context = bundleContext;
String[] valueMap = config.placeHolderKeyValuePairs();
- // Clear out any existing values
- placeholderEntries.clear();
+ Map<String, String> newMap = new HashMap<>();
for(String line: valueMap) {
// Ignore no or empty lines
if(line != null && !line.isEmpty()) {
@@ -70,11 +70,12 @@
} else if (index > line.length() - 2) {
logger.warn("Placeholder Entry does not contain a value: '{}' -> ignored", line);
} else {
- placeholderEntries.put(line.substring(0, index), line.substring(index + 1));
+ newMap.put(line.substring(0, index), line.substring(index + 1));
}
}
}
}
+ this.placeholderEntries = newMap;
}
/**
@@ -109,7 +110,7 @@
v = getVariableFromEnvironment(name);
} else if (TYPE_PROP.equals(type)) {
v = getVariableFromProperty(name);
- } else {
+ } else if(TYPE_CONFIG.equals(type)){
v = getVariableFromBundleConfiguration(name);
}
if (v == null) {
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/EtcMappingResourceResolverTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/EtcMappingResourceResolverTest.java
index c021c99..ca2d6a6 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/EtcMappingResourceResolverTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/EtcMappingResourceResolverTest.java
@@ -285,7 +285,7 @@
@Test
public void simple_node_string_interpolation() throws Exception {
- buildResource("$[siv.one]", http, resourceResolver, resourceProvider,PROP_REDIRECT_EXTERNAL, "/content/simple-node");
+ buildResource("$[config:siv.one]", http, resourceResolver, resourceProvider,PROP_REDIRECT_EXTERNAL, "/content/simple-node");
setupStringInterpolationProvider(stringInterpolationProvider, stringInterpolationProviderConfiguration, new String[] {"siv.one=test-simple-node.80"});
refreshMapEntries("/etc/map", true);
@@ -304,7 +304,7 @@
@Test
public void simple_match_string_interpolation() throws Exception {
buildResource("test-node", http, resourceResolver, resourceProvider,
- PROP_REG_EXP, "$[siv.one]/",
+ PROP_REG_EXP, "$[config:siv.one]/",
PROP_REDIRECT_EXTERNAL, "/content/simple-match/"
);
setupStringInterpolationProvider(stringInterpolationProvider, stringInterpolationProviderConfiguration, new String[] {"siv.one=test-simple-match.80"});
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
index c0b4b69..b960a82 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
@@ -428,17 +428,6 @@
return resource;
}
-
-// /**
-// * extract the name from a path.
-// * @param fullpath
-// * @return
-// */
-// private String getName(String fullpath) {
-// int n = fullpath.lastIndexOf("/");
-// return fullpath.substring(n+1);
-// }
-
/**
* Test getting a resolver.
* @throws LoginException
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java
index 1a6a299..9f04b6f 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java
@@ -32,7 +32,7 @@
@Test
public void simple_node_string_interpolation() throws Exception {
// To avoid side effects the String Interpolation uses its own Resource Resolver
- Resource sivOne = setupEtcMapResource("$[siv.one]", http,PROP_REDIRECT_EXTERNAL, "/content/simple-node");
+ Resource sivOne = setupEtcMapResource("$[config:siv.one]", http,PROP_REDIRECT_EXTERNAL, "/content/simple-node");
setupStringInterpolationProvider(stringInterpolationProvider, stringInterpolationProviderConfiguration, new String[] {"siv.one=test-simple-node"});
mapEntries.doInit();
@@ -44,7 +44,7 @@
public void simple_match_string_interpolation() throws Exception {
// To avoid side effects the String Interpolation uses its own Resource Resolver
Resource sivOne = setupEtcMapResource("test-node", http,
- PROP_REG_EXP, "$[siv.one]/",
+ PROP_REG_EXP, "$[config:siv.one]/",
PROP_REDIRECT_EXTERNAL, "/content/simple-match/"
);
setupStringInterpolationProvider(stringInterpolationProvider, stringInterpolationProviderConfiguration, new String[] {"siv.one=test-simple-match"});
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImplTest.java
index 5f0d6db..60fe5e9 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImplTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationProviderImplTest.java
@@ -49,7 +49,7 @@
public void test_interpolator_simple() {
Map<String,String> values = new HashMap<>();
values.put("one", "two");
- String substitute = interpolate("$[one]", values);
+ String substitute = interpolate("$[config:one]", values);
assertEquals("Wrong Replacement", "two", substitute);
}
@@ -57,7 +57,7 @@
public void test_interpolator_with_type() {
Map<String,String> values = new HashMap<>();
values.put("one", "two");
- String substitute = interpolate("$[value:one]", values);
+ String substitute = interpolate("$[config:one]", values);
assertEquals("Wrong Replacement (with type)", "two", substitute);
}
@@ -65,15 +65,15 @@
public void test_interpolator_no_match() {
Map<String,String> values = new HashMap<>();
values.put("one", "two");
- String substitute = interpolate("$[two]", values);
- assertEquals("Should not been replaced", "$[two]", substitute);
+ String substitute = interpolate("$[config:two]", values);
+ assertEquals("Should not been replaced", "$[config:two]", substitute);
}
@Test
public void test_interpolator_no_match_with_default() {
Map<String,String> values = new HashMap<>();
values.put("one", "two");
- String substitute = interpolate("$[two;default=three]", values);
+ String substitute = interpolate("$[config:two;default=three]", values);
assertEquals("Should have been default for no match", "three", substitute);
}
@@ -81,7 +81,7 @@
public void test_interpolator_full_with_match() {
Map<String,String> values = new HashMap<>();
values.put("one", "two");
- String substitute = interpolate("$[value:one;default=three]", values);
+ String substitute = interpolate("$[config:one;default=three]", values);
assertEquals("Wrong Replacement", "two", substitute);
}
@@ -89,7 +89,7 @@
public void test_interpolator_full_with_default() {
Map<String,String> values = new HashMap<>();
values.put("one", "two");
- String substitute = interpolate("$[value:two;default=three]", values);
+ String substitute = interpolate("$[config:two;default=three]", values);
assertEquals("Should have been default for no match", "three", substitute);
}
@@ -113,7 +113,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]";
+ String line = "$[config:one]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "two", substituted);
}
@@ -126,7 +126,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "Here is $[one], too";
+ String line = "Here is $[config:one], too";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "Here is two, too", substituted);
}
@@ -139,7 +139,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one] with another $[three]";
+ String line = "$[config:one] with another $[config:three]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "two with another four", substituted);
}
@@ -153,7 +153,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "Here comes $[one] with another $[three] equals $[five], horray!";
+ String line = "Here comes $[config:one] with another $[config:three] equals $[config:five], horray!";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "Here comes two with another four equals six, horray!", substituted);
}
@@ -181,9 +181,9 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "Here comes $[unkown] placeholders!";
+ String line = "Here comes $[config:unkown] placeholders!";
String substituted = placeholderProvider.substitute(line);
- assertEquals("Wrong resolved line", "Here comes $[unkown] placeholders!", substituted);
+ assertEquals("Wrong resolved line", "Here comes $[config:unkown] placeholders!", substituted);
}
@Test
@@ -195,7 +195,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[siv.one]/";
+ String line = "$[config:siv.one]/";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "test-value/", substituted);
}
@@ -209,9 +209,9 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "\\$[one]=$[one]";
+ String line = "\\$[config:one]=$[config:one]";
String substituted = placeholderProvider.substitute(line);
- assertEquals("Wrong resolved line", "$[one]=two", substituted);
+ assertEquals("Wrong resolved line", "$[config:one]=two", substituted);
}
@Test
@@ -223,7 +223,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[$[one]]";
+ String line = "$[config:$[config:one]]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "three", substituted);
}
@@ -237,7 +237,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one$[one]]";
+ String line = "$[config:one$[config:one]]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "three", substituted);
}
@@ -251,9 +251,9 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]$[two]";
+ String line = "$[config:one]$[config:two]";
String substituted = placeholderProvider.substitute(line);
- assertEquals("Wrong resolved line", "two$[two]", substituted);
+ assertEquals("Wrong resolved line", "two$[config:two]", substituted);
}
@Test
@@ -265,9 +265,9 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]$[two]";
+ String line = "$[config:one]$[config:two]";
String substituted = placeholderProvider.substitute(line);
- assertEquals("Wrong resolved line", "two$[two]", substituted);
+ assertEquals("Wrong resolved line", "two$[config:two]", substituted);
}
@Test
@@ -279,7 +279,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]-$[two]";
+ String line = "$[config:one]-$[config:two]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "two-four", substituted);
}
@@ -293,7 +293,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]-$[two]";
+ String line = "$[config:one]-$[config:two]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "two-four", substituted);
}
@@ -302,7 +302,7 @@
public void test_no_configuration() {
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
- String line = "$[one]$[two]";
+ String line = "$[config:one]$[config:two]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", line, substituted);
}
@@ -320,7 +320,7 @@
);
placeholderProvider.modified(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]-$[two]";
+ String line = "$[config:one]-$[config:two]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "two-three", substituted);
}
@@ -334,7 +334,7 @@
StringInterpolationProviderImpl placeholderProvider = new StringInterpolationProviderImpl();
placeholderProvider.activate(bundleContext, stringInterpolationProviderConfiguration);
- String line = "$[one]-$[two]";
+ String line = "$[config:one]-$[config:two]";
String substituted = placeholderProvider.substitute(line);
assertEquals("Wrong resolved line", "two-four", substituted);
placeholderProvider.deactivate(bundleContext);
