diff --git a/docs/src/contributing/code/architecture-tests.md b/docs/src/contributing/code/architecture-tests.md new file mode 100644 index 00000000..9c9fba15 --- /dev/null +++ b/docs/src/contributing/code/architecture-tests.md @@ -0,0 +1,72 @@ +--- +title: Architecture Tests +description: How ArchUnit guards the hexagonal boundaries and prevents maintainability issues in IDP-Core. +--- + +IDP-Core enforces its architectural guidelines automatically with [ArchUnit](https://www.archunit.org/). The tests fail the build when code drifts from the architecture decided in [ADR-0002](../adrs/0002-code-architecture-pattern.md). + +## What the Tests Enforce + +The rules live in two classes under `src/test/java/com/decathlon/idp_core/architecture/`. `HexagonalArchitectureTest` guards the boundaries defined by our hexagonal architecture. `MaintainabilityArchitectureTest` prevents cyclic dependencies and enforces general code conventions (dependency injection, logging, exceptions, naming). + +### Hexagonal Constraints + +| Rule | Scope | +| ---- | ----- | +| `DOMAIN_MUST_NOT_DEPEND_ON_INFRASTRUCTURE` | The domain never depends on `infrastructure`. | +| `DOMAIN_MUST_NOT_DEPEND_ON_FORBIDDEN_TECHNOLOGY` | The domain never depends on JPA, Spring Web, Spring Data JPA/repositories, Hibernate, MapStruct, JSLT, Jackson, Kafka, or HTTP clients. | +| `DOMAIN_MUST_NOT_DEPEND_ON_JPA_ENTITIES` | The domain never depends on `*JpaEntity` persistence types. | +| `LAYER_DEPENDENCIES_POINT_INWARD` | `infrastructure` may depend on `domain`, never the reverse. | +| `PORTS_MUST_BE_INTERFACES` | Every class in `domain.port` is an interface. | +| `PORT_NAMED_CLASSES_MUST_LIVE_IN_PORT_PACKAGE` | Every `*Port` contract lives in `domain.port`. | +| `PORTS_IMPLEMENTED_ONLY_IN_ADAPTERS` | A port is implemented only by an `infrastructure.adapters` class. | +| `DTOS_RESIDE_IN_API_DTO_PACKAGE` | Every `*Dto`, `*DtoIn`, `*DtoOut` lives in `adapters.api.dto`. | +| `CONTROLLERS_MUST_NOT_ACCESS_PERSISTENCE` | Controllers never depend on the persistence adapter. | +| `TRANSACTIONAL_NOT_ON_CONTROLLERS` | Controller classes are never `@Transactional`. | +| `TRANSACTIONAL_METHODS_NOT_ON_CONTROLLERS` | Controller methods are never `@Transactional`. | + +The domain may use the Java standard library, Jakarta Validation, and Spring stereotype/transaction annotations. The Spring Data pagination value types (`Pageable`, `Page`, `Sort`, `PageRequest`) are also allowed as a pragmatic abstraction the domain exposes on its ports. + +### Maintainability Conventions + +| Rule | Scope | +| ---- | ----- | +| `FREE_OF_CYCLES` | No package cycles across the whole application. | +| `DOMAIN_FREE_OF_CYCLES` | No cycles between `domain` sub-packages. | +| `ADAPTERS_FREE_OF_CYCLES` | No cycles between `infrastructure.adapters` sub-packages. | +| `NO_FIELD_INJECTION` | Beans use constructor injection, never field/`@Value`/setter injection. | +| `NO_STANDARD_STREAMS` | No `System.out`/`System.err`/`printStackTrace`; use the SLF4J logger. | +| `NO_JAVA_UTIL_LOGGING` | No `java.util.logging`; use SLF4J. | +| `DOMAIN_MUST_NOT_LOG` | The domain never depends on a logging framework. | +| `RUNTIME_EXCEPTIONS_ARE_NAMED_EXCEPTION` | Every runtime exception type is named `*Exception`. | +| `EXCEPTIONS_LIVE_IN_DOMAIN_EXCEPTION_PACKAGE` | Every `*Exception` lives in `domain.exception`. | +| `SERVICES_LIVE_IN_DOMAIN_SERVICE_PACKAGE` | Every `*Service` lives in `domain.service`. | +| `CONTROLLERS_LIVE_IN_API_CONTROLLER_PACKAGE` | Every `*Controller` lives in `adapters.api.controller`. | +| `MAPPERS_LIVE_IN_MAPPER_PACKAGE` | Every `*Mapper` lives in a `mapper` package. | + +## Run the Tests + +The rules run automatically inside `mvn clean verify`. To run only the architecture tests: + +```bash +mvn test -Dtest=HexagonalArchitectureTest,MaintainabilityArchitectureTest +``` + +## Read a Failure + +Each rule carries a `because(...)` clause, so a failure names the offending class and the reason. For example, importing a JPA annotation into a domain class fails with: + +```text +Rule 'no classes that reside in a package '..domain..' should depend on classes +that reside in any package ['jakarta.persistence..', ...]' was violated (1 times): +Class <...domain.model.enums.PropertyType> depends on +``` + +To fix a violation, move the technical dependency into an adapter under `infrastructure` and interact with the domain through a port. + +--- + +## Next Steps + +- **[Domain-Infrastructure Separation](domain-infrastructure.md)** +- **[ADR-0002 Code Architecture Pattern](../adrs/0002-code-architecture-pattern.md)** diff --git a/docs/src/contributing/index.md b/docs/src/contributing/index.md index 49c66f5f..0b86393c 100644 --- a/docs/src/contributing/index.md +++ b/docs/src/contributing/index.md @@ -60,7 +60,7 @@ Thank you for your interest in contributing to IDP-Core. This section provides e ## Code of Conduct -Please carefully read and follow our **[Code of Conduct](../../../CODE_OF_CONDUCT.md)** to ensure a welcoming environment for all contributors. +Please carefully read and follow our **[Code of Conduct](https://github.com/Decathlon/internal-developer-platform/blob/main/CODE_OF_CONDUCT.md)** to ensure a welcoming environment for all contributors. ### Our Standards diff --git a/docs/zensical.toml b/docs/zensical.toml index 4ec20b9e..f72132b5 100644 --- a/docs/zensical.toml +++ b/docs/zensical.toml @@ -63,7 +63,8 @@ nav = [ "contributing/code/exception-handling.md", "contributing/code/domain-model-validations.md", "contributing/code/best-practices.md", - "contributing/code/code-conventions.md" + "contributing/code/code-conventions.md", + "contributing/code/architecture-tests.md" ]}, { "Architecture Decision Records" = [ "contributing/adrs/index.md", diff --git a/pom.xml b/pom.xml index 691826e5..02fc200b 100644 --- a/pom.xml +++ b/pom.xml @@ -26,6 +26,7 @@ 0.8.14 1.1.0 1.5.5.Final + 1.3.2 @@ -194,6 +195,14 @@ test + + + com.tngtech.archunit + archunit-junit5 + ${archunit.version} + test + + org.springdoc diff --git a/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/JwtConfiguration.java b/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/JwtConfiguration.java index c90315dd..3a99b4d8 100644 --- a/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/JwtConfiguration.java +++ b/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/JwtConfiguration.java @@ -22,8 +22,12 @@ @Configuration public class JwtConfiguration { - @Value("${spring.security.oauth2.resourceserver.jwt.jwk-set-uri}") - private String jwkSetUri; + private final String jwkSetUri; + + public JwtConfiguration( + @Value("${spring.security.oauth2.resourceserver.jwt.jwk-set-uri}") String jwkSetUri) { + this.jwkSetUri = jwkSetUri; + } @Bean @ConditionalOnMissingBean diff --git a/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SwaggerConfiguration.java b/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SwaggerConfiguration.java index a0e0e7cc..47ce8abb 100644 --- a/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SwaggerConfiguration.java +++ b/src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SwaggerConfiguration.java @@ -53,11 +53,15 @@ public class SwaggerConfiguration { public static final String CLIENT_ID = "clientId"; public static final String BEARER = "bearer"; - @Value("${spring.security.oauth2.client.provider.idp-core.token-uri}") - private String oauth2url; + private final String oauth2url; + private final String idpCorePrefixUrl; - @Value("${app.idp-core-prefix-url}") - private String idpCorePrefixUrl; + public SwaggerConfiguration( + @Value("${spring.security.oauth2.client.provider.idp-core.token-uri}") String oauth2url, + @Value("${app.idp-core-prefix-url}") String idpCorePrefixUrl) { + this.oauth2url = oauth2url; + this.idpCorePrefixUrl = idpCorePrefixUrl; + } @Bean public OpenAPI openAPI() { diff --git a/src/test/java/com/decathlon/idp_core/architecture/HexagonalArchitectureTest.java b/src/test/java/com/decathlon/idp_core/architecture/HexagonalArchitectureTest.java new file mode 100644 index 00000000..75efd01c --- /dev/null +++ b/src/test/java/com/decathlon/idp_core/architecture/HexagonalArchitectureTest.java @@ -0,0 +1,134 @@ +package com.decathlon.idp_core.architecture; + +import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAPackage; +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noMethods; +import static com.tngtech.archunit.library.Architectures.layeredArchitecture; + +import com.tngtech.archunit.core.importer.ImportOption.DoNotIncludeTests; +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.lang.ArchRule; + +/// Architecture guardrails enforcing the "Pragmatic Hexagonal Architecture" decided in +/// ADR-0002 and detailed in the `.github/instructions` layer guidelines. +/// +/// Some rules enforced are: +/// - The domain stays independent from the infrastructure and from third-party integration libraries +/// - Ports are pure contracts +@AnalyzeClasses(packages = HexagonalArchitectureTest.BASE_PACKAGE, importOptions = DoNotIncludeTests.class) +class HexagonalArchitectureTest { + + static final String BASE_PACKAGE = "com.decathlon.idp_core"; + + private static final String DOMAIN_PACKAGE = "..domain.."; + private static final String INFRASTRUCTURE_PACKAGE = "..infrastructure.."; + private static final String PORT_PACKAGE = "..domain.port.."; + private static final String ADAPTERS_PACKAGE = "..infrastructure.adapters.."; + private static final String API_CONTROLLER_PACKAGE = "..adapters.api.controller.."; + private static final String API_DTO_PACKAGE = "..adapters.api.dto.."; + private static final String PERSISTENCE_PACKAGE = "..adapters.persistence.."; + + private static final String SPRING_TRANSACTIONAL = "org.springframework.transaction.annotation.Transactional"; + private static final String JAKARTA_TRANSACTIONAL = "jakarta.transaction.Transactional"; + + /// Third-party / framework integration packages the domain must never depend + /// on. + /// + /// The list is intentionally explicit so the allowed Spring + /// annotations (e.g. `@Service`, `@Component`) and Jakarta Validation + /// (`jakarta.validation..`) are not caught as false positives. + private static final String[] FORBIDDEN_TECHNOLOGY_PACKAGES = {"jakarta.persistence..", // JPA + "org.springframework.web..", "org.springframework.data.jpa..", + "org.springframework.data.repository..", "org.springframework.http..", "org.hibernate..", + "org.mapstruct..", "com.schibsted..", "com.fasterxml.jackson..", "org.apache.kafka..", + "java.net.http..",}; + + // --------------------------------------------------------------------------------------- + // 1. Hexagonal constraints + // --------------------------------------------------------------------------------------- + + /// The domain must never depend on the infrastructure + @ArchTest + static final ArchRule DOMAIN_MUST_NOT_DEPEND_ON_INFRASTRUCTURE = noClasses().that() + .resideInAPackage(DOMAIN_PACKAGE).should().dependOnClassesThat() + .resideInAPackage(INFRASTRUCTURE_PACKAGE) + .because("the domain is the inside the hexagon and must not depend on adapters"); + + /// The domain is pure business logic and must not depend on integration + /// frameworks + /// Jakarta Validation is allowed + @ArchTest + static final ArchRule DOMAIN_MUST_NOT_DEPEND_ON_FORBIDDEN_TECHNOLOGY = noClasses().that() + .resideInAPackage(DOMAIN_PACKAGE).should().dependOnClassesThat() + .resideInAnyPackage(FORBIDDEN_TECHNOLOGY_PACKAGES) + .because("the domain must stay free of integration frameworks"); + + /// Ports are contracts: every class in the port package must be an interface + @ArchTest + static final ArchRule PORTS_MUST_BE_INTERFACES = classes().that().resideInAPackage(PORT_PACKAGE) + .should().beInterfaces() + .because("ports define the contract between domain and adapters and carry no implementation"); + + /// Every `*Port` contract must live in the dedicated port package + @ArchTest + static final ArchRule PORT_NAMED_CLASSES_MUST_LIVE_IN_PORT_PACKAGE = classes().that() + .haveSimpleNameEndingWith("Port").should().resideInAPackage(PORT_PACKAGE) + .because("port contracts must be grouped under domain.port"); + + /// A driven port may only be implemented by an infrastructure adapter, no other + /// layer is allowed to provide a port implementation + @ArchTest + static final ArchRule PORTS_IMPLEMENTED_ONLY_IN_ADAPTERS = classes().that() + .implement(resideInAPackage(PORT_PACKAGE)).should().resideInAPackage(ADAPTERS_PACKAGE) + .because("ports are provided by domain and implemented by infrastructure adapters only"); + + /// The domain must not depend on JPA persistence entities + @ArchTest + static final ArchRule DOMAIN_MUST_NOT_DEPEND_ON_JPA_ENTITIES = noClasses().that() + .resideInAPackage(DOMAIN_PACKAGE).should().dependOnClassesThat() + .haveSimpleNameEndingWith("JpaEntity") + .because("persistence entities are an infrastructure detail, the domain owns its own model"); + + /// The infrastructure may depend on the domain, but the domain must never reach + /// into the infrastructure + @ArchTest + static final ArchRule LAYER_DEPENDENCIES_POINT_INWARD = layeredArchitecture() + .consideringOnlyDependenciesInLayers().layer("Domain").definedBy(DOMAIN_PACKAGE) + .layer("Infrastructure").definedBy(INFRASTRUCTURE_PACKAGE).whereLayer("Infrastructure") + .mayNotBeAccessedByAnyLayer().because( + "dependencies point inward: infrastructure depends on the domain, never the reverse"); + + /// Request/response DTOs are a driving-adapter and must live with the REST API + @ArchTest + static final ArchRule DTOS_RESIDE_IN_API_DTO_PACKAGE = classes().that() + .haveSimpleNameEndingWith("Dto").or().haveSimpleNameEndingWith("DtoIn").or() + .haveSimpleNameEndingWith("DtoOut").should().resideInAPackage(API_DTO_PACKAGE) + .because("DTOs are an API-adapter concern and must stay under adapters.api.dto"); + + /// Controllers must go through domain ports and never reach directly the + /// persistence adapter + @ArchTest + static final ArchRule CONTROLLERS_MUST_NOT_ACCESS_PERSISTENCE = noClasses().that() + .resideInAPackage(API_CONTROLLER_PACKAGE).should().dependOnClassesThat() + .resideInAPackage(PERSISTENCE_PACKAGE) + .because("controllers must call the domain through ports, never the persistence adapter"); + + /// Transaction boundaries belong to the domain service layer, controllers + /// should not + /// be annotated with `@Transactional` at class level + @ArchTest + static final ArchRule TRANSACTIONAL_NOT_ON_CONTROLLERS = noClasses().that() + .resideInAPackage(API_CONTROLLER_PACKAGE).should().beAnnotatedWith(SPRING_TRANSACTIONAL) + .orShould().beAnnotatedWith(JAKARTA_TRANSACTIONAL) + .because("transaction boundaries belong in the domain service layer, not the web layer"); + + /// Transaction boundaries belong to the domain service layer; controller + /// methods should not be annotated with `@Transactional` + @ArchTest + static final ArchRule TRANSACTIONAL_METHODS_NOT_ON_CONTROLLERS = noMethods().that() + .areDeclaredInClassesThat().resideInAPackage(API_CONTROLLER_PACKAGE).should() + .beAnnotatedWith(SPRING_TRANSACTIONAL).orShould().beAnnotatedWith(JAKARTA_TRANSACTIONAL) + .because("transaction boundaries belong in the domain service layer, not the web layer"); +} diff --git a/src/test/java/com/decathlon/idp_core/architecture/MaintainabilityArchitectureTest.java b/src/test/java/com/decathlon/idp_core/architecture/MaintainabilityArchitectureTest.java new file mode 100644 index 00000000..6e582abc --- /dev/null +++ b/src/test/java/com/decathlon/idp_core/architecture/MaintainabilityArchitectureTest.java @@ -0,0 +1,125 @@ +package com.decathlon.idp_core.architecture; + +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; +import static com.tngtech.archunit.library.GeneralCodingRules.NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS; +import static com.tngtech.archunit.library.GeneralCodingRules.NO_CLASSES_SHOULD_USE_FIELD_INJECTION; +import static com.tngtech.archunit.library.GeneralCodingRules.NO_CLASSES_SHOULD_USE_JAVA_UTIL_LOGGING; +import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices; + +import com.tngtech.archunit.core.importer.ImportOption.DoNotIncludeTests; +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.lang.ArchRule; + +/// General maintainability guardrails that keep the codebase consistent, predictable and easy to evolve +/// See `.github/instructions/{java,springboot,domain}.instructions.md` for more details. +/// +/// They cover : +/// - Dependency-injection best practices +/// - Logging discipline +/// - Exception handling +/// - Project naming and placement conventions +@AnalyzeClasses(packages = MaintainabilityArchitectureTest.BASE_PACKAGE, importOptions = DoNotIncludeTests.class) +class MaintainabilityArchitectureTest { + + static final String BASE_PACKAGE = "com.decathlon.idp_core"; + + private static final String DOMAIN_PACKAGE = "..domain.."; + private static final String EXCEPTION_PACKAGE = "..domain.exception.."; + + // --------------------------------------------------------------------------------------- + // 1. Cyclic dependencies + // --------------------------------------------------------------------------------------- + + /// No dependency cycles between the top-level slices of the application. + @ArchTest + static final ArchRule FREE_OF_CYCLES = slices().matching("com.decathlon.idp_core.(*)..").should() + .beFreeOfCycles() + .because("package cycles are the primary symptom of architecture erosion (ADR-0002)"); + + /// No dependency cycles between domain sub-packages (model, service, port, + /// ...). + @ArchTest + static final ArchRule DOMAIN_FREE_OF_CYCLES = slices() + .matching("com.decathlon.idp_core.domain.(*)..").should().beFreeOfCycles() + .because("the domain must stay internally untangled to remain easy to reason about"); + + /// No dependency cycles between infrastructure adapters (api, persistence, + /// formula, ...). + @ArchTest + static final ArchRule ADAPTERS_FREE_OF_CYCLES = slices() + .matching("com.decathlon.idp_core.infrastructure.adapters.(*)..").should().beFreeOfCycles() + .because("adapters must remain independent plugs that can evolve in isolation"); + + // --------------------------------------------------------------------------------------- + // 2. Dependency-injection & logging + // --------------------------------------------------------------------------------------- + + /// Beans must use constructor injection; field/setter injection hides + /// dependencies and makes classes harder to test (springboot.instructions.md) + @ArchTest + static final ArchRule NO_FIELD_INJECTION = NO_CLASSES_SHOULD_USE_FIELD_INJECTION + .because("constructor injection is mandatory; field injection hides dependencies"); + + /// Production code must log through SLF4J, never the console + @ArchTest + static final ArchRule NO_STANDARD_STREAMS = NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS + .because("use the SLF4J logger instead of System.out / System.err / printStackTrace"); + + /// Production code must log through SLF4J, never `java.util.logging` + @ArchTest + static final ArchRule NO_JAVA_UTIL_LOGGING = NO_CLASSES_SHOULD_USE_JAVA_UTIL_LOGGING + .because("SLF4J (with Logback) is the single logging facade used across the project"); + + /// The domain is pure business logic and must not perform any logging; logging + /// is an infrastructure concern (domain.instructions.md) + @ArchTest + static final ArchRule DOMAIN_MUST_NOT_LOG = noClasses().that().resideInAPackage(DOMAIN_PACKAGE) + .should().dependOnClassesThat() + .resideInAnyPackage("org.slf4j..", "ch.qos.logback..", "org.apache.logging..", + "org.apache.commons.logging..") + .because("the domain must stay free of logging frameworks; logging belongs to adapters"); + + // --------------------------------------------------------------------------------------- + // 3. Exception handling + // --------------------------------------------------------------------------------------- + + /// Every runtime exception type must be named `*Exception` + @ArchTest + static final ArchRule RUNTIME_EXCEPTIONS_ARE_NAMED_EXCEPTION = classes().that() + .areAssignableTo(RuntimeException.class).should().haveSimpleNameEndingWith("Exception") + .because("the *Exception suffix is the project convention for throwable types"); + + /// Every `*Exception` type must live in the domain exception package + @ArchTest + static final ArchRule EXCEPTIONS_LIVE_IN_DOMAIN_EXCEPTION_PACKAGE = classes().that() + .haveSimpleNameEndingWith("Exception").should().resideInAPackage(EXCEPTION_PACKAGE) + .because("domain exceptions are grouped under domain.exception"); + + // --------------------------------------------------------------------------------------- + // 4. Naming & placement conventions + // --------------------------------------------------------------------------------------- + + /// `*Service` classes are domain services and must live in the domain service + /// package + @ArchTest + static final ArchRule SERVICES_LIVE_IN_DOMAIN_SERVICE_PACKAGE = classes().that() + .haveSimpleNameEndingWith("Service").should().resideInAPackage("..domain.service..") + .because("business services belong to the domain service layer"); + + /// `*Controller` classes are driving REST adapters and must live in the API + /// package + @ArchTest + static final ArchRule CONTROLLERS_LIVE_IN_API_CONTROLLER_PACKAGE = classes().that() + .haveSimpleNameEndingWith("Controller").should() + .resideInAPackage("..adapters.api.controller..") + .because("REST controllers are a driving adapter and belong under adapters.api.controller"); + + /// `*Mapper` classes translate between layers and must live in a `mapper` + /// package next to their adapter + @ArchTest + static final ArchRule MAPPERS_LIVE_IN_MAPPER_PACKAGE = classes().that() + .haveSimpleNameEndingWith("Mapper").should().resideInAPackage("..mapper..") + .because("mappers are grouped in a dedicated mapper package per adapter"); +}