From 9f744d4a1e6a29d86b53ebd1785905f221159814 Mon Sep 17 00:00:00 2001 From: Onkar Date: Sun, 9 Aug 2026 13:40:01 +0530 Subject: [PATCH 1/2] Add jws-diag diff command: compare effective config between two CATALINA_BASE dirs --- src/main/java/org/jboss/jws/diag/Main.java | 4 +- .../org/jboss/jws/diag/diff/ConfigDiffer.java | 239 ++++++++++++++++++ .../org/jboss/jws/diag/diff/DiffCommand.java | 97 +++++++ .../diff/formatter/DiffHumanFormatter.java | 90 +++++++ .../diff/formatter/DiffJsonFormatter.java | 37 +++ .../jboss/jws/diag/diff/model/ChangeType.java | 10 + .../jboss/jws/diag/diff/model/DiffEntry.java | 44 ++++ .../jboss/jws/diag/diff/model/DiffReport.java | 45 ++++ .../jboss/jws/diag/diff/ConfigDifferTest.java | 194 ++++++++++++++ .../jws/diag/diff/DiffHumanFormatterTest.java | 93 +++++++ .../jws/diag/diff/DiffJsonFormatterTest.java | 89 +++++++ 11 files changed, 941 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/jboss/jws/diag/diff/ConfigDiffer.java create mode 100644 src/main/java/org/jboss/jws/diag/diff/DiffCommand.java create mode 100644 src/main/java/org/jboss/jws/diag/diff/formatter/DiffHumanFormatter.java create mode 100644 src/main/java/org/jboss/jws/diag/diff/formatter/DiffJsonFormatter.java create mode 100644 src/main/java/org/jboss/jws/diag/diff/model/ChangeType.java create mode 100644 src/main/java/org/jboss/jws/diag/diff/model/DiffEntry.java create mode 100644 src/main/java/org/jboss/jws/diag/diff/model/DiffReport.java create mode 100644 src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java create mode 100644 src/test/java/org/jboss/jws/diag/diff/DiffHumanFormatterTest.java create mode 100644 src/test/java/org/jboss/jws/diag/diff/DiffJsonFormatterTest.java diff --git a/src/main/java/org/jboss/jws/diag/Main.java b/src/main/java/org/jboss/jws/diag/Main.java index b11bd88..811e8b1 100644 --- a/src/main/java/org/jboss/jws/diag/Main.java +++ b/src/main/java/org/jboss/jws/diag/Main.java @@ -2,6 +2,7 @@ import org.jboss.jws.diag.bundle.BundleCommand; import org.jboss.jws.diag.config.ConfigCommand; +import org.jboss.jws.diag.diff.DiffCommand; import org.jboss.jws.diag.logs.LogsCommand; import org.jboss.jws.diag.summary.SummaryCommand; import org.jboss.jws.diag.validate.ValidateCommand; @@ -17,7 +18,8 @@ ConfigCommand.class, ValidateCommand.class, BundleCommand.class, - LogsCommand.class + LogsCommand.class, + DiffCommand.class } ) public class Main implements Runnable { diff --git a/src/main/java/org/jboss/jws/diag/diff/ConfigDiffer.java b/src/main/java/org/jboss/jws/diag/diff/ConfigDiffer.java new file mode 100644 index 0000000..06820d0 --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/ConfigDiffer.java @@ -0,0 +1,239 @@ +package org.jboss.jws.diag.diff; + +import org.jboss.jws.diag.config.model.CertificateConfig; +import org.jboss.jws.diag.config.model.ConfigValue; +import org.jboss.jws.diag.config.model.ConnectorConfig; +import org.jboss.jws.diag.config.model.ExecutorConfig; +import org.jboss.jws.diag.config.model.ServerConfig; +import org.jboss.jws.diag.config.model.ServiceConfig; +import org.jboss.jws.diag.config.model.SslHostConfig; +import org.jboss.jws.diag.diff.model.ChangeType; +import org.jboss.jws.diag.diff.model.DiffEntry; +import org.jboss.jws.diag.diff.model.DiffReport; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Computes a structural diff between two parsed {@link ServerConfig} trees. + * + *

Connectors are keyed by port; executors by name. Structural differences + * (ADDED/REMOVED items) are reported before field-level CHANGED entries. + */ +public final class ConfigDiffer { + + public DiffReport diff(Path leftBase, Path rightBase, + ServerConfig left, ServerConfig right) { + List entries = new ArrayList<>(); + + compareServer(left, right, entries); + + Map leftServices = indexServicesByName(left.getServices()); + Map rightServices = indexServicesByName(right.getServices()); + + for (String name : union(leftServices.keySet(), rightServices.keySet())) { + ServiceConfig lSvc = leftServices.get(name); + ServiceConfig rSvc = rightServices.get(name); + String svcPath = "services[" + name + "]"; + if (lSvc == null) { + entries.add(new DiffEntry(svcPath, ChangeType.ADDED, null, name)); + } else if (rSvc == null) { + entries.add(new DiffEntry(svcPath, ChangeType.REMOVED, name, null)); + } else { + compareConnectors(svcPath, lSvc, rSvc, entries); + compareExecutors(svcPath, lSvc, rSvc, entries); + } + } + + return new DiffReport(leftBase, rightBase, entries); + } + + private void compareServer(ServerConfig left, ServerConfig right, List out) { + if (left.getShutdownPort() != right.getShutdownPort()) { + out.add(new DiffEntry("server.shutdownPort", ChangeType.CHANGED, + String.valueOf(left.getShutdownPort()), + String.valueOf(right.getShutdownPort()))); + } + diffNullable("server.shutdownCommand", + left.getShutdownCommand(), right.getShutdownCommand(), out); + } + + private void compareConnectors(String svcPath, + ServiceConfig left, ServiceConfig right, + List out) { + Map lMap = indexConnectorsByPort(left.getConnectors()); + Map rMap = indexConnectorsByPort(right.getConnectors()); + + for (int port : union(lMap.keySet(), rMap.keySet())) { + String cPath = svcPath + ".connectors[" + port + "]"; + ConnectorConfig lc = lMap.get(port); + ConnectorConfig rc = rMap.get(port); + if (lc == null) { + out.add(new DiffEntry(cPath, ChangeType.ADDED, null, "port " + port)); + } else if (rc == null) { + out.add(new DiffEntry(cPath, ChangeType.REMOVED, "port " + port, null)); + } else { + diffCv(cPath + ".protocol", lc.getProtocol(), rc.getProtocol(), out); + diffCv(cPath + ".sslEnabled", lc.getSslEnabled(), rc.getSslEnabled(), out); + diffCv(cPath + ".maxThreads", lc.getMaxThreads(), rc.getMaxThreads(), out); + diffCv(cPath + ".connectionTimeout", lc.getConnectionTimeout(), rc.getConnectionTimeout(), out); + diffCv(cPath + ".maxConnections", lc.getMaxConnections(), rc.getMaxConnections(), out); + diffCv(cPath + ".compression", lc.getCompression(), rc.getCompression(), out); + diffCv(cPath + ".secretRequired", lc.getSecretRequired(), rc.getSecretRequired(), out); + diffNullable(cPath + ".executorRef", lc.getExecutorRef(), rc.getExecutorRef(), out); + diffNullable(cPath + ".proxyName", lc.getProxyName(), rc.getProxyName(), out); + diffNullable(cPath + ".proxyPort", + lc.getProxyPort() == null ? null : String.valueOf(lc.getProxyPort()), + rc.getProxyPort() == null ? null : String.valueOf(rc.getProxyPort()), out); + compareSslHostConfigs(cPath, + lc.getSslHostConfigs() != null ? lc.getSslHostConfigs() : Collections.emptyList(), + rc.getSslHostConfigs() != null ? rc.getSslHostConfigs() : Collections.emptyList(), + out); + } + } + } + + private void compareSslHostConfigs(String cPath, + List left, List right, + List out) { + Map lMap = indexSslByHost(left); + Map rMap = indexSslByHost(right); + + for (String host : union(lMap.keySet(), rMap.keySet())) { + String sPath = cPath + ".ssl[" + host + "]"; + SslHostConfig ls = lMap.get(host); + SslHostConfig rs = rMap.get(host); + if (ls == null) { + out.add(new DiffEntry(sPath, ChangeType.ADDED, null, host)); + } else if (rs == null) { + out.add(new DiffEntry(sPath, ChangeType.REMOVED, host, null)); + } else { + diffNullable(sPath + ".protocols", ls.getProtocols(), rs.getProtocols(), out); + diffNullable(sPath + ".sslEnabledProtocols", + ls.getSslEnabledProtocols(), rs.getSslEnabledProtocols(), out); + diffNullable(sPath + ".ciphers", ls.getCiphers(), rs.getCiphers(), out); + diffNullable(sPath + ".certificateVerification", + ls.getCertificateVerification(), rs.getCertificateVerification(), out); + compareCertificates(sPath, ls.getCertificates(), rs.getCertificates(), out); + } + } + } + + private void compareCertificates(String sPath, + List left, List right, + List out) { + Map lMap = indexCertsByType(left); + Map rMap = indexCertsByType(right); + + for (String type : union(lMap.keySet(), rMap.keySet())) { + String certPath = sPath + ".certificate[" + type + "]"; + CertificateConfig lc = lMap.get(type); + CertificateConfig rc = rMap.get(type); + if (lc == null) { + out.add(new DiffEntry(certPath, ChangeType.ADDED, null, type)); + } else if (rc == null) { + out.add(new DiffEntry(certPath, ChangeType.REMOVED, type, null)); + } else { + diffNullable(certPath + ".keystoreFile", lc.getKeystoreFile(), rc.getKeystoreFile(), out); + diffCv(certPath + ".keystoreType", lc.getKeystoreType(), rc.getKeystoreType(), out); + diffNullable(certPath + ".type", lc.getType(), rc.getType(), out); + } + } + } + + private void compareExecutors(String svcPath, + ServiceConfig left, ServiceConfig right, + List out) { + Map lMap = indexExecutorsByName(left.getExecutors()); + Map rMap = indexExecutorsByName(right.getExecutors()); + + for (String name : union(lMap.keySet(), rMap.keySet())) { + String ePath = svcPath + ".executors[" + name + "]"; + ExecutorConfig le = lMap.get(name); + ExecutorConfig re = rMap.get(name); + if (le == null) { + out.add(new DiffEntry(ePath, ChangeType.ADDED, null, name)); + } else if (re == null) { + out.add(new DiffEntry(ePath, ChangeType.REMOVED, name, null)); + } else { + diffCv(ePath + ".maxThreads", le.getMaxThreads(), re.getMaxThreads(), out); + diffCv(ePath + ".minSpareThreads", le.getMinSpareThreads(), re.getMinSpareThreads(), out); + diffCv(ePath + ".threadPriority", le.getThreadPriority(), re.getThreadPriority(), out); + diffCv(ePath + ".maxIdleTime", le.getMaxIdleTime(), re.getMaxIdleTime(), out); + diffNullable(ePath + ".namePrefix", le.getNamePrefix(), re.getNamePrefix(), out); + } + } + } + + private void diffCv(String path, ConfigValue left, ConfigValue right, + List out) { + if (left == null && right == null) return; + String lStr = left == null ? null : renderCv(left); + String rStr = right == null ? null : renderCv(right); + if (!Objects.equals(lStr, rStr)) { + out.add(new DiffEntry(path, ChangeType.CHANGED, lStr, rStr)); + } + } + + private void diffNullable(String path, String left, String right, List out) { + if (!Objects.equals(left, right)) { + out.add(new DiffEntry(path, ChangeType.CHANGED, left, right)); + } + } + + static String renderCv(ConfigValue cv) { + return cv.getValue() + " (" + (cv.isExplicit() ? "explicit" : "default") + ")"; + } + + private static Map indexServicesByName(List items) { + Map map = new LinkedHashMap<>(); + for (ServiceConfig s : items) map.put(s.getName(), s); + return map; + } + + private static Map indexExecutorsByName(List items) { + Map map = new LinkedHashMap<>(); + for (ExecutorConfig e : items) map.put(e.getName(), e); + return map; + } + + private static Map indexConnectorsByPort(List items) { + Map map = new LinkedHashMap<>(); + for (ConnectorConfig c : items) map.put(c.getPort(), c); + return map; + } + + private static Map indexSslByHost(List items) { + Map map = new LinkedHashMap<>(); + int idx = 0; + for (SslHostConfig s : items) { + String key = s.getHostName() != null ? s.getHostName() : String.valueOf(idx); + map.put(key, s); + idx++; + } + return map; + } + + private static Map indexCertsByType(List items) { + Map map = new LinkedHashMap<>(); + int idx = 0; + for (CertificateConfig c : items) { + String key = c.getType() != null ? c.getType() : String.valueOf(idx); + map.put(key, c); + idx++; + } + return map; + } + + private static Iterable union(Iterable a, Iterable b) { + java.util.LinkedHashSet set = new java.util.LinkedHashSet<>(); + a.forEach(set::add); + b.forEach(set::add); + return set; + } +} diff --git a/src/main/java/org/jboss/jws/diag/diff/DiffCommand.java b/src/main/java/org/jboss/jws/diag/diff/DiffCommand.java new file mode 100644 index 0000000..5b35c5a --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/DiffCommand.java @@ -0,0 +1,97 @@ +package org.jboss.jws.diag.diff; + +import org.jboss.jws.diag.common.ExitCodes; +import org.jboss.jws.diag.common.OutputFormat; +import org.jboss.jws.diag.common.OutputFormatMixin; +import org.jboss.jws.diag.config.model.ServerConfig; +import org.jboss.jws.diag.config.parser.PropertyResolver; +import org.jboss.jws.diag.config.parser.ServerXmlParser; +import org.jboss.jws.diag.diff.formatter.DiffHumanFormatter; +import org.jboss.jws.diag.diff.formatter.DiffJsonFormatter; +import org.jboss.jws.diag.diff.model.DiffReport; +import picocli.CommandLine.Command; +import picocli.CommandLine.Mixin; +import picocli.CommandLine.Option; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +@Command(name = "diff", + description = "Compare effective server.xml configuration between two CATALINA_BASE directories", + mixinStandardHelpOptions = true) +public class DiffCommand implements Runnable { + + @Option(names = "--left", + required = true, + description = "Path to the first CATALINA_BASE directory (or its server.xml)") + private Path left; + + @Option(names = "--right", + required = true, + description = "Path to the second CATALINA_BASE directory (or its server.xml)") + private Path right; + + @Mixin + private OutputFormatMixin outputFormat; + + @Override + public void run() { + Path leftXml = resolveServerXml("--left", left); + Path rightXml = resolveServerXml("--right", right); + if (leftXml == null || rightXml == null) { + System.exit(ExitCodes.ERRORS); + return; + } + + ServerConfig leftConfig = parse(leftXml, left); + ServerConfig rightConfig = parse(rightXml, right); + if (leftConfig == null || rightConfig == null) { + System.exit(ExitCodes.ERRORS); + return; + } + + DiffReport report = new ConfigDiffer().diff(left, right, leftConfig, rightConfig); + + String output; + if (outputFormat.getFormat() == OutputFormat.JSON) { + output = new DiffJsonFormatter().format(report); + } else { + output = new DiffHumanFormatter().format(report); + } + + System.out.println(output); + System.exit(report.hasDifferences() ? ExitCodes.WARNINGS : ExitCodes.OK); + } + + private Path resolveServerXml(String flag, Path path) { + if (Files.isRegularFile(path)) { + return path; + } + if (Files.isDirectory(path)) { + Path xml = path.resolve("conf/server.xml"); + if (Files.exists(xml)) return xml; + System.err.println("ERROR: server.xml not found under " + flag + ": " + path); + return null; + } + System.err.println("ERROR: " + flag + " is not a directory or server.xml file: " + path); + return null; + } + + private ServerConfig parse(Path serverXml, Path base) { + try { + Path resolverBase; + if (Files.isDirectory(base)) { + resolverBase = base; + } else { + Path parent = base.getParent(); + resolverBase = (parent != null) ? parent.getParent() : null; + } + PropertyResolver resolver = PropertyResolver.create(resolverBase); + return new ServerXmlParser(resolver).parse(serverXml); + } catch (IOException e) { + System.err.println("ERROR: Failed to parse " + serverXml + ": " + e.getMessage()); + return null; + } + } +} diff --git a/src/main/java/org/jboss/jws/diag/diff/formatter/DiffHumanFormatter.java b/src/main/java/org/jboss/jws/diag/diff/formatter/DiffHumanFormatter.java new file mode 100644 index 0000000..5df0e7e --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/formatter/DiffHumanFormatter.java @@ -0,0 +1,90 @@ +package org.jboss.jws.diag.diff.formatter; + +import org.jboss.jws.diag.diff.model.ChangeType; +import org.jboss.jws.diag.diff.model.DiffEntry; +import org.jboss.jws.diag.diff.model.DiffReport; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Renders a {@link DiffReport} as human-readable text grouped by path prefix. + * + *

Example: + * 

+ * Diff  left : /opt/tomcat-a
+ *       right: /opt/tomcat-b
+ *
+ * server
+ *   ~ shutdownPort          8005  →  8006
+ *
+ * services[Catalina].connectors[8080]
+ *   ~ maxThreads            200 (default)  →  150 (explicit)
+ *
+ * services[Catalina].connectors[8443]
+ *   + ADDED
+ *
+ * No differences found.
+ *
+ */ +public final class DiffHumanFormatter { + + public String format(DiffReport report) { + StringBuilder sb = new StringBuilder(); + sb.append(String.format("Diff left : %s%n", report.getLeft())); + sb.append(String.format(" right: %s%n", report.getRight())); + + if (!report.hasDifferences()) { + sb.append("\nNo differences found.\n"); + return sb.toString(); + } + + sb.append(String.format("%n%d change(s) found:%n", report.getChangeCount())); + + // Group entries by their section (everything up to the last '.') + Map> groups = new LinkedHashMap<>(); + for (DiffEntry e : report.getEntries()) { + String section = sectionOf(e.getPath()); + groups.computeIfAbsent(section, k -> new ArrayList<>()).add(e); + } + + for (Map.Entry> g : groups.entrySet()) { + sb.append('\n').append(g.getKey()).append('\n'); + for (DiffEntry e : g.getValue()) { + String field = fieldOf(e.getPath()); + switch (e.getType()) { + case ADDED: + sb.append(String.format(" + %-28s (added in right)%n", field)); + break; + case REMOVED: + sb.append(String.format(" - %-28s (only in left)%n", field)); + break; + case CHANGED: + sb.append(String.format(" ~ %-28s %s → %s%n", + field, + nullSafe(e.getLeftValue()), + nullSafe(e.getRightValue()))); + break; + } + } + } + return sb.toString(); + } + + private static String sectionOf(String path) { + int dot = path.lastIndexOf('.'); + return dot < 0 ? path : path.substring(0, dot); + } + + private static String fieldOf(String path) { + int dot = path.lastIndexOf('.'); + // For ADDED/REMOVED whole-section entries (e.g. connectors[8443]) the path IS the section + return dot < 0 ? path : path.substring(dot + 1); + } + + private static String nullSafe(String s) { + return s == null ? "(absent)" : s; + } +} diff --git a/src/main/java/org/jboss/jws/diag/diff/formatter/DiffJsonFormatter.java b/src/main/java/org/jboss/jws/diag/diff/formatter/DiffJsonFormatter.java new file mode 100644 index 0000000..649100e --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/formatter/DiffJsonFormatter.java @@ -0,0 +1,37 @@ +package org.jboss.jws.diag.diff.formatter; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import org.jboss.jws.diag.diff.model.DiffReport; + +/** + * Serializes a {@link DiffReport} as indented JSON. + * + *

Schema: + * 

+ * {
+ *   "schemaVersion": "1.0",
+ *   "left": "/opt/tomcat-a",
+ *   "right": "/opt/tomcat-b",
+ *   "changeCount": 2,
+ *   "changes": [
+ *     { "path": "server.shutdownPort", "type": "CHANGED", "left": "8005", "right": "8006" },
+ *     ...
+ *   ]
+ * }
+ *
+ */ +public final class DiffJsonFormatter { + + private static final ObjectMapper MAPPER = new ObjectMapper() + .enable(SerializationFeature.INDENT_OUTPUT); + + public String format(DiffReport report) { + try { + return MAPPER.writeValueAsString(report); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Failed to serialize DiffReport to JSON", e); + } + } +} diff --git a/src/main/java/org/jboss/jws/diag/diff/model/ChangeType.java b/src/main/java/org/jboss/jws/diag/diff/model/ChangeType.java new file mode 100644 index 0000000..a0b8bfd --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/model/ChangeType.java @@ -0,0 +1,10 @@ +package org.jboss.jws.diag.diff.model; + +public enum ChangeType { + /** Present only in right; absent in left. */ + ADDED, + /** Present only in left; absent in right. */ + REMOVED, + /** Present in both but values differ. */ + CHANGED +} diff --git a/src/main/java/org/jboss/jws/diag/diff/model/DiffEntry.java b/src/main/java/org/jboss/jws/diag/diff/model/DiffEntry.java new file mode 100644 index 0000000..5bbe587 --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/model/DiffEntry.java @@ -0,0 +1,44 @@ +package org.jboss.jws.diag.diff.model; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * A single difference between two server.xml configurations. + * + *

{@code path} uses dot-notation, e.g. + * {@code services[Catalina].connectors[8080].maxThreads}. + * {@code leftValue} is null for ADDED entries; {@code rightValue} is null for REMOVED entries. + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public final class DiffEntry { + + @JsonProperty("path") + private final String path; + + @JsonProperty("type") + private final ChangeType type; + + @JsonProperty("left") + private final String leftValue; + + @JsonProperty("right") + private final String rightValue; + + public DiffEntry(String path, ChangeType type, String leftValue, String rightValue) { + this.path = path; + this.type = type; + this.leftValue = leftValue; + this.rightValue = rightValue; + } + + public String getPath() { return path; } + public ChangeType getType() { return type; } + public String getLeftValue() { return leftValue; } + public String getRightValue() { return rightValue; } + + @Override + public String toString() { + return type + " " + path + " [" + leftValue + " → " + rightValue + "]"; + } +} diff --git a/src/main/java/org/jboss/jws/diag/diff/model/DiffReport.java b/src/main/java/org/jboss/jws/diag/diff/model/DiffReport.java new file mode 100644 index 0000000..edaf5d3 --- /dev/null +++ b/src/main/java/org/jboss/jws/diag/diff/model/DiffReport.java @@ -0,0 +1,45 @@ +package org.jboss.jws.diag.diff.model; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; + +/** + * Full diff result between two server.xml configurations. + */ +@JsonPropertyOrder({"schemaVersion", "left", "right", "changeCount", "changes"}) +public final class DiffReport { + + @JsonProperty("schemaVersion") + private static final String SCHEMA_VERSION = "1.0"; + + private final Path leftBase; + private final Path rightBase; + private final List entries; + + public DiffReport(Path leftBase, Path rightBase, List entries) { + this.leftBase = leftBase; + this.rightBase = rightBase; + this.entries = Collections.unmodifiableList(entries); + } + + @JsonProperty("schemaVersion") + public String getSchemaVersion() { return SCHEMA_VERSION; } + + @JsonProperty("left") + public String getLeft() { return leftBase.toString(); } + + @JsonProperty("right") + public String getRight() { return rightBase.toString(); } + + @JsonProperty("changeCount") + public int getChangeCount() { return entries.size(); } + + @JsonProperty("changes") + public List getEntries() { return entries; } + + public boolean hasDifferences() { return !entries.isEmpty(); } +} diff --git a/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java b/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java new file mode 100644 index 0000000..83a4fb6 --- /dev/null +++ b/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java @@ -0,0 +1,194 @@ +package org.jboss.jws.diag.diff; + +import org.jboss.jws.diag.config.model.ConfigValue; +import org.jboss.jws.diag.config.model.ConnectorConfig; +import org.jboss.jws.diag.config.model.ExecutorConfig; +import org.jboss.jws.diag.config.model.ServerConfig; +import org.jboss.jws.diag.config.model.ServiceConfig; +import org.jboss.jws.diag.diff.model.ChangeType; +import org.jboss.jws.diag.diff.model.DiffEntry; +import org.jboss.jws.diag.diff.model.DiffReport; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class ConfigDifferTest { + + private static final Path LEFT = Path.of("/left"); + private static final Path RIGHT = Path.of("/right"); + private final ConfigDiffer differ = new ConfigDiffer(); + + // ── helpers ───────────────────────────────────────────────────────────── + + private static ServerConfig server(int shutdownPort, List connectors, + List executors) { + return ServerConfig.builder() + .shutdownPort(shutdownPort) + .shutdownCommand("SHUTDOWN") + .listeners(Collections.emptyList()) + .services(List.of(ServiceConfig.builder() + .name("Catalina") + .connectors(connectors) + .executors(executors) + .build())) + .build(); + } + + private static ConnectorConfig connector(int port, int maxThreads, boolean explicit) { + return ConnectorConfig.builder() + .port(port) + .protocol(ConfigValue.defaulted("HTTP/1.1")) + .sslEnabled(ConfigValue.defaulted(false)) + .maxThreads(explicit + ? ConfigValue.explicit(maxThreads) + : ConfigValue.defaulted(maxThreads)) + .connectionTimeout(ConfigValue.defaulted(20000)) + .maxConnections(ConfigValue.defaulted(8192)) + .compression(ConfigValue.defaulted("off")) + .secretRequired(ConfigValue.defaulted(false)) + .build(); + } + + private static ExecutorConfig executor(String name, int maxThreads) { + return ExecutorConfig.builder() + .name(name) + .maxThreads(ConfigValue.explicit(maxThreads)) + .minSpareThreads(ConfigValue.defaulted(10)) + .threadPriority(ConfigValue.defaulted(5)) + .maxIdleTime(ConfigValue.defaulted(60000)) + .build(); + } + + // ── tests ──────────────────────────────────────────────────────────────── + + @Test + void identicalConfigs_noEntries() { + ServerConfig left = server(8005, List.of(connector(8080, 200, false)), Collections.emptyList()); + ServerConfig right = server(8005, List.of(connector(8080, 200, false)), Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + assertThat(report.hasDifferences()).isFalse(); + assertThat(report.getEntries()).isEmpty(); + } + + @Test + void shutdownPortChange_reportedAsChanged() { + ServerConfig left = server(8005, Collections.emptyList(), Collections.emptyList()); + ServerConfig right = server(8006, Collections.emptyList(), Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + assertThat(report.hasDifferences()).isTrue(); + DiffEntry entry = report.getEntries().get(0); + assertThat(entry.getPath()).isEqualTo("server.shutdownPort"); + assertThat(entry.getType()).isEqualTo(ChangeType.CHANGED); + assertThat(entry.getLeftValue()).isEqualTo("8005"); + assertThat(entry.getRightValue()).isEqualTo("8006"); + } + + @Test + void connectorMaxThreadsChange_reportedWithProvenance() { + ServerConfig left = server(8005, List.of(connector(8080, 200, false)), Collections.emptyList()); + ServerConfig right = server(8005, List.of(connector(8080, 150, true)), Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + assertThat(report.getEntries()) + .extracting(DiffEntry::getPath) + .contains("services[Catalina].connectors[8080].maxThreads"); + + DiffEntry e = report.getEntries().stream() + .filter(x -> x.getPath().endsWith(".maxThreads")) + .findFirst().orElseThrow(); + assertThat(e.getLeftValue()).isEqualTo("200 (default)"); + assertThat(e.getRightValue()).isEqualTo("150 (explicit)"); + } + + @Test + void connectorOnlyInRight_reportedAsAdded() { + ServerConfig left = server(8005, List.of(connector(8080, 200, false)), Collections.emptyList()); + ServerConfig right = server(8005, + List.of(connector(8080, 200, false), connector(8443, 200, false)), + Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry e = report.getEntries().stream() + .filter(x -> x.getPath().equals("services[Catalina].connectors[8443]")) + .findFirst().orElseThrow(); + assertThat(e.getType()).isEqualTo(ChangeType.ADDED); + assertThat(e.getLeftValue()).isNull(); + } + + @Test + void connectorOnlyInLeft_reportedAsRemoved() { + ServerConfig left = server(8005, + List.of(connector(8080, 200, false), connector(8443, 200, false)), + Collections.emptyList()); + ServerConfig right = server(8005, List.of(connector(8080, 200, false)), Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry e = report.getEntries().stream() + .filter(x -> x.getPath().equals("services[Catalina].connectors[8443]")) + .findFirst().orElseThrow(); + assertThat(e.getType()).isEqualTo(ChangeType.REMOVED); + assertThat(e.getRightValue()).isNull(); + } + + @Test + void executorOnlyInRight_reportedAsAdded() { + ServerConfig left = server(8005, Collections.emptyList(), Collections.emptyList()); + ServerConfig right = server(8005, Collections.emptyList(), List.of(executor("pool", 150))); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry e = report.getEntries().stream() + .filter(x -> x.getPath().equals("services[Catalina].executors[pool]")) + .findFirst().orElseThrow(); + assertThat(e.getType()).isEqualTo(ChangeType.ADDED); + } + + @Test + void executorMaxThreadsChange_detected() { + ServerConfig left = server(8005, Collections.emptyList(), List.of(executor("pool", 100))); + ServerConfig right = server(8005, Collections.emptyList(), List.of(executor("pool", 200))); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry e = report.getEntries().stream() + .filter(x -> x.getPath().endsWith("executors[pool].maxThreads")) + .findFirst().orElseThrow(); + assertThat(e.getType()).isEqualTo(ChangeType.CHANGED); + assertThat(e.getLeftValue()).isEqualTo("100 (explicit)"); + assertThat(e.getRightValue()).isEqualTo("200 (explicit)"); + } + + @Test + void multipleChanges_allReported() { + ServerConfig left = server(8005, + List.of(connector(8080, 200, false)), + List.of(executor("pool", 100))); + ServerConfig right = server(8006, + List.of(connector(8080, 150, true)), + List.of(executor("pool", 200))); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + assertThat(report.getChangeCount()).isGreaterThanOrEqualTo(3); + } + + @Test + void leftAndRightPathsPreservedInReport() { + ServerConfig cfg = server(8005, Collections.emptyList(), Collections.emptyList()); + DiffReport report = differ.diff(LEFT, RIGHT, cfg, cfg); + + assertThat(report.getLeft()).isEqualTo(LEFT.toString()); + assertThat(report.getRight()).isEqualTo(RIGHT.toString()); + } +} diff --git a/src/test/java/org/jboss/jws/diag/diff/DiffHumanFormatterTest.java b/src/test/java/org/jboss/jws/diag/diff/DiffHumanFormatterTest.java new file mode 100644 index 0000000..6e6cba8 --- /dev/null +++ b/src/test/java/org/jboss/jws/diag/diff/DiffHumanFormatterTest.java @@ -0,0 +1,93 @@ +package org.jboss.jws.diag.diff; + +import org.jboss.jws.diag.diff.formatter.DiffHumanFormatter; +import org.jboss.jws.diag.diff.model.ChangeType; +import org.jboss.jws.diag.diff.model.DiffEntry; +import org.jboss.jws.diag.diff.model.DiffReport; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class DiffHumanFormatterTest { + + private static final Path LEFT = Path.of("/opt/tomcat-a"); + private static final Path RIGHT = Path.of("/opt/tomcat-b"); + private final DiffHumanFormatter formatter = new DiffHumanFormatter(); + + private DiffReport report(List entries) { + return new DiffReport(LEFT, RIGHT, entries); + } + + @Test + void noDifferences_showsNoDifferencesMessage() { + String out = formatter.format(report(Collections.emptyList())); + + assertThat(out).contains("No differences found."); + assertThat(out).contains("/opt/tomcat-a"); + assertThat(out).contains("/opt/tomcat-b"); + } + + @Test + void changedEntry_showsArrowBetweenValues() { + DiffEntry e = new DiffEntry("server.shutdownPort", ChangeType.CHANGED, "8005", "8006"); + String out = formatter.format(report(List.of(e))); + + assertThat(out).contains("8005"); + assertThat(out).contains("8006"); + assertThat(out).contains("→"); + assertThat(out).contains("shutdownPort"); + } + + @Test + void addedEntry_showsAddedLabel() { + DiffEntry e = new DiffEntry("services[Catalina].connectors[8443]", + ChangeType.ADDED, null, "port 8443"); + String out = formatter.format(report(List.of(e))); + + assertThat(out).contains("+"); + assertThat(out).contains("added in right"); + } + + @Test + void removedEntry_showsRemovedLabel() { + DiffEntry e = new DiffEntry("services[Catalina].connectors[8443]", + ChangeType.REMOVED, "port 8443", null); + String out = formatter.format(report(List.of(e))); + + assertThat(out).contains("-"); + assertThat(out).contains("only in left"); + } + + @Test + void changeCountShownInHeader() { + List entries = List.of( + new DiffEntry("server.shutdownPort", ChangeType.CHANGED, "8005", "8006"), + new DiffEntry("services[Catalina].connectors[8080].maxThreads", + ChangeType.CHANGED, "200 (default)", "150 (explicit)") + ); + String out = formatter.format(report(entries)); + + assertThat(out).contains("2 change(s) found"); + } + + @Test + void groupingBySection_relatedEntriesUnderSameSection() { + List entries = List.of( + new DiffEntry("services[Catalina].connectors[8080].maxThreads", + ChangeType.CHANGED, "200 (default)", "150 (explicit)"), + new DiffEntry("services[Catalina].connectors[8080].connectionTimeout", + ChangeType.CHANGED, "20000 (default)", "60000 (explicit)") + ); + String out = formatter.format(report(entries)); + + // Both entries should appear under the same section header + long sectionHeaderCount = out.lines() + .filter(l -> l.contains("connectors[8080]") && !l.stripLeading().startsWith("~")) + .count(); + assertThat(sectionHeaderCount).isEqualTo(1); + } +} diff --git a/src/test/java/org/jboss/jws/diag/diff/DiffJsonFormatterTest.java b/src/test/java/org/jboss/jws/diag/diff/DiffJsonFormatterTest.java new file mode 100644 index 0000000..622a187 --- /dev/null +++ b/src/test/java/org/jboss/jws/diag/diff/DiffJsonFormatterTest.java @@ -0,0 +1,89 @@ +package org.jboss.jws.diag.diff; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.jboss.jws.diag.diff.formatter.DiffJsonFormatter; +import org.jboss.jws.diag.diff.model.ChangeType; +import org.jboss.jws.diag.diff.model.DiffEntry; +import org.jboss.jws.diag.diff.model.DiffReport; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class DiffJsonFormatterTest { + + private static final Path LEFT = Path.of("/opt/tomcat-a"); + private static final Path RIGHT = Path.of("/opt/tomcat-b"); + private static final ObjectMapper MAPPER = new ObjectMapper(); + private final DiffJsonFormatter formatter = new DiffJsonFormatter(); + + private JsonNode parse(DiffReport report) throws Exception { + return MAPPER.readTree(formatter.format(report)); + } + + @Test + void schemaVersionPresentAtRoot() throws Exception { + DiffReport report = new DiffReport(LEFT, RIGHT, Collections.emptyList()); + JsonNode root = parse(report); + + assertThat(root.get("schemaVersion").asText()).isEqualTo("1.0"); + } + + @Test + void leftAndRightPathsInRoot() throws Exception { + DiffReport report = new DiffReport(LEFT, RIGHT, Collections.emptyList()); + JsonNode root = parse(report); + + assertThat(root.get("left").asText()).isEqualTo("/opt/tomcat-a"); + assertThat(root.get("right").asText()).isEqualTo("/opt/tomcat-b"); + } + + @Test + void changeCountMatchesEntriesSize() throws Exception { + List entries = List.of( + new DiffEntry("server.shutdownPort", ChangeType.CHANGED, "8005", "8006") + ); + DiffReport report = new DiffReport(LEFT, RIGHT, entries); + JsonNode root = parse(report); + + assertThat(root.get("changeCount").asInt()).isEqualTo(1); + assertThat(root.get("changes").size()).isEqualTo(1); + } + + @Test + void changedEntry_hasAllFields() throws Exception { + DiffEntry entry = new DiffEntry("server.shutdownPort", ChangeType.CHANGED, "8005", "8006"); + JsonNode root = parse(new DiffReport(LEFT, RIGHT, List.of(entry))); + + JsonNode change = root.get("changes").get(0); + assertThat(change.get("path").asText()).isEqualTo("server.shutdownPort"); + assertThat(change.get("type").asText()).isEqualTo("CHANGED"); + assertThat(change.get("left").asText()).isEqualTo("8005"); + assertThat(change.get("right").asText()).isEqualTo("8006"); + } + + @Test + void addedEntry_rightPresentLeftAbsent() throws Exception { + DiffEntry entry = new DiffEntry("services[Catalina].connectors[8443]", + ChangeType.ADDED, null, "port 8443"); + JsonNode root = parse(new DiffReport(LEFT, RIGHT, List.of(entry))); + + JsonNode change = root.get("changes").get(0); + assertThat(change.get("type").asText()).isEqualTo("ADDED"); + assertThat(change.has("left")).isFalse(); + assertThat(change.get("right").asText()).isEqualTo("port 8443"); + } + + @Test + void emptyDiff_changesIsEmptyArray() throws Exception { + JsonNode root = parse(new DiffReport(LEFT, RIGHT, Collections.emptyList())); + + assertThat(root.get("changes").isArray()).isTrue(); + assertThat(root.get("changes").size()).isEqualTo(0); + assertThat(root.get("changeCount").asInt()).isEqualTo(0); + } +} From 1ff2129cdc83d4e9d50087d9b6de000fc9478582 Mon Sep 17 00:00:00 2001 From: Onkar Date: Wed, 12 Aug 2026 23:05:46 +0530 Subject: [PATCH 2/2] Address PR review: add SSL, certificate, and shutdownCommand diff tests --- .../jboss/jws/diag/diff/ConfigDifferTest.java | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java b/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java index 83a4fb6..8d28f9a 100644 --- a/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java +++ b/src/test/java/org/jboss/jws/diag/diff/ConfigDifferTest.java @@ -1,10 +1,12 @@ package org.jboss.jws.diag.diff; +import org.jboss.jws.diag.config.model.CertificateConfig; import org.jboss.jws.diag.config.model.ConfigValue; import org.jboss.jws.diag.config.model.ConnectorConfig; import org.jboss.jws.diag.config.model.ExecutorConfig; import org.jboss.jws.diag.config.model.ServerConfig; import org.jboss.jws.diag.config.model.ServiceConfig; +import org.jboss.jws.diag.config.model.SslHostConfig; import org.jboss.jws.diag.diff.model.ChangeType; import org.jboss.jws.diag.diff.model.DiffEntry; import org.jboss.jws.diag.diff.model.DiffReport; @@ -191,4 +193,148 @@ void leftAndRightPathsPreservedInReport() { assertThat(report.getLeft()).isEqualTo(LEFT.toString()); assertThat(report.getRight()).isEqualTo(RIGHT.toString()); } + + // ── SSL / certificate / shutdownCommand ────────────────────────────────── + + @Test + void shutdownCommandChange_reportedAsChanged() { + ServerConfig left = ServerConfig.builder() + .shutdownPort(8005).shutdownCommand("SHUTDOWN") + .listeners(Collections.emptyList()) + .services(List.of(ServiceConfig.builder() + .name("Catalina").connectors(Collections.emptyList()) + .executors(Collections.emptyList()).build())) + .build(); + ServerConfig right = ServerConfig.builder() + .shutdownPort(8005).shutdownCommand("STOP") + .listeners(Collections.emptyList()) + .services(List.of(ServiceConfig.builder() + .name("Catalina").connectors(Collections.emptyList()) + .executors(Collections.emptyList()).build())) + .build(); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry entry = report.getEntries().stream() + .filter(e -> e.getPath().equals("server.shutdownCommand")) + .findFirst().orElseThrow(); + assertThat(entry.getType()).isEqualTo(ChangeType.CHANGED); + assertThat(entry.getLeftValue()).isEqualTo("SHUTDOWN"); + assertThat(entry.getRightValue()).isEqualTo("STOP"); + } + + @Test + void sslHostConfigAddedInRight_reportedAsAdded() { + ServerConfig left = server(8005, + List.of(connector(8443, 200, false)), Collections.emptyList()); + ServerConfig right = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2,TLSv1.3", + Collections.emptyList())))), + Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry entry = report.getEntries().stream() + .filter(e -> e.getPath().contains(".ssl[") && e.getType() == ChangeType.ADDED) + .findFirst().orElseThrow(); + assertThat(entry.getRightValue()).isEqualTo("_default_"); + } + + @Test + void sslHostConfigProtocolChange_reportedAsChanged() { + ServerConfig left = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2", Collections.emptyList())))), + Collections.emptyList()); + ServerConfig right = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2,TLSv1.3", + Collections.emptyList())))), + Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry entry = report.getEntries().stream() + .filter(e -> e.getPath().endsWith(".protocols")) + .findFirst().orElseThrow(); + assertThat(entry.getType()).isEqualTo(ChangeType.CHANGED); + assertThat(entry.getLeftValue()).isEqualTo("TLSv1.2"); + assertThat(entry.getRightValue()).isEqualTo("TLSv1.2,TLSv1.3"); + } + + @Test + void certificateAddedInRight_reportedAsAdded() { + CertificateConfig cert = CertificateConfig.builder() + .keystoreFile("conf/server.jks") + .keystoreType(ConfigValue.explicit("JKS")) + .type("RSA") + .build(); + ServerConfig left = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2", + Collections.emptyList())))), + Collections.emptyList()); + ServerConfig right = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2", + List.of(cert))))), + Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry entry = report.getEntries().stream() + .filter(e -> e.getPath().contains(".certificate[") && e.getType() == ChangeType.ADDED) + .findFirst().orElseThrow(); + assertThat(entry.getRightValue()).isEqualTo("RSA"); + } + + @Test + void certificateKeystoreFileChange_reportedAsChanged() { + CertificateConfig leftCert = CertificateConfig.builder() + .keystoreFile("conf/old.jks") + .keystoreType(ConfigValue.explicit("JKS")) + .type("RSA") + .build(); + CertificateConfig rightCert = CertificateConfig.builder() + .keystoreFile("conf/new.jks") + .keystoreType(ConfigValue.explicit("JKS")) + .type("RSA") + .build(); + ServerConfig left = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2", List.of(leftCert))))), + Collections.emptyList()); + ServerConfig right = server(8005, + List.of(sslConnector(8443, List.of(ssl("_default_", "TLSv1.2", List.of(rightCert))))), + Collections.emptyList()); + + DiffReport report = differ.diff(LEFT, RIGHT, left, right); + + DiffEntry entry = report.getEntries().stream() + .filter(e -> e.getPath().endsWith(".keystoreFile")) + .findFirst().orElseThrow(); + assertThat(entry.getType()).isEqualTo(ChangeType.CHANGED); + assertThat(entry.getLeftValue()).isEqualTo("conf/old.jks"); + assertThat(entry.getRightValue()).isEqualTo("conf/new.jks"); + } + + // ── helpers ────────────────────────────────────────────────────────────── + + private static ConnectorConfig sslConnector(int port, List sslHostConfigs) { + return ConnectorConfig.builder() + .port(port) + .protocol(ConfigValue.explicit("org.apache.coyote.http11.Http11NioProtocol")) + .sslEnabled(ConfigValue.explicit(true)) + .maxThreads(ConfigValue.defaulted(200)) + .connectionTimeout(ConfigValue.defaulted(20000)) + .maxConnections(ConfigValue.defaulted(8192)) + .compression(ConfigValue.defaulted("off")) + .secretRequired(ConfigValue.defaulted(false)) + .sslHostConfigs(sslHostConfigs) + .build(); + } + + private static SslHostConfig ssl(String hostName, String protocols, + List certs) { + return SslHostConfig.builder() + .hostName(hostName) + .protocols(protocols) + .certificates(certs) + .build(); + } }