Skip to content

Sometimes deposits are not marked as validated even when document is created #925

Description

@PascalRepond

Bug description

When validating (approving) a deposit, the system creates the document but
sometimes does not mark the deposit as validated. The moderator then retries,
which creates the document a second time (double validation).

Example: document https://folia.unifr.ch/manage/records/documents/detail/324501
was created, but the corresponding deposit stayed "to validate":
https://folia.unifr.ch/manage/deposit/324490/create

Root cause

The approve flow is neither atomic nor idempotent.

In DepositRecord.create_document()
(sonar/modules/deposits/api.py):

  1. DocumentRecord.create(metadata, dbcommit=True, ...) commits the document
    to the DB immediately
    (independently of the deposit).
  2. Files are added, then document.reindex() runs — this dumps all resolved
    fields and, if there are more than 1000 fields, ES throws here.
  3. self["document"] = {"$ref": ...} (the deposit→document link) is only set
    after the reindex.

In the review endpoint (sonar/modules/deposits/rest.py):
create_document()deposit["status"] = VALIDATED → email → final
db.session.commit().

So if document.reindex() (or any later step) raises:

  • the document is already committed (step 1),
  • the deposit→document link (step 3) and status = VALIDATED + final commit
    never run → the deposit stays "to validate",
  • the request returns 500.

Result: document created and committed, deposit not validated, no link
between them. On retry, create_document() runs again with no guard checking
self.get("document") → a second document is created.

The >1000-fields ES error is only the trigger that was observed; the
underlying defect is the non-atomic, non-idempotent design, which any
post-create_document() failure (email, network to ES, DB hiccup, a future
mapping change) can re-trigger.

Relation to #1014

#1014 (trim dumped linked fields) removes the specific >1000-fields trigger,
and the ES field limit was already raised in prod. Those reduce the
occurrences but do not fix the structural fragility addressed here.

Expected behavior

Validating a deposit either fully succeeds (document created and deposit
marked validated, atomically) or fully fails (nothing created, deposit
unchanged) — and can never create a duplicate document.

What should be done

  • Atomicity (full rollback on failure): make document creation +
    deposit status update a single transaction. If any step fails (including
    indexing), roll everything back so neither a document nor a validated
    deposit remains; the moderator can retry cleanly. Do not commit the
    document independently before the deposit status is ready to commit.
  • Idempotency guard: in create_document(), if the deposit already has a
    linked document (self.get("document")), do not create another one.

Acceptance criteria

  • On a successful approve: document created, deposit validated, and the
    deposit.document link set — all committed together
  • On a failure during approve (incl. indexing error): no document is
    persisted and the deposit stays to_validate
  • Re-running approve on a deposit that already has a linked document does
    not create a second document
  • Test reproducing a failure mid-validation and asserting consistent state

Server: production (folia) — v1.8.2

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBreaks something but is not blockingclient request

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions