Skip to content

chore: convert Conference Scheduling to Models Service structure - #1126

Merged
triceo merged 36 commits into
developmentfrom
chore/conference-scheduling-refactor
Aug 19, 2026
Merged

chore: convert Conference Scheduling to Models Service structure#1126
triceo merged 36 commits into
developmentfrom
chore/conference-scheduling-refactor

Conversation

@larsbeck

Copy link
Copy Markdown

Converts the conference-scheduling quickstart to the Models Service SDK structure:

  • Move solver justifications to domain/justification with a shared ConferenceSchedulingJustification interface
  • Add DTO layer (input/output, metrics, config overrides, validation issues)
  • Add DemoDataBuilder/DemoDataGenerator for demo data
  • Update pom.xml for platform model conventions

Review feedback incorporated

Addresses the review comments from @triceo, @TomCools and Copilot:

Less code, readable construction (net ~-970 lines)

  • Nested builders on domain Talk/Speaker and on TalkDTO/SpeakerDTO; the telescoping 22-/11-arg constructors and their PMD.ExcessiveParameterList suppressions are gone, so the PMD parameter-list rule is enforced again
  • All generated withXxx methods deleted except the three actually used; the ArchUnit wither-per-component rule (which manufactured the bloat) is removed, with_methods_must_be_used stays and now bans dead withers
  • Repeated normalization ternaries replaced by reused immutableCopy/orEmpty/nonNegative helpers
  • Static imports restored in the ConstraintProvider (PMD TooManyStaticImports raised to 30)
  • ConfigOverrides: the 20 withers are gone and the no-arg constructor now yields all-null "no overrides" (previously it silently overrode every weight with 1). The constructor itself must stay because the SDK requires it reflectively — tracked in Service SDK: ModelConfigOverrides records are forced to declare an all-null no-arg constructor timefold-solver#2493

Build & quality gates

  • Custom coverage harness (CoverageReportSummary, ShadowSourcesCoverageCheck, RecordWithMethodCoverageTest) removed — plain JaCoCo gates unchanged (30% instruction / 20% branch bundle, 100% line on the ConstraintProvider)
  • Node/npm fully out of the maven build; markdownlint removed (spotless still formats markdown)
  • The vacuum OpenAPI-description lint moved to CI: .github/workflows/lint_openapi.yml lints the committed spec of every model shipping a .vacuum-ruleset.yaml (covers future models automatically). src/build/openapi.json and .vacuum-ruleset.yaml are now actually committed — the root .gitignore's .* and build patterns had been silently excluding them, so the lint could never run before
  • Unused WebJars dependencies and quarkus-web-dependency-locator dropped (UI loads assets from CDN)
  • Parent set to 999-SNAPSHOT (replaced on release; CI builds the solver from source), JFrog repositories removed — everything resolves from Maven Central

Correctness

  • Model conversion fails fast with IllegalArgumentException("Unknown <kind> '<key>'.") on unknown talk-type/speaker/timeslot/room/prerequisite references instead of leaking nulls into the solver model
  • Validator rejects blank talkTypeName, not just null
  • ConferenceScheduleIssue got its missing OpenAPI description (vacuum scores 100/100); small UI/test fixes (unclosed span, stray jQuery .fail argument, assertNull), README cd path corrected

Note on CI: the failing Maven Long-Running jobs are pre-existing repo infrastructure breakage — actions/checkout v7 refuses fork-PR checkouts under pull_request_target (see this comment); unrelated to this PR.

🤖 Generated with Claude Code

larsbeck and others added 8 commits July 14, 2026 14:13
- Move solver justifications to domain/justification with shared
  ConferenceSchedulingJustification interface
