This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch feature/streamline-hiding-redundant-properties in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-installer-factory-configuration.git
commit 89958d7ab1e8f139b0f580080fd6d13ccc69b573 Author: Konrad Windszus <[email protected]> AuthorDate: Fri Nov 22 12:38:39 2024 +0100 SLING-12495 Clean up logic to hide redundant properties No longer consider metatype default values Either hide both merged/inherited and component properties from description or don't hide anything. Individual hiding may lead to false configurations due to the order of precedence. Update to Parent 62 --- pom.xml | 8 +- .../ConfigurationSerializerWebConsolePlugin.java | 238 +++++++++------------ 2 files changed, 114 insertions(+), 132 deletions(-) diff --git a/pom.xml b/pom.xml index 243282b..e5a3411 100644 --- a/pom.xml +++ b/pom.xml @@ -22,7 +22,7 @@ <parent> <groupId>org.apache.sling</groupId> <artifactId>sling-bundle-parent</artifactId> - <version>61</version> + <version>62</version> <relativePath /> </parent> @@ -78,6 +78,12 @@ <artifactId>org.osgi.service.component</artifactId> <scope>provided</scope> </dependency> + <!-- transitive dependency of DS 1.4--> + <dependency> + <groupId>org.osgi</groupId> + <artifactId>org.osgi.dto</artifactId> + <scope>provided</scope> + </dependency> <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.service.cm</artifactId> diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigurationSerializerWebConsolePlugin.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigurationSerializerWebConsolePlugin.java index 7eb8ecc..7cc4ede 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigurationSerializerWebConsolePlugin.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigurationSerializerWebConsolePlugin.java @@ -27,27 +27,22 @@ import java.io.IOException; import java.io.PrintWriter; import java.net.URL; import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; import java.util.Dictionary; import java.util.Enumeration; +import java.util.HashMap; import java.util.Hashtable; -import java.util.stream.Collectors; +import java.util.Map; import org.apache.sling.installer.api.info.InfoProvider; import org.apache.sling.installer.api.serializer.ConfigurationSerializerFactory; import org.apache.sling.installer.api.serializer.ConfigurationSerializerFactory.Format; -import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; import org.osgi.service.cm.Configuration; import org.osgi.service.cm.ConfigurationAdmin; -import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; import org.osgi.service.component.runtime.ServiceComponentRuntime; import org.osgi.service.component.runtime.dto.ComponentDescriptionDTO; -import org.osgi.service.metatype.MetaTypeService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,9 +62,7 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { private static final String RES_LOC = LABEL + "/res/ui/"; private static final String PARAMETER_PID = "pid"; private static final String PARAMETER_FORMAT = "format"; - private static final String PARAMETER_REMOVE_COMPONENT_DEFAULT_PROPERTIES = "removeComponentDefaultProps"; - private static final String PARAMETER_REMOVE_METATYPE_DEFAULT_PROPERTIES = "removeMetatypeDefaultProps"; - private static final String PARAMETER_REMOVE_MERGED_DEFAULT_PROPERTIES = "removeMergedDefaultProps"; + private static final String PARAMETER_HIDE_REDUNDANT_PROPERTIES = "hideRedundantProperties"; /** The logger */ private final Logger LOGGER = LoggerFactory.getLogger(ConfigurationSerializerWebConsolePlugin.class); @@ -77,52 +70,24 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { @Reference ConfigurationAdmin configurationAdmin; - @Reference - MetaTypeService metatypeService; - @Reference private InfoProvider infoProvider; @Reference private ServiceComponentRuntime scr; - private final BundleContext bundleContext; - - @Activate - public ConfigurationSerializerWebConsolePlugin(BundleContext bundleContext) { - this.bundleContext = bundleContext; - } - @Override public void service(final ServletRequest request, final ServletResponse response) throws IOException { - final String pid = request.getParameter(PARAMETER_PID); - final String format = request.getParameter(PARAMETER_FORMAT); - // initial loading - final boolean removeMetatypeDefaultProperties; - final boolean removeComponentDefaultProperties; - final boolean removeMergedDefaultProperties; - // initial loading? - if (format == null) { - removeMetatypeDefaultProperties = true; - removeComponentDefaultProperties = true; - removeMergedDefaultProperties = true; - } else { - removeMetatypeDefaultProperties = - Boolean.parseBoolean(request.getParameter(PARAMETER_REMOVE_METATYPE_DEFAULT_PROPERTIES)); - removeComponentDefaultProperties = - Boolean.parseBoolean(request.getParameter(PARAMETER_REMOVE_COMPONENT_DEFAULT_PROPERTIES)); - removeMergedDefaultProperties = - Boolean.parseBoolean(request.getParameter(PARAMETER_REMOVE_MERGED_DEFAULT_PROPERTIES)); - } - Collection<ComponentDescriptionDTO> allComponentDescriptions; - if (removeComponentDefaultProperties) { - allComponentDescriptions = scr.getComponentDescriptionDTOs(); + final Configuration configuration; + if (pid != null && !pid.trim().isEmpty()) { + // normalize PID to detect factory configurations with "-" separator + + configuration = configurationAdmin.getConfiguration(pid, null); } else { - allComponentDescriptions = Collections.emptyList(); + configuration = null; } - - MetatypeHandler metatypeHandler = new MetatypeHandler(metatypeService, bundleContext); + final String format = request.getParameter(PARAMETER_FORMAT); ConfigurationSerializerFactory.Format serializationFormat = Format.JSON; if (format != null && !format.trim().isEmpty()) { try { @@ -131,7 +96,28 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { LOGGER.warn("Illegal parameter 'format' given, falling back to default '{}'", serializationFormat, e); } } - final PrintWriter pw = response.getWriter(); + final boolean hideRedundantProperties; + if (format == null) { + hideRedundantProperties = true; + } else { + hideRedundantProperties = Boolean.parseBoolean(request.getParameter(PARAMETER_HIDE_REDUNDANT_PROPERTIES)); + } + dumpConfiguration(configuration, serializationFormat, hideRedundantProperties, response.getWriter()); + } + + private void dumpConfiguration( + Configuration configuration, + ConfigurationSerializerFactory.Format serializationFormat, + boolean hideRedundantProperties, + PrintWriter pw) { + // map with key = configuration pid and value = ComponentDescriptionDTO + Map<String, ComponentDescriptionDTO> allComponentDescriptions = new HashMap<>(); + String pid = configuration != null ? configuration.getPid() : ""; + scr.getComponentDescriptionDTOs().stream().forEach(dto -> { + for (String configPid : dto.configurationPid) { + allComponentDescriptions.put(configPid, dto); + } + }); pw.println("<script type=\"text/javascript\" src=\"" + RES_LOC + "clipboard.js\"></script>"); pw.print("<form method='get'>"); @@ -146,66 +132,70 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { tdLabel(pw, "PID"); tdContent(pw); - pw.print("<input type='text' name='"); - pw.print(PARAMETER_PID); - pw.print("' value='"); - if (pid != null) { - pw.print(escapeXml(pid)); - } - - pw.println("' class='input' size='120' minlength='3'>"); + pw.printf( + "<input type='text' name='%s' ' value='%s' class='input' size='120' minlength='3'>", + PARAMETER_PID, escapeXml(pid)); + pw.println(); + pw.println( + "<p>For factory PIDs use the factory PID followed by a tilde and the configuration name, e.g. 'my.factory.pid~myname'</p>"); closeTd(pw); closeTr(pw); tr(pw); - tdLabel(pw, "Remove Properties"); + + tdLabel(pw, "Hide Properties"); tdContent(pw); - pw.print("<input type='checkbox' name='"); - pw.print(PARAMETER_REMOVE_METATYPE_DEFAULT_PROPERTIES); - pw.print("'"); - if (removeMetatypeDefaultProperties) { + pw.append("<input type='checkbox' name='"); + pw.append(PARAMETER_HIDE_REDUNDANT_PROPERTIES); + pw.append("'"); + if (hideRedundantProperties) { pw.print(" checked"); } - pw.println(" id='"); - pw.print(PARAMETER_REMOVE_METATYPE_DEFAULT_PROPERTIES); - pw.println("' class='input' value='true'>"); - pw.println("<label for='"); - pw.println(PARAMETER_REMOVE_METATYPE_DEFAULT_PROPERTIES); - pw.println("'>Metatype Default Properties</label>"); - - pw.print("<input type='checkbox' name='"); - pw.print(PARAMETER_REMOVE_COMPONENT_DEFAULT_PROPERTIES); - pw.print("'"); - if (removeComponentDefaultProperties) { - pw.print(" checked"); + pw.append(" id='"); + pw.append(PARAMETER_HIDE_REDUNDANT_PROPERTIES); + pw.append("' class='input' value='true'>").println(); + pw.append("<label for='"); + pw.append(PARAMETER_HIDE_REDUNDANT_PROPERTIES); + pw.append("'>"); + + StringBuilder sb = new StringBuilder("Redundant Properties ("); + // these configs come from inherited sources + Dictionary<String, Object> mergedProperties = ConfigTaskCreator.getDefaultProperties(infoProvider, pid); + if (mergedProperties == null) { + mergedProperties = new Hashtable<>(); } - - pw.println(" id='"); - pw.print(PARAMETER_REMOVE_COMPONENT_DEFAULT_PROPERTIES); - pw.println("' class='input' value='true'>"); - pw.println("<label for='"); - pw.println(PARAMETER_REMOVE_COMPONENT_DEFAULT_PROPERTIES); - pw.println("'>Declarative Services Component Properties</label>"); - - if (Activator.MERGE_SCHEMES != null) { - pw.print("<input type='checkbox' name='"); - pw.print(PARAMETER_REMOVE_MERGED_DEFAULT_PROPERTIES); - pw.print("'"); - if (removeMergedDefaultProperties) { - pw.print(" checked"); + if (mergedProperties.size() > 0) { + sb.append( + "from <a href=\"https://sling.apache.org/documentation/bundles/configuration-installer-factory.html#merging-of-configurations\";>Merge Schemes</a> "); + sb.append("\"").append(String.join(", ", Activator.MERGE_SCHEMES)).append("\""); + } + final String pidReferencedFromComponentDescription; + if (configuration != null) { + pidReferencedFromComponentDescription = + configuration.getFactoryPid() != null ? configuration.getFactoryPid() : configuration.getPid(); + ComponentDescriptionDTO componentDescription = + allComponentDescriptions.get(pidReferencedFromComponentDescription); + if (componentDescription != null) { + if (sb.length() > 0) { + sb.append(" and "); + } + sb.append(String.format( + "from component description of <a href=\"components/%s\">component \"%s\" with configuration pid(s) \"%s\"</a>", + componentDescription.name, + componentDescription.name, + String.join(", ", componentDescription.configurationPid))); } - - pw.println(" id='"); - pw.print(PARAMETER_REMOVE_MERGED_DEFAULT_PROPERTIES); - pw.println("' class='input' value='true'>"); - pw.println("<label for='"); - pw.println(PARAMETER_REMOVE_MERGED_DEFAULT_PROPERTIES); - pw.println("'>Merged Properties</label>"); + } else { + pidReferencedFromComponentDescription = ""; + } + if (sb.length() == 0) { + sb.append("no fallback sources found"); } + pw.append(sb.toString()).append(")</label>").println(); pw.println( - "<p>Selecting any of these options strips those properties which have the same name and value as one from any of the selected sources. The removed properties are very likely being redundant and therefore do not need to be added to serialized configs.</a>"); + "<p>Enabling it hides those properties which have the same name and value as any of their fallback values.</p>"); closeTd(pw); closeTr(pw); @@ -215,10 +205,10 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { pw.print("<select name='"); pw.print(PARAMETER_FORMAT); pw.println("'>"); - option(pw, "JSON", "OSGi Configurator JSON", format); - option(pw, "CONFIG", "Apache Felix Config", format); - option(pw, "PROPERTIES", "Java Properties", format); - option(pw, "PROPERTIES_XML", "Java Properties (XML)", format); + option(pw, "JSON", "OSGi Configurator JSON", serializationFormat.name()); + option(pw, "CONFIG", "Apache Felix Config", serializationFormat.name()); + option(pw, "PROPERTIES", "Java Properties", serializationFormat.name()); + option(pw, "PROPERTIES_XML", "Java Properties (XML)", serializationFormat.name()); pw.println("</select>"); pw.println(" <input type='submit' value='Print' class='submit'>"); @@ -226,15 +216,11 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { closeTd(pw); closeTr(pw); - if (pid != null && !pid.trim().isEmpty()) { + if (configuration != null) { tr(pw); tdLabel(pw, "Serialized Configuration Properties"); tdContent(pw); - Configuration configuration = configurationAdmin.getConfiguration(pid, null); - Dictionary<String, Object> mergedProperties = ConfigTaskCreator.getDefaultProperties(infoProvider, pid); - if (mergedProperties == null) { - mergedProperties = new Hashtable<>(); - } + Dictionary<String, Object> properties = configuration.getProperties(); if (properties == null) { pw.print("<p class='ui-state-error-text'>"); @@ -242,22 +228,16 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { pw.println("</p>"); } else { properties = ConfigUtil.cleanConfiguration(properties); + if (hideRedundantProperties) { + removeComponentDefaultProperties( + allComponentDescriptions, + pidReferencedFromComponentDescription, + properties, + mergedProperties); + ConfigUtil.removeRedundantProperties(properties, mergedProperties); + } + try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { - if (removeMetatypeDefaultProperties) { - metatypeHandler.updateConfiguration( - configuration.getFactoryPid(), configuration.getPid(), properties, mergedProperties); - } - if (removeComponentDefaultProperties) { - removeComponentDefaultProperties( - allComponentDescriptions, - configuration.getPid(), - configuration.getFactoryPid(), - properties, - mergedProperties); - } - if (removeMergedDefaultProperties) { - ConfigUtil.removeRedundantProperties(properties, mergedProperties); - } // always emit in alphabetical order of keys ConfigurationSerializerFactory.create(serializationFormat) .serialize(new SortedDictionary<>(properties), baos); @@ -275,7 +255,6 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { closeTd(pw); closeTr(pw); } - pw.println("</table>"); pw.print("</form>"); } @@ -371,25 +350,22 @@ public class ConfigurationSerializerWebConsolePlugin extends GenericServlet { * @param mergedProperties */ private void removeComponentDefaultProperties( - final Collection<ComponentDescriptionDTO> componentDescriptions, - final String pid, - final String factoryPid, + final Map<String, ComponentDescriptionDTO> componentDescriptions, + final String pidReferencedFromComponentDescription, final Dictionary<String, Object> dict, final Dictionary<String, Object> mergedProperties) { - String effectivePid = factoryPid != null ? factoryPid : pid; - Collection<ComponentDescriptionDTO> relevantComponentDescriptions = componentDescriptions.stream() - // find all with a matching pid - .filter(c -> Arrays.asList(c.configurationPid).contains(effectivePid)) - .collect(Collectors.toList()); + ComponentDescriptionDTO relevantComponentDescriptions = + componentDescriptions.get(pidReferencedFromComponentDescription); final Enumeration<String> e = dict.keys(); while (e.hasMoreElements()) { final String key = e.nextElement(); final Object newValue = dict.get(key); - if (relevantComponentDescriptions.stream() - .allMatch(c -> ConfigUtil.isSameValue(newValue, c.properties.get(key)) - && mergedProperties.get(key) == null)) { - dict.remove(key); + if (relevantComponentDescriptions != null) { + Object defaultValue = relevantComponentDescriptions.properties.get(key); + if (ConfigUtil.isSameValue(newValue, defaultValue) && mergedProperties.get(key) == null) { + dict.remove(key); + } } } }
