Skip to content

test(architecture): a rule that selects nothing fails instead of passing - #1165

Merged
ExtraToast merged 3 commits into
mainfrom
test/rules-that-select-something
Sep 5, 2026
Merged

test(architecture): a rule that selects nothing fails instead of passing#1165
ExtraToast merged 3 commits into
mainfrom
test/rules-that-select-something

Conversation

@ExtraToast

@ExtraToast ExtraToast commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #1159.

Why

ArchitecturePackages declared 51 globs. Measured with ArchUnit's own matcher against the
imported classes, 32 matched nothing, and thirteen of those were still read by rules.

The issue's table was measured by expanding the globs by hand and overcounts the damage in one
direction while missing where it actually sits. .. matches zero packages as well as many, so
net.blueshell.api..web.. does match net.blueshell.api.user.webWEB, PERSISTENCE, DOMAIN
and DTO are live, not dead, and the four rule files the table blames them for were mostly fine.
The dead ones are the application layer, the domain.model/domain.service split,
persistence.repository, web.dto and the whole domain.<feature> prefix.

Where the dead globs sat matters more than how many there were. ArchUnit already fails a rule
whose selection is empty — failOnEmptyShould is on by default, which is why several rules here
explicitly opt out with allowEmptyShould(true). Almost none of these thirteen were on that side.
They sat in dependOnClassesThat, in onlyBeAccessed().byAnyPackage, in a
resideOutsideOfPackages exemption — positions where an empty match makes the rule permissive
and ArchUnit is structurally unable to notice. persistence must not depend on application layer
built its entire target predicate out of five dead globs and passed.

What this achieves

A controller taking a repository, an input type without @Schema, a Query filed in persistence
— all three now fail, and none of them did. A glob that stops matching fails the build with the
constant's name in the message rather than turning its rules green.

How

Rule by rule. Five rules are gone, four are repointed, three had dead entries pruned from a list
that was quietly widening them.

Rule Answer
dto only accessed at api boundary deleted — no DTO package to ring-fence
controllers do not access repositories directly deleted — weaker sibling
persistence must not depend on application layer deleted — the boundary is gone
web DTOs must not be entities (ApiBoundary) deleted — exact duplicate
controllers must not depend on Spring Data repositories deleted — weaker sibling
repositories do not depend on services repointed, and widened to all of persistence
application layer does not depend on controllers repointed to SERVICE_LAYER + persistence
response DTOs are decorated with Schema repointed to <module>/web + name suffix
request DTOs are decorated with Schema repointed to <module>/web + name suffix
dtos must not be entities / must not be Spring components repointed to <module>/web
query objects in application layer not persistence dead exemption pruned
repository only accessed by... / jobs only accessed from... dead allowances pruned
transactional annotations in application layer only dead exemptions pruned

The three deletions worth arguing about:

persistence must not depend on application layer cannot be restated. It split a module's
application package into query objects, which persistence may use under ADR-015, and everything
else, which it may not. The flattening put both in <module>/domain. There is no package boundary
left that draws that line, and inventing one would be a rule nobody asked for. Its nameable half —
persistence must not touch a *Service — is repositories do not depend on services, repointed
and widened from repositories to the whole persistence folder so nothing is lost.

Two controller-to-repository rules, one in each of AccessArchitectureTest and
ApiBoundaryArchitectureTest, are both retired.
I repointed the ApiBoundary one first, planted
the probe, and it fired — but so did DataOwnershipArchitectureTest's web validators should use services not repositories, which was already live against today's packages and is strictly
stronger: every class in a web package, not just the *Controller ones. Keeping a repointed
third copy would have been a rule kept alive for its own sake.

dto only accessed at api boundary guarded web/dto, a folder the flattening emptied.
Input and response types sit directly in <module>/web, where
application services do not depend on DTOs covers the inside of a module and
CrossModuleWebAccessArchitectureTest covers the outside.

Every deletion is recorded in the class header next to the two #1158 already retired, so the file
says why a rule is absent rather than leaving a gap.

PackageConstantsArchitectureTest is the mechanism. It reflects over every constant — arrays
contribute one entry per element — and fails naming any glob that matches no class.

