diff --git a/.dockerignore b/.dockerignore index d5c390e..c3dab4b 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,7 +1,6 @@ **/target .git .idea -.mvn *.iml **/node_modules @@ -16,3 +15,9 @@ agiledancers-be/target/* !agiledancers-be/target/quarkus-app **/unit-results + +# CI sources are build inputs; runner credentials and local state are not. +ci/runner/.env +ci/runner/.runner +ci/runner/data +ci/runner/local diff --git a/AGENTS.md b/AGENTS.md index 1a549ca..fb11189 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,3 +12,11 @@ `grilling` automatically to ordinary development requests. - Keep the upstream skill files unchanged. Source revision and license are recorded in `skills/upstream.json` and `skills/LICENSE.mattpocock`. + +# Backend changes and reviews + +- Before changing backend Java, read `CODING_STANDARDS.md`. +- For reviews, use `skills/code-review/SKILL.md` together with + `docs/backend-review.md`. Keep upstream skills unchanged. +- Obtain maintainer agreement before adding any standards waiver; record its + narrow scope and reason alongside the affected code or configuration. diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md new file mode 100644 index 0000000..27ba6ec --- /dev/null +++ b/CODING_STANDARDS.md @@ -0,0 +1,255 @@ +# Backend coding standards + +This document is the authoritative policy for handwritten production and test Java +in `agiledancers-be`. Generated code and Java helpers outside that module are out +of scope. Existing frontend conventions in README.md and agiledancers-fe remain in force. + +## Scope and authority + +- Cover handwritten production and test Java in `agiledancers-be`. Exclude + generated code. Dependency injection and production architecture rules apply + where relevant. Frontend and Java helpers outside the backend are out of scope. +- [Java coding guidelines](https://www.geeksforgeeks.org/java/coding-guidelines-in-java/) + supplied source material. Only the rules adopted in this local document apply; + changes to the article do not change project policy. +- The project's pinned Palantir formatter wins formatting conflicts, including + comment layout. S8 remains responsible for formatter integration. +- Do not adopt unrelated static-analysis rule bundles. + +## Java conventions + +- Types use UpperCamelCase; methods and variables use lowerCamelCase; constants + use UPPER_SNAKE_CASE; package names are lowercase. +- One top-level type per file; explicit member visibility; `@Override` on + overriding methods. Do not invent visibility modifiers where Java forbids them. +- Opening braces stay on the declaration/control-statement line. All `if`, `for` + and `while` bodies require braces, including enhanced for and do/while bodies. +- No wildcard imports. Static imports are allowed only in test classes. +- Comments explain intent or constraints. Document public contracts with Javadoc + where their meaning is not clear from the declaration; do not require boilerplate + documentation for every accessor. +- Block comments use the following layout, subject to Palantir formatting: + + ```java + /* + This is an example of a block comment. + It may have more than one line. + */ + ``` +- Prefer records for immutable data carriers. Classes are allowed when mutation, + framework requirements or inheritance make records unsuitable. Preserve API and + serialization behavior. Record immutability is shallow: protect mutable + components appropriately. +- Require `final` for parameters, local variables and fields wherever they can + remain assigned once, including loop variables and catch parameters where Java + permits it. Field finality must remain compatible with framework injection and + lifecycle requirements. Permit reassignment when the algorithm needs it; do not + introduce artificial holder objects to avoid reassignment. This rule does not + require final classes or methods. +- Use explicit types unless the initializer visibly identifies the type. `var` is + allowed with `new School(...)` or an explicit cast, but not for method results, + literals or other inferred expressions. For example, + `final var school = new School(...)` is allowed, while a repository result uses + `final School school = repository.find(id)`. Borderline cases require review. +- No additional member-ordering or line-width policy beyond Palantir. + +## Metrics + +Targets guide development. Review signals require assessment, not automatic +failure. Blocking limits fail verification. Strict inequalities matter: a value +exactly at a blocking limit passes. + +| Metric | Target | Review signal | Blocking limit | +| --- | ---: | ---: | ---: | +| Cyclomatic complexity per method/constructor | ≤ 5 | > 7 | > 10 | +| Cognitive complexity per method/constructor | ≤ 10 | > 10 | > 15 | +| Method/constructor length, code lines | ≤ 20 | > 30 | > 50 | +| Parameters per method | ≤ 3 | > 4 | > 5 | +| Parameters per explicitly written constructor | ≤ 4 | > 5 | > 7 | +| Injected dependencies per class | ≤ 4 | > 5 | > 7 | +| Fan-out per class, unique used types | ≤ 10 | > 15 | Review only | +| Own superclass depth, excluding Object | ≤ 1 | > 2 | > 3 | +| Control-flow nesting | ≤ 2 | > 3 | > 4 | +| Production duplicated lines density | ≤ 1% | > 1% | > 3% | +| Class length, code lines | ≤ 200 | > 300 | > 500 | +| Declared public methods per class, excluding trivial accessors | ≤ 10 | > 15 | Review only | + +### Measurement definitions + +- Use the pinned PMD algorithms as authoritative for cyclomatic and cognitive + complexity. Document their exact configuration, including switch and boolean + treatment. Do not accidentally introduce a class-level aggregate complexity gate. +- Measure each method, constructor and explicitly written record constructor + independently. Lambda bodies contribute to their enclosing executable; + methods in nested/local classes are separate executables. +- Parameter limits exclude implicit record constructors and generated members. + Record component counts remain a design review concern. +- Length means physical source lines after Palantir formatting, excluding blank + and comment-only lines. Declaration and brace-only lines count; a line containing + both code and comments counts. Do not substitute statement counts or NCSS. + Measure nested types separately and exclude them from enclosing class length. +- Count distinct injected collaborators by resolved type and qualifier. Include + constructor, field, setter, inherited and implicit constructor injection. + Repeated injection of the same type and qualifier counts once; differently + qualified instances of the same type count separately. +- Fan-out counts distinct explicitly referenced resolved types, including JDK and + framework types. Exclude primitives, the class itself and generated references. +- Inheritance depth counts project-owned superclass layers; exclude Object, + interfaces and external framework ancestry. +- Nesting combines `if`, loops, `switch` and `catch`, with the first structure at + depth one. An `else if` chain stays at one level. `else`, `try` and `finally` + add no level themselves. Lambdas retain surrounding depth; methods in nested + classes start fresh. +- Public-method counts include explicitly declared public instance and static + methods; exclude constructors, inherited methods and trivial accessors. A + trivial accessor only reads or assigns its corresponding field, with no + validation, branching or other work. +- Duplication limits apply only to production code. Duplication in tests is allowed + and has no target, review signal or blocking limit. +- Detect production clones with CPD, minimum 100 tokens, without ignoring + identifier or literal differences. Density is + `100 × distinct duplicated code-line locations / all production code-line locations`. + Exclude blank/comment-only lines from numerator and denominator. Count every + occurrence, including the first; merge overlapping ranges to count each source + line once. This project metric need not equal a future Sonar metric. + +## Enforcement and developer workflow + +- Use explicitly pinned Java-25-compatible Checkstyle and PMD/CPD versions. + Retain ArchUnit for architecture checks and add small targeted custom checks + where standard rules cannot accurately express the agreed semantics. +- Automate every blocking metric and explicit rule where reliable checking is + available. Identify rules requiring manual review explicitly; do not claim + they are automated. Review signals must appear in reports even when builds pass. +- Run standards checks during `mvn verify`; provide documented standalone commands. + Ordinary compile/test commands retain the current S8 formatting behavior. +- Standards checks report violations without rewriting code. CI passes these + checks before image publication. Parser failures or unsupported Java constructs + fail with the affected path and actionable diagnostics, rather than being skipped. +- Add a root AGENTS.md instruction requiring agents to read CODING_STANDARDS.md + before changing backend Java. Project review instructions must explicitly cover + manual rules, review signals in changed code and approved waivers, without + repeating checks already enforced by tooling. +- Keep upstream skill files unchanged. Put project-specific code-review guidance + in a companion file referenced by project instructions and used alongside + `skills/code-review/SKILL.md`. +- Waivers must be explicit, narrowly scoped and carry a recorded reason alongside + code or configuration. Agents need maintainer agreement before introducing one. + No blanket suppressions, exclusions of difficult files or weakened thresholds + merely to make verification pass. + +## Coverage + +- Require at least 80% backend line coverage across handwritten production code + for the module as a whole, not separately per class or package. Compare the + unrounded percentage against 80%. +- Local `mvn verify` must reach 80% using backend unit/integration tests alone. + This intentionally tightens the original combined-tests-only requirement. +- CI additionally instruments the backend exercised by Playwright, merges that + coverage with backend unit/integration coverage and gates the combined result. + The Playwright contribution need not separately reach 80%. +- Exclude generated code only; do not broadly exclude DTOs, configuration or + difficult code. Publish line and branch coverage reports; branch coverage has + no agreed blocking percentage. +- PIT measures mutation-test effectiveness, not line coverage, and is outside + S7's required implementation. Configuring Sonar in the pipeline remains out of + scope for S14. FindBugs/SpotBugs is not an additional required check suite. + + +## Implemented checks and manual review + +Checkstyle **12.3.1**, PMD/CPD **7.28.0**, and JaCoCo **0.8.14** are pinned. +`tooling/standards/checkstyle.xml` selects individual naming, structure, import and brace checks; no general analysis bundle is enabled. +`tooling/standards/src/main/java/Standards.java` supplies the targeted measurements. +ArchUnit remains in the backend test suite for architectural boundaries. + +PMD uses Java 25, the running JDK's `jrt-fs.jar`, compiled backend classes and the +resolved test dependency classpath. Cyclomatic complexity uses PMD's default +options: boolean paths count, assertions do not. Decisions include if/loops, +catch, ternary and throw; non-default switch alternatives count individually, +including comma-separated labels. There is no aggregate class-complexity gate. +Cognitive complexity uses PMD's boolean-operation sequences, nesting, switch, +recursion and flow rules. The scoped visitors retain those algorithms while +including lambdas in their enclosing executable and excluding nested/local type +bodies, whose methods are measured independently. Compact record constructors +use the same visitors and start with cyclomatic complexity one. + +Custom checks measure physical code lines from regular parser tokens, including +brace/declaration lines and mixed code/comment lines. They count multiline literal +lines while excluding physically blank lines. Class length excludes nested-type +tokens, preserving lines shared with enclosing code. Other custom measurements +implement the definitions above: nesting, resolved type fan-out, project-owned +superclass depth, public non-accessor methods, and injected collaborators. +Injection includes inherited fields/setters, explicit `@Inject` constructors, +single constructors on bean classes, records' implicit canonical constructors, +and Lombok required/all-argument constructor injection. Qualifier keys use resolved +annotation names and values, including defaults; repeat sites are deduplicated. +The CPD configuration preserves identifiers/literals, uses 100 tokens and disables +skip blocks. Its line-location union counts overlapping occurrences only once. + +Checkstyle and targeted checks enforce import/brace/naming/visibility rules, +`@Override`, visible constructor/cast/new-array `var` initializer forms, final parameters, locals (including +loops/catches and lambdas) and initialized fields. PMD's three selected finality +rules additionally cover control-flow assignment and private constructor-initialized +fields. Framework/lifecycle requirements, generated Lombok setters, and external +writes to public/protected fields need review before changing field finality. +No tool can infer whether a comment explains intent, whether a public contract +needs Javadoc, whether a record is appropriate, or whether mutable components are +sufficiently protected. These are **manual rules**, as are ambiguous type inference +and architecture/DI design decisions. Review all `DESIGN` record-component entries +and `REVIEW` signals, even when a build passes, using `docs/backend-review.md`. + +Reports never rewrite Java. Formatter application remains S8's separate validate +execution. Generated output under `target` and generated source directories are +outside the handwritten `src/main/java` and `src/test/java` scope. No maintained +file, DTO, configuration class or package is excluded from standards or coverage. +JaCoCo filters compiler/Lombok-generated members; no handwritten-code exclusions +are configured. Line and branch results are published, with only lines gated. + +Exact metric waivers live in `tooling/standards/waivers.tsv`; each records a metric, +fully resolved executable signature (or class), exact retained value and the +maintainer agreement/reason. No wildcard waivers are allowed, and a changed value +fails again. A waived blocking result remains visible as `WAIVER`. PMD suppression +annotations/comments cannot silently waive a rule. The only approved waiver is the eight-parameter `Group` canonical constructor, +which preserves its JSON contract while defensively copying both lists. Adding a waiver always requires maintainer agreement. + +## Commands and reports + +Run from the root with JDK 25: + +```sh +mvn -Dformat.mode=check clean verify +# Prepare standalone analysis without running tests: +mvn -pl agiledancers-be test-compile dependency:build-classpath@standards-classpath +mvn -f tooling/standards/pom.xml test exec:java +# Exact local coverage gate after a test/report build: +java tooling/coverage/Coverage.java agiledancers-be/target/site/jacoco/jacoco.xml +``` + +`verify` runs backend tests, builds resolved analysis inputs, generates JaCoCo +reports, runs checker regression tests and standards analysis, then gates exact +module-wide line coverage. `compile` and `test` retain existing formatting behavior +and do not run the verify-only standards checks. Standalone analysis requires its +classpath preparation; unresolved types fail with the path and preparation advice. + +Full metrics and findings: `agiledancers-be/target/standards/report.txt`. +Checker regression results: `tooling/standards/target/surefire-reports`. +Coverage HTML/XML/CSV: `agiledancers-be/target/site/jacoco` (local backend tests) +and `agiledancers-be/target/site/jacoco-combined` (merged browser contribution). +The gate compares integer line counts (`5 * covered >= 4 * total`), rejecting an +empty report and percentages that would round up to 80%. + +CI first passes local verify. It mounts the pinned JaCoCo agent into its disposable +backend test container, exposes the agent only on the isolated Docker network, +and uses the pinned CLI to dump `target/playwright.exec` after Playwright. A missing +or empty dump fails. The following commands merge both datasets and gate the result +before the verified-commit marker can authorize publication: + +```sh +mvn -pl agiledancers-be jacoco:merge@combined jacoco:report@combined-report exec:exec@combined-coverage +``` + +CI retains both coverage reports, execution data, standards findings and regression +results with its usual diagnostics artifacts. The agent is mounted only for tests; +published images contain no agent. PIT and Sonar configuration remain outside S7. +See `specs/S7-validation.md` for migration evidence and retained-signal assessment. diff --git a/Dockerfile b/Dockerfile index 9f31df8..18de3e0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,9 @@ FROM maven:3.9-eclipse-temurin-25 AS build WORKDIR /workspace COPY pom.xml ./ +COPY .mvn .mvn +COPY tooling tooling +COPY ci ci COPY agiledancers-be/pom.xml agiledancers-be/pom.xml COPY agiledancers-be/src agiledancers-be/src RUN mvn -B -ntp clean verify diff --git a/README.md b/README.md index 928c2a4..c83ec3e 100644 --- a/README.md +++ b/README.md @@ -308,3 +308,12 @@ UTF-8, import, cache and idempotence checks with `python3 tooling/format/test.py`, and the sequential end-to-end build measurements with `python3 tooling/format/benchmark.py` (requires the ordinary backend test prerequisites). Results are written under `target/`. + +## Backend coding standards (S7) + +Read [CODING_STANDARDS.md](CODING_STANDARDS.md) before changing backend Java. +It defines the agreed rules, exact metric limits, manual review duties, pinned +analysis tools, coverage policy and standalone commands. `mvn verify` runs standards +checks and requires at least 80% module-wide backend line coverage. CI also collects +and merges coverage from Playwright before publication. Reports are retained with +CI diagnostics. See [S7 validation](specs/S7-validation.md) for current migration status. diff --git a/agiledancers-be/pom.xml b/agiledancers-be/pom.xml index d6ae746..6d850c0 100644 --- a/agiledancers-be/pom.xml +++ b/agiledancers-be/pom.xml @@ -32,6 +32,7 @@ + io.quarkusquarkus-jacocotest com.tngtech.archunit archunit-junit5 @@ -62,6 +63,60 @@ + + org.apache.maven.pluginsmaven-dependency-plugin3.8.1 + standards-classpathprepare-packagebuild-classpath + ${project.build.directory}/standards-classpath.txt + + coverage-toolsprepare-packagecopy + + org.jacocoorg.jacoco.agent0.8.14runtimeagent.jar + org.jacocoorg.jacoco.cli0.8.14nodepscli.jar + ${project.build.directory}/coverage-tools + + + + org.codehaus.mojoexec-maven-plugin3.6.3 + standardsverifyexec + + ${maven.home}/bin/mvn + ${maven.multiModuleProjectDirectory} + -B-ntp-f + ${maven.multiModuleProjectDirectory}/tooling/standards/pom.xml + testexec:java + + + + coverageverifyexec + ${java.home}/bin/java + ${maven.multiModuleProjectDirectory}/tooling/coverage/Coverage.java + ${project.build.directory}/site/jacoco/jacoco.xml + + + combined-coveragenoneexec + ${java.home}/bin/java + ${maven.multiModuleProjectDirectory}/tooling/coverage/Coverage.java + ${project.build.directory}/site/jacoco-combined/jacoco.xml + + + + + org.jacocojacoco-maven-plugin0.8.14 + + agentprepare-agent + *QuarkusClassLoader${project.build.directory}/jacoco.exectrue + + reportpost-integration-testreport + combinednonemerge + ${project.build.directory}jacoco.execplaywright.exec + ${project.build.directory}/jacoco-combined.exec + + combined-reportnonereport + ${project.build.directory}/jacoco-combined.exec + ${project.build.directory}/site/jacoco-combined + + + io.quarkus quarkus-maven-plugin diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/AbstractEntity.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/AbstractEntity.java index 0e0e3f9..9e4ff8d 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/AbstractEntity.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/AbstractEntity.java @@ -3,7 +3,11 @@ package nl.agilecc.agiledancers.model; import java.util.Collections; import java.util.HashSet; import java.util.Set; -import lombok.*; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; @Getter @Setter @@ -13,15 +17,15 @@ import lombok.*; public abstract class AbstractEntity { private Long id; - protected void addNullSafe(Set set, T t) { + protected void addNullSafe(final Set set, final T t) { emptyIfNull(set).add(t); } - private Set emptyIfNull(Set set) { + private Set emptyIfNull(final Set set) { return set == null ? new HashSet<>() : set; } - protected Set unmodifiableNullSafe(Set set) { + protected Set unmodifiableNullSafe(final Set set) { return set == null ? Collections.emptySet() : Collections.unmodifiableSet(set); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Coach.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Coach.java index 326b14c..6abe811 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Coach.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Coach.java @@ -1,6 +1,10 @@ package nl.agilecc.agiledancers.model; -import lombok.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.ToString; @Getter @AllArgsConstructor(access = AccessLevel.PRIVATE) diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceGroup.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceGroup.java index ec15a75..94be1d4 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceGroup.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceGroup.java @@ -2,7 +2,12 @@ package nl.agilecc.agiledancers.model; import java.util.LinkedHashSet; import java.util.Set; -import lombok.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; @@ -42,7 +47,7 @@ public class DanceGroup extends AbstractEntity { return super.unmodifiableNullSafe(dancers); } - public void addDancer(Dancer dancer) { + public void addDancer(final Dancer dancer) { super.addNullSafe(dancers, dancer); } @@ -50,15 +55,15 @@ public class DanceGroup extends AbstractEntity { return super.unmodifiableNullSafe(styles); } - public void addStyle(DanceGroupStyle style) { + public void addStyle(final DanceGroupStyle style) { super.addNullSafe(styles, style); } - public void removeDancer(Dancer dancer) { + public void removeDancer(final Dancer dancer) { dancers.remove(dancer); } - public void replaceStyles(Set styles) { + public void replaceStyles(final Set styles) { this.styles.clear(); this.styles.addAll(styles); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceSchool.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceSchool.java index 1408c03..d2d3ca1 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceSchool.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceSchool.java @@ -2,7 +2,12 @@ package nl.agilecc.agiledancers.model; import java.util.LinkedHashSet; import java.util.Set; -import lombok.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; @Getter @AllArgsConstructor(access = AccessLevel.PRIVATE) @@ -43,31 +48,31 @@ public class DanceSchool extends AbstractEntity { return super.unmodifiableNullSafe(coaches); } - public void addDancer(Dancer dancer) { + public void addDancer(final Dancer dancer) { super.addNullSafe(dancers, dancer); } - public void addDanceGroup(DanceGroup danceGroup) { + public void addDanceGroup(final DanceGroup danceGroup) { super.addNullSafe(danceGroups, danceGroup); } - public void addShow(Show show) { + public void addShow(final Show show) { super.addNullSafe(shows, show); } - public void addCoach(Coach coach) { + public void addCoach(final Coach coach) { super.addNullSafe(coaches, coach); } - public void removeDancer(Dancer entity) { + public void removeDancer(final Dancer entity) { dancers.remove(entity); } - public void removeDanceGroup(DanceGroup entity) { + public void removeDanceGroup(final DanceGroup entity) { danceGroups.remove(entity); } - public void removeShow(Show entity) { + public void removeShow(final Show entity) { shows.remove(entity); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Dancer.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Dancer.java index a57ccae..9ada576 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Dancer.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Dancer.java @@ -2,7 +2,12 @@ package nl.agilecc.agiledancers.model; import java.time.LocalDate; import java.time.Period; -import lombok.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; @Getter @AllArgsConstructor(access = AccessLevel.PRIVATE) diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DayPart.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DayPart.java index 2b58c58..9a280fc 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DayPart.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DayPart.java @@ -2,7 +2,12 @@ package nl.agilecc.agiledancers.model; import java.time.LocalDate; import java.util.List; -import lombok.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; import nl.agilecc.agiledancers.model.specifications.DayPartType; @Getter @@ -19,7 +24,9 @@ public class DayPart extends AbstractEntity { @Setter private List danceGroups; - public void removeDanceGroup(DanceGroup group) { - danceGroups = danceGroups.stream().filter(value -> value != group).toList(); + public void removeDanceGroup(final DanceGroup group) { + danceGroups = danceGroups.stream() + .filter((final DanceGroup value) -> value != group) + .toList(); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Show.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Show.java index a4a53e2..d931b0f 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Show.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/Show.java @@ -2,7 +2,12 @@ package nl.agilecc.agiledancers.model; import java.util.List; import java.util.Map; -import lombok.*; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; @Getter diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/SchedulingResult.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/SchedulingResult.java index f99ecc7..b69d016 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/SchedulingResult.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/SchedulingResult.java @@ -1,5 +1,16 @@ package nl.agilecc.agiledancers.model.scheduling; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; -public record SchedulingResult(boolean optimal, List> assignments) {} +public record SchedulingResult(boolean optimal, List> assignments) { + public SchedulingResult { + assignments = assignments == null + ? null + : assignments.stream() + .map((final List part) -> + part == null ? null : Collections.unmodifiableList(new ArrayList<>(part))) + .toList(); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/repository/AbstractRepository.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/repository/AbstractRepository.java index d200964..5da8a3c 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/repository/AbstractRepository.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/repository/AbstractRepository.java @@ -1,18 +1,20 @@ package nl.agilecc.agiledancers.repository; -import java.util.*; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import nl.agilecc.agiledancers.model.AbstractEntity; public abstract class AbstractRepository { private long sequence = 0L; - protected Map entities = new LinkedHashMap<>(); + protected final Map entities = new LinkedHashMap<>(); public long nextId() { return sequence++; } - public T add(T entity) { + public T add(final T entity) { if (entity.getId() == null) { entity.setId(nextId()); } @@ -24,11 +26,11 @@ public abstract class AbstractRepository { return List.copyOf(entities.values()); } - public T findById(long id) { + public T findById(final long id) { return entities.get(id); } - public void delete(long id) { + public void delete(final long id) { entities.remove(id); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceGroupResource.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceGroupResource.java index 44373a4..c79f423 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceGroupResource.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceGroupResource.java @@ -3,22 +3,32 @@ package nl.agilecc.agiledancers.rest; import jakarta.inject.Inject; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import jakarta.ws.rs.*; -import jakarta.ws.rs.core.*; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriInfo; import java.util.List; +import nl.agilecc.agiledancers.model.DanceGroup; import nl.agilecc.agiledancers.rest.apimodel.GroupRequest; import nl.agilecc.agiledancers.rest.apimodel.GroupView; import nl.agilecc.agiledancers.service.SchoolManagementService; +import nl.agilecc.agiledancers.service.model.GroupDetails; @Path("/api/dance-schools/{schoolId}/dance-groups") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) public class DanceGroupResource { - @Inject - SchoolManagementService service; + private @Inject SchoolManagementService service; - @PathParam("schoolId") - long schoolId; + private @PathParam("schoolId") long schoolId; @GET public List list() { @@ -27,14 +37,16 @@ public class DanceGroupResource { @GET @Path("/{id}") - public GroupView get(@PathParam("id") long id) { + public GroupView get(final @PathParam("id") long id) { return GroupView.of(service.group(schoolId, id)); } @POST - public Response create(@NotNull @Valid GroupRequest request, @Context UriInfo uriInfo) { - var entity = service.createGroup( - schoolId, request.category(), request.type(), request.startAge(), request.endAge(), request.styles()); + public Response create(final @NotNull @Valid GroupRequest request, final @Context UriInfo uriInfo) { + final DanceGroup entity = service.createGroup( + schoolId, + new GroupDetails( + request.category(), request.type(), request.startAge(), request.endAge(), request.styles())); return Response.created(uriInfo.getAbsolutePathBuilder() .path(entity.getId().toString()) .build()) @@ -44,34 +56,31 @@ public class DanceGroupResource { @PUT @Path("/{id}") - public GroupView update(@PathParam("id") long id, @NotNull @Valid GroupRequest request) { + public GroupView update(final @PathParam("id") long id, final @NotNull @Valid GroupRequest request) { return GroupView.of(service.updateGroup( schoolId, id, - request.category(), - request.type(), - request.startAge(), - request.endAge(), - request.styles())); + new GroupDetails( + request.category(), request.type(), request.startAge(), request.endAge(), request.styles()))); } @DELETE @Path("/{id}") - public Response delete(@PathParam("id") long id) { + public Response delete(final @PathParam("id") long id) { service.deleteGroup(schoolId, id); return Response.noContent().build(); } @PUT @Path("/{id}/dancers/{dancerId}") - public Response addDancer(@PathParam("id") long id, @PathParam("dancerId") long dancerId) { + public Response addDancer(final @PathParam("id") long id, final @PathParam("dancerId") long dancerId) { service.addGroupDancer(schoolId, id, dancerId); return Response.noContent().build(); } @DELETE @Path("/{id}/dancers/{dancerId}") - public Response removeDancer(@PathParam("id") long id, @PathParam("dancerId") long dancerId) { + public Response removeDancer(final @PathParam("id") long id, final @PathParam("dancerId") long dancerId) { service.removeGroupDancer(schoolId, id, dancerId); return Response.noContent().build(); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceSchoolResource.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceSchoolResource.java index 5593f00..b69fea4 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceSchoolResource.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceSchoolResource.java @@ -3,9 +3,20 @@ package nl.agilecc.agiledancers.rest; import jakarta.inject.Inject; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import jakarta.ws.rs.*; -import jakarta.ws.rs.core.*; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriInfo; import java.util.List; +import nl.agilecc.agiledancers.model.DanceSchool; import nl.agilecc.agiledancers.rest.apimodel.SchoolRequest; import nl.agilecc.agiledancers.rest.apimodel.SchoolView; import nl.agilecc.agiledancers.service.SchoolManagementService; @@ -14,8 +25,7 @@ import nl.agilecc.agiledancers.service.SchoolManagementService; @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) public class DanceSchoolResource { - @Inject - SchoolManagementService service; + private @Inject SchoolManagementService service; @GET public List list() { @@ -24,13 +34,13 @@ public class DanceSchoolResource { @GET @Path("/{id}") - public SchoolView get(@PathParam("id") long id) { + public SchoolView get(final @PathParam("id") long id) { return SchoolView.of(service.school(id)); } @POST - public Response create(@NotNull @Valid SchoolRequest request, @Context UriInfo uriInfo) { - var entity = service.createSchool(request.name()); + public Response create(final @NotNull @Valid SchoolRequest request, final @Context UriInfo uriInfo) { + final DanceSchool entity = service.createSchool(request.name()); return Response.created(uriInfo.getAbsolutePathBuilder() .path(entity.getId().toString()) .build()) @@ -40,13 +50,13 @@ public class DanceSchoolResource { @PUT @Path("/{id}") - public SchoolView update(@PathParam("id") long id, @NotNull @Valid SchoolRequest request) { + public SchoolView update(final @PathParam("id") long id, final @NotNull @Valid SchoolRequest request) { return SchoolView.of(service.updateSchool(id, request.name())); } @DELETE @Path("/{id}") - public Response delete(@PathParam("id") long id) { + public Response delete(final @PathParam("id") long id) { service.deleteSchool(id); return Response.noContent().build(); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DancerResource.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DancerResource.java index 7c03ee3..9d34588 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DancerResource.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DancerResource.java @@ -3,9 +3,20 @@ package nl.agilecc.agiledancers.rest; import jakarta.inject.Inject; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import jakarta.ws.rs.*; -import jakarta.ws.rs.core.*; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriInfo; import java.util.List; +import nl.agilecc.agiledancers.model.Dancer; import nl.agilecc.agiledancers.rest.apimodel.DancerRequest; import nl.agilecc.agiledancers.rest.apimodel.DancerView; import nl.agilecc.agiledancers.service.SchoolManagementService; @@ -14,11 +25,9 @@ import nl.agilecc.agiledancers.service.SchoolManagementService; @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) public class DancerResource { - @Inject - SchoolManagementService service; + private @Inject SchoolManagementService service; - @PathParam("schoolId") - long schoolId; + private @PathParam("schoolId") long schoolId; @GET public List list() { @@ -27,13 +36,13 @@ public class DancerResource { @GET @Path("/{id}") - public DancerView get(@PathParam("id") long id) { + public DancerView get(final @PathParam("id") long id) { return DancerView.of(service.dancer(schoolId, id)); } @POST - public Response create(@NotNull @Valid DancerRequest request, @Context UriInfo uriInfo) { - var entity = service.createDancer(schoolId, request.name(), request.birthDate()); + public Response create(final @NotNull @Valid DancerRequest request, final @Context UriInfo uriInfo) { + final Dancer entity = service.createDancer(schoolId, request.name(), request.birthDate()); return Response.created(uriInfo.getAbsolutePathBuilder() .path(entity.getId().toString()) .build()) @@ -43,13 +52,13 @@ public class DancerResource { @PUT @Path("/{id}") - public DancerView update(@PathParam("id") long id, @NotNull @Valid DancerRequest request) { + public DancerView update(final @PathParam("id") long id, final @NotNull @Valid DancerRequest request) { return DancerView.of(service.updateDancer(schoolId, id, request.name(), request.birthDate())); } @DELETE @Path("/{id}") - public Response delete(@PathParam("id") long id) { + public Response delete(final @PathParam("id") long id) { service.deleteDancer(schoolId, id); return Response.noContent().build(); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/JacksonConfiguration.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/JacksonConfiguration.java index 83ee000..2a02edf 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/JacksonConfiguration.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/JacksonConfiguration.java @@ -8,7 +8,7 @@ import jakarta.inject.Singleton; @Singleton public class JacksonConfiguration implements ObjectMapperCustomizer { @Override - public void customize(ObjectMapper mapper) { + public void customize(final ObjectMapper mapper) { mapper.disable(DeserializationFeature.ACCEPT_FLOAT_AS_INT); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ManagementExceptionMapper.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ManagementExceptionMapper.java index a61ee29..89183ec 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ManagementExceptionMapper.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ManagementExceptionMapper.java @@ -10,8 +10,8 @@ import nl.agilecc.agiledancers.service.ManagementException; @Provider public class ManagementExceptionMapper implements ExceptionMapper { @Override - public Response toResponse(ManagementException exception) { - int status = switch (exception.getKind()) { + public Response toResponse(final ManagementException exception) { + final int status = switch (exception.getKind()) { case NOT_FOUND -> 404; case INVALID_INPUT -> 400; case CONFLICT -> 409; diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/SchoolSnapshotResource.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/SchoolSnapshotResource.java index aa512e0..7416e6b 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/SchoolSnapshotResource.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/SchoolSnapshotResource.java @@ -1,27 +1,37 @@ package nl.agilecc.agiledancers.rest; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.inject.Inject; -import jakarta.ws.rs.*; -import jakarta.ws.rs.core.*; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; import nl.agilecc.agiledancers.rest.apimodel.SchoolView; -import nl.agilecc.agiledancers.service.*; -import nl.agilecc.agiledancers.service.model.*; +import nl.agilecc.agiledancers.service.ManagementException; +import nl.agilecc.agiledancers.service.ManagementExceptionKind; +import nl.agilecc.agiledancers.service.SchoolManagementService; +import nl.agilecc.agiledancers.service.model.ImportPreview; +import nl.agilecc.agiledancers.service.model.SchoolSnapshot; @Path("/api/dance-schools") @Produces(MediaType.APPLICATION_JSON) public class SchoolSnapshotResource { - @Inject - SchoolManagementService service; + private @Inject SchoolManagementService service; - @Inject - ObjectMapper mapper; + private @Inject ObjectMapper mapper; @GET @Path("/{id}/export") - public Response export(@PathParam("id") long id) throws Exception { - String json = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(service.exportSchool(id)); + public Response export(final @PathParam("id") long id) throws Exception { + final String json = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(service.exportSchool(id)); return Response.ok(json) .header("Content-Disposition", "attachment; filename=school-" + id + ".json") .build(); @@ -30,29 +40,31 @@ public class SchoolSnapshotResource { @POST @Path("/import/preview") @Consumes(MediaType.APPLICATION_JSON) - public ImportPreview preview(String json) { + public ImportPreview preview(final String json) { return service.preview(parse(json)); } @POST @Path("/import") @Consumes(MediaType.APPLICATION_JSON) - public SchoolView apply(String json, @QueryParam("replaceId") Long replaceId) { + public SchoolView apply(final String json, final @QueryParam("replaceId") Long replaceId) { return SchoolView.of(service.importSchool(parse(json), replaceId)); } - private SchoolSnapshot parse(String json) { + private SchoolSnapshot parse(final String json) { try { - SchoolSnapshot snapshot = mapper.copy() + final SchoolSnapshot snapshot = mapper.copy() .enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) .enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS) .disable(MapperFeature.ALLOW_COERCION_OF_SCALARS) .disable(DeserializationFeature.ACCEPT_FLOAT_AS_INT) .enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION) .readValue(json, SchoolSnapshot.class); - if (snapshot == null) throw new IllegalArgumentException("Het bestand bevat geen school"); + if (snapshot == null) { + throw new IllegalArgumentException("Het bestand bevat geen school"); + } return snapshot; - } catch (Exception e) { + } catch (final Exception e) { throw new ManagementException( ManagementExceptionKind.INVALID_INPUT, "Ongeldig JSON-bestand: " + e.getMessage()); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ShowResource.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ShowResource.java index b082a48..ea4d997 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ShowResource.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ShowResource.java @@ -3,10 +3,23 @@ package nl.agilecc.agiledancers.rest; import jakarta.inject.Inject; import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; -import jakarta.ws.rs.*; -import jakarta.ws.rs.core.*; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.UriInfo; import java.util.List; import nl.agilecc.agiledancers.model.DayPart; +import nl.agilecc.agiledancers.model.Show; +import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; +import nl.agilecc.agiledancers.rest.apimodel.DayPartRequest; import nl.agilecc.agiledancers.rest.apimodel.SchedulingView; import nl.agilecc.agiledancers.rest.apimodel.ShowRequest; import nl.agilecc.agiledancers.rest.apimodel.ShowView; @@ -17,14 +30,11 @@ import nl.agilecc.agiledancers.service.SchoolManagementService; @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) public class ShowResource { - @Inject - SchoolManagementService service; + private @Inject SchoolManagementService service; - @Inject - SchedulingService scheduling; + private @Inject SchedulingService scheduling; - @PathParam("schoolId") - long schoolId; + private @PathParam("schoolId") long schoolId; @GET public List list() { @@ -33,13 +43,13 @@ public class ShowResource { @GET @Path("/{id}") - public ShowView get(@PathParam("id") long id) { + public ShowView get(final @PathParam("id") long id) { return ShowView.of(service.show(schoolId, id)); } @POST - public Response create(@NotNull @Valid ShowRequest request, @Context UriInfo uriInfo) { - var entity = service.createShow(schoolId, request.name(), parts(request), request.appearances()); + public Response create(final @NotNull @Valid ShowRequest request, final @Context UriInfo uriInfo) { + final Show entity = service.createShow(schoolId, request.name(), parts(request), request.appearances()); return Response.created(uriInfo.getAbsolutePathBuilder() .path(entity.getId().toString()) .build()) @@ -49,13 +59,13 @@ public class ShowResource { @PUT @Path("/{id}") - public ShowView update(@PathParam("id") long id, @NotNull @Valid ShowRequest request) { + public ShowView update(final @PathParam("id") long id, final @NotNull @Valid ShowRequest request) { return ShowView.of(service.updateShow(schoolId, id, request.name(), parts(request), request.appearances())); } - private static List parts(ShowRequest request) { + private static List parts(final ShowRequest request) { return request.dayParts().stream() - .map(p -> DayPart.builder() + .map((final DayPartRequest p) -> DayPart.builder() .date(p.date()) .type(p.type()) .danceGroups(List.of()) @@ -66,15 +76,15 @@ public class ShowResource { @POST @Path("/{id}/schedule") @Consumes(MediaType.WILDCARD) - public SchedulingView schedule(@PathParam("id") long id) { - var result = scheduling.schedule(schoolId, id); + public SchedulingView schedule(final @PathParam("id") long id) { + final SchedulingResult result = scheduling.schedule(schoolId, id); return new SchedulingView( result.optimal(), result.optimal() ? "Optimal balance" : "Valid result; optimal balance not proven"); } @DELETE @Path("/{id}") - public Response delete(@PathParam("id") long id) { + public Response delete(final @PathParam("id") long id) { service.deleteShow(schoolId, id); return Response.noContent().build(); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DancerView.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DancerView.java index 417aa5f..0631a37 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DancerView.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DancerView.java @@ -4,7 +4,7 @@ import java.time.LocalDate; import nl.agilecc.agiledancers.model.Dancer; public record DancerView(long id, String name, LocalDate birthDate) { - public static DancerView of(Dancer dancer) { + public static DancerView of(final Dancer dancer) { return new DancerView(dancer.getId(), dancer.getName(), dancer.getBirthDate()); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DayPartView.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DayPartView.java index 45379cf..afae00d 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DayPartView.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DayPartView.java @@ -1,7 +1,13 @@ package nl.agilecc.agiledancers.rest.apimodel; import java.time.LocalDate; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import nl.agilecc.agiledancers.model.specifications.DayPartType; -public record DayPartView(LocalDate date, DayPartType type, List danceGroupIds) {} +public record DayPartView(LocalDate date, DayPartType type, List danceGroupIds) { + public DayPartView { + danceGroupIds = danceGroupIds == null ? null : Collections.unmodifiableList(new ArrayList<>(danceGroupIds)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupRequest.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupRequest.java index 794851d..1258a36 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupRequest.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupRequest.java @@ -3,6 +3,8 @@ package nl.agilecc.agiledancers.rest.apimodel; import jakarta.validation.constraints.Min; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.Set; import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; @@ -12,4 +14,8 @@ public record GroupRequest( @NotNull DanceGroupType type, @NotNull @Min(0) Integer startAge, @NotNull @Min(0) Integer endAge, - @NotNull Set<@NotNull DanceGroupStyle> styles) {} + @NotNull Set<@NotNull DanceGroupStyle> styles) { + public GroupRequest { + styles = styles == null ? null : Collections.unmodifiableSet(new LinkedHashSet<>(styles)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupView.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupView.java index c5e3514..bf5f84d 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupView.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupView.java @@ -1,5 +1,8 @@ package nl.agilecc.agiledancers.rest.apimodel; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import nl.agilecc.agiledancers.model.DanceGroup; @@ -15,7 +18,12 @@ public record GroupView( int endAge, Set styles, List dancerIds) { - public static GroupView of(DanceGroup group) { + public GroupView { + styles = styles == null ? null : Collections.unmodifiableSet(new LinkedHashSet<>(styles)); + dancerIds = dancerIds == null ? null : Collections.unmodifiableList(new ArrayList<>(dancerIds)); + } + + public static GroupView of(final DanceGroup group) { return new GroupView( group.getId(), group.getCategory(), diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchoolView.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchoolView.java index 42c1e87..1f1b5ea 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchoolView.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchoolView.java @@ -1,5 +1,7 @@ package nl.agilecc.agiledancers.rest.apimodel; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import nl.agilecc.agiledancers.model.DanceGroup; import nl.agilecc.agiledancers.model.DanceSchool; @@ -7,7 +9,13 @@ import nl.agilecc.agiledancers.model.Dancer; import nl.agilecc.agiledancers.model.Show; public record SchoolView(long id, String name, List dancerIds, List danceGroupIds, List showIds) { - public static SchoolView of(DanceSchool school) { + public SchoolView { + dancerIds = dancerIds == null ? null : Collections.unmodifiableList(new ArrayList<>(dancerIds)); + danceGroupIds = danceGroupIds == null ? null : Collections.unmodifiableList(new ArrayList<>(danceGroupIds)); + showIds = showIds == null ? null : Collections.unmodifiableList(new ArrayList<>(showIds)); + } + + public static SchoolView of(final DanceSchool school) { return new SchoolView( school.getId(), school.getName(), diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowRequest.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowRequest.java index 1e256fb..26ffbca 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowRequest.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowRequest.java @@ -1,11 +1,21 @@ package nl.agilecc.agiledancers.rest.apimodel; import jakarta.validation.Valid; -import jakarta.validation.constraints.*; -import java.util.*; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotNull; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; public record ShowRequest( @NotBlank String name, @NotNull List<@NotNull @Valid DayPartRequest> dayParts, - Map appearances) {} + Map appearances) { + public ShowRequest { + dayParts = dayParts == null ? null : Collections.unmodifiableList(new ArrayList<>(dayParts)); + appearances = appearances == null ? null : Collections.unmodifiableMap(new LinkedHashMap<>(appearances)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowView.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowView.java index fcf0e2a..bc2feca 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowView.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowView.java @@ -1,16 +1,27 @@ package nl.agilecc.agiledancers.rest.apimodel; -import java.util.*; -import nl.agilecc.agiledancers.model.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.DayPart; +import nl.agilecc.agiledancers.model.Show; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; public record ShowView(long id, String name, List dayParts, Map appearances) { - public static ShowView of(Show show) { + public ShowView { + dayParts = dayParts == null ? null : Collections.unmodifiableList(new ArrayList<>(dayParts)); + appearances = appearances == null ? null : Collections.unmodifiableMap(new LinkedHashMap<>(appearances)); + } + + public static ShowView of(final Show show) { return new ShowView( show.getId(), show.getName(), show.getDayParts().stream() - .map(p -> new DayPartView( + .map((final DayPart p) -> new DayPartView( p.getDate(), p.getType(), p.getDanceGroups().stream() diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/CoachService.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/CoachService.java index 457ae29..7b942c9 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/CoachService.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/CoachService.java @@ -4,9 +4,9 @@ import nl.agilecc.agiledancers.model.Coach; import nl.agilecc.agiledancers.repository.CoachRepository; public class CoachService { - private CoachRepository coachRepository = new CoachRepository(); + private final CoachRepository coachRepository = new CoachRepository(); - public Coach addCoach(String name, Coach coach) { + public Coach addCoach(final String name, final Coach coach) { return coachRepository.add(Coach.builder().name(name).build()); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceGroupService.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceGroupService.java index 1862a13..696a3f8 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceGroupService.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceGroupService.java @@ -1,31 +1,24 @@ package nl.agilecc.agiledancers.service; +import java.util.LinkedHashSet; import java.util.Set; import nl.agilecc.agiledancers.model.Coach; import nl.agilecc.agiledancers.model.DanceGroup; import nl.agilecc.agiledancers.model.Dancer; -import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; -import nl.agilecc.agiledancers.model.specifications.DanceGroupType; import nl.agilecc.agiledancers.repository.DanceGroupRepository; +import nl.agilecc.agiledancers.service.model.GroupDetails; public class DanceGroupService { private final DanceGroupRepository danceGroupRepository = new DanceGroupRepository(); - public DanceGroup add( - String category, - DanceGroupType type, - int startAge, - int endAge, - Set dancers, - Set styles, - Coach coach) { - DanceGroup danceGroup = DanceGroup.builder() - .category(category) - .type(type) - .startAge(startAge) - .endAge(endAge) + public DanceGroup add(final GroupDetails details, final Set dancers, final Coach coach) { + final DanceGroup danceGroup = DanceGroup.builder() + .category(details.category()) + .type(details.type()) + .startAge(details.startAge()) + .endAge(details.endAge()) .dancers(dancers) - .styles(styles) + .styles(details.styles() == null ? null : new LinkedHashSet<>(details.styles())) .coach(coach) .build(); diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceSchoolService.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceSchoolService.java index a300d0d..e509b9a 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceSchoolService.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DanceSchoolService.java @@ -1,29 +1,33 @@ package nl.agilecc.agiledancers.service; -import nl.agilecc.agiledancers.model.*; +import nl.agilecc.agiledancers.model.Coach; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.DanceSchool; +import nl.agilecc.agiledancers.model.Dancer; +import nl.agilecc.agiledancers.model.Show; import nl.agilecc.agiledancers.repository.DanceSchoolRepository; public class DanceSchoolService { private final DanceSchoolRepository danceSchoolRepository = new DanceSchoolRepository(); - public DanceSchool add(String name) { - DanceSchool danceSchool = DanceSchool.builder().name(name).build(); + public DanceSchool add(final String name) { + final DanceSchool danceSchool = DanceSchool.builder().name(name).build(); return danceSchoolRepository.add(danceSchool); } - public void addDancer(Long danceSchoolId, Dancer dancer) { + public void addDancer(final Long danceSchoolId, final Dancer dancer) { danceSchoolRepository.findById(danceSchoolId).addDancer(dancer); } - public void addDanceGroup(Long danceSchoolId, DanceGroup danceGroup) { + public void addDanceGroup(final Long danceSchoolId, final DanceGroup danceGroup) { danceSchoolRepository.findById(danceSchoolId).addDanceGroup(danceGroup); } - public void addCoach(Long danceSchoolId, Coach coach) { + public void addCoach(final Long danceSchoolId, final Coach coach) { danceSchoolRepository.findById(danceSchoolId).addCoach(coach); } - public void addShow(Long danceSchoolId, Show show) { + public void addShow(final Long danceSchoolId, final Show show) { danceSchoolRepository.findById(danceSchoolId).addShow(show); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DancerService.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DancerService.java index ba66ff1..f016b00 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DancerService.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/DancerService.java @@ -7,8 +7,8 @@ import nl.agilecc.agiledancers.repository.DancerRepository; public class DancerService { private final DancerRepository dancerRepository = new DancerRepository(); - public Dancer addDancer(String name, LocalDate birthDate) { - Dancer dancer = Dancer.builder().name(name).birthDate(birthDate).build(); + public Dancer addDancer(final String name, final LocalDate birthDate) { + final Dancer dancer = Dancer.builder().name(name).birthDate(birthDate).build(); return dancerRepository.add(dancer); } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ManagementException.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ManagementException.java index ffa9ecf..304d9aa 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ManagementException.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ManagementException.java @@ -4,7 +4,7 @@ public class ManagementException extends RuntimeException { private final ManagementExceptionKind kind; - public ManagementException(ManagementExceptionKind kind, String message) { + public ManagementException(final ManagementExceptionKind kind, final String message) { super(message); this.kind = kind; } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchedulingService.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchedulingService.java index 81cb17c..e9f784e 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchedulingService.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchedulingService.java @@ -2,26 +2,35 @@ package nl.agilecc.agiledancers.service; import jakarta.inject.Inject; import jakarta.inject.Singleton; +import nl.agilecc.agiledancers.model.scheduling.SchedulingInput; import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; import nl.agilecc.agiledancers.service.port.SchedulingEngine; @Singleton public class SchedulingService { @Inject - SchoolManagementService schools; + private SchoolManagementService schools; @Inject - SchedulingEngine engine; + private SchedulingEngine engine; - public SchedulingResult schedule(long schoolId, long showId) { - var input = schools.beginScheduling(schoolId, showId); + public SchedulingService() {} + + /** Supplies collaborators directly for callers outside the CDI lifecycle. */ + public SchedulingService(final SchoolManagementService schools, final SchedulingEngine engine) { + this.schools = schools; + this.engine = engine; + } + + public SchedulingResult schedule(final long schoolId, final long showId) { + final SchedulingInput input = schools.beginScheduling(schoolId, showId); try { - var result = engine.solve(input); + final SchedulingResult result = engine.solve(input); schools.finishScheduling(schoolId, input, result); return result; - } catch (ManagementException e) { + } catch (final ManagementException e) { throw e; - } catch (RuntimeException | LinkageError e) { + } catch (final RuntimeException | LinkageError e) { throw new ManagementException( ManagementExceptionKind.INVALID_INPUT, "Solver failed. Existing assignments were preserved. Please run scheduling again."); diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java index 8253896..acf1304 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java @@ -1,17 +1,34 @@ package nl.agilecc.agiledancers.service; -import static nl.agilecc.agiledancers.service.ManagementExceptionKind.*; - import jakarta.inject.Singleton; import java.time.LocalDate; -import java.util.*; -import nl.agilecc.agiledancers.model.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import nl.agilecc.agiledancers.model.AbstractEntity; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.DanceSchool; +import nl.agilecc.agiledancers.model.Dancer; +import nl.agilecc.agiledancers.model.DayPart; +import nl.agilecc.agiledancers.model.Show; import nl.agilecc.agiledancers.model.scheduling.GroupRequirement; import nl.agilecc.agiledancers.model.scheduling.SchedulingInput; import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; -import nl.agilecc.agiledancers.model.specifications.*; -import nl.agilecc.agiledancers.repository.*; -import nl.agilecc.agiledancers.service.model.*; +import nl.agilecc.agiledancers.model.specifications.DanceGroupType; +import nl.agilecc.agiledancers.repository.DanceGroupRepository; +import nl.agilecc.agiledancers.repository.DanceSchoolRepository; +import nl.agilecc.agiledancers.repository.DancerRepository; +import nl.agilecc.agiledancers.repository.ShowRepository; +import nl.agilecc.agiledancers.service.model.GroupDetails; +import nl.agilecc.agiledancers.service.model.ImportPreview; +import nl.agilecc.agiledancers.service.model.SchoolSnapshot; /** Owns the in-memory school aggregates; all writes go through this shared service. */ @Singleton @@ -24,18 +41,23 @@ public class SchoolManagementService { private final Map revisions = new HashMap<>(); private final Set running = new HashSet<>(); - private void touch(Show show) { + private void touch(final Show show) { revisions.merge(show.getId(), 1L, Long::sum); } - public synchronized SchedulingInput beginScheduling(long schoolId, long showId) { - Show show = show(schoolId, showId); - if (running.contains(showId)) - throw new ManagementException(CONFLICT, "Scheduling is already running for this show."); - List current = groups(schoolId); - if (current.isEmpty() || show.getDayParts().isEmpty()) + /** Reserves this show's run slot and snapshots its inputs; always release it with endScheduling. */ + public synchronized SchedulingInput beginScheduling(final long schoolId, final long showId) { + final Show show = show(schoolId, showId); + if (running.contains(showId)) { throw new ManagementException( - INVALID_INPUT, "Scheduling requires at least one school group and one daypart."); + ManagementExceptionKind.CONFLICT, "Scheduling is already running for this show."); + } + final List current = groups(schoolId); + if (current.isEmpty() || show.getDayParts().isEmpty()) { + throw new ManagementException( + ManagementExceptionKind.INVALID_INPUT, + "Scheduling requires at least one school group and one daypart."); + } ShowConfiguration.validate(show.getDayParts(), show.getAppearances()); running.add(showId); return new SchedulingInput( @@ -43,42 +65,29 @@ public class SchoolManagementService { revisions.getOrDefault(showId, 0L), show.getDayParts().size(), current.stream() - .map(g -> new GroupRequirement( + .map((final DanceGroup g) -> new GroupRequirement( g.getId(), show.getAppearances().get(g.getType()))) .toList()); } - public synchronized void finishScheduling(long schoolId, SchedulingInput input, SchedulingResult result) { - Show show; + /** Validates the complete result and input revision before replacing any saved assignments. */ + public synchronized void finishScheduling( + final long schoolId, final SchedulingInput input, final SchedulingResult result) { + final Show show; try { show = show(schoolId, input.showId()); - } catch (ManagementException e) { - throw new ManagementException(CONFLICT, "Scheduling inputs changed. Run scheduling again."); - } - if (revisions.getOrDefault(input.showId(), 0L) != input.revision()) - throw new ManagementException(CONFLICT, "Scheduling inputs changed. Run scheduling again."); - // Validate the complete solver output before mutating any assignments. - if (result == null - || result.assignments() == null - || result.assignments().size() != input.dayParts()) + } catch (final ManagementException e) { throw new ManagementException( - INVALID_INPUT, "Solver returned an invalid result. Existing assignments were preserved."); - Map current = new HashMap<>(); - groups(schoolId).forEach(g -> current.put(g.getId(), g)); - for (var part : result.assignments()) { - if (part == null - || new HashSet<>(part).size() != part.size() - || !current.keySet().containsAll(part)) - throw new ManagementException(INVALID_INPUT, "Solver returned invalid group assignments."); + ManagementExceptionKind.CONFLICT, "Scheduling inputs changed. Run scheduling again."); } - for (GroupRequirement group : input.groups()) { - long id = group.groupId(); - if (result.assignments().stream().filter(p -> p.contains(id)).count() != group.appearances()) - throw new ManagementException(INVALID_INPUT, "Solver did not satisfy every required appearance."); + if (revisions.getOrDefault(input.showId(), 0L) != input.revision()) { + throw new ManagementException( + ManagementExceptionKind.CONFLICT, "Scheduling inputs changed. Run scheduling again."); } - List replacement = new ArrayList<>(); + final Map current = validateAssignments(schoolId, input, result); + final List replacement = new ArrayList<>(); for (int p = 0; p < input.dayParts(); p++) { - DayPart old = show.getDayParts().get(p); + final DayPart old = show.getDayParts().get(p); replacement.add(DayPart.builder() .date(old.getDate()) .type(old.getType()) @@ -90,7 +99,43 @@ public class SchoolManagementService { show.setDayParts(List.copyOf(replacement)); } - public synchronized void endScheduling(long showId) { + private Map validateAssignments( + final long schoolId, final SchedulingInput input, final SchedulingResult result) { + // Validate the complete solver output before mutating any assignments. + if (result == null + || result.assignments() == null + || result.assignments().size() != input.dayParts()) { + throw new ManagementException( + ManagementExceptionKind.INVALID_INPUT, + "Solver returned an invalid result. Existing assignments were preserved."); + } + final Map current = new HashMap<>(); + groups(schoolId).forEach((final DanceGroup g) -> current.put(g.getId(), g)); + validateParts(result.assignments(), current.keySet()); + for (final GroupRequirement group : input.groups()) { + final long id = group.groupId(); + if (result.assignments().stream() + .filter((final List p) -> p.contains(id)) + .count() + != group.appearances()) { + throw new ManagementException( + ManagementExceptionKind.INVALID_INPUT, "Solver did not satisfy every required appearance."); + } + } + return current; + } + + private static void validateParts(final List> assignments, final Set groupIds) { + for (final List part : assignments) { + if (part == null || new HashSet<>(part).size() != part.size() || !groupIds.containsAll(part)) { + throw new ManagementException( + ManagementExceptionKind.INVALID_INPUT, "Solver returned invalid group assignments."); + } + } + } + + /** Releases the run slot after success or failure, without changing saved assignments. */ + public synchronized void endScheduling(final long showId) { running.remove(showId); } @@ -98,139 +143,134 @@ public class SchoolManagementService { return schools.findAll(); } - public synchronized DanceSchool school(long schoolId) { + public synchronized DanceSchool school(final long schoolId) { return required(schools.findById(schoolId), "Dance school", schoolId); } - public synchronized DanceSchool createSchool(String name) { + public synchronized DanceSchool createSchool(final String name) { return schools.add(DanceSchool.builder().name(name).build()); } - public synchronized DanceSchool updateSchool(long schoolId, String name) { - DanceSchool school = school(schoolId); + public synchronized DanceSchool updateSchool(final long schoolId, final String name) { + final DanceSchool school = school(schoolId); school.setName(name); return school; } - public synchronized void deleteSchool(long schoolId) { - DanceSchool school = school(schoolId); - school.getDanceGroups().forEach(group -> groups.delete(group.getId())); - school.getDancers().forEach(dancer -> dancers.delete(dancer.getId())); - school.getShows().forEach(show -> shows.delete(show.getId())); + public synchronized void deleteSchool(final long schoolId) { + final DanceSchool school = school(schoolId); + school.getDanceGroups().forEach((final DanceGroup group) -> groups.delete(group.getId())); + school.getDancers().forEach((final Dancer dancer) -> dancers.delete(dancer.getId())); + school.getShows().forEach((final Show show) -> shows.delete(show.getId())); schools.delete(schoolId); } - public synchronized List dancers(long schoolId) { + public synchronized List dancers(final long schoolId) { return List.copyOf(school(schoolId).getDancers()); } - public synchronized Dancer dancer(long schoolId, long dancerId) { + public synchronized Dancer dancer(final long schoolId, final long dancerId) { return member(school(schoolId).getDancers(), dancerId, "Dancer"); } - public synchronized Dancer createDancer(long schoolId, String name, LocalDate birthDate) { - DanceSchool school = school(schoolId); - Dancer dancer = + public synchronized Dancer createDancer(final long schoolId, final String name, final LocalDate birthDate) { + final DanceSchool school = school(schoolId); + final Dancer dancer = dancers.add(Dancer.builder().name(name).birthDate(birthDate).build()); school.addDancer(dancer); return dancer; } - public synchronized Dancer updateDancer(long schoolId, long dancerId, String name, LocalDate birthDate) { - Dancer dancer = dancer(schoolId, dancerId); + public synchronized Dancer updateDancer( + final long schoolId, final long dancerId, final String name, final LocalDate birthDate) { + final Dancer dancer = dancer(schoolId, dancerId); dancer.setName(name); dancer.setBirthDate(birthDate); return dancer; } - public synchronized void deleteDancer(long schoolId, long dancerId) { - DanceSchool school = school(schoolId); - Dancer dancer = dancer(schoolId, dancerId); - school.getDanceGroups().forEach(group -> group.removeDancer(dancer)); + public synchronized void deleteDancer(final long schoolId, final long dancerId) { + final DanceSchool school = school(schoolId); + final Dancer dancer = dancer(schoolId, dancerId); + school.getDanceGroups().forEach((final DanceGroup group) -> group.removeDancer(dancer)); school.removeDancer(dancer); dancers.delete(dancerId); } - public synchronized List groups(long schoolId) { + public synchronized List groups(final long schoolId) { return List.copyOf(school(schoolId).getDanceGroups()); } - public synchronized DanceGroup group(long schoolId, long groupId) { + public synchronized DanceGroup group(final long schoolId, final long groupId) { return member(school(schoolId).getDanceGroups(), groupId, "Dance group"); } - public synchronized DanceGroup createGroup( - long schoolId, - String category, - DanceGroupType type, - int startAge, - int endAge, - Set styles) { - DanceSchool school = school(schoolId); - validateAgeRange(startAge, endAge); - DanceGroup group = groups.add(DanceGroup.builder() - .category(category) - .type(type) - .startAge(startAge) - .endAge(endAge) - .styles(new LinkedHashSet<>(styles)) + public synchronized DanceGroup createGroup(final long schoolId, final GroupDetails details) { + final DanceSchool school = school(schoolId); + validateAgeRange(details.startAge(), details.endAge()); + final DanceGroup group = groups.add(DanceGroup.builder() + .category(details.category()) + .type(details.type()) + .startAge(details.startAge()) + .endAge(details.endAge()) + .styles(new LinkedHashSet<>(details.styles())) .build()); school.addDanceGroup(group); school.getShows().forEach(this::touch); return group; } - public synchronized DanceGroup updateGroup( - long schoolId, - long groupId, - String category, - DanceGroupType type, - int startAge, - int endAge, - Set styles) { - DanceGroup group = group(schoolId, groupId); - validateAgeRange(startAge, endAge); - group.setCategory(category); - if (group.getType() != type) school(schoolId).getShows().forEach(this::touch); - group.setType(type); - group.setStartAge(startAge); - group.setEndAge(endAge); - group.replaceStyles(styles); + public synchronized DanceGroup updateGroup(final long schoolId, final long groupId, final GroupDetails details) { + final DanceGroup group = group(schoolId, groupId); + validateAgeRange(details.startAge(), details.endAge()); + group.setCategory(details.category()); + if (group.getType() != details.type()) { + school(schoolId).getShows().forEach(this::touch); + } + group.setType(details.type()); + group.setStartAge(details.startAge()); + group.setEndAge(details.endAge()); + group.replaceStyles(details.styles()); return group; } - public synchronized void deleteGroup(long schoolId, long groupId) { - DanceGroup group = group(schoolId, groupId); - school(schoolId).getShows().forEach(show -> show.getDayParts().forEach(part -> part.removeDanceGroup(group))); + public synchronized void deleteGroup(final long schoolId, final long groupId) { + final DanceGroup group = group(schoolId, groupId); + school(schoolId) + .getShows() + .forEach((final Show show) -> + show.getDayParts().forEach((final DayPart part) -> part.removeDanceGroup(group))); school(schoolId).getShows().forEach(this::touch); school(schoolId).removeDanceGroup(group); groups.delete(groupId); } - public synchronized void addGroupDancer(long schoolId, long groupId, long dancerId) { - DanceGroup group = group(schoolId, groupId); + public synchronized void addGroupDancer(final long schoolId, final long groupId, final long dancerId) { + final DanceGroup group = group(schoolId, groupId); group.addDancer(dancer(schoolId, dancerId)); } - public synchronized void removeGroupDancer(long schoolId, long groupId, long dancerId) { - DanceGroup group = group(schoolId, groupId); + public synchronized void removeGroupDancer(final long schoolId, final long groupId, final long dancerId) { + final DanceGroup group = group(schoolId, groupId); group.removeDancer(dancer(schoolId, dancerId)); } - public synchronized List shows(long schoolId) { + public synchronized List shows(final long schoolId) { return List.copyOf(school(schoolId).getShows()); } - public synchronized Show show(long schoolId, long showId) { + public synchronized Show show(final long schoolId, final long showId) { return member(school(schoolId).getShows(), showId, "Show"); } public synchronized Show createShow( - long schoolId, String name, List parts, Map counts) { - DanceSchool school = school(schoolId); - if (counts == null) counts = ShowConfiguration.defaults(parts.size()); + final long schoolId, final String name, final List parts, Map counts) { + final DanceSchool school = school(schoolId); + if (counts == null) { + counts = ShowConfiguration.defaults(parts.size()); + } ShowConfiguration.validate(parts, counts); - Show show = shows.add(Show.builder() + final Show show = shows.add(Show.builder() .name(name) .dayParts(List.copyOf(parts)) .appearances(Map.copyOf(counts)) @@ -240,19 +280,25 @@ public class SchoolManagementService { } public synchronized Show updateShow( - long schoolId, long showId, String name, List parts, Map counts) { - Show show = show(schoolId, showId); - if (counts == null) + final long schoolId, + final long showId, + final String name, + final List parts, + Map counts) { + final Show show = show(schoolId, showId); + if (counts == null) { counts = show.getAppearances().isEmpty() ? ShowConfiguration.defaults(parts.size()) : show.getAppearances(); + } ShowConfiguration.validate(parts, counts); - boolean changed = + final boolean changed = !counts.equals(show.getAppearances()) || !partKeys(parts).equals(partKeys(show.getDayParts())); - List retained = parts.stream() - .map(p -> DayPart.builder() + final List retained = parts.stream() + .map((final DayPart p) -> DayPart.builder() .date(p.getDate()) .type(p.getType()) .danceGroups(show.getDayParts().stream() - .filter(old -> old.getDate().equals(p.getDate()) && old.getType() == p.getType()) + .filter((final DayPart old) -> + old.getDate().equals(p.getDate()) && old.getType() == p.getType()) .findFirst() .map(DayPart::getDanceGroups) .orElse(List.of())) @@ -261,28 +307,32 @@ public class SchoolManagementService { show.setName(name); show.setDayParts(retained); show.setAppearances(Map.copyOf(counts)); - if (changed) touch(show); + if (changed) { + touch(show); + } return show; } - private static List partKeys(List parts) { - return parts.stream().map(p -> p.getDate() + ":" + p.getType()).toList(); + private static List partKeys(final List parts) { + return parts.stream() + .map((final DayPart p) -> p.getDate() + ":" + p.getType()) + .toList(); } - public synchronized void deleteShow(long schoolId, long showId) { - Show show = show(schoolId, showId); + public synchronized void deleteShow(final long schoolId, final long showId) { + final Show show = show(schoolId, showId); school(schoolId).removeShow(show); shows.delete(showId); } - public synchronized SchoolSnapshot exportSchool(long id) { + public synchronized SchoolSnapshot exportSchool(final long id) { return SchoolSnapshot.of(school(id)); } - public synchronized ImportPreview preview(SchoolSnapshot snapshot) { + public synchronized ImportPreview preview(final SchoolSnapshot snapshot) { snapshot.restore(); - DanceSchool existing = schools.findAll().stream() - .filter(s -> s.getKey().equals(snapshot.key())) + final DanceSchool existing = schools.findAll().stream() + .filter((final DanceSchool s) -> s.getKey().equals(snapshot.key())) .findFirst() .orElse(null); return new ImportPreview( @@ -294,58 +344,30 @@ public class SchoolManagementService { existing == null ? null : existing.getName()); } - public synchronized DanceSchool importSchool(SchoolSnapshot snapshot, Long replaceId) { - DanceSchool staged = snapshot.restore(); - ImportPreview preview = preview(snapshot); + /** Validates and stages the complete graph before replacing repositories; file IDs become runtime IDs. */ + public synchronized DanceSchool importSchool(final SchoolSnapshot snapshot, final Long replaceId) { + final DanceSchool staged = snapshot.restore(); + final ImportPreview preview = preview(snapshot); if (!Objects.equals(preview.existingId(), replaceId)) { - throw new ManagementException(INVALID_INPUT, "De bestaande school is gewijzigd. Bekijk de import opnieuw."); + throw new ManagementException( + ManagementExceptionKind.INVALID_INPUT, + "De bestaande school is gewijzigd. Bekijk de import opnieuw."); } // All validation and construction finish before changing repository state. // Rebuild sets after assigning runtime IDs: entity hash codes include the ID. - List importedDancers = new ArrayList<>(staged.getDancers()); - List importedGroups = new ArrayList<>(staged.getDanceGroups()); - List importedShows = new ArrayList<>(staged.getShows()); - Map> memberships = new IdentityHashMap<>(); - importedGroups.forEach(g -> memberships.put(g, new ArrayList<>(g.getDancers()))); - importedDancers.forEach(d -> d.setId(dancers.nextId())); - importedGroups.forEach(g -> { + final List importedDancers = new ArrayList<>(staged.getDancers()); + final List importedGroups = new ArrayList<>(staged.getDanceGroups()); + final List importedShows = new ArrayList<>(staged.getShows()); + final Map> memberships = new IdentityHashMap<>(); + importedGroups.forEach((final DanceGroup g) -> memberships.put(g, new ArrayList<>(g.getDancers()))); + importedDancers.forEach((final Dancer d) -> d.setId(dancers.nextId())); + importedGroups.forEach((final DanceGroup g) -> { g.setId(groups.nextId()); }); - List rebuiltGroups = new ArrayList<>(); - Map replacements = new IdentityHashMap<>(); - for (DanceGroup g : importedGroups) { - DanceGroup rebuilt = DanceGroup.builder() - .category(g.getCategory()) - .type(g.getType()) - .startAge(g.getStartAge()) - .endAge(g.getEndAge()) - .styles(new LinkedHashSet<>(g.getStyles())) - .coach(g.getCoach()) - .dancers(new LinkedHashSet<>(memberships.get(g))) - .build(); - rebuilt.setId(g.getId()); - rebuiltGroups.add(rebuilt); - replacements.put(g, rebuilt); - } - List rebuiltShows = new ArrayList<>(); - for (Show show : importedShows) { - Show rebuilt = Show.builder() - .name(show.getName()) - .appearances(show.getAppearances()) - .dayParts(show.getDayParts().stream() - .map(p -> DayPart.builder() - .date(p.getDate()) - .type(p.getType()) - .danceGroups(p.getDanceGroups().stream() - .map(replacements::get) - .toList()) - .build()) - .toList()) - .build(); - rebuilt.setId(shows.nextId()); - rebuiltShows.add(rebuilt); - } - DanceSchool imported = DanceSchool.builder() + final Map replacements = new IdentityHashMap<>(); + final List rebuiltGroups = rebuildGroups(importedGroups, memberships, replacements); + final List rebuiltShows = rebuildShows(importedShows, replacements); + final DanceSchool imported = DanceSchool.builder() .key(staged.getKey()) .name(staged.getName()) .dancers(new LinkedHashSet<>(importedDancers)) @@ -363,22 +385,67 @@ public class SchoolManagementService { return schools.add(imported); } - private static void validateAgeRange(int startAge, int endAge) { + private static List rebuildGroups( + final List importedGroups, + final Map> memberships, + final Map replacements) { + final List rebuiltGroups = new ArrayList<>(); + for (final DanceGroup g : importedGroups) { + final DanceGroup rebuilt = DanceGroup.builder() + .category(g.getCategory()) + .type(g.getType()) + .startAge(g.getStartAge()) + .endAge(g.getEndAge()) + .styles(new LinkedHashSet<>(g.getStyles())) + .coach(g.getCoach()) + .dancers(new LinkedHashSet<>(memberships.get(g))) + .build(); + rebuilt.setId(g.getId()); + rebuiltGroups.add(rebuilt); + replacements.put(g, rebuilt); + } + return rebuiltGroups; + } + + private List rebuildShows(final List importedShows, final Map replacements) { + final List rebuiltShows = new ArrayList<>(); + for (final Show show : importedShows) { + final Show rebuilt = Show.builder() + .name(show.getName()) + .appearances(show.getAppearances()) + .dayParts(show.getDayParts().stream() + .map((final DayPart p) -> DayPart.builder() + .date(p.getDate()) + .type(p.getType()) + .danceGroups(p.getDanceGroups().stream() + .map(replacements::get) + .toList()) + .build()) + .toList()) + .build(); + rebuilt.setId(shows.nextId()); + rebuiltShows.add(rebuilt); + } + return rebuiltShows; + } + + private static void validateAgeRange(final int startAge, final int endAge) { if (startAge > endAge) { - throw new ManagementException(INVALID_INPUT, "startAge must not exceed endAge"); + throw new ManagementException(ManagementExceptionKind.INVALID_INPUT, "startAge must not exceed endAge"); } } - private static T member(Collection entities, long id, String type) { + private static T member(final Collection entities, final long id, final String type) { return entities.stream() - .filter(entity -> entity.getId() == id) + .filter((final T entity) -> entity.getId() == id) .findFirst() - .orElseThrow(() -> new ManagementException(NOT_FOUND, type + " " + id + " not found in this school")); + .orElseThrow(() -> new ManagementException( + ManagementExceptionKind.NOT_FOUND, type + " " + id + " not found in this school")); } - private static T required(T value, String type, long id) { + private static T required(final T value, final String type, final long id) { if (value == null) { - throw new ManagementException(NOT_FOUND, type + " " + id + " not found"); + throw new ManagementException(ManagementExceptionKind.NOT_FOUND, type + " " + id + " not found"); } return value; } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ShowConfiguration.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ShowConfiguration.java index 75db05a..e6f3c5b 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ShowConfiguration.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/ShowConfiguration.java @@ -1,16 +1,19 @@ package nl.agilecc.agiledancers.service; -import java.util.*; +import java.util.Arrays; +import java.util.EnumMap; +import java.util.List; +import java.util.Map; import nl.agilecc.agiledancers.model.DayPart; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; public final class ShowConfiguration { private ShowConfiguration() {} - public static Map defaults(int n) { - Map counts = new EnumMap<>(DanceGroupType.class); - if (n > 0) - for (var type : DanceGroupType.values()) + public static Map defaults(final int n) { + final Map counts = new EnumMap<>(DanceGroupType.class); + if (n > 0) { + for (final DanceGroupType type : DanceGroupType.values()) { counts.put( type, switch (type) { @@ -18,28 +21,41 @@ public final class ShowConfiguration { case SHOW_CASE, PRE_EDUCATION -> n; default -> Math.min(2, n); }); + } + } return counts; } - public static void validate(List parts, Map counts) { + public static void validate(final List parts, final Map counts) { + validateParts(parts); + if (counts == null) { + invalidCounts(); + } + if (parts.isEmpty() && counts.isEmpty()) { + return; + } + if (counts.size() != DanceGroupType.values().length + || Arrays.stream(DanceGroupType.values()) + .anyMatch((final DanceGroupType t) -> counts.get(t) == null + || counts.get(t) < 1 + || (!parts.isEmpty() && counts.get(t) > parts.size()))) { + invalidCounts(); + } + } + + private static void validateParts(final List parts) { if (parts == null || parts.size() > 6 - || parts.stream().anyMatch(p -> p == null || p.getDate() == null || p.getType() == null) + || parts.stream().anyMatch((final DayPart p) -> p == null || p.getDate() == null || p.getType() == null) || parts.stream() - .map(p -> p.getDate() + ":" + p.getType()) + .map((final DayPart p) -> p.getDate() + ":" + p.getType()) .distinct() .count() - != parts.size()) + != parts.size()) { throw new ManagementException( ManagementExceptionKind.INVALID_INPUT, "Configure zero to six dayparts with unique date/type combinations."); - if (counts == null) invalidCounts(); - if (parts.isEmpty() && counts.isEmpty()) return; - if (counts.size() != DanceGroupType.values().length - || Arrays.stream(DanceGroupType.values()) - .anyMatch(t -> counts.get(t) == null - || counts.get(t) < 1 - || (!parts.isEmpty() && counts.get(t) > parts.size()))) invalidCounts(); + } } private static void invalidCounts() { diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Group.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Group.java index 2367e5a..93fd4cd 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Group.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Group.java @@ -1,5 +1,7 @@ package nl.agilecc.agiledancers.service.model; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; @@ -12,4 +14,10 @@ public record Group( Integer endAge, List styles, List dancerIds, - String coachId) {} + String coachId) { + // The maintainer-approved parameter waiver preserves this version-2 JSON contract. + public Group { + styles = styles == null ? null : Collections.unmodifiableList(new ArrayList<>(styles)); + dancerIds = dancerIds == null ? null : Collections.unmodifiableList(new ArrayList<>(dancerIds)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/GroupDetails.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/GroupDetails.java new file mode 100644 index 0000000..d9b1413 --- /dev/null +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/GroupDetails.java @@ -0,0 +1,14 @@ +package nl.agilecc.agiledancers.service.model; + +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Set; +import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; +import nl.agilecc.agiledancers.model.specifications.DanceGroupType; + +public record GroupDetails( + String category, DanceGroupType type, int startAge, int endAge, Set styles) { + public GroupDetails { + styles = styles == null ? null : Collections.unmodifiableSet(new LinkedHashSet<>(styles)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Part.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Part.java index 2068cb0..e9896a0 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Part.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Part.java @@ -1,7 +1,13 @@ package nl.agilecc.agiledancers.service.model; import java.time.LocalDate; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import nl.agilecc.agiledancers.model.specifications.DayPartType; -public record Part(LocalDate date, DayPartType type, List groupIds) {} +public record Part(LocalDate date, DayPartType type, List groupIds) { + public Part { + groupIds = groupIds == null ? null : Collections.unmodifiableList(new ArrayList<>(groupIds)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Performance.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Performance.java index 0cd8641..66fcb39 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Performance.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Performance.java @@ -1,7 +1,15 @@ package nl.agilecc.agiledancers.service.model; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import nl.agilecc.agiledancers.model.specifications.DanceGroupType; -public record Performance(String id, String name, List dayParts, Map appearances) {} +public record Performance(String id, String name, List dayParts, Map appearances) { + public Performance { + dayParts = dayParts == null ? null : Collections.unmodifiableList(new ArrayList<>(dayParts)); + appearances = appearances == null ? null : Collections.unmodifiableMap(new LinkedHashMap<>(appearances)); + } +} diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java index 0fa488c..8ab4d2f 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java @@ -1,9 +1,24 @@ package nl.agilecc.agiledancers.service.model; import java.time.LocalDate; -import java.util.*; -import nl.agilecc.agiledancers.model.*; -import nl.agilecc.agiledancers.model.specifications.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.UUID; +import nl.agilecc.agiledancers.model.Coach; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.DanceSchool; +import nl.agilecc.agiledancers.model.Dancer; +import nl.agilecc.agiledancers.model.DayPart; +import nl.agilecc.agiledancers.model.Show; import nl.agilecc.agiledancers.service.ManagementException; import nl.agilecc.agiledancers.service.ManagementExceptionKind; import nl.agilecc.agiledancers.service.ShowConfiguration; @@ -17,23 +32,30 @@ public record SchoolSnapshot( List coaches, List groups, List shows) { - public static SchoolSnapshot of(DanceSchool s) { - Map dancers = ids(s.getDancers()); - Map coaches = ids(s.getCoaches()); - Map groups = ids(s.getDanceGroups()); - Map shows = ids(s.getShows()); + public SchoolSnapshot { + dancers = dancers == null ? null : Collections.unmodifiableList(new ArrayList<>(dancers)); + coaches = coaches == null ? null : Collections.unmodifiableList(new ArrayList<>(coaches)); + groups = groups == null ? null : Collections.unmodifiableList(new ArrayList<>(groups)); + shows = shows == null ? null : Collections.unmodifiableList(new ArrayList<>(shows)); + } + + public static SchoolSnapshot of(final DanceSchool s) { + final Map dancers = ids(s.getDancers()); + final Map coaches = ids(s.getCoaches()); + final Map groups = ids(s.getDanceGroups()); + final Map shows = ids(s.getShows()); return new SchoolSnapshot( 2, s.getKey(), s.getName(), s.getDancers().stream() - .map(d -> new Person(dancers.get(d), d.getName(), d.getBirthDate())) + .map((final Dancer d) -> new Person(dancers.get(d), d.getName(), d.getBirthDate())) .toList(), s.getCoaches().stream() - .map(c -> new Trainer(coaches.get(c), c.getName())) + .map((final Coach c) -> new Trainer(coaches.get(c), c.getName())) .toList(), s.getDanceGroups().stream() - .map(g -> new Group( + .map((final DanceGroup g) -> new Group( groups.get(g), g.getCategory(), g.getType(), @@ -44,11 +66,11 @@ public record SchoolSnapshot( g.getCoach() == null ? null : coaches.get(g.getCoach()))) .toList(), s.getShows().stream() - .map(show -> new Performance( + .map((final Show show) -> new Performance( shows.get(show), show.getName(), show.getDayParts().stream() - .map(p -> new Part( + .map((final DayPart p) -> new Part( p.getDate(), p.getType(), p.getDanceGroups().stream() @@ -59,42 +81,62 @@ public record SchoolSnapshot( .toList()); } - private static Map ids(Collection values) { - Map result = new IdentityHashMap<>(); - for (T value : values) result.put(value, Integer.toString(result.size() + 1)); + private static Map ids(final Collection values) { + final Map result = new IdentityHashMap<>(); + for (final T value : values) { + result.put(value, Integer.toString(result.size() + 1)); + } return result; } + /** Validates all references and values, then rebuilds an independent graph with temporary IDs. */ public DanceSchool restore() { check(Objects.equals(formatVersion, 2), "Niet-ondersteunde formatVersion: " + formatVersion); check(key != null, "key ontbreekt"); text(name, "schoolnaam"); - DanceSchool school = DanceSchool.builder().key(key).name(name).build(); - Map dancerMap = new LinkedHashMap<>(); - for (Person d : list(dancers, "dancers")) { + final DanceSchool school = DanceSchool.builder().key(key).name(name).build(); + final Map dancerMap = restoreDancers(school); + final Map coachMap = restoreCoaches(school); + final Map groupMap = restoreGroups(school, dancerMap, coachMap); + restoreShows(school, groupMap); + return school; + } + + private Map restoreDancers(final DanceSchool school) { + final Map dancerMap = new LinkedHashMap<>(); + for (final Person d : list(dancers, "dancers")) { check(d != null, "Lege danser"); text(d.name(), "dansernaam"); check( d.birthDate() != null && !d.birthDate().isAfter(LocalDate.now()), "Ongeldige geboortedatum voor " + d.name()); - Dancer dancer = + final Dancer dancer = Dancer.builder().name(d.name()).birthDate(d.birthDate()).build(); // Assign temporary distinct IDs before inserting entities into sets. dancer.setId((long) dancerMap.size()); put(dancerMap, d.id(), dancer); school.addDancer(dancer); } - Map coachMap = new LinkedHashMap<>(); - for (Trainer c : list(coaches, "coaches")) { + return dancerMap; + } + + private Map restoreCoaches(final DanceSchool school) { + final Map coachMap = new LinkedHashMap<>(); + for (final Trainer c : list(coaches, "coaches")) { check(c != null, "Lege coach"); text(c.name(), "coachnaam"); - Coach coach = Coach.builder().name(c.name()).build(); + final Coach coach = Coach.builder().name(c.name()).build(); coach.setId((long) coachMap.size()); put(coachMap, c.id(), coach); school.addCoach(coach); } - Map groupMap = new LinkedHashMap<>(); - for (Group g : list(groups, "groups")) { + return coachMap; + } + + private Map restoreGroups( + final DanceSchool school, final Map dancerMap, final Map coachMap) { + final Map groupMap = new LinkedHashMap<>(); + for (final Group g : list(groups, "groups")) { check(g != null, "Lege groep"); text(g.category(), "groepsnaam"); check( @@ -104,7 +146,7 @@ public record SchoolSnapshot( && g.startAge() >= 0 && g.endAge() >= g.startAge(), "Ongeldig type of leeftijdsbereik voor groep " + g.category()); - DanceGroup group = DanceGroup.builder() + final DanceGroup group = DanceGroup.builder() .category(g.category()) .type(g.type()) .startAge(g.startAge()) @@ -120,20 +162,25 @@ public record SchoolSnapshot( .build(); check(!group.getStyles().contains(null), "Ongeldige dansstijl"); group.setId((long) groupMap.size()); - for (String id : unique(g.dancerIds(), "dancerIds")) + for (final String id : unique(g.dancerIds(), "dancerIds")) { group.addDancer(ref(dancerMap, id, "groep " + g.category() + " verwijst naar een onbekende danser")); + } put(groupMap, g.id(), group); school.addDanceGroup(group); } - Set showIds = new HashSet<>(); - for (Performance show : list(shows, "shows")) { + return groupMap; + } + + private void restoreShows(final DanceSchool school, final Map groupMap) { + final Set showIds = new HashSet<>(); + for (final Performance show : list(shows, "shows")) { check(show != null, "Lege show"); text(show.id(), "show-id"); check(showIds.add(show.id()), "Dubbele show-id " + show.id()); text(show.name(), "shownaam"); - List parts = new ArrayList<>(); - Set types = new HashSet<>(); - for (Part part : list(show.dayParts(), "dayParts")) { + final List parts = new ArrayList<>(); + final Set types = new HashSet<>(); + for (final Part part : list(show.dayParts(), "dayParts")) { check( part != null && part.type() != null @@ -144,13 +191,13 @@ public record SchoolSnapshot( .date(part.date()) .type(part.type()) .danceGroups(unique(part.groupIds(), "groupIds").stream() - .map(id -> + .map((final String id) -> ref(groupMap, id, "Show " + show.name() + " verwijst naar een onbekende groep")) .toList()) .build()); } ShowConfiguration.validate(parts, show.appearances()); - Show entity = Show.builder() + final Show entity = Show.builder() .name(show.name()) .dayParts(parts) .appearances(Map.copyOf(show.appearances())) @@ -158,19 +205,18 @@ public record SchoolSnapshot( entity.setId((long) showIds.size()); school.addShow(entity); } - return school; } - private static void text(String value, String field) { + private static void text(final String value, final String field) { check(value != null && !value.isBlank(), field + " ontbreekt"); } - private static List list(List value, String field) { + private static List list(final List value, final String field) { check(value != null, field + " ontbreekt"); return value; } - private static List unique(List value, String field) { + private static List unique(final List value, final String field) { list(value, field); check( value.stream().noneMatch(Objects::isNull) && new HashSet<>(value).size() == value.size(), @@ -178,18 +224,20 @@ public record SchoolSnapshot( return value; } - private static void put(Map map, String id, T value) { + private static void put(final Map map, final String id, final T value) { text(id, "id"); check(map.putIfAbsent(id, value) == null, "Dubbele id " + id); } - private static T ref(Map map, String id, String error) { - T value = map.get(id); + private static T ref(final Map map, final String id, final String error) { + final T value = map.get(id); check(value != null, error); return value; } - private static void check(boolean valid, String message) { - if (!valid) throw new ManagementException(ManagementExceptionKind.INVALID_INPUT, message); + private static void check(final boolean valid, final String message) { + if (!valid) { + throw new ManagementException(ManagementExceptionKind.INVALID_INPUT, message); + } } } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/port/SchedulingEngine.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/port/SchedulingEngine.java index 6921771..592d499 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/port/SchedulingEngine.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/port/SchedulingEngine.java @@ -4,5 +4,6 @@ import nl.agilecc.agiledancers.model.scheduling.SchedulingInput; import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; public interface SchedulingEngine { - SchedulingResult solve(SchedulingInput input); + /** Computes assignments without mutating the captured inputs; the caller validates and commits the result. */ + public SchedulingResult solve(final SchedulingInput input); } diff --git a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/solver/OrToolsSchedulingEngine.java b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/solver/OrToolsSchedulingEngine.java index 869e211..4280bb8 100644 --- a/agiledancers-be/src/main/java/nl/agilecc/agiledancers/solver/OrToolsSchedulingEngine.java +++ b/agiledancers-be/src/main/java/nl/agilecc/agiledancers/solver/OrToolsSchedulingEngine.java @@ -1,45 +1,71 @@ package nl.agilecc.agiledancers.solver; import com.google.ortools.Loader; -import com.google.ortools.sat.*; +import com.google.ortools.sat.BoolVar; +import com.google.ortools.sat.CpModel; +import com.google.ortools.sat.CpSolver; +import com.google.ortools.sat.CpSolverStatus; +import com.google.ortools.sat.IntVar; +import com.google.ortools.sat.LinearExpr; import jakarta.inject.Singleton; -import java.util.*; +import java.util.ArrayList; +import java.util.List; import nl.agilecc.agiledancers.model.scheduling.SchedulingInput; import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; -import nl.agilecc.agiledancers.service.*; +import nl.agilecc.agiledancers.service.ManagementException; +import nl.agilecc.agiledancers.service.ManagementExceptionKind; import nl.agilecc.agiledancers.service.port.SchedulingEngine; @Singleton public class OrToolsSchedulingEngine implements SchedulingEngine { @Override - public SchedulingResult solve(SchedulingInput input) { + public SchedulingResult solve(final SchedulingInput input) { Loader.loadNativeLibraries(); - CpModel model = new CpModel(); - int n = input.dayParts(), groups = input.groups().size(); - BoolVar[][] assigned = new BoolVar[groups][n]; - for (int g = 0; g < groups; g++) { - for (int p = 0; p < n; p++) assigned[g][p] = model.newBoolVar("g" + g + "p" + p); - model.addEquality(LinearExpr.sum(assigned[g]), input.groups().get(g).appearances()); - } - IntVar[] loads = new IntVar[n]; - for (int p = 0; p < n; p++) { - BoolVar[] column = new BoolVar[groups]; - for (int g = 0; g < groups; g++) column[g] = assigned[g][p]; - loads[p] = model.newIntVar(0, groups, "load" + p); - model.addEquality(loads[p], LinearExpr.sum(column)); - } - IntVar max = model.newIntVar(0, groups, "max"), min = model.newIntVar(0, groups, "min"); - model.addMaxEquality(max, loads); - model.addMinEquality(min, loads); - model.minimize(LinearExpr.weightedSum(new IntVar[] {max, min}, new long[] {1, -1})); - CpSolver solver = new CpSolver(); + final CpModel model = new CpModel(); + final BoolVar[][] assigned = assignments(model, input); + balance(model, assigned, input.dayParts()); + final CpSolver solver = new CpSolver(); solver.getParameters().setMaxTimeInSeconds(1800); // Quarkus owns process shutdown; do not install a native SIGINT handler. solver.getParameters().setCatchSigintSignal(false); // Single-worker search with a fixed seed makes identical inputs repeatable. solver.getParameters().setNumSearchWorkers(1).setRandomSeed(0); - CpSolverStatus status = solver.solve(model); - if (status != CpSolverStatus.OPTIMAL && status != CpSolverStatus.FEASIBLE) + final CpSolverStatus status = solver.solve(model); + requireSolution(status); + return new SchedulingResult(status == CpSolverStatus.OPTIMAL, readAssignments(input, assigned, solver)); + } + + private static BoolVar[][] assignments(final CpModel model, final SchedulingInput input) { + final BoolVar[][] assigned = new BoolVar[input.groups().size()][input.dayParts()]; + for (int g = 0; g < assigned.length; g++) { + for (int p = 0; p < input.dayParts(); p++) { + assigned[g][p] = model.newBoolVar("g" + g + "p" + p); + } + model.addEquality(LinearExpr.sum(assigned[g]), input.groups().get(g).appearances()); + } + return assigned; + } + + private static void balance(final CpModel model, final BoolVar[][] assigned, final int dayParts) { + final int groups = assigned.length; + final IntVar[] loads = new IntVar[dayParts]; + for (int p = 0; p < dayParts; p++) { + final BoolVar[] column = new BoolVar[groups]; + for (int g = 0; g < groups; g++) { + column[g] = assigned[g][p]; + } + loads[p] = model.newIntVar(0, groups, "load" + p); + model.addEquality(loads[p], LinearExpr.sum(column)); + } + final IntVar max = model.newIntVar(0, groups, "max"); + final IntVar min = model.newIntVar(0, groups, "min"); + model.addMaxEquality(max, loads); + model.addMinEquality(min, loads); + model.minimize(LinearExpr.weightedSum(new IntVar[] {max, min}, new long[] {1, -1})); + } + + private static void requireSolution(final CpSolverStatus status) { + if (status != CpSolverStatus.OPTIMAL && status != CpSolverStatus.FEASIBLE) { throw new ManagementException( ManagementExceptionKind.INVALID_INPUT, switch (status) { @@ -49,14 +75,21 @@ public class OrToolsSchedulingEngine implements SchedulingEngine { + " preserved."; default -> "Solver failed (" + status + "). Existing assignments were preserved."; }); - List> parts = new ArrayList<>(); - for (int p = 0; p < n; p++) { - List ids = new ArrayList<>(); - for (int g = 0; g < groups; g++) - if (solver.booleanValue(assigned[g][p])) + } + } + + private static List> readAssignments( + final SchedulingInput input, final BoolVar[][] assigned, final CpSolver solver) { + final List> parts = new ArrayList<>(); + for (int p = 0; p < input.dayParts(); p++) { + final List ids = new ArrayList<>(); + for (int g = 0; g < assigned.length; g++) { + if (solver.booleanValue(assigned[g][p])) { ids.add(input.groups().get(g).groupId()); + } + } parts.add(List.copyOf(ids)); } - return new SchedulingResult(status == CpSolverStatus.OPTIMAL, List.copyOf(parts)); + return List.copyOf(parts); } } diff --git a/agiledancers-be/src/main/resources/application.properties b/agiledancers-be/src/main/resources/application.properties index d59cafd..43d28ef 100644 --- a/agiledancers-be/src/main/resources/application.properties +++ b/agiledancers-be/src/main/resources/application.properties @@ -4,3 +4,8 @@ quarkus.http.host=0.0.0.0 # Scheduling is synchronous with a 30-minute solver limit. quarkus.http.idle-timeout=31M quarkus.jackson.fail-on-unknown-properties=true + +# Share execution data from ordinary and Quarkus tests. Reports are generated at verify. +%test.quarkus.jacoco.data-file=target/jacoco.exec +%test.quarkus.jacoco.reuse-data-file=true +%test.quarkus.jacoco.report=false diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/ArchitectureTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/ArchitectureTest.java index 5924f87..33f4b69 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/ArchitectureTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/ArchitectureTest.java @@ -17,8 +17,7 @@ class ArchitectureTest { private static final String API_MODEL = ROOT + ".rest.apimodel"; // Exclude Lombok builders and compiler-generated enum-switch helper classes. - @ArchTest - static final ArchRule handwritten_types_are_top_level = classes() + private @ArchTest static final ArchRule HANDWRITTEN_TYPES_ARE_TOP_LEVEL = classes() .that() .areNotAnnotatedWith("lombok.Generated") .and() @@ -26,8 +25,7 @@ class ArchitectureTest { .should() .beTopLevelClasses(); - @ArchTest - static final ArchRule service_records_reside_in_service_model = classes() + private @ArchTest static final ArchRule SERVICE_RECORDS_RESIDE_IN_SERVICE_MODEL = classes() .that() .resideInAPackage(ROOT + ".service..") .and() @@ -35,8 +33,7 @@ class ArchitectureTest { .should() .resideInAPackage(ROOT + ".service.model"); - @ArchTest - static final ArchRule service_interfaces_reside_in_service_port = classes() + private @ArchTest static final ArchRule SERVICE_INTERFACES_RESIDE_IN_SERVICE_PORT = classes() .that() .resideInAPackage(ROOT + ".service..") .and() @@ -44,8 +41,7 @@ class ArchitectureTest { .should() .resideInAPackage(ROOT + ".service.port"); - @ArchTest - static final ArchRule scheduling_port_uses_domain_types = classes() + private @ArchTest static final ArchRule SCHEDULING_PORT_USES_DOMAIN_TYPES = classes() .that() .resideInAPackage(ROOT + ".service.port..") .should() @@ -53,15 +49,13 @@ class ArchitectureTest { .resideInAnyPackage(ROOT + ".service.port..", ROOT + ".model..", "java.."); // API model classes and records use the Request, Response, View, Dto or DTO suffix. - @ArchTest - static final ArchRule api_models_reside_in_the_api_model_package = classes() + private @ArchTest static final ArchRule API_MODELS_RESIDE_IN_THE_API_MODEL_PACKAGE = classes() .that() .haveNameMatching(".*(Request|Response|View|Dto|DTO)$") .should() .resideInAPackage(API_MODEL); - @ArchTest - static final ArchRule rest_records_reside_in_the_api_model_package = classes() + private @ArchTest static final ArchRule REST_RECORDS_RESIDE_IN_THE_API_MODEL_PACKAGE = classes() .that() .resideInAPackage(ROOT + ".rest..") .and() @@ -69,8 +63,7 @@ class ArchitectureTest { .should() .resideInAPackage(API_MODEL); - @ArchTest - static final ArchRule api_models_are_public_top_level_types = classes() + private @ArchTest static final ArchRule API_MODELS_ARE_PUBLIC_TOP_LEVEL_TYPES = classes() .that() .resideInAPackage(API_MODEL + "..") .should() @@ -82,8 +75,7 @@ class ArchitectureTest { // The existing services depend on concrete repositories. This rule preserves that // arrangement until repository ports and dependency injection are introduced. - @ArchTest - static final ArchRule layer_dependencies = layeredArchitecture() + private @ArchTest static final ArchRule LAYER_DEPENDENCIES = layeredArchitecture() .consideringOnlyDependenciesInLayers() .layer("Model") .definedBy(ROOT + ".model..") @@ -106,8 +98,7 @@ class ArchitectureTest { .whereLayer("Solver") .mayOnlyAccessLayers("Services", "Model"); - @ArchTest - static final ArchRule production_classes_have_an_architectural_home = classes() + private @ArchTest static final ArchRule PRODUCTION_CLASSES_HAVE_AN_ARCHITECTURAL_HOME = classes() .should() .resideInAnyPackage( ROOT + ".model..", @@ -116,31 +107,27 @@ class ArchitectureTest { ROOT + ".rest..", ROOT + ".solver.."); - @ArchTest - static final ArchRule model_is_independent_of_frameworks = classes() + private @ArchTest static final ArchRule MODEL_IS_INDEPENDENT_OF_FRAMEWORKS = classes() .that() .resideInAPackage(ROOT + ".model..") .should() .onlyDependOnClassesThat() .resideInAnyPackage(ROOT + ".model..", "java..", "lombok.."); - @ArchTest - static final ArchRule ortools_is_encapsulated_in_solver = noClasses() + private @ArchTest static final ArchRule ORTOOLS_IS_ENCAPSULATED_IN_SOLVER = noClasses() .that() .resideOutsideOfPackage(ROOT + ".solver..") .should() .dependOnClassesThat() .resideInAPackage("com.google.ortools.."); - @ArchTest - static final ArchRule http_api_is_encapsulated_in_rest = noClasses() + private @ArchTest static final ArchRule HTTP_API_IS_ENCAPSULATED_IN_REST = noClasses() .that() .resideOutsideOfPackage(ROOT + ".rest..") .should() .dependOnClassesThat() .resideInAnyPackage("jakarta.ws.rs..", "org.jboss.resteasy.."); - @ArchTest - static final ArchRule packages_are_free_of_cycles = + private @ArchTest static final ArchRule PACKAGES_ARE_FREE_OF_CYCLES = slices().matching(ROOT + ".(*)..").should().beFreeOfCycles(); } diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/HealthTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/HealthTest.java index e6c3472..dd08837 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/HealthTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/HealthTest.java @@ -8,8 +8,7 @@ import org.junit.jupiter.api.Test; @QuarkusTest class HealthTest { - @Test - void applicationIsReady() { + public @Test void applicationIsReady() { given().when().get("/q/health/ready").then().statusCode(200).body("status", equalTo("UP")); } } diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchedulingTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchedulingTest.java index 8fc2d31..6edc84c 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchedulingTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchedulingTest.java @@ -1,14 +1,28 @@ package nl.agilecc.agiledancers; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.LocalDate; -import java.util.*; -import nl.agilecc.agiledancers.model.*; +import java.util.EnumMap; +import java.util.IntSummaryStatistics; +import java.util.List; +import java.util.Map; +import java.util.Set; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.DayPart; +import nl.agilecc.agiledancers.model.Show; +import nl.agilecc.agiledancers.model.scheduling.SchedulingInput; import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; -import nl.agilecc.agiledancers.model.specifications.*; -import nl.agilecc.agiledancers.service.*; -import nl.agilecc.agiledancers.service.model.*; +import nl.agilecc.agiledancers.model.specifications.DanceGroupType; +import nl.agilecc.agiledancers.model.specifications.DayPartType; +import nl.agilecc.agiledancers.service.ManagementException; +import nl.agilecc.agiledancers.service.SchoolManagementService; +import nl.agilecc.agiledancers.service.ShowConfiguration; +import nl.agilecc.agiledancers.service.model.GroupDetails; +import nl.agilecc.agiledancers.service.model.SchoolSnapshot; import nl.agilecc.agiledancers.solver.OrToolsSchedulingEngine; import org.junit.jupiter.api.Test; @@ -16,9 +30,9 @@ class SchedulingTest { private final SchoolManagementService service = new SchoolManagementService(); private final long school = service.createSchool("Studio").getId(); - private List parts(int n) { + private List parts(final int n) { return java.util.stream.IntStream.range(0, n) - .mapToObj(i -> DayPart.builder() + .mapToObj((final int i) -> DayPart.builder() .date(LocalDate.of(2027, 1, 1).plusDays(i)) .type(DayPartType.AFTERNOON) .danceGroups(List.of()) @@ -26,12 +40,12 @@ class SchedulingTest { .toList(); } - private DanceGroup group(DanceGroupType type) { - return service.createGroup(school, "Group", type, 0, 99, Set.of()); + private DanceGroup group(final DanceGroupType type) { + return service.createGroup(school, new GroupDetails("Group", type, 0, 99, Set.of())); } - private void run(Show show) { - var input = service.beginScheduling(school, show.getId()); + private void run(final Show show) { + final SchedulingInput input = service.beginScheduling(school, show.getId()); try { service.finishScheduling(school, input, new OrToolsSchedulingEngine().solve(input)); } finally { @@ -39,41 +53,42 @@ class SchedulingTest { } } - @Test - void exactAppearancesAndOptimalBalanceIncludingEmptyParts() { + public @Test void exactAppearancesAndOptimalBalanceIncludingEmptyParts() { group(DanceGroupType.PRE_SCHOOLER); - var show = service.createShow(school, "Show", parts(6), null); + final Show show = service.createShow(school, "Show", parts(6), null); run(show); - var loads = show.getDayParts().stream() - .mapToInt(p -> p.getDanceGroups().size()) + IntSummaryStatistics loads = show.getDayParts().stream() + .mapToInt((final DayPart p) -> p.getDanceGroups().size()) .summaryStatistics(); assertEquals(1, loads.getSum()); assertEquals(0, loads.getMin()); assertEquals(1, loads.getMax()); - for (var type : DanceGroupType.values()) group(type); + for (final DanceGroupType type : DanceGroupType.values()) { + group(type); + } run(show); - for (var group : service.groups(school)) + for (final DanceGroup group : service.groups(school)) { assertEquals( show.getAppearances().get(group.getType()).longValue(), show.getDayParts().stream() - .filter(p -> p.getDanceGroups().contains(group)) + .filter((final DayPart p) -> p.getDanceGroups().contains(group)) .count()); + } loads = show.getDayParts().stream() - .mapToInt(p -> p.getDanceGroups().size()) + .mapToInt((final DayPart p) -> p.getDanceGroups().size()) .summaryStatistics(); assertTrue(loads.getMax() - loads.getMin() <= 1); - var snapshot = service.exportSchool(school); + final SchoolSnapshot snapshot = service.exportSchool(school); assertEquals(snapshot, SchoolSnapshot.of(snapshot.restore())); } - @Test - void defaultsInitialConfigurationAndPreservationOnEdits() { - var show = service.createShow(school, "Show", List.of(), null); + public @Test void defaultsInitialConfigurationAndPreservationOnEdits() { + final Show show = service.createShow(school, "Show", List.of(), null); assertTrue(show.getAppearances().isEmpty()); service.updateShow(school, show.getId(), "Show", parts(3), null); assertEquals(3, show.getAppearances().get(DanceGroupType.PRE_EDUCATION)); assertEquals(1, show.getAppearances().get(DanceGroupType.PRE_SCHOOLER)); - var counts = new EnumMap<>(show.getAppearances()); + final var counts = new EnumMap<>(show.getAppearances()); counts.put(DanceGroupType.PRE_SCHOOLER, 2); service.updateShow(school, show.getId(), "Show", parts(4), counts); assertEquals(counts, show.getAppearances()); @@ -85,36 +100,35 @@ class SchedulingTest { assertEquals(counts, show.getAppearances()); } - @Test - void rejectsDuplicatePartsMissingCountsAndOutOfRangeCounts() { - var duplicate = List.of(parts(1).getFirst(), parts(1).getFirst()); + public @Test void rejectsDuplicatePartsMissingCountsAndOutOfRangeCounts() { + final List duplicate = List.of(parts(1).getFirst(), parts(1).getFirst()); assertThrows(ManagementException.class, () -> service.createShow(school, "Show", duplicate, null)); assertThrows(ManagementException.class, () -> service.createShow(school, "Show", parts(7), null)); assertThrows(ManagementException.class, () -> service.createShow(school, "Show", parts(2), Map.of())); - var counts = ShowConfiguration.defaults(2); + final Map counts = ShowConfiguration.defaults(2); counts.put(DanceGroupType.REGULAR, 3); assertThrows(ManagementException.class, () -> service.createShow(school, "Show", parts(2), counts)); counts.put(DanceGroupType.REGULAR, 0); assertThrows(ManagementException.class, () -> service.createShow(school, "Show", parts(2), counts)); } - @Test - void duplicateRunsStaleResultsAndMalformedResultsPreserveAssignments() { - var group = group(DanceGroupType.REGULAR); - var show = service.createShow(school, "Show", parts(2), null); + public @Test void duplicateRunsStaleResultsAndMalformedResultsPreserveAssignments() { + final DanceGroup group = group(DanceGroupType.REGULAR); + final Show show = service.createShow(school, "Show", parts(2), null); run(show); - var before = show.getDayParts().stream().map(DayPart::getDanceGroups).toList(); - var input = service.beginScheduling(school, show.getId()); + final List> before = + show.getDayParts().stream().map(DayPart::getDanceGroups).toList(); + final SchedulingInput input = service.beginScheduling(school, show.getId()); assertThrows(ManagementException.class, () -> service.beginScheduling(school, show.getId())); - var result = new OrToolsSchedulingEngine().solve(input); - service.updateGroup(school, group.getId(), "Renamed", group.getType(), 0, 99, Set.of()); + final SchedulingResult result = new OrToolsSchedulingEngine().solve(input); + service.updateGroup(school, group.getId(), new GroupDetails("Renamed", group.getType(), 0, 99, Set.of())); assertDoesNotThrow(() -> service.finishScheduling(school, input, result)); group(DanceGroupType.PRE_SCHOOLER); assertThrows(ManagementException.class, () -> service.finishScheduling(school, input, result)); assertEquals( before, show.getDayParts().stream().map(DayPart::getDanceGroups).toList()); service.endScheduling(show.getId()); - var fresh = service.beginScheduling(school, show.getId()); + final SchedulingInput fresh = service.beginScheduling(school, show.getId()); assertThrows( ManagementException.class, () -> service.finishScheduling( @@ -122,18 +136,17 @@ class SchedulingTest { assertEquals( before, show.getDayParts().stream().map(DayPart::getDanceGroups).toList()); // Feasible results are accepted without requiring proof of optimality. - var valid = new OrToolsSchedulingEngine().solve(fresh); + final SchedulingResult valid = new OrToolsSchedulingEngine().solve(fresh); assertDoesNotThrow( () -> service.finishScheduling(school, fresh, new SchedulingResult(false, valid.assignments()))); service.endScheduling(show.getId()); } - @Test - void staleDaypartCountChangesAndDeletedShowsAreRejected() { + public @Test void staleDaypartCountChangesAndDeletedShowsAreRejected() { group(DanceGroupType.REGULAR); - var show = service.createShow(school, "Show", parts(2), null); - var input = service.beginScheduling(school, show.getId()); - var result = new OrToolsSchedulingEngine().solve(input); + final Show show = service.createShow(school, "Show", parts(2), null); + final SchedulingInput input = service.beginScheduling(school, show.getId()); + final SchedulingResult result = new OrToolsSchedulingEngine().solve(input); service.updateShow(school, show.getId(), "Show", parts(3), null); assertThrows(ManagementException.class, () -> service.finishScheduling(school, input, result)); service.deleteShow(school, show.getId()); @@ -141,13 +154,12 @@ class SchedulingTest { service.endScheduling(show.getId()); } - @Test - void needsGroupsAndDayparts() { - var show = service.createShow(school, "Show", parts(1), null); + public @Test void needsGroupsAndDayparts() { + final Show show = service.createShow(school, "Show", parts(1), null); assertThrows(ManagementException.class, () -> service.beginScheduling(school, show.getId())); group(DanceGroupType.REGULAR); - var empty = service.createShow(school, "Empty", List.of(), null); + final Show empty = service.createShow(school, "Empty", List.of(), null); assertThrows(ManagementException.class, () -> service.beginScheduling(school, empty.getId())); - assertTrue(show.getAppearances().values().stream().allMatch(n -> n == 1)); + assertTrue(show.getAppearances().values().stream().allMatch((final Integer n) -> n == 1)); } } diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java index efc8043..621f969 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java @@ -1,7 +1,13 @@ package nl.agilecc.agiledancers; import static io.restassured.RestAssured.given; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.matchesPattern; import io.quarkus.test.junit.QuarkusTest; import io.restassured.response.ValidatableResponse; @@ -17,59 +23,26 @@ class SchoolManagementResourceTest { private static final Map SHOW = Map.of("name", "Annual show", "dayParts", new Object[] {Map.of("date", "2027-06-12", "type", "AFTERNOON")}); - @Test - void managesSchoolAndItsResources() { - long schoolId = create(SCHOOLS, Map.of("name", "Dance Studio")); - String school = SCHOOLS + "/" + schoolId; + public @Test void managesSchoolAndItsResources() { + final long schoolId = create(SCHOOLS, Map.of("name", "Dance Studio")); + final String school = SCHOOLS + "/" + schoolId; try { given().get(school).then().statusCode(200).body("name", equalTo("Dance Studio")); given().get(SCHOOLS).then().statusCode(200).body("id", hasItem((int) schoolId)); put(school, Map.of("name", "Renamed studio")).statusCode(200).body("name", equalTo("Renamed studio")); - long dancerId = create(school + "/dancers", DANCER); - String dancer = school + "/dancers/" + dancerId; + final long dancerId = create(school + "/dancers", DANCER); + final String dancer = school + "/dancers/" + dancerId; put(dancer, Map.of("name", "Sam updated", "birthDate", "2011-06-13")) .statusCode(200) .body("birthDate", equalTo("2011-06-13")); given().get(dancer).then().statusCode(200).body("name", equalTo("Sam updated")); given().get(school + "/dancers").then().statusCode(200).body("id", contains((int) dancerId)); - long groupId = create(school + "/dance-groups", GROUP); - String group = school + "/dance-groups/" + groupId; - given().put(group + "/dancers/" + dancerId).then().statusCode(204); - put( - group, - Map.of( - "category", - "Selection", - "type", - "SELECTION", - "startAge", - 10, - "endAge", - 20, - "styles", - new String[] {"JAZZ", "MODERN"})) - .statusCode(200) - .body("category", equalTo("Selection")) - .body("dancerIds", contains((int) dancerId)) - .body("styles", containsInAnyOrder("JAZZ", "MODERN")); - given().get(group).then().statusCode(200).body("type", equalTo("SELECTION")); - given().get(school + "/dance-groups").then().statusCode(200).body("id", contains((int) groupId)); - - long showId = create(school + "/shows", SHOW); - String show = school + "/shows/" + showId; - given().get(show) - .then() - .statusCode(200) - .body("dayParts.type", contains("AFTERNOON")) - .body("dayParts.danceGroupIds", everyItem(empty())); - put(show, Map.of("name", "Summer show", "dayParts", new Object[] { - Map.of("date", "2027-07-01", "type", "EVENING") - })) - .statusCode(200) - .body("dayParts.date", contains("2027-07-01")); - given().get(school + "/shows").then().statusCode(200).body("id", contains((int) showId)); + final String group = manageGroup(school, dancerId); + final long groupId = Long.parseLong(group.substring(group.lastIndexOf('/') + 1)); + final String show = manageShow(school); + final long showId = Long.parseLong(show.substring(show.lastIndexOf('/') + 1)); given().get(school) .then() .statusCode(200) @@ -77,7 +50,7 @@ class SchoolManagementResourceTest { .body("danceGroupIds", contains((int) groupId)) .body("showIds", contains((int) showId)); - for (String path : new String[] {dancer, group, show}) { + for (final String path : new String[] {dancer, group, show}) { given().delete(path).then().statusCode(204); given().get(path).then().statusCode(404); given().delete(path).then().statusCode(404); @@ -93,14 +66,13 @@ class SchoolManagementResourceTest { given().get(school).then().statusCode(404); } - @Test - void dancerCanBelongToMultipleGroupsAndDeletingDancerRemovesMemberships() { - String school = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Membership school")); + public @Test void dancerCanBelongToMultipleGroupsAndDeletingDancerRemovesMemberships() { + final String school = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Membership school")); try { - long dancerId = create(school + "/dancers", DANCER); - String first = school + "/dance-groups/" + create(school + "/dance-groups", GROUP); - String second = school + "/dance-groups/" + create(school + "/dance-groups", GROUP); - for (String group : new String[] {first, second}) { + final long dancerId = create(school + "/dancers", DANCER); + final String first = school + "/dance-groups/" + create(school + "/dance-groups", GROUP); + final String second = school + "/dance-groups/" + create(school + "/dance-groups", GROUP); + for (final String group : new String[] {first, second}) { given().put(group + "/dancers/" + dancerId).then().statusCode(204); given().put(group + "/dancers/" + dancerId).then().statusCode(204); given().get(group).then().body("dancerIds", contains((int) dancerId)); @@ -118,16 +90,15 @@ class SchoolManagementResourceTest { } } - @Test - void isolatesSchoolsAndCascadesSchoolDeletion() { - String first = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "First")); - String second = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Second")); + public @Test void isolatesSchoolsAndCascadesSchoolDeletion() { + final String first = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "First")); + final String second = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Second")); try { - long dancer = create(first + "/dancers", DANCER); - long group = create(first + "/dance-groups", GROUP); - long show = create(first + "/shows", SHOW); - long otherGroup = create(second + "/dance-groups", GROUP); - for (String path : new String[] {"/dancers/" + dancer, "/dance-groups/" + group, "/shows/" + show}) { + final long dancer = create(first + "/dancers", DANCER); + final long group = create(first + "/dance-groups", GROUP); + final long show = create(first + "/shows", SHOW); + final long otherGroup = create(second + "/dance-groups", GROUP); + for (final String path : new String[] {"/dancers/" + dancer, "/dance-groups/" + group, "/shows/" + show}) { given().get(second + path).then().statusCode(404); given().delete(second + path).then().statusCode(404); } @@ -149,42 +120,19 @@ class SchoolManagementResourceTest { } } - @Test - void rejectsInvalidInputWithoutChangingStoredData() { + public @Test void rejectsInvalidInputWithoutChangingStoredData() { given().contentType("application/json").body("{}").post(SCHOOLS).then().statusCode(400); given().contentType("application/json") .body("null") .post(SCHOOLS) .then() .statusCode(400); - String school = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Validation school")); + final String school = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Validation school")); try { put(school, Map.of("name", " ")).statusCode(400); given().get(school).then().body("name", equalTo("Validation school")); - for (Object body : new Object[] { - Map.of("name", "Sam"), - Map.of("name", "Sam", "birthDate", "2999-01-01"), - Map.of("name", "Sam", "birthDate", "not-a-date") - }) { - given().contentType("application/json") - .body(body) - .post(school + "/dancers") - .then() - .statusCode(400); - } - for (String body : new String[] { - "{}", - "{\"category\":\"Teens\",\"type\":\"INVALID\"}", - "{\"category\":\"Teens\",\"type\":\"REGULAR\",\"startAge\":-1,\"endAge\":18,\"styles\":[]}", - "{\"category\":\"Teens\",\"type\":\"REGULAR\",\"startAge\":18,\"endAge\":12,\"styles\":[]}" - }) { - given().contentType("application/json") - .body(body) - .post(school + "/dance-groups") - .then() - .statusCode(400); - } - long group = create(school + "/dance-groups", GROUP); + rejectsInvalidDancersAndGroups(school); + final long group = create(school + "/dance-groups", GROUP); put( school + "/dance-groups/" + group, Map.of( @@ -217,25 +165,25 @@ class SchoolManagementResourceTest { } } - @Test - void schedulesAndValidatesTheNewShowConfiguration() { - String school = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Scheduling school")); + public @Test void schedulesAndValidatesTheNewShowConfiguration() { + final String school = SCHOOLS + "/" + create(SCHOOLS, Map.of("name", "Scheduling school")); try { - String show = school + "/shows/" + create(school + "/shows", SHOW); + final String show = school + "/shows/" + create(school + "/shows", SHOW); given().post(show + "/schedule").then().statusCode(400); - long group = create(school + "/dance-groups", GROUP); + final long group = create(school + "/dance-groups", GROUP); given().post(show + "/schedule").then().statusCode(200).body("optimal", equalTo(true)); given().get(show).then().body("dayParts[0].danceGroupIds", contains((int) group)); - String before = given().get(show).asString(); - String counts = "{\"SHOW_CASE\":1,\"PRE_EDUCATION\":1,\"SELECTION\":1,\"REGULAR\":%s,\"PRE_SCHOOLER\":1}"; - for (String count : new String[] {"0", "2", "1.5", "null"}) { - String body = + final String before = given().get(show).asString(); + final String counts = + "{\"SHOW_CASE\":1,\"PRE_EDUCATION\":1,\"SELECTION\":1,\"REGULAR\":%s,\"PRE_SCHOOLER\":1}"; + for (final String count : new String[] {"0", "2", "1.5", "null"}) { + final String body = "{\"name\":\"Show\",\"dayParts\":[{\"date\":\"2027-06-12\",\"type\":\"AFTERNOON\"}],\"appearances\":" + counts.formatted(count) + "}"; put(show, body).statusCode(400); given().get(show).then().body(equalTo(before)); } - String exported = given().get(school + "/export") + final String exported = given().get(school + "/export") .then() .statusCode(200) .extract() @@ -256,7 +204,77 @@ class SchoolManagementResourceTest { } } - private static long create(String path, Object body) { + private static String manageGroup(final String school, final long dancerId) { + final long groupId = create(school + "/dance-groups", GROUP); + final String group = school + "/dance-groups/" + groupId; + given().put(group + "/dancers/" + dancerId).then().statusCode(204); + put( + group, + Map.of( + "category", + "Selection", + "type", + "SELECTION", + "startAge", + 10, + "endAge", + 20, + "styles", + new String[] {"JAZZ", "MODERN"})) + .statusCode(200) + .body("category", equalTo("Selection")) + .body("dancerIds", contains((int) dancerId)) + .body("styles", containsInAnyOrder("JAZZ", "MODERN")); + given().get(group).then().statusCode(200).body("type", equalTo("SELECTION")); + given().get(school + "/dance-groups").then().statusCode(200).body("id", contains((int) groupId)); + + return group; + } + + private static String manageShow(final String school) { + final long showId = create(school + "/shows", SHOW); + final String show = school + "/shows/" + showId; + given().get(show) + .then() + .statusCode(200) + .body("dayParts.type", contains("AFTERNOON")) + .body("dayParts.danceGroupIds", everyItem(empty())); + put(show, Map.of("name", "Summer show", "dayParts", new Object[] { + Map.of("date", "2027-07-01", "type", "EVENING") + })) + .statusCode(200) + .body("dayParts.date", contains("2027-07-01")); + given().get(school + "/shows").then().statusCode(200).body("id", contains((int) showId)); + return show; + } + + private static void rejectsInvalidDancersAndGroups(final String school) { + for (final Object body : new Object[] { + Map.of("name", "Sam"), + Map.of("name", "Sam", "birthDate", "2999-01-01"), + Map.of("name", "Sam", "birthDate", "not-a-date") + }) { + given().contentType("application/json") + .body(body) + .post(school + "/dancers") + .then() + .statusCode(400); + } + for (final String body : new String[] { + "{}", + "{\"category\":\"Teens\",\"type\":\"INVALID\"}", + "{\"category\":\"Teens\",\"type\":\"REGULAR\",\"startAge\":-1,\"endAge\":18,\"styles\":[]}", + "{\"category\":\"Teens\",\"type\":\"REGULAR\",\"startAge\":18,\"endAge\":12,\"styles\":[]}" + }) { + given().contentType("application/json") + .body(body) + .post(school + "/dance-groups") + .then() + .statusCode(400); + } + } + + private static long create(final String path, final Object body) { return given().contentType("application/json") .body(body) .post(path) @@ -268,7 +286,7 @@ class SchoolManagementResourceTest { .getLong("id"); } - private static ValidatableResponse put(String path, Object body) { + private static ValidatableResponse put(final String path, final Object body) { return given().contentType("application/json").body(body).put(path).then(); } } diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotResourceTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotResourceTest.java index f7d83c0..819abfd 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotResourceTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotResourceTest.java @@ -1,7 +1,10 @@ package nl.agilecc.agiledancers; import static io.restassured.RestAssured.given; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyOrNullString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; import io.quarkus.test.junit.QuarkusTest; import org.junit.jupiter.api.Test; @@ -10,8 +13,7 @@ import org.junit.jupiter.api.Test; class SchoolSnapshotResourceTest { private static final String BASE = "/api/dance-schools"; - @Test - void downloadPreviewRestoreAndRejectInvalidFiles() { + public @Test void downloadPreviewRestoreAndRejectInvalidFiles() { long id = given().contentType("application/json") .body("{\"name\":\"JSON school\"}") .post(BASE) @@ -20,7 +22,7 @@ class SchoolSnapshotResourceTest { .extract() .jsonPath() .getLong("id"); - String json = given().get(BASE + "/" + id + "/export") + final String json = given().get(BASE + "/" + id + "/export") .then() .statusCode(200) .header("Content-Disposition", containsString(".json")) @@ -35,21 +37,7 @@ class SchoolSnapshotResourceTest { .statusCode(200) .body("existingId", equalTo((int) id)) .body("dancers", equalTo(0)); - for (String bad : new String[] { - "{", - "null", - json.replace("\"formatVersion\" : 2", "\"formatVersion\" : 1"), - json.replace("\"name\" :", "\"unknown\" :"), - json + " {}", - json.replace("\"dancers\" : [ ]", "\"dancers\" : null") - }) { - given().contentType("application/json") - .body(bad) - .post(BASE + "/import?replaceId=" + id) - .then() - .statusCode(400) - .body("message", not(emptyOrNullString())); - } + rejectInvalidFiles(id, json); given().get(BASE + "/" + id + "/export").then().body(equalTo(json)); given().delete(BASE + "/" + id).then().statusCode(204); id = given().contentType("application/json") @@ -65,4 +53,22 @@ class SchoolSnapshotResourceTest { given().delete(BASE + "/" + id); } } + + private static void rejectInvalidFiles(final long id, final String json) { + for (final String bad : new String[] { + "{", + "null", + json.replace("\"formatVersion\" : 2", "\"formatVersion\" : 1"), + json.replace("\"name\" :", "\"unknown\" :"), + json + " {}", + json.replace("\"dancers\" : [ ]", "\"dancers\" : null") + }) { + given().contentType("application/json") + .body(bad) + .post(BASE + "/import?replaceId=" + id) + .then() + .statusCode(400) + .body("message", not(emptyOrNullString())); + } + } } diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotTest.java index 947395c..8b98fb9 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotTest.java @@ -1,12 +1,30 @@ package nl.agilecc.agiledancers; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.LocalDate; -import java.util.*; -import nl.agilecc.agiledancers.model.specifications.*; -import nl.agilecc.agiledancers.service.*; -import nl.agilecc.agiledancers.service.model.*; +import java.util.List; +import java.util.UUID; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.DanceSchool; +import nl.agilecc.agiledancers.model.Dancer; +import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; +import nl.agilecc.agiledancers.model.specifications.DanceGroupType; +import nl.agilecc.agiledancers.model.specifications.DayPartType; +import nl.agilecc.agiledancers.service.ManagementException; +import nl.agilecc.agiledancers.service.SchoolManagementService; +import nl.agilecc.agiledancers.service.ShowConfiguration; +import nl.agilecc.agiledancers.service.model.Group; +import nl.agilecc.agiledancers.service.model.Part; +import nl.agilecc.agiledancers.service.model.Performance; +import nl.agilecc.agiledancers.service.model.Person; +import nl.agilecc.agiledancers.service.model.SchoolSnapshot; +import nl.agilecc.agiledancers.service.model.Trainer; import org.junit.jupiter.api.Test; class SchoolSnapshotTest { @@ -35,26 +53,27 @@ class SchoolSnapshotTest { ShowConfiguration.defaults(1)))); } - @Test - void roundTripAfterRestartPreservesAllFieldsAndSharedRelationships() { - var snapshot = complete(); - var service = new SchoolManagementService(); - var existing = service.createSchool("Other"); + public @Test void roundTripAfterRestartPreservesAllFieldsAndSharedRelationships() { + final SchoolSnapshot snapshot = complete(); + final var service = new SchoolManagementService(); + final DanceSchool existing = service.createSchool("Other"); service.createDancer(existing.getId(), "Other dancer", LocalDate.of(2000, 1, 1)); - var imported = service.importSchool(snapshot, null); + final DanceSchool imported = service.importSchool(snapshot, null); assertEquals(2, imported.getDanceGroups().size()); - var dancer = imported.getDancers().iterator().next(); + final Dancer dancer = imported.getDancers().iterator().next(); imported.getDanceGroups() - .forEach(g -> assertSame(dancer, g.getDancers().iterator().next())); + .forEach((final DanceGroup g) -> + assertSame(dancer, g.getDancers().iterator().next())); assertEquals(SchoolSnapshot.of(snapshot.restore()), service.exportSchool(imported.getId())); - var restarted = new SchoolManagementService(); - var restored = restarted.importSchool(service.exportSchool(imported.getId()), null); + final var restarted = new SchoolManagementService(); + final DanceSchool restored = restarted.importSchool(service.exportSchool(imported.getId()), null); assertEquals(SchoolSnapshot.of(snapshot.restore()), restarted.exportSchool(restored.getId())); // Membership removal must work after remapping hash-based sets. service.deleteDancer(imported.getId(), dancer.getId()); - imported.getDanceGroups().forEach(g -> assertTrue(g.getDancers().isEmpty())); + imported.getDanceGroups() + .forEach((final DanceGroup g) -> assertTrue(g.getDancers().isEmpty())); assertEquals(1, existing.getDancers().size()); - var group = imported.getDanceGroups().iterator().next(); + final DanceGroup group = imported.getDanceGroups().iterator().next(); service.deleteGroup(imported.getId(), group.getId()); assertEquals( 1, @@ -68,19 +87,18 @@ class SchoolSnapshotTest { assertDoesNotThrow(() -> service.exportSchool(imported.getId()).restore()); } - @Test - void previewIsReadOnlyAndReplacementRemovesPreviousConfiguration() { - var service = new SchoolManagementService(); - var snapshot = complete(); + public @Test void previewIsReadOnlyAndReplacementRemovesPreviousConfiguration() { + final var service = new SchoolManagementService(); + final SchoolSnapshot snapshot = complete(); assertNull(service.preview(snapshot).existingId()); assertTrue(service.schools().isEmpty()); - var original = service.importSchool(snapshot, null); + final DanceSchool original = service.importSchool(snapshot, null); service.createDancer(original.getId(), "Extra", LocalDate.of(2000, 1, 1)); - var other = service.createSchool(snapshot.name()); + final DanceSchool other = service.createSchool(snapshot.name()); assertEquals(original.getId(), service.preview(snapshot).existingId()); assertThrows(ManagementException.class, () -> service.importSchool(snapshot, null)); assertEquals(2, original.getDancers().size()); - var replaced = service.importSchool(snapshot, original.getId()); + final DanceSchool replaced = service.importSchool(snapshot, original.getId()); assertEquals(SchoolSnapshot.of(snapshot.restore()), service.exportSchool(replaced.getId())); assertSame(other, service.school(other.getId())); service.deleteSchool(replaced.getId()); @@ -89,23 +107,21 @@ class SchoolSnapshotTest { service.exportSchool(service.importSchool(snapshot, null).getId())); } - @Test - void unresolvedReferenceLeavesExistingAndOtherSchoolsIntact() { - var service = new SchoolManagementService(); - var valid = complete(); - var original = service.importSchool(valid, null); - var bad = new SchoolSnapshot( + public @Test void unresolvedReferenceLeavesExistingAndOtherSchoolsIntact() { + final var service = new SchoolManagementService(); + final SchoolSnapshot valid = complete(); + final DanceSchool original = service.importSchool(valid, null); + final var bad = new SchoolSnapshot( 2, valid.key(), valid.name(), List.of(), valid.coaches(), valid.groups(), valid.shows()); assertThrows(ManagementException.class, () -> service.preview(bad)); assertThrows(ManagementException.class, () -> service.importSchool(bad, original.getId())); assertEquals(SchoolSnapshot.of(valid.restore()), service.exportSchool(original.getId())); } - @Test - void emptySchoolRoundTrips() { - var service = new SchoolManagementService(); - var empty = service.createSchool("Empty"); - var snapshot = service.exportSchool(empty.getId()); + public @Test void emptySchoolRoundTrips() { + final var service = new SchoolManagementService(); + final DanceSchool empty = service.createSchool("Empty"); + final SchoolSnapshot snapshot = service.exportSchool(empty.getId()); service.deleteSchool(empty.getId()); assertEquals( SchoolSnapshot.of(snapshot.restore()), diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SnapshotCollectionsTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SnapshotCollectionsTest.java new file mode 100644 index 0000000..e2ee85d --- /dev/null +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/SnapshotCollectionsTest.java @@ -0,0 +1,103 @@ +package nl.agilecc.agiledancers; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import nl.agilecc.agiledancers.model.DanceGroup; +import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; +import nl.agilecc.agiledancers.model.specifications.DanceGroupStyle; +import nl.agilecc.agiledancers.model.specifications.DanceGroupType; +import nl.agilecc.agiledancers.rest.apimodel.GroupRequest; +import nl.agilecc.agiledancers.rest.apimodel.ShowRequest; +import nl.agilecc.agiledancers.service.DanceGroupService; +import nl.agilecc.agiledancers.service.model.Group; +import nl.agilecc.agiledancers.service.model.GroupDetails; +import nl.agilecc.agiledancers.service.model.Person; +import nl.agilecc.agiledancers.service.model.SchoolSnapshot; +import org.junit.jupiter.api.Test; + +class SnapshotCollectionsTest { + @Test + public void snapshotCopiesCallerOwnedCollectionsAndRetainsInvalidInputForValidation() { + final List dancers = new ArrayList<>(); + final SchoolSnapshot snapshot = + new SchoolSnapshot(2, UUID.randomUUID(), "School", dancers, List.of(), List.of(), List.of()); + dancers.add(new Person("1", "Dancer", null)); + assertEquals(List.of(), snapshot.dancers()); + assertThrows( + UnsupportedOperationException.class, () -> snapshot.dancers().add(null)); + final SchoolSnapshot invalid = + new SchoolSnapshot(2, UUID.randomUUID(), "School", null, List.of(), List.of(), List.of()); + assertNull(invalid.dancers()); + } + + @Test + public void schedulingResultCopiesBothCollectionLevelsAndPreservesMalformedSolverResults() { + final List part = new ArrayList<>(List.of(1L)); + final List> assignments = new ArrayList<>(List.of(part)); + final SchedulingResult result = new SchedulingResult(true, assignments); + part.clear(); + assignments.clear(); + assertEquals(List.of(List.of(1L)), result.assignments()); + assertThrows( + UnsupportedOperationException.class, + () -> result.assignments().getFirst().clear()); + assertNull(new SchedulingResult(false, null).assignments()); + final List> invalid = new ArrayList<>(); + invalid.add(null); + assertNull(new SchedulingResult(false, invalid).assignments().getFirst()); + } + + @Test + public void requestCollectionsRetainTheirJsonValuesAndCannotBeMutatedThroughAliases() { + final Map counts = new LinkedHashMap<>(Map.of(DanceGroupType.REGULAR, 1)); + final ShowRequest show = new ShowRequest("Show", List.of(), counts); + counts.clear(); + assertEquals(1, show.appearances().get(DanceGroupType.REGULAR)); + assertThrows( + UnsupportedOperationException.class, () -> show.appearances().clear()); + final var styles = new LinkedHashSet<>(List.of(DanceGroupStyle.JAZZ)); + final GroupRequest group = new GroupRequest("Group", DanceGroupType.REGULAR, 0, 99, styles); + styles.clear(); + assertEquals(1, group.styles().size()); + assertThrows(UnsupportedOperationException.class, () -> group.styles().clear()); + } + + @Test + public void groupCopiesBothListsWithoutChangingNullableSnapshotValues() { + final List styles = new ArrayList<>(List.of(DanceGroupStyle.JAZZ)); + final List dancerIds = new ArrayList<>(List.of("dancer-1")); + final Group group = new Group("group-1", "Group", DanceGroupType.REGULAR, 0, 99, styles, dancerIds, null); + styles.clear(); + dancerIds.clear(); + assertEquals(List.of(DanceGroupStyle.JAZZ), group.styles()); + assertEquals(List.of("dancer-1"), group.dancerIds()); + assertThrows(UnsupportedOperationException.class, () -> group.styles().clear()); + assertThrows( + UnsupportedOperationException.class, () -> group.dancerIds().clear()); + final Group invalid = new Group("group-2", "Group", DanceGroupType.REGULAR, 0, 99, null, null, null); + assertNull(invalid.styles()); + assertNull(invalid.dancerIds()); + } + + @Test + public void groupDetailsCopiesTheStylesPassedAcrossTheServiceBoundary() { + final var styles = new LinkedHashSet<>(List.of(DanceGroupStyle.JAZZ)); + final GroupDetails details = new GroupDetails("Group", DanceGroupType.REGULAR, 0, 99, styles); + styles.clear(); + assertEquals(1, details.styles().size()); + assertThrows(UnsupportedOperationException.class, () -> details.styles().clear()); + final DanceGroup group = new DanceGroupService().add(details, new LinkedHashSet<>(), null); + group.addStyle(DanceGroupStyle.HIPHOP); + assertEquals(2, group.getStyles().size()); + assertEquals(1, details.styles().size()); + assertNull(new GroupDetails("Group", DanceGroupType.REGULAR, 0, 99, null).styles()); + } +} diff --git a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java index 0d91c0b..0f0270f 100644 --- a/agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java +++ b/agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java @@ -1,23 +1,36 @@ package nl.agilecc.agiledancers.service; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.LocalDate; -import java.util.*; -import java.util.concurrent.*; -import nl.agilecc.agiledancers.model.*; -import nl.agilecc.agiledancers.model.specifications.*; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import nl.agilecc.agiledancers.model.DayPart; +import nl.agilecc.agiledancers.model.Show; +import nl.agilecc.agiledancers.model.scheduling.SchedulingInput; +import nl.agilecc.agiledancers.model.scheduling.SchedulingResult; +import nl.agilecc.agiledancers.model.specifications.DanceGroupType; +import nl.agilecc.agiledancers.model.specifications.DayPartType; +import nl.agilecc.agiledancers.service.model.GroupDetails; +import nl.agilecc.agiledancers.service.model.SchoolSnapshot; import nl.agilecc.agiledancers.service.port.SchedulingEngine; import nl.agilecc.agiledancers.solver.OrToolsSchedulingEngine; import org.junit.jupiter.api.Test; class SchedulingServiceTest { - @Test - void failedAndNoResultRunsPreserveAssignmentsAndReleaseTheRunGuard() { - var schools = new SchoolManagementService(); - long school = schools.createSchool("School").getId(); - schools.createGroup(school, "Group", DanceGroupType.REGULAR, 0, 99, Set.of()); - var show = schools.createShow( + public @Test void failedAndNoResultRunsPreserveAssignmentsAndReleaseTheRunGuard() { + final var schools = new SchoolManagementService(); + final long school = schools.createSchool("School").getId(); + schools.createGroup(school, new GroupDetails("Group", DanceGroupType.REGULAR, 0, 99, Set.of())); + final Show show = schools.createShow( school, "Show", List.of(DayPart.builder() @@ -26,34 +39,31 @@ class SchedulingServiceTest { .danceGroups(List.of()) .build()), null); - var scheduling = new SchedulingService(); - scheduling.schools = schools; - scheduling.engine = new OrToolsSchedulingEngine(); + final var scheduling = new SchedulingService(schools, new OrToolsSchedulingEngine()); scheduling.schedule(school, show.getId()); - var before = schools.exportSchool(school); - for (SchedulingEngine failure : List.of( - input -> { + final SchoolSnapshot before = schools.exportSchool(school); + for (final SchedulingEngine failure : List.of( + (final SchedulingInput input) -> { throw new IllegalStateException("Native failure"); }, - input -> { + (final SchedulingInput input) -> { throw new ManagementException( ManagementExceptionKind.INVALID_INPUT, "No valid result before the limit"); }, - input -> null)) { - scheduling.engine = failure; - assertThrows(ManagementException.class, () -> scheduling.schedule(school, show.getId())); + (final SchedulingInput input) -> null)) { + final var failedScheduling = new SchedulingService(schools, failure); + assertThrows(ManagementException.class, () -> failedScheduling.schedule(school, show.getId())); assertEquals(before, schools.exportSchool(school)); - var next = schools.beginScheduling(school, show.getId()); + final SchedulingInput next = schools.beginScheduling(school, show.getId()); schools.endScheduling(next.showId()); } } - @Test - void concurrentRunIsRejectedWhileEditsRemainPossibleAndInvalidateTheFirstResult() throws Exception { - var schools = new SchoolManagementService(); - long school = schools.createSchool("School").getId(); - schools.createGroup(school, "Group", DanceGroupType.REGULAR, 0, 99, Set.of()); - var show = schools.createShow( + public @Test void concurrentRunIsRejectedWhileEditsRemainPossibleAndInvalidateTheFirstResult() throws Exception { + final var schools = new SchoolManagementService(); + final long school = schools.createSchool("School").getId(); + schools.createGroup(school, new GroupDetails("Group", DanceGroupType.REGULAR, 0, 99, Set.of())); + final Show show = schools.createShow( school, "Show", List.of(DayPart.builder() @@ -62,39 +72,45 @@ class SchedulingServiceTest { .danceGroups(List.of()) .build()), null); - var scheduling = new SchedulingService(); - scheduling.schools = schools; - var entered = new CountDownLatch(1); - var release = new CountDownLatch(1); - scheduling.engine = input -> { - entered.countDown(); - try { - if (!release.await(10, TimeUnit.SECONDS)) throw new IllegalStateException("Test timeout"); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IllegalStateException(e); - } - return new OrToolsSchedulingEngine().solve(input); - }; - try (var executor = Executors.newSingleThreadExecutor()) { - var first = executor.submit(() -> scheduling.schedule(school, show.getId())); + final var entered = new CountDownLatch(1); + final var release = new CountDownLatch(1); + final SchedulingEngine engine = blockingEngine(entered, release); + final var scheduling = new SchedulingService(schools, engine); + try (final java.util.concurrent.ExecutorService executor = Executors.newSingleThreadExecutor()) { + final Future first = executor.submit(() -> scheduling.schedule(school, show.getId())); try { assertTrue(entered.await(10, TimeUnit.SECONDS)); assertEquals( ManagementExceptionKind.CONFLICT, assertThrows(ManagementException.class, () -> scheduling.schedule(school, show.getId())) .getKind()); - schools.createGroup(school, "New group", DanceGroupType.REGULAR, 0, 99, Set.of()); + schools.createGroup(school, new GroupDetails("New group", DanceGroupType.REGULAR, 0, 99, Set.of())); } finally { release.countDown(); } - var exception = assertThrows(ExecutionException.class, () -> first.get(10, TimeUnit.SECONDS)); + final ExecutionException exception = + assertThrows(ExecutionException.class, () -> first.get(10, TimeUnit.SECONDS)); assertInstanceOf(ManagementException.class, exception.getCause()); assertTrue(exception.getCause().getMessage().contains("inputs changed")); assertTrue(show.getDayParts().getFirst().getDanceGroups().isEmpty()); } - scheduling.engine = new OrToolsSchedulingEngine(); - scheduling.schedule(school, show.getId()); + new SchedulingService(schools, new OrToolsSchedulingEngine()).schedule(school, show.getId()); assertEquals(2, show.getDayParts().getFirst().getDanceGroups().size()); } + + private static SchedulingEngine blockingEngine(final CountDownLatch entered, final CountDownLatch release) { + final SchedulingEngine engine = (final SchedulingInput input) -> { + entered.countDown(); + try { + if (!release.await(10, TimeUnit.SECONDS)) { + throw new IllegalStateException("Test timeout"); + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + return new OrToolsSchedulingEngine().solve(input); + }; + return engine; + } } diff --git a/agiledancers-be/src/test/java/sit/DanceSchoolSit.java b/agiledancers-be/src/test/java/sit/DanceSchoolSit.java index 20751ac..3fa2976 100644 --- a/agiledancers-be/src/test/java/sit/DanceSchoolSit.java +++ b/agiledancers-be/src/test/java/sit/DanceSchoolSit.java @@ -6,19 +6,18 @@ import nl.agilecc.agiledancers.service.DanceSchoolService; import org.junit.jupiter.api.Test; class DanceSchoolSit { - Faker faker = new Faker(); - int numberOfDanceGroups = 42; + private final Faker faker = new Faker(); + private final int numberOfDanceGroups = 42; - int aantalDansersTm5 = 10; - int aantalDansers6tm10 = 30; - int aantalDansers11tot14 = 70; - int aantalDansers14tm17 = 140; - int aantalDansers18Plus = 100; + private final int aantalDansersTm5 = 10; + private final int aantalDansers6tm10 = 30; + private final int aantalDansers11tot14 = 70; + private final int aantalDansers14tm17 = 140; + private final int aantalDansers18Plus = 100; - private DanceSchoolService danceSchoolService = new DanceSchoolService(); + private final DanceSchoolService danceSchoolService = new DanceSchoolService(); - @Test - void testDanceSchoolModel() { - DanceSchool danceSchool = danceSchoolService.add("24 Dance Studio"); + public @Test void testDanceSchoolModel() { + final DanceSchool danceSchool = danceSchoolService.add("24 Dance Studio"); } } diff --git a/ci/Pipeline.java b/ci/Pipeline.java index b5ea06c..447ca7e 100644 --- a/ci/Pipeline.java +++ b/ci/Pipeline.java @@ -148,6 +148,10 @@ class Pipeline { "1g", "--cpus", "2", + "--volume", + root.resolve("agiledancers-be/target/coverage-tools/agent.jar") + ":/jacoco-agent.jar:ro", + "--env", + "JAVA_TOOL_OPTIONS=-javaagent:/jacoco-agent.jar=output=tcpserver,address=*,port=6300", backendImage); run( "docker", @@ -202,6 +206,37 @@ class Pipeline { "npm", "run", "e2e"); + run( + "docker", + "run", + "--rm", + "--network", + runName, + "--user", + user, + "--volume", + root.resolve("agiledancers-be/target") + ":/coverage", + "eclipse-temurin:25-jre", + "java", + "-jar", + "/coverage/coverage-tools/cli.jar", + "dump", + "--address", + "backend", + "--destfile", + "/coverage/playwright.exec"); + check( + Files.size(root.resolve("agiledancers-be/target/playwright.exec")) > 0, + "Playwright backend coverage is missing"); + run( + "mvn", + "-B", + "-ntp", + "-pl", + "agiledancers-be", + "jacoco:merge@combined", + "jacoco:report@combined-report", + "exec:exec@combined-coverage"); Files.writeString(artifacts.resolve("verified-commit"), commit + "\n"); } catch (Exception e) { failure = e; @@ -270,6 +305,10 @@ class Pipeline { List failures = new ArrayList<>(); for (String path : List.of( "agiledancers-be/target/surefire-reports", + "agiledancers-be/target/standards", + "tooling/standards/target/surefire-reports", + "agiledancers-be/target/site/jacoco", + "agiledancers-be/target/site/jacoco-combined", "agiledancers-be/target/failsafe-reports", "agiledancers-fe/unit-results", "agiledancers-fe/test-results", @@ -282,6 +321,11 @@ class Pipeline { } } } + for (String data : List.of("jacoco.exec", "playwright.exec", "jacoco-combined.exec")) { + final Path source = root.resolve("agiledancers-be/target").resolve(data); + if (Files.isRegularFile(source)) + Files.copy(source, artifacts.resolve(data), StandardCopyOption.REPLACE_EXISTING); + } if (result(false, "docker", "info").code() == 0) { for (String role : List.of("backend", "frontend", "browser")) { String container = runName + "-" + role; diff --git a/docs/backend-review.md b/docs/backend-review.md new file mode 100644 index 0000000..cdddb86 --- /dev/null +++ b/docs/backend-review.md @@ -0,0 +1,16 @@ +# Backend review companion + +Use alongside `skills/code-review/SKILL.md` and `CODING_STANDARDS.md`. + +Review changed code for intent-revealing comments, missing public-contract Javadoc, +appropriate record use, defensive copying of mutable record components, and +compatibility with APIs, JSON and persistence. Assess ambiguous `var` initializers +and framework/lifecycle exceptions to field finality. Confirm architecture and +injection decisions make sense for production and tests. + +Read standards reports even when verification passes. Assess every review signal +in changed code, including fan-out, public methods and record component counts. +Record a rationale for retained signals in the S7 evidence or change description. +Check narrowly scoped waivers against recorded maintainer agreement and reason. +Never introduce blanket exclusions or weaken thresholds to pass a build. +Do not repeat mechanical checks already enforced by tooling; verify their result. diff --git a/specs/S7-review-signals.txt b/specs/S7-review-signals.txt new file mode 100644 index 0000000..bf1ead6 --- /dev/null +++ b/specs/S7-review-signals.txt @@ -0,0 +1,54 @@ +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/GroupRequirement.java:3 recordComponents=2 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/SchedulingInput.java:5 recordComponents=4 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/scheduling/SchedulingResult.java:7 recordComponents=2 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DancerRequest.java:8 recordComponents=2 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DancerView.java:6 recordComponents=3 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DayPartRequest.java:7 recordComponents=2 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/DayPartView.java:9 recordComponents=3 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ErrorView.java:6 recordComponents=1 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupRequest.java:12 recordComponents=5 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupView.java:13 recordComponents=7 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchedulingView.java:3 recordComponents=2 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchoolRequest.java:5 recordComponents=1 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/SchoolView.java:11 recordComponents=5 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowRequest.java:13 recordComponents=3 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/ShowView.java:13 recordComponents=4 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Group.java:9 recordComponents=8 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/GroupDetails.java:9 recordComponents=5 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/ImportPreview.java:3 recordComponents=6 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Part.java:9 recordComponents=3 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Performance.java:10 recordComponents=4 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Person.java:5 recordComponents=3 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java:27 recordComponents=7 (manual design review) +DESIGN agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Trainer.java:3 recordComponents=2 (manual design review) +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceGroup.java:18 fanOut=16.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/model/DanceSchool.java:16 fanOut=16.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceGroupResource.java:28 fanOut=21.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DanceSchoolResource.java:27 fanOut=20.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/DancerResource.java:27 fanOut=20.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/SchoolSnapshotResource.java:26 fanOut=25.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/ShowResource.java:32 fanOut=25.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/rest/apimodel/GroupView.java:21 parameters=7.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java:102 cyclomatic=8.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java:282 executableLines=33.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java:282 parameters=5.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java:35 fanOut=35.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/SchoolManagementService.java:35 publicMethods=28.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java:136 executableLines=37.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java:174 executableLines=35.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java:27 fanOut=29.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java:35 parameters=7.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/SchoolSnapshot.java:42 executableLines=41.0 +REVIEW agiledancers-be/src/main/java/nl/agilecc/agiledancers/solver/OrToolsSchedulingEngine.java:20 fanOut=17.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchedulingTest.java:29 fanOut=22.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java:123 executableLines=44.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java:168 executableLines=38.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolManagementResourceTest.java:26 executableLines=39.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotResourceTest.java:16 executableLines=40.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotTest.java:30 fanOut=19.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SchoolSnapshotTest.java:56 executableLines=32.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/SnapshotCollectionsTest.java:26 fanOut=22.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java:28 fanOut=28.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java:29 executableLines=32.0 +REVIEW agiledancers-be/src/test/java/nl/agilecc/agiledancers/service/SchedulingServiceTest.java:62 executableLines=38.0 +WAIVER agiledancers-be/src/main/java/nl/agilecc/agiledancers/service/model/Group.java:19 parameters=8.0 Maintainer approved in the S7 implementation conversation: preserve the eight-component version-2 JSON shape while defensively copying styles and dancerIds. diff --git a/specs/S7-validation.md b/specs/S7-validation.md new file mode 100644 index 0000000..437f45c --- /dev/null +++ b/specs/S7-validation.md @@ -0,0 +1,113 @@ +# S7 implementation and validation evidence + +Status: IN_REVIEW — implementation complete; awaiting project review. +Date: 2026-10-30. Environment: Ubuntu amd64, OpenJDK 25.0.4.1, Maven 3.9.16. + +## Delivered changes + +Enforcement/configuration: root coding standards and agent discovery instructions; +project review companion (upstream skills unchanged); individually selected +Checkstyle 12.3.1 checks; PMD/CPD 7.28.0 with targeted measurements and regression +fixtures; JaCoCo 0.8.14 with exact line-count gates; verify integration; Docker +build inputs; CI instrumentation, merge and diagnostic retention. No broad +standards/coverage exclusions or unrelated rule bundles were introduced. + +Migration: explicit imports and visibility, required finality and explicit types; +an explicit collaborator constructor for scheduling tests while preserving CDI field injection and the public no-argument constructor; smaller snapshot-restoration, +import-rebuilding, solver and validation methods; smaller HTTP test helpers; +null-tolerant defensive collection copies for records, including both levels of +solver assignments. Existing entity classes retain their mutation, Lombok and +framework contracts. API paths, payloads, snapshot version and persistence behavior +are unchanged. Enforcement and migration can be reviewed separately by path. + +## Scope, tools and measurements + +58 handwritten production files and 9 handwritten test files are checked. +Generated target output, frontend code and Java helpers outside the backend are +outside that standards scope. All handwritten production packages participate in +coverage, including DTOs and configuration. JaCoCo's generated-member filtering is +used; no handwritten class/package exclusions are configured. + +See CODING_STANDARDS.md for complete targets, strict thresholds, algorithm options, +manual rules, scope and standalone commands. The full current metrics report is +`agiledancers-be/target/standards/report.txt`. Production CPD currently reports +**0%** duplicated code lines at its 100-token minimum. Test duplication is not gated. + +The selected PMD finality rules complement custom checks. PMD cyclomatic and +cognitive visitors are scoped explicitly: lambdas contribute, nested/local type +methods are independent, and no aggregate class complexity gate is introduced. +Resolved collaborator identity includes qualifier values/defaults and inherited +generic type substitution. Exact metric waivers are supported and regression tested; +`tooling/standards/waivers.tsv` records the sole approved waiver: the exact +eight-parameter Group constructor needed to preserve its version-2 JSON shape. + +## Reproducible validation + +Set JAVA_HOME to JDK 25 and run from the repository root. + +| Command/check | Result | +| --- | --- | +| `mvn -Dformat.mode=check clean verify` | Passed: 38 backend tests, 17 checker tests, standards checks and the exact backend coverage gate. | +| `mvn -pl agiledancers-be package -DskipTests` | Passed; Quarkus packaged app successfully produced and subsequently exercised by Playwright. | +| `mvn -f tooling/standards/pom.xml test exec:java` | 17 checker tests passed; no unwaived blocking violations. | +| `java tooling/coverage/Coverage.java agiledancers-be/target/site/jacoco/jacoco.xml` | Clean backend-only line gate passed: 791 covered / 818 total = 96.69926650366749%. | +| `PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/google/chrome/chrome npm run e2e` in frontend | Both browser tests passed against the packaged instrumented backend. | +| Pinned CLI `dump --address 127.0.0.1 --destfile agiledancers-be/target/playwright.exec` | Successfully collected execution data from the agent's TCP server. | +| `mvn -pl agiledancers-be jacoco:merge@combined jacoco:report@combined-report exec:exec@combined-coverage` | Passed with matching clean-build application classes: combined 791/818 lines, 96.69926650366749%. | +| `java ci/Ci.java test` | 17 CI orchestration tests passed. | +| `git diff --check` | Passed. | + +Both local and combined branch reports contain 195 covered / 257 total branches +(75.87548638132296%); branches have no blocking percentage. Browser coverage is +successfully collected and merged, but does not add uncovered lines in this run. +Local and combined HTML/XML/CSV reports are under `agiledancers-be/target/site`. +Execution data is in `target/jacoco.exec`, `target/playwright.exec` and +`target/jacoco-combined.exec` within the backend module. CI copies these reports, +execution data and checker results into its retained diagnostics. Local validation +exercised collection and merge; a remote Gitea publication run was not performed. + +The checker fixtures cover strict complexity, length, parameter, nesting, +inheritance and dependency boundaries; implicit/compact record constructors; +lambda/nested-type scope; comments/brace lines; distinct type fan-out; nontrivial +accessors; qualifiers, repeated and inherited injection; exact waivers; parser +failure diagnostics; report-only behavior; CPD occurrence/overlap accounting and +test exclusion; and the unrounded 80% line gate, including a near miss. + +## Retained review signals + +`S7-review-signals.txt` records the current individual REVIEW and DESIGN entries. +The following assessment covers every retained review-signal group. The approved +constructor waiver is documented separately below. + +| Signal/location | Assessment and reason retained | +| --- | --- | +| DanceGroup and DanceSchool fan-out 16 | Domain collection/entity references and explicitly used Lombok annotations are intrinsic to these mutable aggregates. Converting them to records would break their mutation/inheritance contracts. | +| Five REST resources, fan-out 20–25 | HTTP annotations, response/request types and service/domain mapping account for the references. These are thin adapters; splitting them to reduce annotation counts would obscure endpoint ownership. | +| GroupView and SchoolSnapshot compact constructors, 7 parameters | Their components preserve existing JSON contracts. Constructors only protect collections and are within the allowed constructor limit. | +| SchoolManagementService fan-out 35, public methods 28 | One shared synchronization boundary owns in-memory aggregate writes and scheduling revisions. Splitting state/locks needs its own concurrency design; the class passes blocking length/dependency limits. | +| validateAssignments cyclomatic 8 | Independent input/result guards and required-appearance validation are explicit and tested; per-part validation has already been extracted. It remains below the blocking limit. | +| updateShow, 33 lines and 5 parameters | Retains assignments by matching daypart keys before replacing the aggregate. Keeping the transaction visible and preserving its existing signature is preferable to artificial wrappers; both blocking limits pass. | +| SchoolSnapshot fan-out 29; of 41 lines, restoreGroups 37, restoreShows 35 | Each method performs one graph conversion phase. Explicit type/reference checks protect imported data and preserve assignment identity. Refactoring has removed the over-limit monolithic restore method. | +| Solver fan-out 17 | OR-Tools types are confined to this adapter and its dedicated helpers. Further indirection would spread solver details without improving the domain boundary. | +| SchedulingTest, SchoolSnapshotTest, SnapshotCollectionsTest and SchedulingServiceTest fan-out | These behavior tests intentionally exercise multiple collaborating models, services, ports, exceptions and assertions. Production architecture remains covered separately by ArchUnit. | +| HTTP, snapshot and scheduling test methods at 32–44 lines | Arrange/action/assert and try/finally cleanup remain together. Repeated setup was extracted where methods exceeded 50 physical code lines; retained methods pass all blocking limits. | +| All DESIGN record-component entries | Request/view/snapshot/domain records model existing named contracts; their sizes are not automatically gated. Mutable components are copied, including both Group lists under its approved constructor waiver. Framework-backed mutable entities remain classes. | + +## Approved compatibility decisions + +The maintainer approved both proposals in the S7 implementation conversation: + +1. `GroupDetails` groups category, type, age bounds and styles. The Java signatures + of `DanceGroupService.add`, `SchoolManagementService.createGroup` and + `updateGroup` now take 3, 2 and 3 parameters respectively. All callers were + migrated. REST endpoints, JSON payloads and snapshot version remain unchanged. + The record copies its styles, and service construction takes a mutable copy + for the domain group; regression coverage verifies later style edits still work. +2. The explicit eight-component `service.model.Group` constructor defensively + copies styles and dancer IDs while preserving null values for existing import + validation. Its sole waiver matches the fully resolved constructor signature, + metric and exact retained value 8. The agreement and reason are recorded in + `tooling/standards/waivers.tsv` and the result remains visible as `WAIVER`. + +There are no parameter waivers for the three service methods, blanket suppressions +or weakened thresholds. The completed specification resides in `specs/in_review`. diff --git a/specs/refined/S7_define-coding-standards-to-be b/specs/in_review/S7_define-coding-standards-to-be similarity index 90% rename from specs/refined/S7_define-coding-standards-to-be rename to specs/in_review/S7_define-coding-standards-to-be index c5cf45a..87a1256 100644 --- a/specs/refined/S7_define-coding-standards-to-be +++ b/specs/in_review/S7_define-coding-standards-to-be @@ -1,6 +1,6 @@ # S7 — Backend coding standards -Status: REFINED +Status: IN_REVIEW Shared understanding confirmed after the grill-me review. @@ -176,21 +176,21 @@ exactly at a blocking limit passes. ## Acceptance criteria -- [ ] CODING_STANDARDS.md contains all agreed rules and preserves frontend guidance. -- [ ] Agent and project code-review instructions enforce discovery and review of +- [x] CODING_STANDARDS.md contains all agreed rules and preserves frontend guidance. +- [x] Agent and project code-review instructions enforce discovery and review of these standards without modifying upstream skills. -- [ ] Pinned tools, configurations and targeted checks implement the exact agreed +- [x] Pinned tools, configurations and targeted checks implement the exact agreed metrics and identify manual-only rules. -- [ ] Verify and standalone checks report actionable blocking failures and review +- [x] Verify and standalone checks report actionable blocking failures and review signals; CI prevents publication after a failure. -- [ ] Representative violations demonstrate that their checks fail, including +- [x] Representative violations demonstrate that their checks fail, including relevant threshold boundaries and custom measurement semantics. -- [ ] Existing handwritten backend production and test code passes all blocking +- [x] Existing handwritten backend production and test code passes all blocking rules; retained review signals and approved waivers are documented. -- [ ] Local backend tests and combined CI coverage meet the agreed line-coverage +- [x] Local backend tests and combined CI coverage meet the agreed line-coverage gates; line and branch reports are available. -- [ ] Clean verification and behavior-preservation tests pass after migration. -- [ ] Evidence records tool versions, commands, scope, exclusions, metric and +- [x] Clean verification and behavior-preservation tests pass after migration. +- [x] Evidence records tool versions, commands, scope, exclusions, metric and coverage reports, approved waivers and verification results. ## Implementation handoff @@ -198,3 +198,14 @@ exactly at a blocking limit passes. This specification records the agreed design. Updating it does not implement checks, change agent instructions, refactor application code or establish current coverage. Those are implementation work, with evidence required above. + +## Implementation evidence + +Implementation and verification are recorded in `specs/S7-validation.md`; retained +review signals and the sole approved waiver are recorded in +`specs/S7-review-signals.txt` and `tooling/standards/waivers.tsv`. + +The maintainer approved GroupDetails-based Java service signatures and the exact +eight-parameter Group constructor waiver in the implementation conversation. +REST/JSON contracts remain unchanged. Clean verify and browser/combined coverage +checks pass. The implementation is ready for project review. diff --git a/tooling/coverage/Coverage.java b/tooling/coverage/Coverage.java new file mode 100644 index 0000000..5ef2be3 --- /dev/null +++ b/tooling/coverage/Coverage.java @@ -0,0 +1,30 @@ +import java.nio.file.Path; +import javax.xml.parsers.DocumentBuilderFactory; +import org.w3c.dom.Element; + +/** Exact module-wide line gate; consumes JaCoCo XML without rounding percentages. */ +public class Coverage { + public static void main(String[] args) throws Exception { + final var factory = DocumentBuilderFactory.newInstance(); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + final Element report = + factory.newDocumentBuilder().parse(Path.of(args[0]).toFile()).getDocumentElement(); + for (var child = report.getFirstChild(); child != null; child = child.getNextSibling()) { + if (child instanceof Element counter + && counter.getTagName().equals("counter") + && counter.getAttribute("type").equals("LINE")) { + final long covered = Long.parseLong(counter.getAttribute("covered")); + final long missed = Long.parseLong(counter.getAttribute("missed")); + final long total = covered + missed; + System.out.println("Backend line coverage: " + covered + "/" + total + " (" + + 100.0 * covered / total + "%)"); + if (total == 0 || 5 * covered < 4 * total) + throw new IllegalStateException("Backend line coverage must reach 80%: " + args[0]); + return; + } + } + throw new IllegalStateException("Missing module LINE counter: " + args[0]); + } +} diff --git a/tooling/standards/checkstyle.xml b/tooling/standards/checkstyle.xml new file mode 100644 index 0000000..621373e --- /dev/null +++ b/tooling/standards/checkstyle.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/tooling/standards/pom.xml b/tooling/standards/pom.xml new file mode 100644 index 0000000..6fcec2d --- /dev/null +++ b/tooling/standards/pom.xml @@ -0,0 +1,16 @@ + + 4.0.0 + nl.agileccbackend-standards1 + 25UTF-8 + + jakarta.injectjakarta.inject-api2.0.1test + net.sourceforge.pmdpmd-java7.28.0 + com.puppycrawl.toolscheckstyle12.3.1 + org.junit.jupiterjunit-jupiter5.14.1test + + + org.apache.maven.pluginsmaven-compiler-plugin3.16.0 + org.apache.maven.pluginsmaven-surefire-plugin3.6.1 + org.codehaus.mojoexec-maven-plugin3.6.3Standards${project.basedir}/../.. + + diff --git a/tooling/standards/src/main/java/Standards.java b/tooling/standards/src/main/java/Standards.java new file mode 100644 index 0000000..98c4e2c --- /dev/null +++ b/tooling/standards/src/main/java/Standards.java @@ -0,0 +1,588 @@ +import com.puppycrawl.tools.checkstyle.*; +import com.puppycrawl.tools.checkstyle.api.*; +import java.io.*; +import java.nio.file.*; +import java.util.*; +import net.sourceforge.pmd.*; +import net.sourceforge.pmd.cpd.*; +import net.sourceforge.pmd.lang.*; +import net.sourceforge.pmd.lang.ast.*; +import net.sourceforge.pmd.lang.java.ast.*; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.symbols.*; +import net.sourceforge.pmd.lang.java.types.*; +import net.sourceforge.pmd.lang.metrics.MetricOptions; +import net.sourceforge.pmd.lang.rule.RuleSet; + +/** Deliberately independent of the application build and its annotation processors. */ +public class Standards { + static final List findings = new ArrayList<>(); + static final Map> codeLines = new HashMap<>(); + static final Set ownedTypes = new HashSet<>(); + static final Map types = new HashMap<>(); + static final List units = new ArrayList<>(); + static Path root; + + record Waiver(String metric, String executable, double value, String reason) {} + + static final List waivers = new ArrayList<>(); + + public static void main(String[] args) throws Exception { + findings.clear(); + codeLines.clear(); + types.clear(); + units.clear(); + ownedTypes.clear(); + root = Path.of(args[0]).toAbsolutePath().normalize(); + waivers.clear(); + final Path waiverFile = root.resolve("tooling/standards/waivers.tsv"); + if (Files.exists(waiverFile)) + for (String line : Files.readAllLines(waiverFile)) { + if (line.isBlank() || line.startsWith("#")) continue; + final String[] fields = line.split("\t", -1); + if (fields.length != 4 + || fields[0].contains("*") + || fields[1].contains("*") + || fields[3].isBlank()) + throw new IllegalArgumentException("Invalid exact waiver in " + waiverFile + ": " + line); + waivers.add(new Waiver(fields[0], fields[1], Double.parseDouble(fields[2]), fields[3])); + } + final Path output = root.resolve("agiledancers-be/target/standards"); + Files.createDirectories(output); + final List files = new ArrayList<>(); + for (String scope : List.of("main", "test")) { + try (var paths = Files.walk(root.resolve("agiledancers-be/src/" + scope + "/java"))) { + paths.filter(p -> p.toString().endsWith(".java")).sorted().forEach(files::add); + } + } + for (Path file : files) { + final var matcher = java.util.regex.Pattern.compile("package\\s+([\\w.]+)\\s*;") + .matcher(Files.readString(file)); + if (matcher.find()) + ownedTypes.add(matcher.group(1) + "." + file.getFileName().toString().replace(".java", "")); + } + checkstyle(files); + final PMDConfiguration config = new PMDConfiguration(); + config.setDefaultLanguageVersion( + LanguageRegistry.PMD.getLanguageById("java").getVersion("25")); + config.setThreads(1); + config.prependAuxClasspath( + Path.of(System.getProperty("java.home"), "lib/jrt-fs.jar").toString()); + config.setIgnoreIncrementalAnalysis(true); + // Compiled application/test dependencies permit PMD to resolve external types. + final Path cp = root.resolve("agiledancers-be/target/standards-classpath.txt"); + if (Files.exists(cp)) + config.prependAuxClasspath(Files.readString(cp) + + File.pathSeparator + + root.resolve("agiledancers-be/target/classes")); + try (var analysis = PmdAnalysis.create(config)) { + analysis.addRuleSet(RuleSet.forSingleRule(new Measurements())); + for (String rule : List.of( + "category/java/codestyle.xml/LocalVariableCouldBeFinal", + "category/java/codestyle.xml/MethodArgumentCouldBeFinal", + "category/java/design.xml/ImmutableField")) + analysis.addRuleSet(analysis.newRuleSetLoader().loadFromResource(rule)); + for (Path file : files) analysis.files().addFile(file); + final var report = analysis.performAnalysisAndCollectReport(); + for (var error : report.getProcessingErrors()) + findings.add("BLOCK parser " + error.getFileId() + " " + error.getMsg()); + for (var violation : report.getViolations()) + findings.add("BLOCK " + violation.getFileId() + ":" + violation.getBeginLine() + " " + + violation.getRule().getName() + " " + violation.getDescription()); + for (var violation : report.getSuppressedViolations()) + findings.add("BLOCK Unapproved suppression: " + violation); + for (var error : report.getConfigurationErrors()) + findings.add("BLOCK configuration " + error); + } + duplication(files); + Collections.sort(findings); + Files.write(output.resolve("report.txt"), findings); + findings.stream().filter(f -> !f.startsWith("METRIC ")).forEach(System.out::println); + System.out.println("Backend standards: " + files.size() + " source files; full metrics: " + + output.resolve("report.txt")); + if (findings.stream().anyMatch(s -> s.startsWith("BLOCK"))) + throw new IllegalStateException( + "Backend standards failed; see " + output.resolve("report.txt")); + } + + static void checkstyle(List files) throws Exception { + final Checker checker = new Checker(); + checker.setModuleClassLoader(Standards.class.getClassLoader()); + checker.configure(ConfigurationLoader.loadConfiguration( + root.resolve("tooling/standards/checkstyle.xml").toString(), + new PropertiesExpander(new Properties()))); + checker.addListener(new AuditListener() { + public void auditStarted(AuditEvent e) {} + + public void auditFinished(AuditEvent e) {} + + public void fileStarted(AuditEvent e) {} + + public void fileFinished(AuditEvent e) {} + + public void addError(AuditEvent e) { + findings.add("BLOCK " + e.getFileName() + ":" + e.getLine() + " " + e.getMessage()); + } + + public void addException(AuditEvent e, Throwable t) { + findings.add("BLOCK " + e.getFileName() + " parser: " + t); + } + }); + try { + checker.process(files.stream().map(Path::toFile).toList()); + } finally { + checker.destroy(); + } + } + + public static class Measurements extends AbstractJavaRule { + public Measurements() { + setName("BackendMeasurements"); + setMessage("Backend standards"); + setLanguage(LanguageRegistry.PMD.getLanguageById("java")); + } + + @Override + public Object visit(ASTCompilationUnit unit, Object data) { + units.add(unit); + for (var type : all(unit, ASTTypeDeclaration.class)) + types.put(type.getSymbol().getBinaryName(), type); + final Set lines = lines(unit); + codeLines.put(path(unit), lines); + for (var node : all(unit, JavaNode.class)) { + if (node instanceof ASTExecutableDeclaration executable) executable(executable); + if (node instanceof ASTCompactConstructorDeclaration compact) { + final var cyclo = new org.apache.commons.lang3.mutable.MutableInt(1); + compact.acceptVisitor(new ScopedCyclo(compact), cyclo); + final var cognitive = + new net.sourceforge.pmd.lang.java.metrics.internal.CognitiveComplexityVisitor.State( + compact); + compact.acceptVisitor(new ScopedCognitive(), cognitive); + metric(compact, "cyclomatic", cyclo.intValue(), 7, 10); + metric(compact, "cognitive", cognitive.getComplexity(), 10, 15); + metric(compact, "executableLines", lines(compact).size(), 30, 50); + metric(compact, "nesting", nesting(compact.getBody(), 0), 3, 4); + metric( + compact, + "parameters", + compact.getEnclosingType().getRecordComponents().size(), + 5, + 7); + visibility(compact); + } + if (node instanceof ASTVariableId variable) variable(variable); + if (node instanceof ASTTypeDeclaration nested + && nested.isNested() + && !nested.isLocal() + && !nested.isAnonymous()) visibility(nested); + if (node instanceof ASTImportDeclaration imp && imp.isStatic() && production(unit)) + block(imp, "Static imports are test-only"); + if (node instanceof ASTExecutableDeclaration || node instanceof ASTFieldDeclaration) + visibility((ModifierOwner) node); + if (node instanceof ASTMethodDeclaration method + && method.isOverride() + && !method.isAnnotationPresent("java.lang.Override")) + block(method, "Overriding methods require @Override"); + } + for (var type : all(unit, ASTTypeDeclaration.class)) typeMetrics(type); + return data; + } + } + + static Path path(Node node) { + return Path.of(node.getTextDocument().getFileId().getAbsolutePath()) + .toAbsolutePath() + .normalize(); + } + + static boolean production(Node node) { + return path(node).startsWith(root.resolve("agiledancers-be/src/main/java")); + } + + static List all(Node node, Class clazz) { + final List result = new ArrayList<>(); + if (clazz.isInstance(node)) result.add(clazz.cast(node)); + for (int i = 0; i < node.getNumChildren(); i++) result.addAll(all(node.getChild(i), clazz)); + return result; + } + + static void block(Node node, String message) { + findings.add("BLOCK " + location(node) + " " + message); + } + + static String location(Node node) { + return root.relativize(path(node)) + ":" + node.getBeginLine(); + } + + static void metric(Node node, String name, double value, double review, double limit) { + if (value > limit) + for (Waiver waiver : waivers) { + if (waiver.metric().equals(name) + && waiver.executable().equals(identity(node)) + && waiver.value() == value) { + findings.add( + "WAIVER " + location(node) + " " + name + "=" + value + " " + waiver.reason()); + return; + } + } + findings.add((value > limit ? "BLOCK " : value > review ? "REVIEW " : "METRIC ") + + location(node) + " " + name + "=" + value); + } + + static String identity(Node node) { + if (node instanceof ASTCompactConstructorDeclaration compact) { + final List parameters = new ArrayList<>(); + for (var component : compact.getEnclosingType().getRecordComponents()) + parameters.add(component.getTypeMirror().toString()); + return compact.getEnclosingType().getBinaryName() + "#(" + String.join(",", parameters) + + ")"; + } + if (node instanceof ASTExecutableDeclaration executable) { + final List parameters = new ArrayList<>(); + for (var parameter : executable.getFormalParameters()) + parameters.add(parameter.getTypeMirror().toString()); + return executable.getEnclosingType().getBinaryName() + "#" + + (executable instanceof ASTConstructorDeclaration ? "" : executable.getName()) + + "(" + String.join(",", parameters) + ")"; + } + return node instanceof ASTTypeDeclaration type ? type.getBinaryName() : location(node); + } + + static Set lines(JavaNode node) { + final Set result = new HashSet<>(); + var token = node.getFirstToken(); + while (token != null && !token.isEof()) { + final var loc = token.getReportLocation(); + for (int line = loc.getStartLine(); line <= loc.getEndLine(); line++) result.add(line); + if (token == node.getLastToken()) break; + token = token.getNext(); + } + final String[] sourceLines = node.getTextDocument().getText().toString().split("\\R", -1); + result.removeIf(line -> line > sourceLines.length || sourceLines[line - 1].isBlank()); + return result; + } + + static void executable(ASTExecutableDeclaration method) { + { + metric( + method, + "parameters", + method.getArity(), + method instanceof ASTMethodDeclaration ? 4 : 5, + method instanceof ASTMethodDeclaration ? 5 : 7); + } + if (method.getBody() == null) return; + metric(method, "cyclomatic", cyclomatic(method), 7, 10); + metric(method, "cognitive", cognitive(method), 10, 15); + metric(method, "executableLines", lines(method).size(), 30, 50); + metric(method, "nesting", nesting(method.getBody(), 0), 3, 4); + } + + static final class ScopedCyclo + extends net.sourceforge.pmd.lang.java.metrics.internal.CycloVisitor { + ScopedCyclo(JavaNode node) { + super(MetricOptions.emptyOptions(), node); + } + + @Override + public Void visit( + ASTLambdaExpression lambda, org.apache.commons.lang3.mutable.MutableInt value) { + for (int i = 0; i < lambda.getNumChildren(); i++) + lambda.getChild(i).acceptVisitor(this, value); + return null; + } + } + + static final class ScopedCognitive + extends net.sourceforge.pmd.lang.java.metrics.internal.CognitiveComplexityVisitor { + @Override + public Void visit(ASTClassBody body, State state) { + return null; + } + + @Override + public Void visitJavaNode(JavaNode node, State state) { + return node instanceof ASTTypeDeclaration || node instanceof ASTTypeBody + ? null + : super.visitJavaNode(node, state); + } + } + + static int cyclomatic(ASTExecutableDeclaration method) { + final var value = new org.apache.commons.lang3.mutable.MutableInt(); + method.acceptVisitor(new ScopedCyclo(method), value); + return value.intValue(); + } + + static int cognitive(ASTExecutableDeclaration method) { + final var state = + new net.sourceforge.pmd.lang.java.metrics.internal.CognitiveComplexityVisitor.State(method); + method.acceptVisitor(new ScopedCognitive(), state); + return state.getComplexity(); + } + + static int nesting(Node node, int depth) { + if (node instanceof ASTTypeDeclaration) return depth; + final String kind = node.getXPathNodeName(); + final boolean control = Set.of( + "IfStatement", + "ForStatement", + "ForeachStatement", + "WhileStatement", + "DoStatement", + "SwitchStatement", + "SwitchExpression", + "CatchClause") + .contains(kind); + final boolean elseIf = node instanceof ASTIfStatement + && node.getParent() instanceof ASTIfStatement p + && p.getElseBranch() == node; + final int current = depth + (control && !elseIf ? 1 : 0); + int maximum = current; + for (int i = 0; i < node.getNumChildren(); i++) + maximum = Math.max(maximum, nesting(node.getChild(i), current)); + return maximum; + } + + static void visibility(ModifierOwner member) { + final ASTTypeDeclaration owner = ((JavaNode) member).getEnclosingType(); + if (owner == null || owner.isAnnotation()) return; + if (!member + .getModifiers() + .hasAnyExplicitly(JModifier.PUBLIC, JModifier.PROTECTED, JModifier.PRIVATE)) + block(member, "Declare explicit member visibility"); + } + + static void variable(ASTVariableId id) { + if (id.isEnumConstant() && !id.getName().matches("[A-Z][A-Z0-9]*(_[A-Z0-9]+)*")) + block(id, "Enum constants require UPPER_SNAKE_CASE"); + if (id.isRecordComponent() || id.isEnumConstant() || id.isPatternBinding() || id.isUnnamed()) + return; + if (id.isTypeInferred() && !id.isLambdaParameter()) { + final ASTExpression initializer = id.getInitializer(); + if (!(initializer instanceof ASTConstructorCall) + && !(initializer instanceof ASTCastExpression) + && !(initializer instanceof ASTArrayAllocation)) { + block(id, "var requires a visible constructor or cast type"); + } + } + if (id.isLambdaParameter() + && id.getParent() instanceof ASTLambdaParameter parameter + && parameter.hasVarKeyword()) block(id, "Use an explicit lambda parameter type with final"); + if (id.isFinal()) return; + final boolean written = id.getLocalUsages().stream() + .anyMatch(u -> u.getAccessType() != ASTAssignableExpr.AccessType.READ); + if (written) return; + if (id.isLambdaParameter()) { + block(id, "Assigned-once lambda parameter requires final: " + id.getName()); + return; + } + if (id.isField()) { + final ASTFieldDeclaration field = id.ancestors(ASTFieldDeclaration.class).first(); + if (field == null + || id.getInitializer() == null + || field.isAnnotationPresent("jakarta.inject.Inject") + || field.isAnnotationPresent("lombok.Setter") + || id.getEnclosingType().isAnnotationPresent("lombok.Setter") + || id.getEnclosingType().isAnnotationPresent("lombok.Data")) return; + } + block(id, "Assigned-once variable requires final: " + id.getName()); + } + + static boolean owned(String name) { + return types.containsKey(name) + || ownedTypes.stream().anyMatch(n -> name.equals(n) || name.startsWith(n + "$")); + } + + static void typeMetrics(ASTTypeDeclaration type) { + final Set ownLines = new HashSet<>(); + final Set scopeLines = codeLines.getOrDefault(path(type), lines(type)); + final var nestedTypes = + all(type, ASTTypeDeclaration.class).stream().filter(t -> t != type).toList(); + var token = type.getFirstToken(); + while (token != null && !token.isEof()) { + final var region = token.getRegion(); + if (nestedTypes.stream().noneMatch(t -> t.getTextRegion().contains(region))) { + final var loc = token.getReportLocation(); + for (int line = loc.getStartLine(); line <= loc.getEndLine(); line++) + if (scopeLines.contains(line)) ownLines.add(line); + } + if (token == type.getLastToken()) break; + token = token.getNext(); + } + metric(type, "classLines", ownLines.size(), 300, 500); + if (type.isRecord()) + findings.add("DESIGN " + location(type) + " recordComponents=" + + type.getRecordComponents().size() + " (manual design review)"); + int depth = 0; + var superclass = type.getSymbol().getSuperclass(); + while (superclass != null && owned(superclass.getBinaryName())) { + depth++; + superclass = superclass.getSuperclass(); + } + metric(type, "ownSuperclassDepth", depth, 2, 3); + final Set fanout = new HashSet<>(); + for (var ref : all(type, ASTClassType.class)) { + if (ref.getEnclosingType() != type) continue; + final var symbol = ref.getTypeMirror().getSymbol(); + if (symbol != null && symbol.isUnresolved()) + block( + ref, + "Cannot resolve type " + ref.getSimpleName() + + "; compile the backend and regenerate target/standards-classpath.txt before" + + " checking"); + if (symbol instanceof JClassSymbol resolved && !resolved.equals(type.getSymbol())) + fanout.add(resolved.getBinaryName()); + } + metric(type, "fanOut", fanout.size(), 15, Double.POSITIVE_INFINITY); + int publicMethods = 0; + for (var method : type.getOperations()) + if (method instanceof ASTMethodDeclaration + && method.hasExplicitModifiers(JModifier.PUBLIC) + && !trivial(method)) publicMethods++; + metric(type, "publicMethods", publicMethods, 15, Double.POSITIVE_INFINITY); + final Set dependencies = new HashSet<>(); + injected(type, dependencies, new HashSet<>()); + metric(type, "injectedDependencies", dependencies.size(), 5, 7); + } + + static boolean trivial(ASTExecutableDeclaration method) { + if (method.getBody() == null) return false; + final String body = method + .getTextDocument() + .sliceOriginalText(method.getBody().getTextRegion()) + .toString() + .replaceAll("\\s+", ""); + final String name = method.getName(); + String property = name; + if (name.startsWith("get") || name.startsWith("set")) property = name.substring(3); + else if (name.startsWith("is")) property = name.substring(2); + if (property.isEmpty()) return false; + property = Character.toLowerCase(property.charAt(0)) + property.substring(1); + final String fieldName = property; + if (method.getEnclosingType().getSymbol().getDeclaredFields().stream() + .noneMatch(f -> f.getSimpleName().equals(fieldName))) return false; + if (method.getArity() == 0) + return body.equals("{return" + property + ";}") + || body.equals("{returnthis." + property + ";}"); + if (method.getArity() != 1 || !name.startsWith("set")) return false; + final String parameter = method.getFormalParameters().get(0).getVarId().getName(); + return body.equals("{this." + property + "=" + parameter + ";}") + || !property.equals(parameter) && body.equals("{" + property + "=" + parameter + ";}"); + } + + record Dependency(JTypeMirror type, Set qualifiers) {} + + static boolean annotated(AnnotableSymbol symbol, String annotation) { + return symbol.getDeclaredAnnotations().stream() + .anyMatch(a -> a.getBinaryName().equals(annotation)); + } + + static Dependency dependency(JVariableSymbol variable) { + return dependency(variable, Substitution.EMPTY); + } + + static Dependency dependency(JVariableSymbol variable, Substitution substitution) { + final Set qualifiers = new HashSet<>(); + for (var annotation : variable.getDeclaredAnnotations()) { + if (annotation.getBinaryName().equals("jakarta.inject.Named") + || annotated(annotation.getAnnotationSymbol(), "jakarta.inject.Qualifier")) { + final Map attributes = new TreeMap<>(); + for (String name : annotation.getAttributeNames()) + attributes.put(name, String.valueOf(annotation.getAttribute(name))); + qualifiers.add(annotation.getBinaryName() + attributes); + } + } + return new Dependency(variable.getTypeMirror(substitution), qualifiers); + } + + static void injected(ASTTypeDeclaration type, Set dependencies, Set visited) { + if (!visited.add(type.getBinaryName())) return; + final var constructors = type.getSymbol().getConstructors(); + final boolean bean = type.getDeclaredAnnotations().any(a -> { + final var symbol = a.getTypeMirror().getSymbol(); + return Set.of( + "jakarta.inject.Singleton", + "jakarta.enterprise.context.ApplicationScoped", + "jakarta.enterprise.context.RequestScoped", + "jakarta.enterprise.context.Dependent", + "jakarta.enterprise.context.SessionScoped") + .contains(symbol.getCanonicalName()) + || annotated(symbol, "jakarta.inject.Scope") + || annotated(symbol, "jakarta.enterprise.context.NormalScope") + || annotated(symbol, "jakarta.enterprise.inject.Stereotype"); + }); + for (var constructor : constructors) { + if (annotated(constructor, "jakarta.inject.Inject") || bean && constructors.size() == 1) { + for (var parameter : constructor.getFormalParameters()) + dependencies.add(dependency(parameter)); + } + } + // PMD deliberately sees handwritten source, so account for Lombok's generated constructor. + if (bean && type.getOperations().filterIs(ASTConstructorDeclaration.class).isEmpty()) { + final boolean allArgs = type.isAnnotationPresent("lombok.AllArgsConstructor"); + final boolean requiredArgs = type.isAnnotationPresent("lombok.RequiredArgsConstructor"); + if (allArgs || requiredArgs) + for (var field : type.getDeclarations(ASTFieldDeclaration.class)) { + if (!field.isStatic()) + for (var id : field.getVarIds()) + if (allArgs + || id.getInitializer() == null + && (id.isFinal() || field.isAnnotationPresent("lombok.NonNull"))) + dependencies.add(dependency(id.getSymbol())); + } + } + inheritedInjection(type.getTypeMirror(), dependencies, new HashSet<>()); + } + + static void inheritedInjection( + JClassType resolvedType, Set dependencies, Set visited) { + if (resolvedType == null) return; + final JClassSymbol symbol = resolvedType.getSymbol(); + final Substitution substitution = resolvedType.getTypeParamSubst(); + if (!visited.add(symbol.getBinaryName())) return; + for (var field : symbol.getDeclaredFields()) + if (annotated(field, "jakarta.inject.Inject")) + dependencies.add(dependency(field, substitution)); + for (var method : symbol.getDeclaredMethods()) + if (annotated(method, "jakarta.inject.Inject")) + for (var parameter : method.getFormalParameters()) + dependencies.add(dependency(parameter, substitution)); + inheritedInjection(resolvedType.getSuperClass(), dependencies, visited); + } + + static void duplication(List files) throws Exception { + final var config = new CPDConfiguration(); + config.setMinimumTileSize(100); + config.setIgnoreIdentifiers(false); + config.setIgnoreLiterals(false); + config.setSkipLexicalErrors(false); + config.setNoSkipBlocks(true); + final Set duplicated = new HashSet<>(); + try (var cpd = CpdAnalysis.create(config)) { + for (var file : files) + if (file.startsWith(root.resolve("agiledancers-be/src/main/java"))) + cpd.files().addFile(file); + cpd.performAnalysis(report -> { + for (var error : report.getProcessingErrors()) findings.add("BLOCK CPD parser " + error); + for (var match : report.getMatches()) + for (var mark : match) { + final var loc = mark.getLocation(); + final Path path = + Path.of(loc.getFileId().getAbsolutePath()).toAbsolutePath().normalize(); + for (int line = loc.getStartLine(); line <= loc.getEndLine(); line++) + if (codeLines.getOrDefault(path, Set.of()).contains(line)) + duplicated.add(path + ":" + line); + findings.add("CLONE " + path + ":" + loc.getStartLine() + "-" + loc.getEndLine()); + } + }); + } + final long total = codeLines.entrySet().stream() + .filter(e -> e.getKey().startsWith(root.resolve("agiledancers-be/src/main/java"))) + .mapToLong(e -> e.getValue().size()) + .sum(); + final double density = total == 0 ? 0 : 100.0 * duplicated.size() / total; + findings.add((density > 3 ? "BLOCK " : density > 1 ? "REVIEW " : "METRIC ") + + "productionDuplication=" + density + "% (" + duplicated.size() + "/" + total + ")"); + } +} diff --git a/tooling/standards/src/test/java/StandardsTest.java b/tooling/standards/src/test/java/StandardsTest.java new file mode 100644 index 0000000..09f83ac --- /dev/null +++ b/tooling/standards/src/test/java/StandardsTest.java @@ -0,0 +1,357 @@ +import static org.junit.jupiter.api.Assertions.*; + +import java.nio.file.*; +import java.util.*; +import net.sourceforge.pmd.*; +import net.sourceforge.pmd.lang.*; +import net.sourceforge.pmd.lang.document.FileId; +import net.sourceforge.pmd.lang.java.ast.*; +import net.sourceforge.pmd.lang.rule.RuleSet; +import org.junit.jupiter.api.*; + +class StandardsTest { + private List analyze(String text) throws Exception { + Standards.root = Path.of("").toAbsolutePath(); + Standards.ownedTypes.clear(); + Standards.findings.clear(); + Standards.units.clear(); + Standards.types.clear(); + Standards.codeLines.clear(); + final var config = new PMDConfiguration(); + config.setThreads(1); + config.prependAuxClasspath( + Path.of(System.getProperty("java.home"), "lib/jrt-fs.jar").toString()); + config.prependAuxClasspath( + System.getProperty("surefire.test.class.path", System.getProperty("java.class.path"))); + config.setIgnoreIncrementalAnalysis(true); + config.setDefaultLanguageVersion( + LanguageRegistry.PMD.getLanguageById("java").getVersion("25")); + try (var analysis = PmdAnalysis.create(config)) { + analysis.addRuleSet(RuleSet.forSingleRule(new Standards.Measurements())); + analysis.files().addSourceFile(FileId.fromPath(Standards.root.resolve("Fixture.java")), text); + var report = analysis.performAnalysisAndCollectReport(); + assertTrue( + report.getProcessingErrors().isEmpty(), report.getProcessingErrors().toString()); + } + return List.copyOf(Standards.findings); + } + + private boolean contains(List findings, String prefix, String metric) { + return findings.stream().anyMatch(s -> s.startsWith(prefix) && s.endsWith(metric)); + } + + @Test + void cyclomaticAndCognitiveBoundaries() throws Exception { + var at = analyze( + "class Fixture { public void run(final boolean b) { " + "if (b) {}".repeat(9) + "} }"); + assertTrue(contains(at, "REVIEW", "cyclomatic=10.0")); + var over = analyze( + "class Fixture { public void run(final boolean b) { " + "if (b) {}".repeat(10) + "} }"); + assertTrue(contains(over, "BLOCK", "cyclomatic=11.0")); + assertTrue(contains( + analyze( + "class Fixture { public void run(final boolean b) {" + "if (b) {}".repeat(15) + "} }"), + "REVIEW", + "cognitive=15.0")); + assertTrue(contains( + analyze( + "class Fixture { public void run(final boolean b) {" + "if (b) {}".repeat(16) + "} }"), + "BLOCK", + "cognitive=16.0")); + } + + @Test + void methodAndConstructorParameterBoundariesAndImplicitRecords() throws Exception { + assertTrue(contains( + analyze( + "class Fixture { public void run(final int a, final int b, final int c, final int d," + + " final int e) {} }"), + "REVIEW", + "parameters=5.0")); + assertTrue(contains( + analyze( + "class Fixture { public void run(final int a, final int b, final int c, final int d," + + " final int e, final int f) {} }"), + "BLOCK", + "parameters=6.0")); + var implicit = + analyze("record Fixture(int a, int b, int c, int d, int e, int f, int g, int h) {}"); + assertFalse(implicit.stream().anyMatch(s -> s.contains("parameters="))); + var compact = analyze( + "record Fixture(int a, int b, int c, int d, int e, int f, int g, int h) { public Fixture" + + " {} }"); + assertTrue(contains(compact, "BLOCK", "parameters=8.0")); + } + + @Test + void physicalLinesExcludeCommentsAndCountBraces() throws Exception { + var findings = analyze("class Fixture {\n" + + "public void run() {\n" + + "// comment\n" + + "/* multi\n" + + " comment */\n\n" + + "System.out.println(1); // code\n" + + "}\n" + + "}"); + assertTrue(contains(findings, "METRIC", "executableLines=3.0")); + assertTrue(contains(findings, "METRIC", "classLines=5.0")); + assertTrue(contains( + analyze("class Fixture {\npublic void run() {\n" + "System.out.println(1);\n".repeat(48) + + "}\n}"), + "REVIEW", + "executableLines=50.0")); + assertTrue(contains( + analyze("class Fixture {\npublic void run() {\n" + "System.out.println(1);\n".repeat(49) + + "}\n}"), + "BLOCK", + "executableLines=51.0")); + } + + @Test + void nestingElseIfCatchSwitchLambdaAndNestedMethods() throws Exception { + var chain = analyze( + "class Fixture { public void run(final boolean b) { if (b) {} else if (b) {} else if (b)" + + " {} } }"); + assertTrue(contains(chain, "METRIC", "nesting=1.0")); + var nested = analyze( + "class Fixture { public void run(final boolean b) { if (b) { while (b) { switch (1) {" + + " default -> { try {} catch (Exception e) { Runnable r = () -> { if (b) {} }; } } }" + + " } } } }"); + assertTrue(contains(nested, "BLOCK", "nesting=5.0")); + var separate = analyze( + "class Fixture { public void run(final boolean b) { if (b) { class Inner { public void" + + " go() { if (b) {} } } } } }"); + assertFalse(separate.stream().anyMatch(s -> s.endsWith("nesting=2.0"))); + } + + @Test + void lambdaComplexityContributesAndNestedClassDoesNot() throws Exception { + var findings = analyze( + "class Fixture { public void run(final boolean b) { Runnable r = () -> { if (b) {} };" + + " class Inner { public void nested() { if (b) {} } } } }"); + assertEquals( + 2, + findings.stream().filter(s -> s.endsWith("cyclomatic=2.0")).count(), + findings.toString()); + } + + @Test + void varAndFinalRules() throws Exception { + var findings = analyze( + "class Fixture { public void run(int p) { var a = new Object(); var b = (Object) a; var c" + + " = name(); int count = 0; count++; } private String name() { return \"x\"; } }"); + assertEquals(1, findings.stream().filter(s -> s.contains("var requires")).count()); + assertTrue(findings.stream().anyMatch(s -> s.contains("requires final: p"))); + assertFalse(findings.stream().anyMatch(s -> s.contains("requires final: count"))); + } + + @Test + void inheritanceCountsOnlyProjectSuperclasses() throws Exception { + var findings = analyze( + "class A {} class B extends A {} class C extends B {} class D extends C {} class Fixture" + + " extends D {} class External extends java.util.ArrayList {}"); + assertTrue(contains(findings, "REVIEW", "ownSuperclassDepth=3.0")); + assertTrue(contains(findings, "BLOCK", "ownSuperclassDepth=4.0")); + assertTrue(contains(findings, "METRIC", "ownSuperclassDepth=0.0")); + } + + @Test + void injectionUsesResolvedTypesQualifiersInheritedMembersAndImplicitConstructors() + throws Exception { + var findings = analyze(""" + class Parent { + @jakarta.inject.Inject private String inherited; + @jakarta.inject.Inject public void setValue(final Integer value) {} + } + @jakarta.inject.Singleton class Fixture extends Parent { + private final String value; + public Fixture(@jakarta.inject.Named("a") final String first, + @jakarta.inject.Named("a") final String repeated, + @jakarta.inject.Named("b") final String second) { value = first; } + @jakarta.inject.Inject private String repeatedInherited; + } + """); + assertTrue(contains(findings, "METRIC", "injectedDependencies=4.0"), findings.toString()); + String fields = ""; + for (int i = 0; i < 7; i++) + fields += + "@jakarta.inject.Inject @jakarta.inject.Named(\"" + i + "\") private String d" + i + ";"; + assertTrue( + contains(analyze("class Fixture {" + fields + "}"), "REVIEW", "injectedDependencies=7.0")); + assertTrue(contains( + analyze("class Fixture {" + fields + "@jakarta.inject.Inject private Integer eighth;}"), + "BLOCK", + "injectedDependencies=8.0")); + } + + @Test + void fanoutCountsDistinctJdkTypesAndPublicMethodsExcludeOnlyTrueAccessors() throws Exception { + var findings = analyze(""" + class Fixture { + private String name; + private java.util.List values; + private java.util.List repeated; + public String getName() { return name; } + public void setName(final String value) { this.name = value; } + public String getOther() { return name; } + public String getValidatedName() { if (name == null) { throw new IllegalStateException(); } return name; } + public static int calculate() { return 1; } + } + """); + assertTrue(contains(findings, "METRIC", "fanOut=3.0"), findings.toString()); + assertTrue(contains(findings, "METRIC", "publicMethods=3.0"), findings.toString()); + } + + @Test + void classLengthBoundaryAndNestedTypeLines() throws Exception { + assertTrue(contains( + analyze("class Fixture {\n" + "private int field;\n".repeat(498) + "}"), + "REVIEW", + "classLines=500.0")); + assertTrue(contains( + analyze("class Fixture {\n" + "private int field;\n".repeat(499) + "}"), + "BLOCK", + "classLines=501.0")); + var separate = analyze( + "class Fixture {\nprivate class Inner {\nprivate int value;\n}\nprivate int other;\n}"); + assertEquals(2, separate.stream().filter(s -> s.endsWith("classLines=3.0")).count()); + assertTrue( + contains(analyze("class Fixture { private class Inner {} }"), "METRIC", "classLines=1.0")); + } + + @Test + void switchBooleanAndThrowTreatmentIsPinned() throws Exception { + var findings = analyze( + "class Fixture { public int run(final boolean a, final boolean b) { if (a && b) { throw" + + " new IllegalStateException(); } return switch (1) { case 0, 1 -> 1; default -> 2;" + + " }; } }"); + assertTrue(contains(findings, "METRIC", "cyclomatic=6.0"), findings.toString()); + } + + private Path project(String production, String tests) throws Exception { + Path directory = Files.createTempDirectory("s7-fixture-"); + Path prod = directory.resolve("agiledancers-be/src/main/java"); + Path test = directory.resolve("agiledancers-be/src/test/java"); + Files.createDirectories(prod); + Files.createDirectories(test); + Path config = directory.resolve("tooling/standards/checkstyle.xml"); + Files.createDirectories(config.getParent()); + Files.copy(Path.of(System.getProperty("basedir"), "checkstyle.xml"), config); + if (production != null) Files.writeString(prod.resolve("Fixture.java"), production); + if (tests != null) Files.writeString(test.resolve("ExampleTest.java"), tests); + return directory; + } + + @Test + void checksFailWithoutRewritingSourcesAndExposeReviewSignals() throws Exception { + String source = "package example;\n" + + "import java.util.*;\n" + + "class Fixture { public void run(int x) { if (x == 0) System.out.println(x); } }"; + Path directory = project(source, null); + assertThrows( + IllegalStateException.class, () -> Standards.main(new String[] {directory.toString()})); + assertEquals( + source, Files.readString(directory.resolve("agiledancers-be/src/main/java/Fixture.java"))); + String report = + Files.readString(directory.resolve("agiledancers-be/target/standards/report.txt")); + assertTrue(report.contains("form of import"), report); + assertTrue(report.contains("must use '{}'s"), report); + assertTrue(report.contains("requires final: x"), report); + } + + @Test + void productionCpdCountsEveryOccurrenceAndTestClonesDoNotGate() throws Exception { + String method = "public void first() {\n" + "System.out.println(123);\n".repeat(30) + "}\n"; + String source = + "package example;\nclass Fixture {\n" + method + method.replace("first", "second") + "}"; + Path directory = project(source, null); + assertThrows( + IllegalStateException.class, () -> Standards.main(new String[] {directory.toString()})); + String report = + Files.readString(directory.resolve("agiledancers-be/target/standards/report.txt")); + assertTrue( + report.contains("BLOCK productionDuplication=") && report.contains("(64/67)"), report); + assertTrue(report.lines().filter(l -> l.startsWith("CLONE")).count() >= 2, report); + Path testProject = project( + "package example; class Fixture {}", source.replace("class Fixture", "class ExampleTest")); + Standards.main(new String[] {testProject.toString()}); + String testReport = + Files.readString(testProject.resolve("agiledancers-be/target/standards/report.txt")); + assertTrue(testReport.contains("productionDuplication=0.0%"), testReport); + } + + @Test + void parserFailureNamesTheAffectedFileAndFails() throws Exception { + Path directory = project("package example; class Fixture { public void broken( }", null); + Exception failure = + assertThrows(Exception.class, () -> Standards.main(new String[] {directory.toString()})); + assertTrue( + failure.toString().contains("Fixture.java") + || Standards.findings.stream().anyMatch(f -> f.contains("Fixture.java")), + failure.toString()); + } + + @Test + void exactCoverageGateAcceptsEightyPercentAndRejectsRoundedNearMiss() throws Exception { + Path directory = Files.createTempDirectory("s7-coverage-"); + Path xml = directory.resolve("report.xml"); + Path helper = + Path.of(System.getProperty("basedir"), "../coverage/Coverage.java").normalize(); + for (long[] counts : + List.of(new long[] {4, 1, 0}, new long[] {799999, 200001, 1}, new long[] {0, 0, 1})) { + Files.writeString( + xml, + ""); + Process process = new ProcessBuilder( + Path.of(System.getProperty("java.home"), "bin/java").toString(), + helper.toString(), + xml.toString()) + .redirectErrorStream(true) + .redirectOutput(directory.resolve("output.log").toFile()) + .start(); + assertEquals(counts[2], process.waitFor(), Files.readString(directory.resolve("output.log"))); + } + } + + @Test + void overridesAndImplicitRecordInjectionAreMeasured() throws Exception { + var override = + analyze("class Fixture extends java.util.ArrayList { public void clear() {} }"); + assertTrue( + override.stream().anyMatch(f -> f.contains("require @Override")), override.toString()); + var record = analyze( + "@jakarta.inject.Singleton record Fixture(String one, String repeated, Integer two) {}"); + assertTrue(contains(record, "METRIC", "injectedDependencies=2.0"), record.toString()); + var inherited = analyze( + "class Parent { @jakarta.inject.Inject private T dependency; } class Fixture extends" + + " Parent { @jakarta.inject.Inject private String repeated; }"); + assertEquals( + 2, + inherited.stream().filter(f -> f.endsWith("injectedDependencies=1.0")).count(), + inherited.toString()); + } + + @Test + void waiverMatchesOnlyTheExactMetricSignatureAndRetainedValue() throws Exception { + Standards.waivers.add(new Standards.Waiver( + "parameters", + "Fixture#run(int,int,int,int,int,int)", + 6, + "Test fixture agreement: preserve an example signature")); + try { + var waived = analyze( + "class Fixture { public void run(final int a, final int b, final int c, final int d," + + " final int e, final int f) {} }"); + assertTrue( + waived.stream().anyMatch(f -> f.startsWith("WAIVER") && f.contains("parameters=6.0")), + waived.toString()); + var changed = analyze( + "class Fixture { public void run(final int a, final int b, final int c, final int d," + + " final int e, final int f, final int g) {} }"); + assertTrue(contains(changed, "BLOCK", "parameters=7.0")); + } finally { + Standards.waivers.clear(); + } + } +} diff --git a/tooling/standards/waivers.tsv b/tooling/standards/waivers.tsv new file mode 100644 index 0000000..0fb7f72 --- /dev/null +++ b/tooling/standards/waivers.tsv @@ -0,0 +1,3 @@ +# Maintainer-approved, exact metric waivers only. +# metricowner#method(type1,type2) or owner#(types)valueagreement and reason +parameters nl.agilecc.agiledancers.service.model.Group#(java.lang.String,java.lang.String,nl.agilecc.agiledancers.model.specifications.DanceGroupType,java.lang.Integer,java.lang.Integer,java.util.List,java.util.List,java.lang.String) 8 Maintainer approved in the S7 implementation conversation: preserve the eight-component version-2 JSON shape while defensively copying styles and dancerIds.