From 5aab8a6455b482fddf570d4424f40e432c887708 Mon Sep 17 00:00:00 2001 From: qqeasonchen Date: Tue, 1 Sep 2026 18:03:29 +0800 Subject: [PATCH 1/4] feat(arch-guard): enforce rules via rule.check(classes) (#5305) Switches ArchitectureRulesTest from WARN mode to FAIL mode. WARN mode was silently useless. The arch-guard module has no SLF4J binding on its test runtime classpath, so LoggerFactory returns the NOP logger and every `LOG.warn(r.getFailureReport())` call is discarded. Verified empirically: the architectureCheck test-results XML contains an empty block even though all 10 *_warn methods ran and passed. With `rule.check(classes)`, a violation throws AssertionError carrying the rule description and every violating location, so the build itself reports the real state of the codebase. This is the API the official ArchUnit examples use and it does not depend on logging at all. Also drops the now-unused Logger/LoggerFactory/EvaluationResult imports, the LOG field, and the stale "From 1.14.0" Javadoc paragraph. --- .../guard/ArchitectureRulesTest.java | 67 ++++++++----------- 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java b/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java index 1b8803051f..cb397c0525 100644 --- a/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java +++ b/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java @@ -17,85 +17,72 @@ package org.apache.eventmesh.architecture.guard; - import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.tngtech.archunit.core.domain.JavaClasses; -import com.tngtech.archunit.lang.EvaluationResult; /** * Test class for {@link ArchitectureRules}. * - *

Each rule is asserted in WARN mode: violations are logged at - * {@code WARN} level but the test passes. From 1.14.0 the - * {@code *_warn} test methods will be replaced with hard - * {@code rule.check(classes)} assertions that fail on violation. + *

Each rule is asserted in FAIL mode: {@code rule.check(classes)} + * throws {@code AssertionError} listing every violating location, so + * any architecture violation breaks the build. + * + *

This module intentionally carries no SLF4J binding, so the + * earlier WARN mode discarded every violation report (SLF4J falls back + * to the NOP logger). FAIL mode does not depend on logging at all. */ class ArchitectureRulesTest { - private static final Logger LOG = LoggerFactory.getLogger(ArchitectureRulesTest.class); - private final JavaClasses classes = ArchitectureRules.loadProductionClasses(); @Test - void ruleInternalHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleInternalHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleInternalHidden violations:\n{}", r.getFailureReport()); + void ruleInternalHidden() { + ArchitectureRules.ruleInternalHidden.check(classes); } @Test - void ruleHttpProtocolHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleHttpProtocolHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleHttpProtocolHidden violations:\n{}", r.getFailureReport()); + void ruleHttpProtocolHidden() { + ArchitectureRules.ruleHttpProtocolHidden.check(classes); } @Test - void ruleGrpcProtocolHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleGrpcProtocolHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleGrpcProtocolHidden violations:\n{}", r.getFailureReport()); + void ruleGrpcProtocolHidden() { + ArchitectureRules.ruleGrpcProtocolHidden.check(classes); } @Test - void ruleTcpProtocolHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleTcpProtocolHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleTcpProtocolHidden violations:\n{}", r.getFailureReport()); + void ruleTcpProtocolHidden() { + ArchitectureRules.ruleTcpProtocolHidden.check(classes); } @Test - void ruleOldUtilsRenamed_warn() { - EvaluationResult r = ArchitectureRules.ruleOldUtilsRenamed.evaluate(classes); - LOG.warn("ArchitectureRules.ruleOldUtilsRenamed violations:\n{}", r.getFailureReport()); + void ruleOldUtilsRenamed() { + ArchitectureRules.ruleOldUtilsRenamed.check(classes); } @Test - void ruleRuntimeTcpInternalHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeTcpInternalHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeTcpInternalHidden violations:\n{}", r.getFailureReport()); + void ruleRuntimeTcpInternalHidden() { + ArchitectureRules.ruleRuntimeTcpInternalHidden.check(classes); } @Test - void ruleRuntimeTcpInternalNoReverse_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeTcpInternalNoReverse.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeTcpInternalNoReverse violations:\n{}", r.getFailureReport()); + void ruleRuntimeTcpInternalNoReverse() { + ArchitectureRules.ruleRuntimeTcpInternalNoReverse.check(classes); } @Test - void ruleRuntimeEngineIsolatedFromInfra_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra violations:\n{}", r.getFailureReport()); + void ruleRuntimeEngineIsolatedFromInfra() { + ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra.check(classes); } @Test - void ruleRuntimePushDoesNotImportCodec_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimePushDoesNotImportCodec.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimePushDoesNotImportCodec violations:\n{}", r.getFailureReport()); + void ruleRuntimePushDoesNotImportCodec() { + ArchitectureRules.ruleRuntimePushDoesNotImportCodec.check(classes); } @Test - void ruleRuntimeSubscriptionStateIsolated_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeSubscriptionStateIsolated.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeSubscriptionStateIsolated violations:\n{}", r.getFailureReport()); + void ruleRuntimeSubscriptionStateIsolated() { + ArchitectureRules.ruleRuntimeSubscriptionStateIsolated.check(classes); } } From e9115ee45fbb6447ebc1d2ab7dba1b076fa226c8 Mon Sep 17 00:00:00 2001 From: qqeasonchen Date: Tue, 1 Sep 2026 18:11:50 +0800 Subject: [PATCH 2/4] fix(arch-guard): correct meshmessage package in protocol-hidden rules (#5305) The three protocol-hidden rules (http / grpc / tcp) excluded "org.apache.eventmesh.protocol.plugin.meshmessage..", but the real package is "org.apache.eventmesh.protocol.meshmessage" -- there is no ".plugin" segment. Because the exclusion never matched, all 7 protocol adapter classes were judged as ordinary offenders. In FAIL mode that produced 167 of the 192 reported violations: ruleHttpProtocolHidden 107 ruleTcpProtocolHidden 46 ruleGrpcProtocolHidden 14 No rule semantics change; only the three package strings are corrected. --- .../eventmesh/architecture/guard/ArchitectureRules.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java b/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java index c9bd406439..ce6d26dc6c 100644 --- a/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java +++ b/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java @@ -56,21 +56,21 @@ public static JavaClasses loadProductionClasses() { public static ArchRule ruleHttpProtocolHidden = noClasses() .that().resideOutsideOfPackage("org.apache.eventmesh.common.protocol.http..") .and().resideOutsideOfPackage("org.apache.eventmesh.common..") - .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.plugin.meshmessage..") + .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.meshmessage..") .and().resideOutsideOfPackage("org.apache.eventmesh.client..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.common.protocol.http.."); public static ArchRule ruleGrpcProtocolHidden = noClasses() .that().resideOutsideOfPackage("org.apache.eventmesh.common.protocol.grpc..") .and().resideOutsideOfPackage("org.apache.eventmesh.common..") - .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.plugin.meshmessage..") + .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.meshmessage..") .and().resideOutsideOfPackage("org.apache.eventmesh.client..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.common.protocol.grpc.."); public static ArchRule ruleTcpProtocolHidden = noClasses() .that().resideOutsideOfPackage("org.apache.eventmesh.common.protocol.tcp..") .and().resideOutsideOfPackage("org.apache.eventmesh.common..") - .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.plugin.meshmessage..") + .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.meshmessage..") .and().resideOutsideOfPackage("org.apache.eventmesh.client..") .and().resideOutsideOfPackage("org.apache.eventmesh.runtime..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.common.protocol.tcp.."); From 3b1fa6f93231bb0f3af08ba03424395ffa9cd839 Mon Sep 17 00:00:00 2001 From: qqeasonchen Date: Tue, 1 Sep 2026 18:16:34 +0800 Subject: [PATCH 3/4] fix(arch-guard): drop broken ruleRuntimeTcpInternalNoReverse (#5305) The rule forbade classes in runtime.tcp.internal from depending on runtime.tcp.., but the trailing ".." covers runtime.tcp.internal itself. The result was that a class depending on its own nested type counted as an architecture violation. All 25 reported violations were of this shape: NettyTcpPushChannel.deliver -> TcpAckRegistry.register (internal -> public, normal) TcpPushChannel.deliver -> TcpFrameCodec.encodePush (internal -> internal) TcpPushChannel.deliver -> TcpPushChannel$TcpSessionSink.write (own nested type) TcpRequest$Kind.values -> TcpRequest$Kind.clone (itself) Narrowing the ".." would not rescue the rule either: internal implementation classes depending on the public types they implement is ordinary layering, not a violation. The boundary worth enforcing is already covered by ruleRuntimeTcpInternalHidden, which keeps everything outside runtime.tcp away from runtime.tcp.internal. That rule stays. Rules in force drop from 10 to 9; all 9 pass in FAIL mode. --- .../eventmesh/architecture/guard/ArchitectureRules.java | 4 ---- .../eventmesh/architecture/guard/ArchitectureRulesTest.java | 5 ----- 2 files changed, 9 deletions(-) diff --git a/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java b/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java index ce6d26dc6c..ffe25736d5 100644 --- a/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java +++ b/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java @@ -82,10 +82,6 @@ public static JavaClasses loadProductionClasses() { .that().resideOutsideOfPackage("org.apache.eventmesh.runtime.tcp..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.runtime.tcp.internal.."); - public static ArchRule ruleRuntimeTcpInternalNoReverse = noClasses() - .that().resideInAPackage("org.apache.eventmesh.runtime.tcp.internal..") - .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.runtime.tcp.."); - public static ArchRule ruleRuntimeEngineIsolatedFromInfra = noClasses() .that().resideInAPackage("org.apache.eventmesh.runtime.boot..") .or().resideInAPackage("org.apache.eventmesh.runtime.ingress..") diff --git a/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java b/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java index cb397c0525..423d93f88d 100644 --- a/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java +++ b/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java @@ -66,11 +66,6 @@ void ruleRuntimeTcpInternalHidden() { ArchitectureRules.ruleRuntimeTcpInternalHidden.check(classes); } - @Test - void ruleRuntimeTcpInternalNoReverse() { - ArchitectureRules.ruleRuntimeTcpInternalNoReverse.check(classes); - } - @Test void ruleRuntimeEngineIsolatedFromInfra() { ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra.check(classes); From 59a992e18aa5fbdd1c27c388622599d7ba6c88a9 Mon Sep 17 00:00:00 2001 From: qqeasonchen Date: Tue, 1 Sep 2026 18:23:02 +0800 Subject: [PATCH 4/4] ci(arch-guard): dedicated Architecture Guard workflow (#5305) Completes the A+B setup for the architecture rules. B (local): :eventmesh-architecture-guard:architectureCheck already fails the build on violation, so a developer gets feedback in seconds without waiting for CI. A (CI): .github/workflows/architecture-guard.yml runs the same task on pushes and pull requests touching the analysed modules. A violation now surfaces as its own "Architecture Guard" check instead of hiding inside the 30-minute Build job, and it finishes in a fraction of the time because it only compiles what the rules need. ci.yml drops :eventmesh-architecture-guard:test from the matrix build so the rules are not executed twice. Failure reporting uses the gradle exit code only - no ::error annotation and no auto PR comment - matching how ci.yml and license.yml already behave. README is updated to the 9 rules actually in force (the 5 common rules from #5298 plus the 4 runtime rules from #5297) and documents why WARN mode was replaced: the module has no SLF4J binding, so the NOP logger silently discarded every violation report. --- .github/workflows/architecture-guard.yml | 70 ++++++++++++++++++++++++ .github/workflows/ci.yml | 5 ++ eventmesh-architecture-guard/README.md | 57 +++++++++++++------ 3 files changed, 115 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/architecture-guard.yml diff --git a/.github/workflows/architecture-guard.yml b/.github/workflows/architecture-guard.yml new file mode 100644 index 0000000000..62af103546 --- /dev/null +++ b/.github/workflows/architecture-guard.yml @@ -0,0 +1,70 @@ +# +# 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. +# + +name: 'Architecture Guard' + +# Runs the ArchUnit rules as their own check so a violation shows up as +# "Architecture Guard" on the PR instead of hiding inside the 30-minute +# Build job. Only the modules that feed the analysis trigger it. +on: + push: + branches: [ '**' ] + paths: + - 'eventmesh-architecture-guard/**' + - 'eventmesh-common/**' + - 'eventmesh-runtime/**' + - 'eventmesh-spi/**' + - 'eventmesh-protocol-plugin/**' + - 'eventmesh-storage-plugin/**' + - '.github/workflows/architecture-guard.yml' + pull_request: + branches: [ '**' ] + paths: + - 'eventmesh-architecture-guard/**' + - 'eventmesh-common/**' + - 'eventmesh-runtime/**' + - 'eventmesh-spi/**' + - 'eventmesh-protocol-plugin/**' + - 'eventmesh-storage-plugin/**' + - '.github/workflows/architecture-guard.yml' + +permissions: + contents: read + +jobs: + arch-guard: + name: Architecture Guard + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + submodules: true + + - name: Set up JDK 21 + uses: actions/setup-java@v5 + with: + distribution: 'zulu' + java-version: 21 + + - name: Setup Gradle + uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e #v6.1.0 + + - name: Run architecture rules + run: ./gradlew :eventmesh-architecture-guard:architectureCheck --no-daemon + env: + DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f0837464c..7f991b595e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,10 +67,15 @@ jobs: java-version: ${{ matrix.java }} # https://docs.gradle.org/current/userguide/performance.html + # :eventmesh-architecture-guard:test is excluded here because the + # dedicated Architecture Guard workflow already runs those rules + # (via architectureCheck) on every relevant change; running them + # twice just lengthens the matrix build. - name: Build run: > ./gradlew clean build dist jacocoTestReport --parallel --daemon --scan -x spotlessJava -x generateGrammarSource -x generateDistLicense -x checkDeniedLicense + -x :eventmesh-architecture-guard:test env: DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }} diff --git a/eventmesh-architecture-guard/README.md b/eventmesh-architecture-guard/README.md index 753ab91fd4..ededcf19ff 100644 --- a/eventmesh-architecture-guard/README.md +++ b/eventmesh-architecture-guard/README.md @@ -1,31 +1,54 @@ # :eventmesh-architecture-guard -Minimum-viable architecture guard for issue #5305. Ships with the #5298 -PR (sub-package split of eventmesh-common). +Architecture guard for issue #5305. Enforces the package boundaries +introduced by #5298 (`eventmesh-common` sub-packages) and #5297 +(`eventmesh-runtime` sub-packages). ## What it does -Five ArchUnit rules that enforce the new org.apache.eventmesh.common -sub-package boundaries declared in #5298: +Nine ArchUnit rules, asserted in FAIL mode: any violation breaks the +build. -| Rule | Forbids | -|----------------------------|------------------------------------------------------------------------------------------| -| ruleInternalHidden | org.apache.eventmesh.common.internal.. reached from outside org.apache.eventmesh.common.. | -| ruleHttpProtocolHidden | org.apache.eventmesh.common.protocol.http.. from modules other than meshmessage / sdks | -| ruleGrpcProtocolHidden | org.apache.eventmesh.common.protocol.grpc.. from modules other than meshmessage / sdks | -| ruleTcpProtocolHidden | org.apache.eventmesh.common.protocol.tcp.. from modules other than meshmessage / sdks / runtime | -| ruleOldUtilsRenamed | org.apache.eventmesh.common.utils.. (renamed to .util.. in #5298) | +`eventmesh-common` boundaries (from #5298): -## Severity: WARN in 1.13.0, FAIL in 1.14.0 +| Rule | Forbids | +|------------------------|-------------------------------------------------------------------------------------------| +| ruleInternalHidden | org.apache.eventmesh.common.internal.. reached from outside org.apache.eventmesh.common.. | +| ruleHttpProtocolHidden | org.apache.eventmesh.common.protocol.http.. from modules other than meshmessage / sdks | +| ruleGrpcProtocolHidden | org.apache.eventmesh.common.protocol.grpc.. from modules other than meshmessage / sdks | +| ruleTcpProtocolHidden | org.apache.eventmesh.common.protocol.tcp.. from modules other than meshmessage / sdks / runtime | +| ruleOldUtilsRenamed | org.apache.eventmesh.common.utils.. (renamed to .util.. in #5298) | -The 1.13.0 release ships the rules in WARN mode (rule evaluations are -logged via System.out.println in ArchitectureRulesTest, but violations -do NOT fail `gradle check`). From 1.14.0 the *_warn test methods will -be replaced with hard rule.check(classes) assertions, at which point -new violations WILL fail `gradle architectureCheck` and thus the build. +`eventmesh-runtime` boundaries (from #5297): + +| Rule | Forbids | +|-------------------------------------|--------------------------------------------------------------------------| +| ruleRuntimeTcpInternalHidden | runtime.tcp.internal.. reached from outside runtime.tcp.. | +| ruleRuntimeEngineIsolatedFromInfra | runtime.boot / ingress / delivery depending on runtime.tcp.internal.. | +| ruleRuntimePushDoesNotImportCodec | runtime.push depending on runtime.tcp.internal.. | +| ruleRuntimeSubscriptionStateIsolated | runtime.ingress depending on runtime.state.internal.. | + +## Severity: FAIL + +`ArchitectureRulesTest` calls `rule.check(classes)` for every rule, so +a violation throws and fails the task. + +This replaced the original WARN mode, which turned out to be silent: +the module has no SLF4J binding on its test runtime classpath, so +`LoggerFactory` hands back the NOP logger and every +`LOG.warn(report)` call was discarded. Violations were reported +nowhere. FAIL mode does not depend on logging at all. ## Running locally ```bash ./gradlew :eventmesh-architecture-guard:architectureCheck ``` + +## Continuous integration + +`.github/workflows/architecture-guard.yml` runs the same task as its +own check on pushes and pull requests that touch the analysed modules, +so a violation appears as "Architecture Guard" on the PR rather than +buried in the Build job log. The matrix build in `ci.yml` excludes +`:eventmesh-architecture-guard:test` to avoid running the rules twice.