Files
sure/test/models/family_export_test.rb
Guillem Arias Fauste 9d4b4bc27c feat(jobs): reap imports and exports stuck by lost background jobs (#2681)
* feat(jobs): reap imports and exports stuck by lost background jobs

A hard-killed worker (OOM, SIGKILL during deploy) loses its in-flight
Sidekiq job permanently. Sync is the only model with a stale sweep;
everything else wedges in a non-terminal status forever:

- Import stuck "importing"/"reverting" (e.g. #2274)
- ImportSession stuck "importing" — unrecoverable, publish_later
  refuses to re-publish while importing
- FamilyExport stuck "pending"/"processing" — exports index polls
  every 3s indefinitely
- PdfImport's AI-processing claim held forever (ProcessPdfJob's
  reclaim only runs if the job is redelivered)
- Provider activities_fetch_pending flags stranded when a
  self-rescheduling fetch chain loses a link

SyncCleanerJob (existing hourly cron) now sweeps all of them, each
isolated so one failure doesn't block the rest, and records every
reaped record as a DebugLogEntry with the family attached.

Idempotency guards so a stray or redelivered job cannot corrupt a
record the reaper (or a user retry) has since moved on:

- Import#publish skips complete/reverting/revert_failed imports —
  a redelivered ImportJob after completion double-applies data for
  import types without row dedup. Failed imports deliberately stay
  publishable: their transaction rolled back, so a re-run is a retry.
- FamilyDataExportJob refuses terminal exports but still allows
  processing ones through, since graceful-shutdown redelivery is what
  completes them.

Reaper thresholds (6h imports, 2h exports) key off updated_at and
dwarf legitimate runtimes, so live jobs are not swept in practice.

* fix(jobs): make the reapers race-safe and commit-aware

Review feedback on #2681 (jjmata, Codex, CodeRabbit):

- Every sweep now mutates under record.with_lock with a staleness
  re-check, mirroring the guard Sync#perform gained in #2680 — a job
  finishing between the sweep query and the write can no longer be
  clobbered mid-flight
- Import.clean distinguishes which side of import!'s single transaction
  the worker died on: rows attached means the data committed and only
  the status write was lost, so the record is finalized as complete
  (marking it failed invited a re-publish that double-imports types
  without row dedup, e.g. TradeImport); no rows means a clean rollback
  and the failed/try-again path stays
- PdfImport.clean applies the same split: no rows → the AI-extraction
  claim died, reclaim to pending; rows → the publish died post-commit,
  finalize complete instead of letting the same extracted rows be
  published twice. Stuck PdfImport reverts (previously unswept by
  either clean) now go to revert_failed like every other import
- ImportSession.clean reconciles chunks whose import! committed but
  never got the complete-status write before failing the session, so
  re-publish skips them instead of duplicating their rows via
  SureImport's split path
- Import#publish redelivery skip is captured via DebugLogEntry instead
  of Rails.logger so support can see it in /settings/debug
- The interrupted-error copy is i18n-backed (imports.errors.interrupted)

* fix(jobs): round-2 review feedback on the reapers

- Import.clean excludes session-owned chunks (import_session_id: nil) —
  SyncCleanerJob runs it before ImportSession.clean, so it could
  finalize or fail a SureImport chunk outside the session flow that
  owns its lifecycle. Regression test added (CodeRabbit major)
- family.sync_later moved outside the row-lock transaction in both
  reap paths — Rails doesn't defer enqueues to after-commit by default,
  so Sidekiq could pick the job up before the status write was visible
- Per-record rescue in Import.clean / PdfImport.clean so one bad record
  doesn't abort the rest of the hourly sweep
- ImportSession.clean uses the importing? enum predicate;
  reconcile_committed_chunks! iterates with each (default-ordered
  association made find_each warn, and chunk counts are tiny)

* fix(jobs): address round-3 reaper review (isolation + commit signal)

Per-record rescue isolation for the three sweeps that lacked it —
FamilyExport.clean, ImportSession.clean, and SyncCleanerJob's
activity-flag loop — mirroring Import.clean/PdfImport.clean. One bad
record (validation error, DB blip) no longer aborts the rest of that
sweep for the hour; the activity-flag guard is per-record so a failure
also stops skipping the models that follow.

data_committed? now covers Category/Rule/Merchant imports, whose
records hang off the family rather than the import (no entries or
accounts), via the new Import#committed_by_named_records? helper. A
committed one is reaped to complete instead of a retryable failed;
nameless RuleImport rows carry no stable key, so a nameless-only file
has no commit signal and stays retryable.

* fix(schema): drop duplicate enable_banking_accounts columns from merge

The merge of main into this branch re-appended product, credit_limit,
and identification_hashes after updated_at, so schema.rb declared each
twice and db:schema:load raised "you can't define an already defined
column 'product'". Removed the duplicate declarations (kept the new
treat_balance_as_available_credit column); the test database loads
again.
2026-07-25 05:04:30 +02:00

