Reset MARC files on settings update (PP-4637)

[tdilauro]

Description

• Adds a marc_export_reset Celery task that sets up the next MARC export as a first run by removing existing files (database records and S3 objects). The records are deleted in one transaction, and S3 deletion is best-effort, so a failure won't leave the reset half-applied.
• Queues the reset when a library's MARC exporter settings change in the admin interface and when a library registry pushes a new web client URL (only if the change would result in a new web client URL).
• First-run full exports now also record a full-content delta pointing at the same S3 object, so (1) consumers that only apply delta files receive the complete refreshed dataset without a second upload and (2) we avoid an extra upload and the doubled storage utilization.
• Export finalization now detects a configuration change that raced an in-flight export (records reset, settings drift, or the library no longer MARC-enabled), discards the stale upload, and immediately re-queues a fresh full export.
• Cleanup and reset only delete a shared S3 object when the last record referencing it is removed, and upload aborts are best-effort so cleanup failures cannot cascade.
• The MARC download page labels the full-content delta as "Full content as of " instead of a date range starting at the full delta sentinel epoch.

Motivation and Context

MARC record exports run periodically, but a configuration change would not trigger an update. Record updates based on those changes might not become available until long after the change was made. Further, some MARC consumers apply only delta files rather than reprocessing full exports. When a library's MARC export configuration changed (organization code, summary/genre inclusion, web client URLs), records exported before the change were never corrected for those consumers, leaving stale data in their catalogs indefinitely. Resetting the library's export state on configuration changes forces a full refresh that reaches both full-file and delta-only consumers.

[Jira PP-4637]

How Has This Been Tested?

• New and updated tests for the new functionality.
• All tests/checks pass locally.
• CI tests/checks pass.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @tdilauro's task in 12m 18s —— View job

---

Code Review

• Read the full diff against origin/main
• Review celery/tasks/marc.py (reset task, stale-export detection, cleanup, first-run delta)
• Review exporter.py helpers (sentinel, reset selection, web-client URL impact)
• Review admin catalog_services.py reset enqueue
• Review opds_registration.py reset enqueue
• Review uploader.py and api/controller/marc.py
• Review test coverage

---

Summary

Solid, carefully-constructed change. I traced the end-to-end refresh flow (admin/registry change → reset → next scheduled marc_export sees last_updated is None → first-run full export → paired full-content delta) and the shared-key cleanup, and the correctness properties hold up. The previously-flagged races are adequately covered: the admin controller dispatches resets before commit so an enqueue failure rolls the settings back via the request teardown (api/routes.py:36), and _finalize_uploads + _verify_uploads_after_commit catch a config change on either side of the export commit (every reset trigger also mutates the compared LibraryInfo, and _export_config_changed's records_gone check covers a concurrent delete). The shared-S3-key ref-counting in both cleanup and reset is correct, and the best-effort abort/delete paths are appropriately swallowed. One robustness issue in the reset task is worth addressing before merge; everything else looks good.

Details

src/palace/manager/celery/tasks/marc.py:513-517

The docstring claims the reset is idempotent so duplicate tasks need no guard, but this per-object delete loop is not idempotent under concurrent execution. Duplicate resets for the same library are readily queued (an admin settings change, a registry web-client change, and _verify_uploads_after_commit can all fire close together). If two run concurrently and both load the same MarcFile rows via files_for_reset before either commits, the first commit succeeds and the second emits DELETE … WHERE id = … matching 0 rows — SQLAlchemy's default confirm_deleted_rows=True then raises StaleDataError, failing the second task (it self-heals only because the deletion already happened, but it produces a spurious task failure / error-log noise). A bulk delete sidesteps the row-count confirmation and is genuinely idempotent (and avoids loading the ORM objects):

keys = set(
    session.scalars(
        select(MarcFile.key).where(
            MarcFile.library == library,
            MarcFile.collection_id.in_(enabled_collection_ids),
        )
    )
)
session.execute(
    delete(MarcFile).where(
        MarcFile.library == library,
        MarcFile.collection_id.in_(enabled_collection_ids),
    )
)
session.commit()

Alternatively, if the guard is considered unnecessary in practice, soften the "the task is idempotent" claim to note the concurrent-duplicate caveat.

circulation/src/palace/manager/celery/tasks/marc.py

Lines 513 to 517 in cfe7cfb

	for file_record in MarcExporter.files_for_reset(session, library):
	task.log.info(f"Deleting MARC export {file_record.key} ({file_record.id}).")
	keys.add(file_record.key)
	session.delete(file_record)
	session.commit()

[greptile-apps]

Greptile Summary

This PR resets MARC exports when export-affecting settings change. The main changes are:

• Adds a reset task that removes MARC file records and best-effort deletes orphaned S3 objects.
• Rechecks in-flight exports before and after finalization so stale uploads can be discarded.
• Creates a first-run full-content delta that shares the full export object.
• Queues resets from MARC admin settings updates and registry web client URL changes.
• Updates cleanup and download-page behavior for shared full-content delta files.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
src/palace/manager/celery/tasks/marc.py	Adds reset handling, stale-export checks, full-content delta creation, and shared-key cleanup.
src/palace/manager/api/admin/controller/catalog_services.py	Queues MARC resets when saved per-library exporter settings change.
src/palace/manager/integration/discovery/opds_registration.py	Queues MARC resets when a registry web client URL change affects exported MARC content.
src/palace/manager/integration/catalog/marc/exporter.py	Adds the full-content delta sentinel, reset helpers, and web client URL de-duplication.
src/palace/manager/integration/catalog/marc/uploader.py	Makes discarded multipart upload aborts best-effort.
src/palace/manager/api/controller/marc.py	Labels full-content deltas separately on the MARC download page.

_{Reviews (5): Last reviewed commit: "CI AI code review feedback" | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 98.52941% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.48%. Comparing base (65b15bf) to head (cfe7cfb).

Files with missing lines	Patch %	Lines
src/palace/manager/celery/tasks/marc.py	98.90%	0 Missing and 1 partial ⚠️
...manager/integration/discovery/opds_registration.py	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3554      +/-   ##
==========================================
+ Coverage   93.46%   93.48%   +0.01%     
==========================================
  Files         512      512              
  Lines       46611    46733     +122     
  Branches     6352     6375      +23     
==========================================
+ Hits        43566    43687     +121     
+ Misses       1969     1968       -1     
- Partials     1076     1078       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.