S12 - turned schedule status into enum and some other review comments
All checks were successful
Build, test and publish / verify (push) Successful in 7m6s
All checks were successful
Build, test and publish / verify (push) Successful in 7m6s
This commit is contained in:
@@ -0,0 +1,8 @@
|
||||
package nl.agilecc.agiledancers.model.scheduling;
|
||||
|
||||
public enum SchedulingPhase {
|
||||
PREPARING,
|
||||
ATTENDANCE,
|
||||
SPREAD,
|
||||
VARIETY
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
package nl.agilecc.agiledancers.model.scheduling;
|
||||
|
||||
/** Progress is observational; intermediate assignments are never published or applied. */
|
||||
public record SchedulingProgress(String phase, boolean feasible) {}
|
||||
public record SchedulingProgress(SchedulingPhase phase, boolean feasible) {}
|
||||
|
||||
@@ -14,6 +14,7 @@ import nl.agilecc.agiledancers.model.scheduling.SchedulingProgress;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingResult;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingSummary;
|
||||
import nl.agilecc.agiledancers.service.model.SchedulingRun;
|
||||
import nl.agilecc.agiledancers.service.model.SchedulingState;
|
||||
import nl.agilecc.agiledancers.service.port.SchedulingEngine;
|
||||
|
||||
/** Owns background tasks and publishes current state independently of any HTTP connection. */
|
||||
@@ -44,7 +45,8 @@ public class SchedulingRuns {
|
||||
executor.submit(() -> calculate(run, input));
|
||||
} catch (final RuntimeException failure) {
|
||||
schools.endScheduling(showId);
|
||||
publish(run.completed("SOLVER_FAILED", "Berekening kon niet starten. Bestaande indeling behouden.", null));
|
||||
publish(run.completed(
|
||||
SchedulingState.SOLVER_FAILED, "Berekening kon niet starten. Bestaande indeling behouden.", null));
|
||||
throw failure;
|
||||
}
|
||||
return run;
|
||||
@@ -86,7 +88,7 @@ public class SchedulingRuns {
|
||||
schools.finishScheduling(run.schoolId(), input, result);
|
||||
completed = latest(run)
|
||||
.completed(
|
||||
result.optimal() ? "OPTIMAL" : "VALID",
|
||||
result.optimal() ? SchedulingState.OPTIMAL : SchedulingState.VALID,
|
||||
result.optimal()
|
||||
? "Optimaal conceptrooster."
|
||||
: "Geldig conceptrooster; optimaliteit niet bewezen.",
|
||||
@@ -94,16 +96,22 @@ public class SchedulingRuns {
|
||||
} catch (final ManagementException failure) {
|
||||
completed = failed(run, failure);
|
||||
} catch (final RuntimeException | LinkageError failure) {
|
||||
completed =
|
||||
latest(run).completed("SOLVER_FAILED", "Berekening mislukt. Bestaande indeling behouden.", null);
|
||||
completed = latest(run)
|
||||
.completed(SchedulingState.SOLVER_FAILED, "Berekening mislukt. Bestaande indeling behouden.", null);
|
||||
}
|
||||
finish(completed);
|
||||
}
|
||||
|
||||
private SchedulingRun failed(final SchedulingRun run, final ManagementException failure) {
|
||||
final String outcome = failure instanceof final SchedulingFailure scheduling
|
||||
? scheduling.getOutcome().name()
|
||||
: (failure.getKind() == ManagementExceptionKind.CONFLICT ? "STALE" : "INVALID_INPUT");
|
||||
final SchedulingState outcome = failure instanceof final SchedulingFailure scheduling
|
||||
? switch (scheduling.getOutcome()) {
|
||||
case INFEASIBLE -> SchedulingState.INFEASIBLE;
|
||||
case NO_SOLUTION -> SchedulingState.NO_SOLUTION;
|
||||
case SOLVER_FAILED -> SchedulingState.SOLVER_FAILED;
|
||||
}
|
||||
: (failure.getKind() == ManagementExceptionKind.CONFLICT
|
||||
? SchedulingState.STALE
|
||||
: SchedulingState.INVALID_INPUT);
|
||||
return latest(run).completed(outcome, failure.getMessage() + " Bestaande indeling behouden.", null);
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package nl.agilecc.agiledancers.service.model;
|
||||
|
||||
import java.time.Instant;
|
||||
import java.util.UUID;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingPhase;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingProgress;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingSummary;
|
||||
|
||||
@@ -13,7 +14,7 @@ public record SchedulingRun(
|
||||
String showName,
|
||||
Instant startedAt,
|
||||
Instant finishedAt,
|
||||
String state,
|
||||
SchedulingState state,
|
||||
SchedulingProgress progress,
|
||||
String message,
|
||||
SchedulingSummary summary) {
|
||||
@@ -25,8 +26,8 @@ public record SchedulingRun(
|
||||
name,
|
||||
Instant.now(),
|
||||
null,
|
||||
"RUNNING",
|
||||
new SchedulingProgress("PREPARING", false),
|
||||
SchedulingState.RUNNING,
|
||||
new SchedulingProgress(SchedulingPhase.PREPARING, false),
|
||||
"Berekening gestart.",
|
||||
null);
|
||||
}
|
||||
@@ -45,7 +46,8 @@ public record SchedulingRun(
|
||||
null);
|
||||
}
|
||||
|
||||
public SchedulingRun completed(final String outcome, final String explanation, final SchedulingSummary scores) {
|
||||
public SchedulingRun completed(
|
||||
final SchedulingState outcome, final String explanation, final SchedulingSummary scores) {
|
||||
return new SchedulingRun(
|
||||
runId,
|
||||
schoolId,
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
package nl.agilecc.agiledancers.service.model;
|
||||
|
||||
public enum SchedulingState {
|
||||
RUNNING,
|
||||
OPTIMAL,
|
||||
VALID,
|
||||
STALE,
|
||||
INVALID_INPUT,
|
||||
INFEASIBLE,
|
||||
NO_SOLUTION,
|
||||
SOLVER_FAILED
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import java.util.List;
|
||||
import java.util.function.Consumer;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingFailureKind;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingInput;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingPhase;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingProgress;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingResult;
|
||||
import nl.agilecc.agiledancers.service.SchedulingFailure;
|
||||
@@ -46,10 +47,11 @@ public class OrToolsSchedulingEngine implements SchedulingEngine {
|
||||
solver.getParameters().setNumSearchWorkers(1).setRandomSeed(0);
|
||||
SchedulingResult best = null;
|
||||
final List<LinearExpr> objectives = List.of(attendance, LinearExpr.term(vars.spread(), 1), variety);
|
||||
final List<String> phases = List.of("ATTENDANCE", "SPREAD", "VARIETY");
|
||||
final List<SchedulingPhase> phases =
|
||||
List.of(SchedulingPhase.ATTENDANCE, SchedulingPhase.SPREAD, SchedulingPhase.VARIETY);
|
||||
for (int stage = 0; stage < objectives.size(); stage++) {
|
||||
final LinearExpr objective = objectives.get(stage);
|
||||
final String phase = phases.get(stage);
|
||||
final SchedulingPhase phase = phases.get(stage);
|
||||
observer.accept(new SchedulingProgress(phase, best != null));
|
||||
final double remaining = (deadline - System.nanoTime()) / 1_000_000_000.0;
|
||||
if (remaining <= 0) {
|
||||
|
||||
@@ -2,15 +2,16 @@ package nl.agilecc.agiledancers.solver;
|
||||
|
||||
import com.google.ortools.sat.CpSolverSolutionCallback;
|
||||
import java.util.function.Consumer;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingPhase;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingProgress;
|
||||
|
||||
/** Publish only the first feasible solution in a phase, avoiding an event per solver improvement. */
|
||||
public class SchedulingSolutionObserver extends CpSolverSolutionCallback {
|
||||
private final Consumer<SchedulingProgress> observer;
|
||||
private final String phase;
|
||||
private final SchedulingPhase phase;
|
||||
private boolean reported;
|
||||
|
||||
public SchedulingSolutionObserver(final Consumer<SchedulingProgress> observer, final String phase) {
|
||||
public SchedulingSolutionObserver(final Consumer<SchedulingProgress> observer, final SchedulingPhase phase) {
|
||||
this.observer = observer;
|
||||
this.phase = phase;
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import java.util.Set;
|
||||
import nl.agilecc.agiledancers.model.scheduling.GroupRequirement;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingFailureKind;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingInput;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingPhase;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingProgress;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingResult;
|
||||
import nl.agilecc.agiledancers.model.scheduling.SchedulingSummary;
|
||||
@@ -26,11 +27,11 @@ class SchedulingProgressTest {
|
||||
final SchedulingResult result = new OrToolsSchedulingEngine().solve(input, updates::add);
|
||||
assertTrue(result.optimal());
|
||||
assertEquals(
|
||||
List.of("ATTENDANCE", "SPREAD", "VARIETY"),
|
||||
List.of(SchedulingPhase.ATTENDANCE, SchedulingPhase.SPREAD, SchedulingPhase.VARIETY),
|
||||
updates.stream().map(SchedulingProgress::phase).distinct().toList());
|
||||
assertEquals(new SchedulingProgress("ATTENDANCE", false), updates.getFirst());
|
||||
assertTrue(updates.contains(new SchedulingProgress("ATTENDANCE", true)));
|
||||
assertTrue(updates.contains(new SchedulingProgress("VARIETY", true)));
|
||||
assertEquals(new SchedulingProgress(SchedulingPhase.ATTENDANCE, false), updates.getFirst());
|
||||
assertTrue(updates.contains(new SchedulingProgress(SchedulingPhase.ATTENDANCE, true)));
|
||||
assertTrue(updates.contains(new SchedulingProgress(SchedulingPhase.VARIETY, true)));
|
||||
}
|
||||
|
||||
public @Test void noSolutionAndInfeasibleHaveStableDistinctOutcomes() {
|
||||
|
||||
@@ -23,6 +23,7 @@ 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.SchedulingRun;
|
||||
import nl.agilecc.agiledancers.service.model.SchedulingState;
|
||||
import nl.agilecc.agiledancers.service.port.SchedulingEngine;
|
||||
import nl.agilecc.agiledancers.solver.OrToolsSchedulingEngine;
|
||||
import org.junit.jupiter.api.Test;
|
||||
@@ -41,7 +42,7 @@ class SchedulingRunsTest {
|
||||
final Runnable unsubscribe = runs.subscribe(updates::set);
|
||||
assertTrue(updates.get().isEmpty());
|
||||
final SchedulingRun started = runs.start(school, show.getId());
|
||||
assertEquals("RUNNING", started.state());
|
||||
assertEquals(SchedulingState.RUNNING, started.state());
|
||||
assertTrue(entered.await(10, TimeUnit.SECONDS));
|
||||
assertEquals(started, runs.status(school, show.getId()));
|
||||
assertThrows(ManagementException.class, () -> runs.start(school, show.getId()));
|
||||
@@ -51,12 +52,12 @@ class SchedulingRunsTest {
|
||||
assertEquals(started, reconnect.get().getFirst());
|
||||
release.countDown();
|
||||
final SchedulingRun finished = await(runs, show);
|
||||
assertEquals("OPTIMAL", finished.state());
|
||||
assertEquals(SchedulingState.OPTIMAL, finished.state());
|
||||
assertEquals(0, finished.summary().performanceSpread());
|
||||
assertTrue(finished.progress().feasible());
|
||||
assertTrue(finished.finishedAt().compareTo(finished.startedAt()) >= 0);
|
||||
assertFalse(show.getDayParts().getFirst().getDanceGroups().isEmpty());
|
||||
assertEquals("RUNNING", updates.get().getFirst().state());
|
||||
assertEquals(SchedulingState.RUNNING, updates.get().getFirst().state());
|
||||
disconnect.run();
|
||||
assertNotEquals(started.runId(), runs.start(school, show.getId()).runId());
|
||||
await(runs, show);
|
||||
@@ -78,9 +79,9 @@ class SchedulingRunsTest {
|
||||
assertTrue(entered.await(10, TimeUnit.SECONDS));
|
||||
schools.touch(first);
|
||||
release.countDown();
|
||||
assertEquals("STALE", await(runs, first).state());
|
||||
assertEquals(SchedulingState.STALE, await(runs, first).state());
|
||||
assertTrue(first.getDayParts().getFirst().getDanceGroups().isEmpty());
|
||||
assertEquals("OPTIMAL", await(runs, second).state());
|
||||
assertEquals(SchedulingState.OPTIMAL, await(runs, second).state());
|
||||
} finally {
|
||||
release.countDown();
|
||||
runs.close();
|
||||
@@ -112,7 +113,7 @@ class SchedulingRunsTest {
|
||||
try {
|
||||
runs.start(school, show.getId());
|
||||
final SchedulingRun failed = await(runs, show);
|
||||
assertNotEquals("RUNNING", failed.state());
|
||||
assertNotEquals(SchedulingState.RUNNING, failed.state());
|
||||
assertTrue(failed.message().contains("behouden"));
|
||||
assertTrue(show.getDayParts().getFirst().getDanceGroups().isEmpty());
|
||||
schools.endScheduling(schools.beginScheduling(school, show.getId()).showId());
|
||||
@@ -127,10 +128,11 @@ class SchedulingRunsTest {
|
||||
false, new OrToolsSchedulingEngine().solve(input).assignments());
|
||||
final var runs = new SchedulingRuns(schools, engine);
|
||||
runs.start(school, show.getId());
|
||||
assertEquals("VALID", await(runs, show).state());
|
||||
assertEquals(SchedulingState.VALID, await(runs, show).state());
|
||||
runs.close();
|
||||
assertThrows(java.util.concurrent.RejectedExecutionException.class, () -> runs.start(school, show.getId()));
|
||||
assertEquals("SOLVER_FAILED", runs.status(school, show.getId()).state());
|
||||
assertEquals(
|
||||
SchedulingState.SOLVER_FAILED, runs.status(school, show.getId()).state());
|
||||
schools.endScheduling(schools.beginScheduling(school, show.getId()).showId());
|
||||
}
|
||||
|
||||
@@ -170,11 +172,11 @@ class SchedulingRunsTest {
|
||||
private SchedulingRun await(final SchedulingRuns runs, final Show show) {
|
||||
final long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
|
||||
SchedulingRun status = runs.status(school, show.getId());
|
||||
while (status.state().equals("RUNNING") && System.nanoTime() < deadline) {
|
||||
while (status.state().equals(SchedulingState.RUNNING) && System.nanoTime() < deadline) {
|
||||
LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(10));
|
||||
status = runs.status(school, show.getId());
|
||||
}
|
||||
assertNotEquals("RUNNING", status.state());
|
||||
assertNotEquals(SchedulingState.RUNNING, status.state());
|
||||
return status;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { SchedulingRun, SchedulingStatus } from './scheduling-status';
|
||||
import { SchedulingRun, SchedulingState, SchedulingStatus } from './scheduling-status';
|
||||
|
||||
class FakeEventSource {
|
||||
static current: FakeEventSource;
|
||||
@@ -14,7 +14,7 @@ class FakeEventSource {
|
||||
}
|
||||
}
|
||||
|
||||
function run(state: string, runId = 'run-1'): SchedulingRun {
|
||||
function run(state: SchedulingState, runId = 'run-1'): SchedulingRun {
|
||||
return {
|
||||
runId,
|
||||
schoolId: 1,
|
||||
@@ -82,6 +82,19 @@ describe('Scheduling status over SSE', () => {
|
||||
expect(status.notifications()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('announces a locally started run completed before the first SSE snapshot', () => {
|
||||
FakeEventSource.current.onerror?.();
|
||||
status.started(run('RUNNING'));
|
||||
vi.advanceTimersByTime(3000);
|
||||
FakeEventSource.current.send([run('OPTIMAL'), run('VALID', 'historical')]);
|
||||
expect(status.notifications().map((item) => item.runId)).toEqual(['run-1']);
|
||||
status.dismiss();
|
||||
FakeEventSource.current.onerror?.();
|
||||
vi.advanceTimersByTime(3000);
|
||||
FakeEventSource.current.send([run('OPTIMAL'), run('VALID', 'historical')]);
|
||||
expect(status.notifications()).toEqual([]);
|
||||
});
|
||||
|
||||
it('retains a fast completion when the start response arrives after SSE', () => {
|
||||
FakeEventSource.current.send([]);
|
||||
FakeEventSource.current.send([run('OPTIMAL')]);
|
||||
|
||||
@@ -1,5 +1,16 @@
|
||||
import { DestroyRef, inject, Injectable, signal } from '@angular/core';
|
||||
|
||||
export type SchedulingState =
|
||||
| 'RUNNING'
|
||||
| 'OPTIMAL'
|
||||
| 'VALID'
|
||||
| 'STALE'
|
||||
| 'INVALID_INPUT'
|
||||
| 'INFEASIBLE'
|
||||
| 'NO_SOLUTION'
|
||||
| 'SOLVER_FAILED';
|
||||
export type SchedulingPhase = 'PREPARING' | 'ATTENDANCE' | 'SPREAD' | 'VARIETY';
|
||||
|
||||
export interface SchedulingRun {
|
||||
runId: string;
|
||||
schoolId: number;
|
||||
@@ -7,8 +18,8 @@ export interface SchedulingRun {
|
||||
showName: string;
|
||||
startedAt: string;
|
||||
finishedAt: string | null;
|
||||
state: string;
|
||||
progress: { phase: string; feasible: boolean };
|
||||
state: SchedulingState;
|
||||
progress: { phase: SchedulingPhase; feasible: boolean };
|
||||
message: string;
|
||||
summary: {
|
||||
attendanceExcess: number;
|
||||
@@ -58,10 +69,16 @@ export class SchedulingStatus {
|
||||
}
|
||||
|
||||
private accept(runs: SchedulingRun[]): void {
|
||||
const activeRuns = new Set(
|
||||
this.runs()
|
||||
.filter((run) => run.state === 'RUNNING')
|
||||
.map((run) => run.runId),
|
||||
);
|
||||
for (const run of runs) {
|
||||
if (run.state === 'RUNNING' || this.seen.has(run.runId)) continue;
|
||||
this.seen.add(run.runId);
|
||||
if (this.initialized) this.notifications.update((items) => [...items, run]);
|
||||
if (this.initialized || activeRuns.has(run.runId))
|
||||
this.notifications.update((items) => [...items, run]);
|
||||
}
|
||||
this.initialized = true;
|
||||
this.runs.set(runs);
|
||||
@@ -96,7 +113,7 @@ export class SchedulingStatus {
|
||||
ATTENDANCE: 'Aanwezigheid',
|
||||
SPREAD: 'Spreiding',
|
||||
VARIETY: 'Variatie',
|
||||
} as Record<string, string>
|
||||
} satisfies Record<SchedulingPhase, string>
|
||||
)[run.progress.phase] ?? run.progress.phase
|
||||
);
|
||||
}
|
||||
|
||||
@@ -22,8 +22,8 @@ without event replay. Run identifiers deduplicate completion notifications.
|
||||
## Validation
|
||||
|
||||
- Maven verify: 60 backend tests passed, plus 19 standards-checker regression tests.
|
||||
- Backend line coverage: 1349/1387, or 97.26%, above the required 80%.
|
||||
- Frontend: 29 unit tests, production build and ESLint.
|
||||
- Backend line coverage: 1346/1384, or 97.25%, above the required 80%.
|
||||
- Frontend: 30 unit tests, production build and ESLint.
|
||||
- Playwright: three real REST/browser scenarios, including scheduling completion,
|
||||
result summaries, a completion snackbar, reloading to recover the latest result
|
||||
and suppression of old notifications after reload.
|
||||
@@ -32,12 +32,17 @@ without event replay. Run identifiers deduplicate completion notifications.
|
||||
- Background tests cover duplicate exclusion, parallel shows, stale-result
|
||||
rejection, preserved assignments, failure outcomes and retry after failure.
|
||||
- Frontend tests cover missed completion after disconnect, reconnection, heartbeat
|
||||
handling, deduplication and completion arriving before the POST response.
|
||||
handling, deduplication, completion arriving before the POST response and a
|
||||
locally started run completing before the first successful SSE connection.
|
||||
- Real solver tests cover phase/discovery callbacks, distinct infeasible/no-solution
|
||||
outcomes and objective-summary definitions including coach exemptions.
|
||||
|
||||
## Manual standards assessment
|
||||
|
||||
Scheduling states and phases use backend enums and frontend string unions while
|
||||
preserving their JSON names. Initial SSE recovery notifies for locally tracked
|
||||
running tasks while suppressing historical completions.
|
||||
|
||||
No waivers or weakened checks were introduced. The architecture tests pass.
|
||||
New records contain immutable values; the SSE snapshot defensively copies its
|
||||
list. The ten-component SchedulingRun record groups run identity, timestamps,
|
||||
@@ -47,14 +52,14 @@ framework-independent domain types; OR-Tools remains encapsulated in solver.
|
||||
|
||||
Retained review signals:
|
||||
|
||||
- SchedulingRuns fan-out (25): task orchestration uses JDK executor/collection
|
||||
- SchedulingRuns fan-out (26): task orchestration uses JDK executor/collection
|
||||
types, the scheduling port and immutable input/progress/result/status models.
|
||||
Its injected dependencies remain two; splitting these short lifecycle methods
|
||||
would distribute the shared publication and run-slot ordering.
|
||||
- SchedulingEventsResource fan-out (16): SSE/Mutiny transport, JSON envelope and
|
||||
service subscription types are confined to the REST adapter.
|
||||
- OrToolsSchedulingEngine fan-out (21), solve cognitive complexity (11) and
|
||||
length (40 code lines): the ordered three-stage loop retains deadline and
|
||||
length (41 code lines): the ordered three-stage loop retains deadline and
|
||||
earlier-feasible-result fallback behavior while publishing phase/discovery.
|
||||
requireSolution cyclomatic complexity (8) retains explicit mappings for stable
|
||||
failure outcomes and their human-readable explanations.
|
||||
|
||||
Reference in New Issue
Block a user