157 lines
4.7 KiB
Ruby

require "test_helper"
class FamilyExportTest < ActiveSupport::TestCase
setup do
@family = families(:dylan_family)
@export = @family.family_exports.create!
end
test "belongs to family" do
assert_equal @family, @export.family
end
test "has default status of pending" do
assert_equal "pending", @export.status
end
test "force_fail! fails a lost export but refuses fresh or terminal ones" do
@export.update_columns(status: "processing", updated_at: 2.hours.ago)
assert @export.force_fail!
assert_equal "failed", @export.reload.status
fresh = @family.family_exports.create!
fresh.update_columns(status: "processing", updated_at: 5.minutes.ago)
assert_not fresh.force_fail!
assert_equal "processing", fresh.reload.status
assert_not @export.force_fail!
assert_equal "failed", @export.reload.status
end
test "can have export file attached" do
@export.export_file.attach(
io: StringIO.new("test content"),
filename: "test.zip",
content_type: "application/zip"
)
assert @export.export_file.attached?
assert_equal "test.zip", @export.export_file.filename.to_s
assert_equal "application/zip", @export.export_file.content_type
end
test "filename is generated correctly" do
travel_to Time.zone.local(2024, 1, 15, 14, 30, 0) do
export = @family.family_exports.create!
expected_filename = "sure_export_20240115_143000.zip"
assert_equal expected_filename, export.filename
end
end
test "downloadable? returns true for completed export with file" do
@export.update!(status: "completed")
@export.export_file.attach(
io: StringIO.new("test content"),
filename: "test.zip",
content_type: "application/zip"
)
assert @export.downloadable?
end
test "downloadable? returns false for pending export" do
@export.update!(status: "pending")
@export.export_file.attach(
io: StringIO.new("test content"),
filename: "test.zip",
content_type: "application/zip"
)
assert_not @export.downloadable?
end
test "downloadable? returns false for completed export without file" do
@export.update!(status: "completed")
assert_not @export.downloadable?
end
test "downloadable? returns false for failed export with file" do
@export.update!(status: "failed")
@export.export_file.attach(
io: StringIO.new("test content"),
filename: "test.zip",
content_type: "application/zip"
)
assert_not @export.downloadable?
end
test "export file is purged when export is destroyed" do
@export.export_file.attach(
io: StringIO.new("test content"),
filename: "test.zip",
content_type: "application/zip"
)
# Verify file is attached
assert @export.export_file.attached?
file_id = @export.export_file.id
blob_id = @export.export_file.blob.id
# Destroy the export
@export.destroy!
# Verify the export record is gone
assert_not FamilyExport.exists?(@export.id)
# Verify the Active Storage attachment is gone
assert_not ActiveStorage::Attachment.exists?(file_id)
# Note: Active Storage purges blobs asynchronously with dependent: :purge_later
# In tests, we can verify the attachment is gone, which is the immediate effect
# The blob will be purged in the background
end
test "can transition through statuses" do
assert_equal "pending", @export.status
@export.processing!
assert_equal "processing", @export.status
@export.completed!
assert_equal "completed", @export.status
@export.failed!
assert_equal "failed", @export.status
end
test "ordered scope returns exports in descending order" do
# Clear existing exports to avoid interference
@family.family_exports.destroy_all
# Create exports with specific timestamps
old_export = @family.family_exports.create!
old_export.update_column(:created_at, 2.days.ago)
new_export = @family.family_exports.create!
new_export.update_column(:created_at, 1.day.ago)
ordered_exports = @family.family_exports.ordered.to_a
assert_equal 2, ordered_exports.length
assert_equal new_export.id, ordered_exports.first.id
assert_equal old_export.id, ordered_exports.last.id
end
test "clean isolates a record whose update! fails so the sweep does not abort" do
@export.update_columns(status: "processing", updated_at: 3.hours.ago)
# A validation/DB error on one stuck record must not abort the sweep for
# the rest (mirrors Import.clean's per-record rescue).
FamilyExport.any_instance.stubs(:update!).raises(ActiveRecord::RecordInvalid.new(FamilyExport.new))
assert_nothing_raised { FamilyExport.clean }
assert_equal "processing", @export.reload.status
end
end