diff --git a/fleetd/src/test/java/dev/ltms/fleet/PackageCyclesTest.java b/fleetd/src/test/java/dev/ltms/fleet/PackageCyclesTest.java index 46f7b3e2..c76a7117 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/PackageCyclesTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/PackageCyclesTest.java @@ -1,96 +1,174 @@ package dev.ltms.fleet; -import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.Dependency; import com.tngtech.archunit.core.domain.JavaClass; -import com.tngtech.archunit.core.domain.JavaClass.Predicates; +import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.core.importer.ImportOption; import com.tngtech.archunit.library.dependencies.SliceRule; import com.tngtech.archunit.library.dependencies.SlicesRuleDefinition; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; + +import static org.junit.jupiter.api.Assertions.fail; + /** - * fleetd #131 (CB-627): enforce package boundaries with an ArchUnit test instead of a - * Maven module split. + * Enforces package boundaries between the top-level {@code dev.ltms.fleet.*} packages. * - *

This test fails the build the moment a NEW cycle appears between the top-level - * {@code dev.ltms.fleet.*} packages. Today's cycles are recorded below as explicit, - * narrow exceptions: each one ignores dependencies between exactly the two named - * packages, in both directions, and nothing else. A cycle through any other pair of - * packages -- or a brand new pair -- still fails this test. + *

{@link #BASELINE_EDGES} names the exact {@code origin class -> target class} + * dependencies allowed to cross a top-level package boundary. Any dependency between two + * top-level packages that is not in that set fails this test, including a brand new + * dependency between a pair of packages that already has other baselined edges. A baseline + * entry whose dependency no longer exists in the code also fails this test, so the baseline + * always names exactly today's exceptions and nothing more. * *

Main code only. The import excludes test classes - * ({@link ImportOption.Predefined#DO_NOT_INCLUDE_TESTS}). Test code legitimately wires - * across many packages for setup and mocking; that is not part of the shipped - * architecture this rule protects. Verified: importing test classes too pulls in a much - * larger, noisier cycle set -- {@code herdr}, {@code member}, {@code peer}, {@code - * config}, {@code guard} and {@code placement} all show up in cycles that disappear the - * moment test classes are excluded. Scanning off the classpath via {@code - * importPackages(...)} (not a hardcoded {@code target/classes} path) also keeps this - * test correct regardless of the working directory the build is invoked from. - * - *

No package moves here -- ticket #131 is explicit that removing a cycle is - * its own, later PR. See the comment on each exception below for which ticket step - * removes it. + * ({@link ImportOption.Predefined#DO_NOT_INCLUDE_TESTS}). Scanning off the classpath via + * {@code importPackages(...)} keeps this test correct regardless of the working directory + * the build is invoked from. */ class PackageCyclesTest { + /** + * Exact {@code "origin -> target"} class dependencies allowed to cross a top-level + * package boundary. Each entry is one directed edge between two specific classes; a + * two-way relationship between a pair of packages is listed as two separate entries, + * one per direction. + */ + private static final Set BASELINE_EDGES = Set.of( + "dev.ltms.fleet.auth.CallerResolver -> dev.ltms.fleet.mcp.ConnectionIdentity", + "dev.ltms.fleet.auth.CallerResolver -> dev.ltms.fleet.mcp.ConnectionIdentity$Caller", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.auth.AuditLog", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.auth.Authz", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.auth.Authz$Action", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.auth.CallerResolver", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.auth.Principal", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.auth.Role", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.LeadChannel", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.LeadChannel$MailboxState", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.LeadMessage", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$AskOutcome", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$AskResult", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$Outcome", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$Outstanding", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$PendingAsk", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$Phase", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$Reply", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$ReplyOutcome", + "dev.ltms.fleet.mcp.FleetMcp -> dev.ltms.fleet.msg.MessageService$TaskView", + "dev.ltms.fleet.mcp.FleetMcp$1 -> dev.ltms.fleet.msg.MessageService$AskOutcome", + "dev.ltms.fleet.mcp.FleetMcp$1 -> dev.ltms.fleet.msg.MessageService$Outcome", + "dev.ltms.fleet.mcp.FleetMcp$1 -> dev.ltms.fleet.msg.MessageService$Phase", + "dev.ltms.fleet.mcp.FleetMcp$CoordinationSource -> dev.ltms.fleet.msg.LeadChannel", + "dev.ltms.fleet.msg.LeadHeartbeatLoop -> dev.ltms.fleet.mcp.PrimaryRegistry", + "dev.ltms.fleet.msg.ReplyPushLoop -> dev.ltms.fleet.mcp.PrimaryRegistry", + "dev.ltms.fleet.inject.CompletionResolver -> dev.ltms.fleet.msg.Rendezvous", + "dev.ltms.fleet.inject.CompletionResolver -> dev.ltms.fleet.msg.Rendezvous$Resolution", + "dev.ltms.fleet.inject.CompletionResolver -> dev.ltms.fleet.msg.TurnToken", + "dev.ltms.fleet.inject.CompletionResolver$InFlight -> dev.ltms.fleet.msg.Rendezvous$Resolution", + "dev.ltms.fleet.inject.Injector -> dev.ltms.fleet.msg.TurnToken", + "dev.ltms.fleet.inject.Injector$Pending -> dev.ltms.fleet.msg.TurnToken", + "dev.ltms.fleet.inject.TurnListener -> dev.ltms.fleet.msg.TurnToken", + "dev.ltms.fleet.inject.TurnRegistrar -> dev.ltms.fleet.msg.TurnToken", + "dev.ltms.fleet.msg.MessageService -> dev.ltms.fleet.inject.Injector", + "dev.ltms.fleet.msg.MessageService -> dev.ltms.fleet.inject.Injector$Cancellation", + "dev.ltms.fleet.msg.MessageService -> dev.ltms.fleet.inject.Injector$Delivery", + "dev.ltms.fleet.metrics.FleetMetrics -> dev.ltms.fleet.msg.ReplyInbox", + "dev.ltms.fleet.msg.LeadHeartbeatLoop -> dev.ltms.fleet.metrics.Metrics", + "dev.ltms.fleet.msg.MessageService -> dev.ltms.fleet.metrics.Metrics", + "dev.ltms.fleet.msg.ReplyPushLoop -> dev.ltms.fleet.metrics.Metrics", + "dev.ltms.fleet.msg.LeadHeartbeatLoop -> dev.ltms.fleet.session.MemberSession", + "dev.ltms.fleet.msg.LeadHeartbeatLoop -> dev.ltms.fleet.session.MemberSession$State", + "dev.ltms.fleet.session.SessionManager -> dev.ltms.fleet.msg.TurnToken" + ); + + private static final String ROOT_PACKAGE = "dev.ltms.fleet."; + @Test void packagesAreFreeOfCycles() { - var classes = new ClassFileImporter() + JavaClasses classes = new ClassFileImporter() .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) .importPackages("dev.ltms.fleet"); + checkBaselineMatchesTodaysEdges(classes); + SliceRule rule = SlicesRuleDefinition.slices() .matching("dev.ltms.fleet.(*)..") .should().beFreeOfCycles(); - - // fleetd #131 step 1: move ConnectionIdentity so authz stops depending on the - // MCP layer. Evidence: auth/CallerResolver.java:3 imports mcp.ConnectionIdentity; - // mcp/FleetMcp.java:3-7 imports auth.AuditLog, Authz, CallerResolver, Principal, - // Role. - rule = ignoreCycle(rule, "auth", "mcp"); - - // fleetd #131 step 2: PrimaryRegistry is used by loops in msg; move it, or put - // an interface between msg and mcp. Evidence: msg/ReplyPushLoop.java:5 and - // msg/LeadHeartbeatLoop.java:5 import mcp.PrimaryRegistry; mcp/FleetMcp.java:15-18 - // imports msg.LeadChannel, LeadMessage, MessageService, Rendezvous. - rule = ignoreCycle(rule, "mcp", "msg"); - - // fleetd #131 -- found while implementing this test, NOT one of the ticket's - // original three; it names its own follow-up step before removal. Evidence: - // inject/CompletionResolver.java:4-5, inject/Injector.java:6 and - // inject/TurnListener.java:3 import msg.Rendezvous / msg.TurnToken; - // msg/MessageService.java:6 imports inject.Injector. - rule = ignoreCycle(rule, "inject", "msg"); - - // fleetd #131 -- same as above, its own follow-up. Evidence: - // metrics/FleetMetrics.java:3 imports msg.ReplyInbox; msg/MessageService.java:7-8, - // msg/LeadHeartbeatLoop.java:6-7 and msg/ReplyPushLoop.java:6-7 import - // metrics.FleetMetrics / metrics.Metrics. - rule = ignoreCycle(rule, "metrics", "msg"); - - // fleetd #131 -- same as above, its own follow-up. Evidence: - // session/SessionManager.java:7 imports msg.TurnToken; - // msg/LeadHeartbeatLoop.java:8 imports session.MemberSession. - rule = ignoreCycle(rule, "msg", "session"); - + for (String edge : BASELINE_EDGES) { + String[] originAndTarget = edge.split(" -> "); + rule = rule.ignoreDependency(originAndTarget[0], originAndTarget[1]); + } rule.check(classes); } /** - * Accepts today's known cycle between two top-level packages, and nothing else. - * Ignoring both directions removes exactly this pair from cycle detection; every - * other dependency -- including any new one added later, between these same two - * packages or any other pair -- is still checked. + * Fails with the exact offending edge when the live code and {@link #BASELINE_EDGES} + * disagree: a dependency crossing a baselined package pair that is not in the baseline, + * or a baseline entry whose dependency no longer exists. */ - private static SliceRule ignoreCycle(SliceRule rule, String packageA, String packageB) { - return rule - .ignoreDependency(residesIn(packageA), residesIn(packageB)) - .ignoreDependency(residesIn(packageB), residesIn(packageA)); + private static void checkBaselineMatchesTodaysEdges(JavaClasses classes) { + Set baselinedPackagePairs = new TreeSet<>(); + for (String edge : BASELINE_EDGES) { + String[] originAndTarget = edge.split(" -> "); + baselinedPackagePairs.add(unorderedPair( + topLevelPackageOf(originAndTarget[0]), topLevelPackageOf(originAndTarget[1]))); + } + + Set liveEdgesInBaselinedPairs = new TreeSet<>(); + for (JavaClass javaClass : classes) { + for (Dependency dependency : javaClass.getDirectDependenciesFromSelf()) { + JavaClass origin = dependency.getOriginClass(); + JavaClass target = dependency.getTargetClass(); + String originPackage = topLevelPackageOf(origin.getFullName()); + String targetPackage = topLevelPackageOf(target.getFullName()); + if (originPackage.isEmpty() || targetPackage.isEmpty() || originPackage.equals(targetPackage)) { + continue; + } + if (baselinedPackagePairs.contains(unorderedPair(originPackage, targetPackage))) { + liveEdgesInBaselinedPairs.add(origin.getFullName() + " -> " + target.getFullName()); + } + } + } + + List problems = new ArrayList<>(); + for (String liveEdge : liveEdgesInBaselinedPairs) { + if (!BASELINE_EDGES.contains(liveEdge)) { + String[] originAndTarget = liveEdge.split(" -> "); + problems.add("new dependency not in the baseline: " + liveEdge + + " (packages " + topLevelPackageOf(originAndTarget[0]) + + " -> " + topLevelPackageOf(originAndTarget[1]) + ")"); + } + } + for (String baselineEdge : BASELINE_EDGES) { + if (!liveEdgesInBaselinedPairs.contains(baselineEdge)) { + String[] originAndTarget = baselineEdge.split(" -> "); + problems.add("stale baseline entry, no such dependency exists: " + baselineEdge + + " (packages " + topLevelPackageOf(originAndTarget[0]) + + " -> " + topLevelPackageOf(originAndTarget[1]) + ")"); + } + } + + if (!problems.isEmpty()) { + fail("PackageCyclesTest baseline is out of date:\n " + String.join("\n ", problems)); + } } - private static DescribedPredicate residesIn(String topLevelPackage) { - return Predicates.resideInAPackage("dev.ltms.fleet." + topLevelPackage + ".."); + private static String unorderedPair(String packageA, String packageB) { + return packageA.compareTo(packageB) <= 0 ? packageA + "|" + packageB : packageB + "|" + packageA; + } + + private static String topLevelPackageOf(String fullyQualifiedClassName) { + if (!fullyQualifiedClassName.startsWith(ROOT_PACKAGE)) { + return ""; + } + String rest = fullyQualifiedClassName.substring(ROOT_PACKAGE.length()); + int dot = rest.indexOf('.'); + return dot < 0 ? "" : rest.substring(0, dot); } }