Skip to content

feat(maintenance): add REST endpoints for system jobs #35207#35831

Open
hassandotcms wants to merge 2 commits into
mainfrom
35207-task-implement-system-jobs-list-and-delete-endpoints
Open

feat(maintenance): add REST endpoints for system jobs #35207#35831
hassandotcms wants to merge 2 commits into
mainfrom
35207-task-implement-system-jobs-list-and-delete-endpoints

Conversation

@hassandotcms
Copy link
Copy Markdown
Member

@hassandotcms hassandotcms commented May 26, 2026

  • GET /v1/maintenance/_systemJobs lists all Quartz scheduler jobs with trigger details (next fire time, misfire policy, running state).
  • DELETE /v1/maintenance/_systemJobs/{group}/{name} removes a Quartz job (returns 404 when the scheduler reports no matching job).
  • Replaces the legacy system_jobs.jsp scriptlet and the Struts deleteScheduler action; behavior is preserved 1:1.
  • Errored jobs surface with an 'error' field so admins can clean up orphans left by upgrades (e.g. ClassNotFoundException).
  • Deletions audit-logged to SecurityLogger.

Proposed Changes

  • change 1
  • change 2

Checklist

  • Tests
  • Translations
  • Security Implications Contemplated (add notes if applicable)

Additional Info

** any additional useful context or info **

Screenshots

Original Updated
** original screenshot ** ** updated screenshot **

This PR fixes: #35207

- GET /v1/maintenance/_systemJobs lists all Quartz scheduler jobs with
  trigger details (next fire time, misfire policy, running state).
- DELETE /v1/maintenance/_systemJobs/{group}/{name} removes a Quartz job
  (returns 404 when the scheduler reports no matching job).
- Replaces the legacy system_jobs.jsp scriptlet and the Struts
  deleteScheduler action; behavior is preserved 1:1.
- Errored jobs surface with an 'error' field so admins can clean up
  orphans left by upgrades (e.g. ClassNotFoundException).
- Deletions audit-logged to SecurityLogger.
- Note: these are Quartz scheduler jobs; the newer JobQueueManager jobs
  at /api/v1/jobs are a separate system and intentionally untouched.
@hassandotcms hassandotcms linked an issue May 26, 2026 that may be closed by this pull request
@hassandotcms hassandotcms changed the title feat(maintenance): add REST endpoints for Quartz system jobs #35207 feat(maintenance): add REST endpoints for system jobs #35207 May 26, 2026
@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label May 26, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 26, 2026

Claude finished @hassandotcms's task in 1m 35s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze code
  • Post review findings

3 issues worth addressing, plus a few minor notes.


1. Null JobDetail causes silent NPE swallowed as an error entry — listSystemJobs line 1558–1567

final JobDetail detail = scheduler.getJobDetail(taskName, group);
// … triggers check …
job.put("className", detail.getJobClass().getSimpleName());  // NPE if detail == null

getJobDetail() returns null when a job disappears between getJobNames() and this call (race). The NPE is silently caught by the outer catch (Exception e) at line 1592, producing a confusing error entry with no class or flags — but error will be "null" (from e.getMessage()). Add an explicit null check and continue (or skip-with-log) rather than relying on exception swallowing.

Fix this →


2. SecurityLogger fires before deletion, so it logs actions that may never complete — deleteSystemJob lines 1653–1659

SecurityLogger.logInfo(…, "is deleting Quartz job name='%s' group='%s'", name, group);

final boolean removed;
try {
    removed = QuartzUtils.removeJob(name, group);   // may throw

If removeJob throws, the audit trail records the deletion but it never happened. Move SecurityLogger.logInfo to after the if (!removed) check (or change the message to "attempted to delete"). Fix this →


3. MISFIRE_INSTRUCTION_SMART_POLICY (value 0, the default) maps to "UNKNOWN"listSystemJobs lines 1581–1589

if (misfire == CronTrigger.MISFIRE_INSTRUCTION_DO_NOTHING) {
    job.put("misfireInstruction", "DO_NOTHING");
} else if (misfire == CronTrigger.MISFIRE_INSTRUCTION_FIRE_ONCE_NOW) {
    job.put("misfireInstruction", "FIRE_ONCE_NOW");
} else {
    job.put("misfireInstruction", "UNKNOWN");   // ← default for most jobs
}

Trigger.MISFIRE_INSTRUCTION_SMART_POLICY = 0 is what most jobs use by default. Operators viewing the list will see every default-policy job as "UNKNOWN" which is confusing. Add a branch for Trigger.MISFIRE_INSTRUCTION_SMART_POLICY"SMART_POLICY". Fix this →


Minor notes (not blockers)

  • Only first trigger reported (line 1560): jobs with multiple triggers have additional fire times silently dropped. The JSP did the same, but worth a doc note or TODO.
  • detail.getJobClass().getSimpleName() returns "" for anonymous classes. getName() is more robust.
  • @Schema on delete response: ResponseEntitySystemJobDeleteView wraps Map<String, Object>. Per the REST CLAUDE.md, this should use @Schema(type = "object", description = "...") on the inner entity rather than leaving it undocumented. The @Schema(implementation = ResponseEntitySystemJobDeleteView.class) on the @ApiResponse is correct for the envelope, but the payload shape is opaque to OpenAPI.
  • Missing @ApiResponse(responseCode = "500") on both new endpoints: both can throw DotRuntimeException which the JAX-RS exception mapper will turn into a 500.

@hassandotcms hassandotcms marked this pull request as ready for review June 2, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Implement system jobs list and delete endpoints

2 participants