SLING-7496 Factory configurations get deleted on update()
The factory prefix was prepended twice which caused the installer to
think that the original configuration being updated was gone, which
triggered a callback to Configuration.delete();
diff --git a/pom.xml b/pom.xml
index c835e26..3ad5109 100644
--- a/pom.xml
+++ b/pom.xml
@@ -90,6 +90,13 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>2.15.0</version>
+ <scope>test</scope>
</dependency>
</dependencies>
</project>
diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java
index afa7293..6e0845c 100644
--- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java
+++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java
@@ -106,7 +106,7 @@
} else {
pid = (event.getPid().startsWith(event.getFactoryPid() + '.') ?
event.getPid().substring(event.getFactoryPid().length() + 1) : event.getPid());
- id = event.getFactoryPid() + '.' + event.getPid();
+ id = event.getFactoryPid() + '.' + pid;
}
if ( event.getType() == ConfigurationEvent.CM_DELETED ) {
final Coordinator.Operation op = Coordinator.SHARED.get(event.getPid(), event.getFactoryPid(), true);
@@ -191,22 +191,7 @@
final String configPid;
int n = pid.indexOf('-');
if (n > 0) {
- // quick check if this is an existing configuration
- final String fString = pid.substring(0, n);
- final String cString = pid.substring(n + 1);
- boolean useExtendedPid = false;
- try {
- if ( ConfigUtil.getConfiguration(this.configAdmin, fString, fString + '.' + cString) != null ) {
- useExtendedPid = true;
- }
- } catch ( final Exception ignore) {
- // ignore this
- }
- if ( useExtendedPid ) {
- configPid = fString + '.' + cString;
- } else {
- configPid = pid.substring(n + 1);
- }
+ configPid = pid.substring(n + 1);
factoryPid = pid.substring(0, n);
} else {
factoryPid = null;
diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java
index 981d590..a9eba9f 100644
--- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java
+++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java
@@ -194,6 +194,12 @@
+ "))");
}
if (configs == null || configs.length == 0) {
+ configs = ca.listConfigurations("(&("
+ + ConfigurationAdmin.SERVICE_FACTORYPID + "=" + encode(factoryPid)
+ + ")(" + Constants.SERVICE_PID + "=" + encode(factoryPid + "." + configPid)
+ + "))");
+ }
+ if (configs == null || configs.length == 0) {
// check for old style with alias pid
configs = ca.listConfigurations(
"(&(" + ConfigurationAdmin.SERVICE_FACTORYPID
diff --git a/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java b/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java
index 7ef7ca9..2b0c9a6 100644
--- a/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java
+++ b/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java
@@ -18,14 +18,19 @@
*/
package org.apache.sling.installer.factories.configuration.impl;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
import java.util.Dictionary;
import java.util.Enumeration;
import java.util.Hashtable;
import org.junit.Test;
+import org.mockito.Mockito;
+import org.osgi.service.cm.Configuration;
+import org.osgi.service.cm.ConfigurationAdmin;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
public class ConfigUtilTest {
@@ -105,4 +110,44 @@
assertTrue(ConfigUtil.isSameData(a, b));
assertTrue(ConfigUtil.isSameData(b, a));
}
+
+ @Test public void testGetOrCreateConfiguration() throws Exception {
+ Configuration c1 = Mockito.mock(Configuration.class);
+ Configuration c2 = Mockito.mock(Configuration.class);
+ ConfigurationAdmin cm = Mockito.mock(ConfigurationAdmin.class);
+ Mockito.when(cm.listConfigurations(
+ "(&(service.factoryPid=a.b.c)(service.pid=c1))"))
+ .thenReturn(new Configuration[] {c1});
+ Mockito.when(cm.listConfigurations(
+ "(&(service.factoryPid=a.b.c)(service.pid=a.b.c.c1))"))
+ .thenReturn(new Configuration[] {c2});
+ Configuration cfg = ConfigUtil.getConfiguration(cm, "a.b.c", "c1");
+ assertSame(c1, cfg);
+ }
+
+ @Test public void testGetOrCreateConfigurationFactoryPrefix() throws Exception {
+ Configuration c1 = Mockito.mock(Configuration.class);
+ Configuration c2 = Mockito.mock(Configuration.class);
+ ConfigurationAdmin cm = Mockito.mock(ConfigurationAdmin.class);
+ Mockito.when(cm.listConfigurations(
+ "(&(service.factoryPid=a.b.c)(service.pid=a.b.c.c1))"))
+ .thenReturn(new Configuration[] {c1});
+ Mockito.when(cm.listConfigurations(
+ "(&(service.factoryPid=a.b.c)(org.apache.sling.installer.osgi.factoryaliaspid=c1))"))
+ .thenReturn(new Configuration[] {c2});
+ Configuration cfg = ConfigUtil.getConfiguration(cm, "a.b.c", "c1");
+ assertSame(c1, cfg);
+
+ assertNull(ConfigUtil.getConfiguration(cm, "a.b.c", "c2"));
+ }
+
+ @Test public void testGetOrCreateConfigurationAliasKey() throws Exception {
+ Configuration c1 = Mockito.mock(Configuration.class);
+ ConfigurationAdmin cm = Mockito.mock(ConfigurationAdmin.class);
+ Mockito.when(cm.listConfigurations(
+ "(&(service.factoryPid=a.b.c)(org.apache.sling.installer.osgi.factoryaliaspid=c1))"))
+ .thenReturn(new Configuration[] {c1});
+ Configuration cfg = ConfigUtil.getConfiguration(cm, "a.b.c", "c1");
+ assertSame(c1, cfg);
+ }
}