- Add DTO layer (input/output/metrics/config overrides/validation)
- Add DemoDataBuilder/DemoDataGenerator for demo data
- Update pom.xml for platform model conventions
- replace generated withXxx bloat and telescoping constructors with builders
- fail fast on unknown references in model conversion
- drop node/npm from the maven build and custom coverage harness
- move to 999-SNAPSHOT parent, drop JFrog repositories

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
Reverts the removal of the vacuum OpenAPI lint and markdownlint from the
maven build and drops the DtoSchemaDescriptionTest that stood in for them.
Also commits the previously missing .vacuum-ruleset.yaml and
.markdownlint-cli2.jsonc (the root .gitignore's ".*" pattern had silently
excluded them, so the lint steps could never run) and adds the missing
@Schema description on ConferenceScheduleIssue that the restored vacuum
lint caught.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
Vacuum is a Go binary; the npm package was only a distribution channel.
Download the pinned platform binary from GitHub releases instead of
installing node/npm via frontend-maven-plugin. Markdownlint is removed
entirely (spotless keeps formatting markdown).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
Moves the vacuum lint out of the maven build into a dedicated GitHub
workflow, following vacuum's documented CI/CD setup. The workflow lints
the committed spec of every model shipping a .vacuum-ruleset.yaml, so
future converted models are covered automatically. The generated
src/build/openapi.json is now committed (force-added past the root
.gitignore's "build" pattern) to keep the lint standalone and make spec
drift visible in diffs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
Unknown prerequisite talk codes now fail fast like every other reference,
the dataset info span is closed properly, a stray jQuery .fail() argument
is removed and DemoDataBuilderTest uses assertNull.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
The UI loads all assets from CDNs, so the WebJars runtime dependencies
and the web-dependency-locator extension were dead weight.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
…leset

The header still pointed at the removed exec-maven-plugin wiring; the
lint now runs in the lint_openapi.yml workflow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request refactors the conference-scheduling quickstart to align with the Timefold Models Service SDK structure, introducing a DTO-first API surface and SDK-compatible model conversion/validation while simplifying build/CI and updating the UI to the new REST contract.

Changes:

  • Introduces a DTO layer (+ validation issues/metrics/config overrides) and a ConferenceScheduleModelConvertor/ConferenceScheduleValidator for SDK model I/O.
  • Replaces the bespoke REST resources with an SDK ModelRest interface and updates the UI + tests to use the /v1/... endpoints and ModelRequest/ModelResponse shapes.
  • Adds OpenAPI spec linting in CI via vacuum rulesets and commits the generated OpenAPI JSON.

Reviewed changes

Copilot reviewed 61 out of 63 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/solver/SolverTestDataFactory.java Adds DTO-based test data factory for solver tests.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/solver/SolverManagerTest.java Adds solver smoke test using model conversion + metrics assertions.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/solver/ConferenceSchedulingConstraintProviderTest.java Minor formatting adjustment in constraint provider tests.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/solver/ConferenceScheduleResourceIT.java Adds native IT targeting /v1 endpoints and async solving status.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/solver/ConferenceScheduleEnvironmentTest.java Updates env-mode test to use DTO input + convertor (no REST demo-data).
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/rest/ConferenceSchedulingResourceIT.java Removes old REST IT for legacy endpoints.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/rest/ConferenceScheduleResourceTest.java Removes old REST test for legacy endpoints.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/ProjectStructureTest.java Adds ArchUnit rules enforcing layered architecture + DTO/record conventions.
use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/demo/DemoDataBuilderTest.java Adds test coverage for new demo input builder.
use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html Switches UI assets to CDN and updates curl examples + platform embed hooks.
use-cases/conference-scheduling/src/main/resources/META-INF/resources/app.js Refactors UI to ModelRest contract, /v1 endpoints, and platform embedding params.
use-cases/conference-scheduling/src/main/resources/application.properties Adopts model metadata/settings + updated termination properties.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/solver/justifications/ConferenceSchedulingJustification.java Removes old solver justification record (moved to domain justification).
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/solver/ConstraintGroupTag.java Introduces tags for constraint group classification.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/solver/ConferenceScheduleConstraintGroup.java Adds SDK-facing constraint group definitions.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleValidator.java Adds SDK ModelValidator producing structured validation issues.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleModelConvertor.java Adds SDK ModelConvertor between DTOs and solver domain model.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleIssues.java Defines concrete validation issue classes for the model.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/exception/ErrorInfo.java Removes legacy error payload type for old REST layer.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/exception/ConferenceScheduleSolverExceptionMapper.java Removes legacy exception mapper (old REST stack).
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/exception/ConferenceScheduleSolverException.java Removes legacy REST exception type.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/DemoDataGenerator.java Removes legacy demo-data generator that returned domain model directly.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/ConferenceSchedulingResource.java Removes legacy bespoke REST resource implementation.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/ConferenceSchedulingDemoResource.java Removes legacy demo-data REST resource.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/rest/ConferenceScheduleResource.java Adds SDK ModelRest resource interface for schedules.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/TimeslotIdDetail.java Adds validation metadata record for timeslot ID issues.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/TimeslotDTO.java Adds timeslot DTO for wire format.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/TalkTypeDTO.java Adds talk type DTO for wire format.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/TalkIdDetail.java Adds validation metadata record for talk ID issues.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/TalkDTO.java Adds talk DTO + builder + minimal withers for solver output merging.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/SpeakerIdDetail.java Adds validation metadata record for speaker ID issues.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/SpeakerDTO.java Adds speaker DTO + builder for wire format.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/RoomIdDetail.java Adds validation metadata record for room ID issues.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/RoomDTO.java Adds room DTO for wire format.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleValidationIssue.java Enumerates validation issue types/codes for the model.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleOutputMetrics.java Adds structured output metrics DTO.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleOutput.java Adds model output DTO for API responses.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleInputMetrics.java Adds structured input metrics DTO.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleInput.java Adds model input DTO for API requests.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleConfigOverrides.java Adds nullable constraint-weight overrides per SDK conventions.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Timeslot.java Adjusts overlap semantics and formatting in equals/overlap helpers.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/TalkType.java Switches compatible sets to linked sets + formatting in equals.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Talk.java Replaces large constructors with builder and adds isScheduled().
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Speaker.java Replaces large constructors with builder.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Room.java Formatting/consistency updates in equals.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/UndesiredTagsJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/UnavailableTimeslotJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/RequiredTagsJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/ProhibitedTagsJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/PreferredTagsJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/DiversityTalkJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/ConflictTalkJustification.java Moves justification to domain + enforces non-null description.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/justification/ConferenceSchedulingJustification.java Adds shared justification record in domain package.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/ConferenceSchedule.java Adapts solution to SDK SolverModel, adds metrics + weight overrides.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/ConferenceConstraintProperties.java Removes redundant explicit no-arg constructor.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/demo/DemoDataGenerator.java Adds SDK demo-data generator returning ModelRequest.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/demo/DemoDataBuilder.java Adds deterministic demo dataset builder producing DTO input.
use-cases/conference-scheduling/README.md Updates CLI snippets and paths for consistency.
use-cases/conference-scheduling/pom.xml Migrates module to platform parent conventions + quality gates + OpenAPI spec copy.
use-cases/conference-scheduling/.vacuum-ruleset.yaml Adds vacuum ruleset enforcing schema/property descriptions for OpenAPI.
.github/workflows/lint_openapi.yml Adds CI job linting committed OpenAPI specs for models with vacuum rulesets.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html Outdated
Comment thread use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html Outdated
@oemebamo

Copy link
Copy Markdown
  • The diff is +5000 lines again, whereas previously it was net negative.

This is imo expected, and it's the direct result of that we want the use-cases folder to contain examples that are feature complete, platform compatible and well structured. Which comes at a cost.

  • Getting started: stays minimal. No DTOs, no metrics, no justifications. This is the "as little code as possible" experience, and it's not what this PR touches.
  • Use cases: deliberately full platform-featured: DTOs, input/output metrics, justifications, constraint weights. Simple in terms of # constraints, but not minimal in code. This is a conscious trade-off, not scope creep.

(We should keep pushing so "fully featured" doesn't have to mean "this much code" forever. But that's a follow-up?)

Error Prone requires -XDaddTypeAnnotationsToSymbol=true on JDK 21 (the
flag is ignored on newer JDKs). Also commits .pmd-ruleset.xml, which the
root .gitignore's ".*" pattern had silently excluded — the PMD check
could not run in CI without it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qae2bExuXdkKjDJjFXynGi
Copilot AI review requested due to automatic review settings July 14, 2026 13:44
@larsbeck

larsbeck commented Jul 14, 2026

Copy link
Copy Markdown
Author

@triceo @pieterdeschepper granted me permission now so I could create a new branch, hence the new PR.

We want these use-cases to be fully featured and be in the right "shape". They are not getting started experiences. They are in fact almost the opposite. Think of them as an opinionated way to build a production ready model (even though they are small models). When someone wants to build a production ready model, they should not need to think about what goes where, are there any building blocks missing - they should be able to take this and start modifying.

See #1126 (comment)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 62 out of 64 changed files in this pull request and generated 5 comments.

Comment thread use-cases/conference-scheduling/src/main/resources/application.properties Outdated
Copilot AI review requested due to automatic review settings July 14, 2026 13:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 62 out of 64 changed files in this pull request and generated 4 comments.

Comment thread use-cases/conference-scheduling/pom.xml Outdated
Comment thread use-cases/conference-scheduling/pom.xml Outdated
Copilot AI review requested due to automatic review settings July 15, 2026 12:03
Copilot AI review requested due to automatic review settings August 18, 2026 22:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 110 out of 111 changed files in this pull request and generated no new comments.

Suppressed comments (4)

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/TalkType.java:30

  • hashCode() and toString() currently assume name is non-null (name.hashCode() / return name). If an invalid input contains a talk type with null name, this will throw a NullPointerException during model conversion (e.g., when adding to a Set) and toString() may also return null, which violates common expectations for toString() implementations. Make both methods null-safe to match the null-tolerant equals() implementation.
    @Override
    public int hashCode() {
        return name.hashCode();
    }

    @Override
    public String toString() {
        return name;
    }

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Timeslot.java:127

  • equals() allows id to be null (via Objects.equals(id, timeslot.id)), but hashCode() calls id.hashCode(), which will throw if id is null (for example, if model conversion runs before validation catches a missing ID). Use a null-safe hashCode implementation consistent with equals.
        if (this == o) {
            return true;
        }
        if (!(o instanceof Timeslot timeslot)) {
            return false;

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleInput.java:25

  • Top-level collections in this DTO are not normalized (unlike the nested DTOs that use immutableCopy). If any of these lists are null (e.g., missing fields in JSON or programmatic construction), ConferenceScheduleValidator and ConferenceScheduleModelConvertor will throw NPEs when calling .stream(). Consider normalizing to immutable, non-null lists in a compact constructor.
    use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleOutput.java:19
  • Similar to ConferenceScheduleInput, this output DTO does not normalize its list fields. If any list is accidentally null, ConferenceScheduleModelConvertor.applyOutputToInput() (and any callers) will throw NPEs when streaming over talks() etc. A compact constructor can defensively normalize these to immutable, non-null lists.

@TomCools

Copy link
Copy Markdown
Collaborator

Project is now mostly in line with other Models, some more work to be done to validate the architecture (or adjust the architecture checks).

I did separate out the Justifications in multiple classes, even though the UI doesn't use the detailed fields yet, we can add that later. I also separated out the Validation issues.

Copilot AI review requested due to automatic review settings August 19, 2026 07:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 110 out of 111 changed files in this pull request and generated no new comments.

Suppressed comments (4)

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/TalkType.java:25

  • TalkType.hashCode() currently calls name.hashCode(), which will throw a NullPointerException if an invalid/partially-deserialized TalkType has a null name. This can surface during model conversion (e.g., when collecting talk types into a Set) and turns an input validation problem into a 500/NPE.
    @Override
    public int hashCode() {
        return name.hashCode();
    }

use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html:197

  • The REST guide documents GET /v1/schedules/{jobId}/score-analysis, but per prior review discussion this endpoint is only available in the Plus / Enterprise edition. Without an in-page disclaimer, Community users will follow this command and get a 404, making the guide misleading.
    use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/TalkDTO.java:15
  • TalkDTO.speakerIds is marked as required in the OpenAPI schema, but the service and tests explicitly accept talks without speakers (speakerIds omitted/null becomes an empty list). This makes the generated OpenAPI contract stricter than the actual behavior.
    use-cases/conference-scheduling/src/test/java/org/acme/conferencescheduling/testhelpers/TestTalkBuilder.java:179
  • TestTalkBuilder can build a Talk with talkType left null, but Talk#getTimeslotRange()/getRoomRange() dereference talkType. If any test path (or future solver-based test) ends up evaluating value ranges, this will NPE. Providing a default TalkType (and wiring in the selected timeslot/room to its compatibility sets) keeps the built Talk structurally valid even when the test doesn’t care about talk type.

Copilot AI review requested due to automatic review settings August 19, 2026 07:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 110 out of 111 changed files in this pull request and generated no new comments.

Suppressed comments (5)

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/TalkType.java:25

  • TalkType.hashCode() calls name.hashCode() directly, which will throw NPE if a TalkType is ever constructed with a null name (and equals() already handles null via Objects.equals). Using a null-safe hashCode keeps the equals/hashCode contract consistent and avoids hard-to-debug NPEs if invalid input slips through validation/conversion.
    @Override
    public int hashCode() {
        return name.hashCode();
    }

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleValidator.java:52

  • validateTalkTypes() currently accepts null/blank (and duplicate) talk type names without reporting an issue. This can later surface as a runtime failure during model conversion (e.g., Collectors.toMap() typically rejects null keys), instead of returning a clear validation error to the caller. Consider adding explicit validation for talk type names (missing/blank/duplicates) similar to timeslots/rooms/speakers/talks.
    use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html:198
  • The REST guide advertises GET /v1/schedules/{jobId}/score-analysis, but the committed OpenAPI spec (src/build/openapi.json) does not define any score-analysis path (verified by searching the spec). This makes the guide misleading for users running with the Community/OpenAPI-described API. Add a short note/disclaimer here (or adjust the command) to clarify availability (e.g., Enterprise-only).
    use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html:8
  • The Font Awesome <link> and the opening <style> tag are currently on the same line, which makes the HTML harder to read and can confuse formatters/linting (and some tools that expect tags to start at line boundaries). Split the <style> tag onto its own line.
    use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/support/ObjectHelper.java:9
  • ObjectHelper is a pure static utility, but it is currently instantiable and not marked final. Making it a non-instantiable utility class helps prevent accidental construction and clarifies intent.

Copilot AI review requested due to automatic review settings August 19, 2026 08:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 110 out of 111 changed files in this pull request and generated no new comments.

Suppressed comments (1)

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleValidator.java:52

  • validateTalkTypes() currently accepts TalkTypeDTO.name() values without checking for null/blank or duplicates. This can cause conversion to fail with a NullPointerException later, because ConferenceScheduleModelConvertor builds a map using Collectors.toMap(TalkTypeDTO::name, ...) which rejects null keys (ConferenceScheduleModelConvertor.java:64-67). It also leaves duplicate talk type names silently unresolved. Consider adding explicit validation for talk type names (missing/blank + duplicate) and reporting them as issues (similar to timeslot/room/speaker/talk ID validation) so invalid datasets are rejected cleanly instead of failing during conversion.

@TomCools TomCools left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having made a lot of changes myself and ran the quickstart locally (succesfully), I think this is as good as we'll get it for now. Next to moving to the service module:

  • OpenAPI spec was added
  • Architecture alignment (ArchUnit) test was added to align all service based use cases.
  • Justifications were added
  • Validation (and issue classes) were added
  • Better test support with TestBuilders
  • Some classes were replaced with modern records
  • Test coverage is much higher now

With this I approve this quickstart modification.

Copilot AI review requested due to automatic review settings August 19, 2026 08:59
@TomCools TomCools changed the title refactor(conference-scheduling): convert to Models Service SDK structure chore: convert to Conference Scheduling to Models Service structure Aug 19, 2026
@TomCools TomCools changed the title chore: convert to Conference Scheduling to Models Service structure chore: convert Conference Scheduling to Models Service structure Aug 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 110 out of 111 changed files in this pull request and generated no new comments.

Suppressed comments (7)

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Timeslot.java:46

  • getOverlapInMinutes() has the same issue as overlapsTime(): it short-circuits on equals() (ID equality), which can return a full duration for different Timeslot instances that happen to share an ID. This method should only short-circuit for object identity.
    public int getOverlapInMinutes(Timeslot other) {
        if (this.equals(other)) {
            return durationInMinutes;
        }
        LocalDateTime startMaximum = startDateTime.isBefore(other.startDateTime) ? other.startDateTime : startDateTime;
        LocalDateTime endMinimum = endDateTime.isBefore(other.endDateTime) ? endDateTime : other.endDateTime;
        return (int) Duration.between(startMaximum, endMinimum).toMinutes();

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleValidator.java:75

  • validateRooms()/validateSpeakers() also assume the incoming collections don’t contain null elements. A null element (from JSON) will currently cause a NullPointerException. Consider treating a null element the same as a missing ID so the API consistently returns validation issues.
    use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/domain/Timeslot.java:38
  • overlapsTime() short-circuits using equals(), but Timeslot.equals() is ID-based. If two distinct Timeslot instances share an ID (e.g. due to invalid input or accidental duplication in mapping), this will report them as overlapping even if their start/end differ. overlapsTime() should only short-circuit for object identity, not ID equality.

This issue also appears on line 40 of the same file.

    public boolean overlapsTime(Timeslot other) {
        if (this.equals(other)) {
            return true;
        }
        return startDateTime.isBefore(other.endDateTime) && other.startDateTime.isBefore(endDateTime);
    }

use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/support/ObjectHelper.java:10

  • There is an extra space in the immutableCopy() method signature (List<T> immutableCopy(...)). This is likely to be reformatted by auto-formatters and can cause noisy diffs.
    use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/dto/ConferenceScheduleInput.java:12
  • Unused imports (java.util.Collections and org.acme.conferencescheduling.support.ObjectHelper) add noise and can trigger style checks in some builds. They can be removed, keeping only List plus the static immutableCopy import.
    use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleValidator.java:65
  • The validation loops assume lists never contain null elements (e.g. a JSON array [null]). If a client sends a null element, validateTalkTypes()/validateTimeslots() will throw NullPointerException instead of reporting issues. It’s better for the validator to be null-tolerant and report the appropriate “...ID_MISSING” issue.

This issue also appears on line 67 of the same file.
use-cases/conference-scheduling/src/main/java/org/acme/conferencescheduling/service/ConferenceScheduleValidator.java:99

  • validateTalks() assumes the talks list contains no null elements; a null entry will throw NullPointerException at talk.code(). Treating a null element as a missing talk ID keeps validation behavior consistent and prevents 500s on malformed JSON.

@triceo
triceo merged commit a38076c into development Aug 19, 2026
49 of 56 checks passed
@triceo
triceo deleted the chore/conference-scheduling-refactor branch August 19, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants