Cancel Schedule

[scottf]

No description provided.

[scottf]

This is unrelated housekeeping.

[scottf]

@claude please review in relation to ADR-51.md, section "Ending/stopping schedules early" https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-51.md#endingstopping-schedules-early

[synadia-claude-reviewer]

Claude finished @scottf's task in 2m 5s —— View job

---

Review: Cancel Schedule vs ADR-51 "Ending/stopping schedules early"

• Fetch ADR-51.md "Ending/stopping schedules early" section
• Read changed files (ScheduleManagement.java, ScheduledMessageBuilder.java, tests)
• Analyze conformance with ADR-51 spec
• Post review findings

---

ADR-51 Spec Summary (Ending/Stopping Schedules Early)

The spec defines two mechanisms:

Basic termination: delete by stream sequence number, purge by exact subject, purge by wildcard subject pattern.

Advanced atomic termination: publish to a different subject (not the schedule subject) while simultaneously stopping the schedule, using:

• Required: Nats-Schedule-Next: purge and Nats-Scheduler: <schedule-subject>
• Optional guard: Nats-Expected-Last-Subject-Sequence + Nats-Expected-Last-Subject-Sequence-Subject

Hard constraint: server rejects with error 10212 if Nats-Scheduler equals the publish subject, or if it's empty/invalid.

---

Overall Conformance: ✅ Correct

The implementation correctly covers all three basic termination modes and both atomic termination variants. One bug and a few observations below.

---

Bug: Header Override Order in makeHeaders

File: ScheduleManagement.java:270–279

private static Headers makeHeaders(String scheduleSubject, @Nullable Headers userHeaders) {
    Headers h = new Headers();
    h.put(NATS_SCHEDULE_NEXT_HDR, "purge");   // required headers set FIRST
    h.put(NATS_SCHEDULER_HDR, scheduleSubject);
    if (userHeaders != null) {
        for (String key : userHeaders.keySet()) {
            h.put(key, userHeaders.get(key)); // user headers applied LAST → they win
        }
    }
    return h;
}

The Javadoc for both publishAndCancelSchedule overloads says:

"The Nats-Scheduler and Nats-Schedule-Next headers are always set by this method and override any conflicting keys from userHeaders"

But the code does the opposite — user headers are applied last, so they would overwrite Nats-Schedule-Next and Nats-Scheduler if a caller passes them in userHeaders. The required headers must be set after user headers to match the documented contract. Fix this →

---

Observations (non-blocking)

1. No client-side guard for targetSubject == scheduleSubject

ScheduleManagement.java:213–227 — the Javadoc correctly documents that the server rejects this with 10212, but there is no client-side validation. A IllegalArgumentException before the network round-trip would improve the developer experience. Optional, but worth considering.

2. Silent ambiguity in findStreamLenient

ScheduleManagement.java:282–288 — when publishOnlyIfScheduleExists=true and the schedule subject matches zero streams or more than one stream, the method returns null (silent no-publish). The multi-stream case is ambiguous and could be surprising to callers; a debug-level log or documented note that ambiguous stream matches are treated as "not found" would help.

3. cancelSchedule(jsm, scheduleSubject, scheduleStream) uses delete-by-sequence for exact subjects, not purge

ScheduleManagement.java:162–177 — the ADR lists "purge a specific schedule via its subject" as a valid basic termination method. The implementation instead resolves the sequence and deletes by sequence. This is functionally equivalent for a single schedule per subject, and arguably more precise (the purge path is kept for wildcards). No issue, just noting it differs from the literal ADR wording.

4. Missing test case: userHeaders conflict with required headers

There is no test that passes Nats-Schedule-Next or Nats-Scheduler in the userHeaders argument to verify which value wins. Once the bug above is fixed this test would serve as a regression guard.

5. Missing test case: publishAndCancelSchedule with targetSubject == scheduleSubject

A test confirming the server rejects this with JetStreamApiException (error 10212) would validate the documented constraint end-to-end.

---

Deleted File

ScheduledStreamUtil.java (removed) — its createSchedulableStream equivalent is now on ScheduleManagement. Clean consolidation, no issues.

[scottf]

All review comments have been addressed

[MauriceVanVeen]

LGTM

[MauriceVanVeen]

Doesn't this overwrite the above message? Should these be + "every"?