Skip to content

Commit 2b45f38

Browse files
bodymindartsclaude
andcommitted
refactor(job): remove cancel_job API
Consumers handle cancellation at their own layer, so the crate-level cancel_job method, Cancelled event variant, CannotCancelJob error, and related tests are no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 616d6ea commit 2b45f38

4 files changed

Lines changed: 2 additions & 195 deletions

File tree

src/entity.rs

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ pub enum JobEvent {
6969
error: String,
7070
},
7171
JobCompleted,
72-
Cancelled,
7372
AttemptCounterReset,
7473
}
7574

@@ -177,20 +176,12 @@ impl Job {
177176
serde_json::from_value(self.config.clone())
178177
}
179178

180-
/// Returns `true` once the job has emitted a `JobCompleted` or `Cancelled` event.
179+
/// Returns `true` once the job has emitted a `JobCompleted` event.
181180
pub fn completed(&self) -> bool {
182181
self.events
183182
.iter_all()
184183
.rev()
185-
.any(|event| matches!(event, JobEvent::JobCompleted | JobEvent::Cancelled))
186-
}
187-
188-
/// Returns `true` if the job was cancelled.
189-
pub fn cancelled(&self) -> bool {
190-
self.events
191-
.iter_all()
192-
.rev()
193-
.any(|event| matches!(event, JobEvent::Cancelled))
184+
.any(|event| matches!(event, JobEvent::JobCompleted))
194185
}
195186

196187
pub(crate) fn inject_tracing_parent(&self) {
@@ -236,14 +227,6 @@ impl Job {
236227
self.events.push(JobEvent::JobCompleted);
237228
}
238229

239-
pub(crate) fn cancel(&mut self) -> es_entity::Idempotent<()> {
240-
if self.completed() {
241-
return es_entity::Idempotent::AlreadyApplied;
242-
}
243-
self.events.push(JobEvent::Cancelled);
244-
es_entity::Idempotent::Executed(())
245-
}
246-
247230
pub(super) fn schedule_retry(
248231
&mut self,
249232
error: String,
@@ -333,7 +316,6 @@ impl TryFromEvents<JobEvent> for Job {
333316
JobEvent::ExecutionAborted { .. } => {}
334317
JobEvent::ExecutionErrored { .. } => {}
335318
JobEvent::JobCompleted => {}
336-
JobEvent::Cancelled => {}
337319
JobEvent::AttemptCounterReset => {}
338320
}
339321
}

src/error.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@ pub enum JobError {
3636
DuplicateId(Option<String>),
3737
#[error("JobError - DuplicateUniqueJobType: {0:?}")]
3838
DuplicateUniqueJobType(Option<String>),
39-
#[error(
40-
"JobError - CannotCancelJob: job is not in pending state (may be running or already completed)"
41-
)]
42-
CannotCancelJob,
4339
#[error("JobError - Config: {0}")]
4440
Config(String),
4541
#[error("JobError - Migration: {0}")]

src/lib.rs

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ mod tracker;
220220

221221
pub mod error;
222222

223-
use es_entity::AtomicOperation;
224223
use tracing::instrument;
225224

226225
use std::sync::{Arc, Mutex};
@@ -481,35 +480,6 @@ impl Jobs {
481480
Ok(self.repo.find_by_id(id).await?)
482481
}
483482