The trade-off. allowEmptyShould(false) is the direct answer the issue suggests, and it is
already the default; it is not enough, because it only sees the classes a rule selects and these
globs were mostly on the target side. Nor can it simply be forced everywhere: two rules here are
honestly "no class does X" and would become false positives. So the two mechanisms are kept
together. The constant check is the wider net, and I removed the four allowEmptyShould(true)
opt-outs that repointing made unnecessary, which puts ArchUnit's own guard back on those rules.
What the constant check does not catch is a constant that still matches but no longer means what
its readers think — DTO matched three platform error DTOs while every module's web/dto had
been emptied. That one needs a human, and it is why DTO and DOMAIN are deleted here despite
matching: they name the pre-flattening shape and had no reader left.

Not in scope

The 32 dead constants are deleted; live-but-unused ones (SHARED_MODEL, SHARED_SECURITY) stay,
since they name today's packages and are the vocabulary a new rule gets written in. WEB and
MODULE_WEB now match the same 267 classes, as do PERSISTENCE and MODULE_PERSISTENCE;
collapsing each pair is a separate tidy.

Worth a reviewer's attention

Repointing request DTOs are decorated with Schema found four real violations, so this PR
touches one production file: CohortSubjectController's four *Request types had no @Schema
while all six of its *Response siblings did. That is the defect this issue describes doing its
job on the first run — the rule had been vacuous since the flattening, so nothing objected. Adding
the annotations is the smallest honest way to land the repoint; the alternative was to keep
tolerating an empty selection.

dtos must not be Spring components picks its subjects by name (*Request/*Response in
<module>/web) rather than by package, because a web package legitimately holds mappers and
argument resolvers that are beans. dtos must not be entities did not need that and selects the
whole web package, which is stronger.

repositories do not depend on services lost its *Repository name filter and now covers
entities and specifications too. It passes today, but it is a genuinely wider rule than the one it
replaces, not just a repointed one.

Verification

  • ./gradlew :services:api:test --tests '*ArchitectureTest*' --tests '*ArchTest*' — exit 0, 67
    passed (71 before, minus 5 deleted rules, plus 1 new).

  • ./gradlew :services:api:test — exit 0, 912 passed.

  • The guard was watched failing. A PROBE = "$ROOT..application.." constant was added:

    java.lang.AssertionError: [these globs match no class, so every rule built on them is vacuous.
    Point each at the package architecture ADR-003 actually puts those types in, or delete it along
    with the rules that read it]
    Expecting empty but was: ["PROBE = net.blueshell.api..application.."]
    
  • A repointed rule was watched failing. A ProbeController taking a BlogRepository, with a
    bare ProbeRequest beside it, was planted in net.blueshell.api.blog.web:

    Rule 'Request DTOs must have @Schema' was violated (1 times):
    Class <net.blueshell.api.blog.web.ProbeRequest> is not annotated with @Schema in
    (ProbeController.kt:0)
    
    Rule 'Controllers must not import repositories' was violated (2 times):
    Constructor <net.blueshell.api.blog.web.ProbeController.<init>(net.blueshell.api.blog.persistence.BlogRepository)>
    has parameter of type <net.blueshell.api.blog.persistence.BlogRepository> in (ProbeController.kt:0)
    

    The second message is what settled the controller-repository question: DataOwnershipArchitectureTest
    reported the same two violations in the same run, which is what made the repointed rule redundant
    rather than useful. Both probes were removed; the runs above are with them gone.


Diff breakdown added removed, scaled to the largest row.

api                                               +139   -197    7
  production         █                              +4     -0    1
  unit tests         ███████████░░░░░░░░░░░░░░░   +135   -197    6
  generated          █░                             +8     -8    1  ~

frontend                                            +0     -0    0
  generated          █░                             +9     -9    2  ~

──────────────────────────────────────────────────────────────────
production                                          +4     -0
tests                                             +135   -197  33.75 test lines per prod line
total (hand-written)                              +139   -197  7 files
~ generated (excluded)                             +17    -17  3 files

ArchitecturePackages declared 32 globs that matched no class after the ADR-003
flattening, and thirteen of them were still read by rules, on the far side where
ArchUnit never looks. Repoint what still means something, delete what the
flattening or a stronger sibling made redundant, and assert every constant
matches at least one class.

Closes #1159.
@ExtraToast
ExtraToast merged commit b75d419 into main Sep 5, 2026
36 of 37 checks passed
@ExtraToast
ExtraToast deleted the test/rules-that-select-something branch September 5, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nineteen architecture rules select a package layout that no longer exists

1 participant