Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ExtraToast merged 3 commits into from
Sep 5, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c56b6fb
test(architecture): a rule that selects nothing fails instead of passing
ExtraToast Sep 5, 2026
55b398c
chore(openapi): the spec and client carry the renamed cohort request …
ExtraToast Sep 5, 2026
5f26cf6
chore(openapi): the generated client keeps the shape ci generates
ExtraToast Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions services/api/openapi.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions services/api/src/main/kotlin/net/blueshell/api/cohort/web/CohortSubjectController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ data class CohortSubjectMemberResponse(
val joinedAt: Instant,
)

@Schema(name = "LinkUser")
data class LinkUserRequest(
@field:NotNull val userId: Long,
@field:NotNull val system: TargetSystem,
Expand All @@ -190,19 +191,22 @@ data class LinkUserRequest(
data class LinkedUserResponse(val userId: Long, val system: TargetSystem, val externalUserId: String)

/** Map the subject's per-system cohort to an external target that already exists. */
@Schema(name = "LinkExistingTarget")
data class LinkExistingTargetRequest(
@field:NotNull val system: TargetSystem,
@field:NotBlank val externalId: String,
)

/** Create a fresh external target and map the subject's per-system cohort to it. */
@Schema(name = "CreateTarget")
data class CreateTargetRequest(
@field:NotNull val system: TargetSystem,
@field:NotBlank val label: String,
val folderHint: String? = null,
)

/** Repoint an existing cohort mapping at a different external target. */
@Schema(name = "SwitchTarget")
data class SwitchTargetRequest(
@field:NotBlank val externalId: String,
val deletePrevious: Boolean = false,
Expand Down
109 changes: 30 additions & 79 deletions services/api/src/test/kotlin/net/blueshell/api/architecture/AccessArchitectureTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,39 +28,40 @@ import org.springframework.security.access.prepost.PreAuthorize
* `net.blueshell.api.domain..web..`, a grouping level the flattening removed, and
* [CrossModuleWebAccessArchitectureTest] now states the stronger rule against today's packages:
* no module reaches another module's web package at all, whatever layer the reach comes from.
*
* Three more went with the stale constants they read (#1159):
*
* `dto only accessed at api boundary` selected `..web.dto..`, a folder the flattening emptied —
* input and response types sit directly in `<module>/web` now, so there is no DTO package left to
* ring-fence. What it was defending is stated against today's packages by
* `application services do not depend on DTOs` inside a module, and by
* [CrossModuleWebAccessArchitectureTest] across modules.
*
* `controllers do not access repositories directly` named `..persistence.repository..` on its
* `should` side, a folder the flattening merged into `<module>/persistence`.
* [DataOwnershipArchitectureTest]'s `web validators should use services not repositories` asks the
* wider question against today's packages — every class in a `web` package rather than the
* controllers alone, and `dependOnClassesThat`, which covers field, parameter and return types as
* well as the calls `accessClassesThat` sees.
*
* `persistence must not depend on application layer` carved a module's `application` package into
* query objects, which persistence could use under ADR-015, and everything else, which it could
* not. The flattening put both in `<module>/domain`, so the line it drew is no longer a line any
* package boundary can express. `repositories do not depend on services` keeps the half that can
* still be named, and now covers all of persistence rather than repositories alone.
*/
class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {

@Test
fun `dto only accessed at api boundary`(): Unit =
arch("DTOs only accessed at web layer boundary") {
classes()
.that().resideInAnyPackage(ArchitecturePackages.DTO)
.and().resideOutsideOfPackages(
"${ArchitecturePackages.DOMAIN_EVENT}web.dto..", // Event domain DTOs - known issue (80% migrated)
"${ArchitecturePackages.DOMAIN_SURVEY}web.dto.." // Survey DTOs used by Event
)
.should().onlyBeAccessed().byAnyPackage(
ArchitecturePackages.WEB,
ArchitecturePackages.DTO,
ArchitecturePackages.PERSISTENCE // Allow for entity -> DTO mappings
)
.because("ADR-001: DTOs should not leak into application layer; use commands instead")
}

@Test
fun `repository only accessed by application and persistence layers`(): Unit =
arch("Repositories only accessed from application layer") {
classes()
.that().resideInAnyPackage(ArchitecturePackages.MODULE_PERSISTENCE)
.and().haveSimpleNameEndingWith("Repository")
.should().onlyBeAccessed().byAnyPackage(
*ArchitecturePackages.SERVICE_LAYER,
*ArchitecturePackages.SERVICE_LAYER, // job handlers included: they live here now
ArchitecturePackages.MODULE_PERSISTENCE,
ArchitecturePackages.DOMAIN_SERVICE, // Domain services can access repositories
ArchitecturePackages.PERSISTENCE,
ArchitecturePackages.JOB, // Per-integration job handlers read DB state
ArchitecturePackages.PLATFORM_MOCK // Mock job handlers in test/dev profile
ArchitecturePackages.PLATFORM_MOCK // Mock job handlers in test/dev profile
)
.because("ADR-016: Repositories are inner layer; only application/domain services access them")
}
Expand All @@ -72,36 +73,20 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
.that().resideInAnyPackage(*ArchitecturePackages.JOB_HOMES)
.and().haveSimpleNameEndingWith("Job")
.should().onlyBeAccessed().byAnyPackage(
ArchitecturePackages.JOB,
ArchitecturePackages.MODULE_DOMAIN,
ArchitecturePackages.MODULE_DOMAIN, // event listeners live here too
ArchitecturePackages.MODULE_API,
ArchitecturePackages.PLATFORM,
ArchitecturePackages.LISTENER // Listeners can dispatch jobs
ArchitecturePackages.PLATFORM
)
.because("Jobs should be triggered by event listeners or scheduling infrastructure")
}

@Test
fun `controllers do not access repositories directly`(): Unit =
arch("Controllers must not access repositories") {
noClasses()
.that().resideInAnyPackage(ArchitecturePackages.WEB)
.and().haveSimpleNameEndingWith("Controller")
.should().accessClassesThat().resideInAnyPackage(ArchitecturePackages.REPOSITORY)
.because("ADR-002: Controllers call use cases and services, never repositories")
}

@Test
fun `application layer does not depend on controllers`(): Unit =
arch("Inner layers must not depend on controllers") {
noClasses()
.that().resideInAnyPackage(
ArchitecturePackages.APPLICATION,
// DOMAIN is the module root ($ROOT..domain..), which also covers each module's
// web package; the domain layer proper is model + service.
ArchitecturePackages.DOMAIN_MODEL,
ArchitecturePackages.DOMAIN_SERVICE,
ArchitecturePackages.PERSISTENCE
*ArchitecturePackages.SERVICE_LAYER,
ArchitecturePackages.MODULE_PERSISTENCE
)
.should().dependOnClassesThat(webControllers)
.because("ADR-016: Inner layers must not depend on web layer")
Expand All @@ -119,10 +104,9 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {

@Test
fun `repositories do not depend on services`(): Unit =
arch("Repositories must not depend on services") {
arch("Persistence must not depend on services") {
noClasses()
.that().resideInAnyPackage(ArchitecturePackages.MODULE_PERSISTENCE)
.and().haveSimpleNameEndingWith("Repository")
.should().dependOnClassesThat(applicationServices)
.because("ADR-016: Dependency direction is Service -> Repository, never the reverse")
}
Expand All @@ -137,28 +121,6 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
.because("ADR-016: Persistence layer must not know about web DTOs or controllers")
}

@Test
fun `persistence must not depend on application layer`(): Unit =
arch("Persistence layer is inner - no application dependencies except queries") {
noClasses()
.that().resideInAnyPackage(ArchitecturePackages.PERSISTENCE)
.should().dependOnClassesThat(
JavaClass.Predicates.resideInAnyPackage(
ArchitecturePackages.APPLICATION_VALIDATION,
ArchitecturePackages.LISTENER,
ArchitecturePackages.EVENT,
ArchitecturePackages.FACTORY
).or(
// Services are named, not packaged: a `*Service` glob matches no package.
JavaClass.Predicates.resideInAnyPackage(ArchitecturePackages.APPLICATION)
.and(JavaClass.Predicates.simpleNameEndingWith("Service"))
).or(applicationOfADomainModuleOtherThanQueries)
.`as`("application services, validators, listeners, events, factories or any other part of a domain module's application package")
)
// ArchitecturePackages.QUERY is exempt (ADR-015: Specs can use query objects)
.because("ADR-016: Persistence can depend on query objects (ADR-015), but not services/handlers/validators")
}

@Test
fun `repositories do not depend on DTOs`(): Unit =
arch("Repositories must not depend on DTOs") {
Expand Down Expand Up @@ -201,12 +163,10 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
.that().haveSimpleNameEndingWith("Query")
.and().resideInAnyPackage("${ArchitecturePackages.ROOT}..") // Within project only
.should().resideOutsideOfPackages(
ArchitecturePackages.QUERY,
ArchitecturePackages.MODULE_DOMAIN, // same layer, once the module is flattened
ArchitecturePackages.MODULE_DOMAIN, // where the flattening puts them
ArchitecturePackages.WEB // Acceptable for web query params
)
.because("ADR-015: Query objects are application concerns, not persistence filters")
.allowEmptyShould(true)
}

@Test
Expand Down Expand Up @@ -251,17 +211,8 @@ class AccessArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {
.`as`("web controllers")

val applicationServices: DescribedPredicate<JavaClass> =
JavaClass.Predicates.resideInAnyPackage(ArchitecturePackages.APPLICATION)
JavaClass.Predicates.resideInAnyPackage(*ArchitecturePackages.SERVICE_LAYER)
.and(JavaClass.Predicates.simpleNameEndingWith("Service"))
.`as`("application services")

val applicationOfADomainModuleOtherThanQueries: DescribedPredicate<JavaClass> =
JavaClass.Predicates.resideInAnyPackage(ArchitecturePackages.DOMAIN_APPLICATION)
.and(
DescribedPredicate.not(
JavaClass.Predicates.resideInAnyPackage(ArchitecturePackages.QUERY)
)
)
.`as`("a domain module's application package other than its query objects")
}
}
32 changes: 12 additions & 20 deletions services/api/src/test/kotlin/net/blueshell/api/architecture/ApiBoundaryArchitectureTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ import org.junit.jupiter.api.Test
/**
* ArchUnit tests enforcing API boundary best practices.
* Aligned with ADR-001, ADR-012.
*
* Two rules left with the stale constants they read (#1159).
*
* `web DTOs must not be entities` was a duplicate: [DecoratorsArchitectureTest]'s
* `dtos must not be entities` is the same assertion, and repointing both at `<module>/web` would
* have left two copies of it.
*
* `controllers must not depend on Spring Data repositories` named `..persistence.repository..`,
* a folder the flattening merged into `<module>/persistence`. Repointing it produced a rule
* [DataOwnershipArchitectureTest]'s `web validators should use services not repositories` already
* states more strongly — that one holds every class in a `web` package to it, not just the
* controllers — so it is retired rather than duplicated.
*/
class ApiBoundaryArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT) {

Expand Down Expand Up @@ -44,17 +56,6 @@ class ApiBoundaryArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT)
.because("ADR-001: Web layer should not know about persistence technology")
}

@Test
fun `controllers must not depend on Spring Data repositories`(): Unit =
arch("Controllers must not import repositories") {
noClasses()
.that().resideInAnyPackage(ArchitecturePackages.WEB)
.and().haveSimpleNameEndingWith("Controller")
.should().dependOnClassesThat()
.resideInAnyPackage(ArchitecturePackages.REPOSITORY)
.because("ADR-002: Controllers reach persistence through the application layer, never directly")
}

@Test
fun `entities implement Identifiable interface`(): Unit =
arch("Entities must implement Identifiable") {
Expand Down Expand Up @@ -88,15 +89,6 @@ class ApiBoundaryArchitectureTest : ArchJUnitTestBase(ArchitecturePackages.ROOT)
.because("Jackson on entities causes lazy-loading and serialization issues - use DTOs instead")
}

@Test
fun `web DTOs must not be entities`(): Unit =
arch("DTOs must not be JPA entities") {
noClasses()
.that().resideInAnyPackage(ArchitecturePackages.DTO)
.should().beAnnotatedWith(Entity::class.java)
.because("DTOs and entities serve different purposes - keep them separate")
}

@Test
fun `ACL adapters isolate external dependencies`(): Unit =
arch("ACL adapters must be in platform integration layer") {
Expand Down
Loading
Loading