484-
/// Cancel a pending job, removing it from the execution queue.
485-
///
486-
/// This operation is idempotent — calling it on an already cancelled or
487-
/// completed job is a no-op. If the job exists but is currently running
488-
/// (not pending), returns [`JobError::CannotCancelJob`].
489-
#[instrument(name = "job.cancel_job", skip(self))]
490-
pub async fn cancel_job(&self, id: JobId) -> Result<(), JobError> {
491-
let mut op = self.repo.begin_op_with_clock(&self.clock).await?;
492-
let mut job = self.repo.find_by_id(id).await?;
493-
494-
if job.cancel().did_execute() {
495-
let result = sqlx::query!(
496-
r#"DELETE FROM job_executions WHERE id = $1 AND state = 'pending'"#,
497-
id as JobId,
498-
)
499-
.execute(op.as_executor())
500-
.await?;
501-
502-
if result.rows_affected() == 0 {
503-
return Err(JobError::CannotCancelJob);
504-
}
505-
506-
self.repo.update_in_op(&mut op, &mut job).await?;
507-
op.commit().await?;
508-
}
509-
510-
Ok(())
511-
}
512-
513483
/// Returns a reference to the clock used by this job service.
514484
pub fn clock(&self) -> &ClockHandle {
515485
&self.clock

tests/job.rs

Lines changed: 0 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -638,144 +638,3 @@ async fn test_bulk_spawn_empty_batch() -> anyhow::Result<()> {
638638

639639
Ok(())
640640
}
641-
642-
#[tokio::test]
643-
async fn test_cancel_pending_job() -> anyhow::Result<()> {
644-
let pool = helpers::init_pool().await?;
645-
let config = JobSvcConfig::builder()
646-
.pool(pool)
647-
.build()
648-
.expect("Failed to build JobsConfig");
649-
650-
let mut jobs = Jobs::init(config).await?;
651-
let spawner = jobs.add_initializer(TestJobInitializer);
652-
653-
// Spawn a job scheduled far in the future so it stays pending
654-
let job_id = JobId::new();
655-
let schedule_at = chrono::Utc::now() + chrono::Duration::hours(24);
656-
spawner
657-
.spawn_at(job_id, TestJobConfig { delay_ms: 50 }, schedule_at)
658-
.await?;
659-
660-
// Cancel the pending job
661-
jobs.cancel_job(job_id).await?;
662-
663-
// Verify it's findable as a cancelled/completed entity
664-
let found = jobs.find(job_id).await?;
665-
assert!(found.completed(), "Cancelled job should be completed");
666-
assert!(found.cancelled(), "Job should be marked as cancelled");
667-
668-
Ok(())
669-
}
670-
671-
#[tokio::test]
672-
async fn test_cancel_running_job_fails() -> anyhow::Result<()> {
673-
let pool = helpers::init_pool().await?;
674-
let config = JobSvcConfig::builder()
675-
.pool(pool)
676-
.build()
677-
.expect("Failed to build JobsConfig");
678-
679-
let mut jobs = Jobs::init(config).await?;
680-
681-
let started = Arc::new(Mutex::new(Vec::<String>::new()));
682-
let completed = Arc::new(Mutex::new(Vec::<String>::new()));
683-
let release = Arc::new(Notify::new());
684-
685-
let spawner = jobs.add_initializer(QueueJobInitializer {
686-
job_type: JobType::new("cancel-running-test"),
687-
started: Arc::clone(&started),
688-
completed: Arc::clone(&completed),
689-
release: Arc::clone(&release),
690-
});
691-
692-
jobs.start_poll()
693-
.await
694-
.expect("Failed to start job polling");
695-
696-
let job_id = JobId::new();
697-
spawner
698-
.spawn(job_id, QueueJobConfig { label: "X".into() })
699-
.await?;
700-
701-
// Wait for the job to start running
702-
let mut attempts = 0;
703-
loop {
704-
tokio::time::sleep(tokio::time::Duration::from_millis(50)).await;
705-
if !started.lock().await.is_empty() {
706-
break;
707-
}
708-
attempts += 1;
709-
assert!(attempts < 100, "Job never started");
710-
}
711-
712-
// Cancel on a running job should fail
713-
let result = jobs.cancel_job(job_id).await;
714-
assert!(
715-
matches!(result, Err(JobError::CannotCancelJob)),
716-
"Cancelling a running job should return JobNotPending, got err: {:?}",
717-
result.err(),
718-
);
719-
720-
// Release the job so it completes normally
721-
release.notify_one();
722-
723-
// Wait for completion
724-
let mut attempts = 0;
725-
loop {
726-
tokio::time::sleep(tokio::time::Duration::from_millis(50)).await;
727-
let job = jobs.find(job_id).await?;
728-
if job.completed() {
729-
break;
730-
}
731-
attempts += 1;
732-
assert!(attempts < 100, "Job never completed");
733-
}
734-
735-
Ok(())
736-
}
737-
738-
#[tokio::test]
739-
async fn test_cancel_already_completed_job_is_idempotent() -> anyhow::Result<()> {
740-
let pool = helpers::init_pool().await?;
741-
let config = JobSvcConfig::builder()
742-
.pool(pool)
743-
.build()
744-
.expect("Failed to build JobsConfig");
745-
746-
let mut jobs = Jobs::init(config).await?;
747-
let spawner = jobs.add_initializer(TestJobInitializer);
748-
749-
jobs.start_poll()
750-
.await
751-
.expect("Failed to start job polling");
752-
753-
let job_id = JobId::new();
754-
spawner
755-
.spawn(job_id, TestJobConfig { delay_ms: 10 })
756-
.await?;
757-
758-
// Wait for the job to complete
759-
let mut attempts = 0;
760-
loop {
761-
tokio::time::sleep(tokio::time::Duration::from_millis(50)).await;
762-
let job = jobs.find(job_id).await?;
763-
if job.completed() {
764-
break;
765-
}
766-
attempts += 1;
767-
assert!(attempts < 100, "Job never completed");
768-
}
769-
770-
// Cancel on an already completed job is a no-op
771-
jobs.cancel_job(job_id).await?;
772-
773-
let job = jobs.find(job_id).await?;
774-
assert!(
775-
!job.cancelled(),
776-
"Completed job should not be marked cancelled"
777-
);
778-
assert!(job.completed(), "Job should still be completed");
779-
780-
Ok(())
781-
}

0 commit comments

Comments
 (0)