From 0e03f35052f9270495c5e94cc4cf483892fce097 Mon Sep 17 00:00:00 2001 From: Oliver Lietz Date: Sun, 11 Apr 2021 22:30:14 +0200 Subject: [PATCH 1/2] KARAF-7099 Provide util for Configuration PIDs --- util/pom.xml | 7 + .../karaf/util/config/ConfigurationPID.java | 122 ++++++++++++++++++ .../util/config/ConfigurationPIDTest.java | 103 +++++++++++++++ 3 files changed, 232 insertions(+) create mode 100644 util/src/main/java/org/apache/karaf/util/config/ConfigurationPID.java create mode 100644 util/src/test/java/org/apache/karaf/util/config/ConfigurationPIDTest.java diff --git a/util/pom.xml b/util/pom.xml index 76ed73d899d..defd4dba323 100644 --- a/util/pom.xml +++ b/util/pom.xml @@ -74,6 +74,13 @@ mail test + + + org.jetbrains + annotations + provided + 20.1.0 + diff --git a/util/src/main/java/org/apache/karaf/util/config/ConfigurationPID.java b/util/src/main/java/org/apache/karaf/util/config/ConfigurationPID.java new file mode 100644 index 00000000000..2af2e091315 --- /dev/null +++ b/util/src/main/java/org/apache/karaf/util/config/ConfigurationPID.java @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.karaf.util.config; + +import java.util.Objects; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class ConfigurationPID { + + private final String pid; + + private final String factoryPid; + + private final String name; + + private static final String TILDE = "~"; + + private static final String DASH = "-"; + + public ConfigurationPID(@NotNull final String pid) { + this.pid = pid; + this.factoryPid = null; + this.name = null; + } + + public ConfigurationPID(@NotNull final String pid, @NotNull final String factoryPid, @Nullable final String name) { + this.pid = pid; + this.factoryPid = factoryPid; + this.name = name; + } + + @NotNull + public String getPid() { + return pid; + } + + @Nullable + public String getFactoryPid() { + return factoryPid; + } + + @Nullable + public String getName() { + return name; + } + + public boolean isFactory() { + return Objects.nonNull(factoryPid); + } + + public boolean isR7() { + return pid.contains(TILDE); + } + + /** + * @param pid the OSGi Persistent Identity (PID) + * @return the ConfigurationPID parsed from pid + */ + @NotNull + public static ConfigurationPID parsePid(@NotNull final String pid) { + final int index = pid.contains(TILDE) ? pid.indexOf(TILDE) : pid.indexOf(DASH); + if (index > 0) { + final String factoryPid = pid.substring(0, index); + final String name = pid.substring(index + 1); + return new ConfigurationPID(pid, factoryPid, name); + } else { + return new ConfigurationPID(pid); + } + } + + /** + * @param filename a filename of a Configuration with a single extension + * @return the ConfigurationPID parsed from filename + */ + @NotNull + public static ConfigurationPID parseFilename(@NotNull final String filename) { + final String pid = filename.substring(0, filename.lastIndexOf('.')); + return parsePid(pid); + } + + /** + * @param filename a filename of a Configuration + * @param extension a filename extension without leading dot, e.g. cfg, config, json, cfg.json or empty string (no extension) + * @return the ConfigurationPID parsed from filename + * @throws IllegalArgumentException if filename does not end with given extension + */ + @NotNull + public static ConfigurationPID parseFilename(@NotNull final String filename, @NotNull final String extension) throws IllegalArgumentException { + final String pid; + if (extension.isEmpty()) { + pid = filename; + } else { + final String ending = String.format(".%s", extension); + if (filename.endsWith(ending)) { + pid = filename.substring(0, filename.length() - ending.length()); + } else { + final String message = String.format("Parsing filename failed. Filename '%s' does not have given extension '%s'.", filename, extension); + throw new IllegalArgumentException(message); + } + } + return parsePid(pid); + } + +} diff --git a/util/src/test/java/org/apache/karaf/util/config/ConfigurationPIDTest.java b/util/src/test/java/org/apache/karaf/util/config/ConfigurationPIDTest.java new file mode 100644 index 00000000000..9d9962188f8 --- /dev/null +++ b/util/src/test/java/org/apache/karaf/util/config/ConfigurationPIDTest.java @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.karaf.util.config; + +import org.junit.Test; + +import static org.apache.karaf.util.config.ConfigurationPID.parseFilename; +import static org.apache.karaf.util.config.ConfigurationPID.parsePid; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class ConfigurationPIDTest { + + @Test + public void testR7FactoryPID() { + final String pid = "org.apache.felix.jaas.Configuration.factory~TokenLoginModule"; + final ConfigurationPID configurationPID = parsePid(pid); + assertEquals(pid, configurationPID.getPid()); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertTrue(configurationPID.isR7()); + } + + @Test + public void testPreR7FactoryPID() { + final String pid = "org.apache.felix.jaas.Configuration.factory-TokenLoginModule"; + final ConfigurationPID configurationPID = parsePid(pid); + assertEquals(pid, configurationPID.getPid()); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertFalse(configurationPID.isR7()); + } + + @Test + public void testR7FactoryPIDFromFilenameWithCfgExtension() { + final String filename = "org.apache.felix.jaas.Configuration.factory~TokenLoginModule.cfg"; + final ConfigurationPID configurationPID = parseFilename(filename); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertTrue(configurationPID.isR7()); + } + + @Test + public void testR7FactoryPIDFromFilenameWithConfigExtension() { + final String filename = "org.apache.felix.jaas.Configuration.factory~TokenLoginModule.config"; + final ConfigurationPID configurationPID = parseFilename(filename); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertTrue(configurationPID.isR7()); + } + + @Test + public void testR7FactoryPIDFromFilenameWithJsonExtension() { + final String filename = "org.apache.felix.jaas.Configuration.factory~TokenLoginModule.json"; + final ConfigurationPID configurationPID = parseFilename(filename); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertTrue(configurationPID.isR7()); + } + + @Test + public void testR7FactoryPIDFromFilenameWithCfgJsonExtension() { + final String filename = "org.apache.felix.jaas.Configuration.factory~TokenLoginModule.cfg.json"; + final ConfigurationPID configurationPID = parseFilename(filename, "cfg.json"); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertTrue(configurationPID.isR7()); + } + + @Test + public void testR7FactoryPIDFromFilenameWithNoExtension() { + final String filename = "org.apache.felix.jaas.Configuration.factory~TokenLoginModule"; + final ConfigurationPID configurationPID = parseFilename(filename, ""); + assertEquals("org.apache.felix.jaas.Configuration.factory", configurationPID.getFactoryPid()); + assertEquals("TokenLoginModule", configurationPID.getName()); + assertTrue(configurationPID.isFactory()); + assertTrue(configurationPID.isR7()); + } + +} From 2742549fab68ff49e68931c08fe384f63dcc939f Mon Sep 17 00:00:00 2001 From: Oliver Lietz Date: Sun, 11 Apr 2021 23:06:00 +0200 Subject: [PATCH 2/2] KARAF-7098 JsonConfigInstaller ignores R7 factory configurations * Use util ConfigurationPID for parsing and representing PIDs * Do not create new configuration when calling deleteConfig(File) * Simplify logging * Rename parameter fileName to configKey where appropriate --- .../config/core/impl/JsonConfigInstaller.java | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/config/src/main/java/org/apache/karaf/config/core/impl/JsonConfigInstaller.java b/config/src/main/java/org/apache/karaf/config/core/impl/JsonConfigInstaller.java index 04661422d11..f29e6393e8a 100644 --- a/config/src/main/java/org/apache/karaf/config/core/impl/JsonConfigInstaller.java +++ b/config/src/main/java/org/apache/karaf/config/core/impl/JsonConfigInstaller.java @@ -20,6 +20,7 @@ import org.apache.felix.fileinstall.ArtifactInstaller; import org.apache.felix.fileinstall.internal.DirectoryWatcher; import org.apache.felix.utils.collections.DictionaryAsMap; +import org.apache.karaf.util.config.ConfigurationPID; import org.osgi.framework.Constants; import org.osgi.service.cm.Configuration; import org.osgi.service.cm.ConfigurationAdmin; @@ -37,12 +38,15 @@ import java.net.URL; import java.util.Dictionary; import java.util.Hashtable; +import java.util.Objects; + +import static org.apache.karaf.util.config.ConfigurationPID.parseFilename; public class JsonConfigInstaller implements ArtifactInstaller, ConfigurationListener { private final static Logger LOGGER = LoggerFactory.getLogger(JsonConfigInstaller.class); - private ConfigurationAdmin configurationAdmin; + private final ConfigurationAdmin configurationAdmin; public JsonConfigInstaller(ConfigurationAdmin configurationAdmin) { this.configurationAdmin = configurationAdmin; @@ -69,9 +73,9 @@ public void uninstall(File artifact) throws Exception { } private void setConfig(File artifact) throws Exception { - String name = artifact.getName(); - String pid[] = parsePid(name); - Configuration configuration = getConfiguration(toConfigKey(artifact), pid[0], pid[1]); + final String filename = artifact.getName(); + final ConfigurationPID configurationPID = parseFilename(filename); + Configuration configuration = getConfiguration(toConfigKey(artifact), configurationPID); Dictionary props = configuration.getProperties(); Hashtable old = props != null ? new Hashtable<>(new DictionaryAsMap<>(props)) : null; Hashtable properties = Configurations.buildReader().build(new FileReader(artifact)).readConfiguration(); @@ -84,20 +88,22 @@ private void setConfig(File artifact) throws Exception { if (old == null || !old.equals(properties)) { properties.put(DirectoryWatcher.FILENAME, toConfigKey(artifact)); if (old == null) { - LOGGER.info("Creating configuration from " + pid[0] + (pid[1] == null ? "" : "-" + pid[1]) + ".json"); + LOGGER.info("Creating configuration from {}", artifact.getName()); } else { - LOGGER.info("Updating configuration from " + pid[0] + (pid[1] == null ? "" : "-" + pid[1]) + ".json"); + LOGGER.info("Updating configuration from {}", artifact.getName()); } configuration.update(properties); } } - boolean deleteConfig(File f) throws Exception { - String pid[] = parsePid(f.getName()); - LOGGER.info("Deleting configuration from " + pid[0] + (pid[1] == null ? "" : "-" + pid[1]) + ".json"); - Configuration config = getConfiguration(toConfigKey(f), pid[0], pid[1]); - config.delete(); - return true; + void deleteConfig(File artifact) throws Exception { + Configuration config = findExistingConfiguration(toConfigKey(artifact)); + if (Objects.nonNull(config)) { + config.delete(); + LOGGER.info("Configuration for {} found and deleted", artifact.getName()); + } else { + LOGGER.info("Configuration for {} not found, unable to delete", artifact.getName()); + } } @Override @@ -140,41 +146,33 @@ private File getCfgFileFromProperty(Object val) throws URISyntaxException, Malfo return null; } - String[] parsePid(String path) { - String pid = path.substring(0, path.lastIndexOf('.')); - int n = pid.indexOf('-'); - if (n > 0) { - String factoryPid = pid.substring(n + 1); - pid = pid.substring(0, n); - return new String[]{pid, factoryPid}; - } else { - return new String[]{pid, null}; - } - } - String toConfigKey(File f) { return f.getAbsoluteFile().toURI().toString(); } - Configuration getConfiguration(String fileName, String pid, String factoryPid) throws Exception { - Configuration oldConfiguration = findExistingConfiguration(fileName); + Configuration getConfiguration(String configKey, ConfigurationPID configurationPID) throws Exception { + Configuration oldConfiguration = findExistingConfiguration(configKey); Configuration cachedConfiguration = oldConfiguration != null ? configurationAdmin.getConfiguration(oldConfiguration.getPid(), null) : null; if (cachedConfiguration != null) { return cachedConfiguration; } else { - Configuration newConfiguration; - if (factoryPid != null) { - newConfiguration = configurationAdmin.createFactoryConfiguration(pid, "?"); + final Configuration newConfiguration; + if (configurationPID.isFactory()) { + if (configurationPID.isR7()) { // use provided name for R7 Factory PIDs + newConfiguration = configurationAdmin.getFactoryConfiguration(configurationPID.getFactoryPid(), configurationPID.getName(), "?"); + } else { // let ConfigurationAdmin create a random string for name part (for backwards compatibility) + newConfiguration = configurationAdmin.createFactoryConfiguration(configurationPID.getFactoryPid(), "?"); + } } else { - newConfiguration = configurationAdmin.getConfiguration(pid, "?"); + newConfiguration = configurationAdmin.getConfiguration(configurationPID.getPid(), "?"); } return newConfiguration; } } - Configuration findExistingConfiguration(String fileName) throws Exception { - String filter = "(" + DirectoryWatcher.FILENAME + "=" + escapeFilterValue(fileName) + ")"; + Configuration findExistingConfiguration(String configKey) throws Exception { + String filter = "(" + DirectoryWatcher.FILENAME + "=" + escapeFilterValue(configKey) + ")"; Configuration[] configurations = configurationAdmin.listConfigurations(filter); if (configurations != null && configurations.length > 0) { return configurations[0];