fix(persistence): a transaction Bundle PUT to a deleted id restores it

crates/persistence/src/backends/mongodb/storage.rs

@@ -86,6 +86,19 @@ enum PendingSearchParameterChange {
     Delete,
 }
 
+/// The version a Bundle writes over a stored document: an update over the live
+/// document, or a restore over a tombstone (`deleted`, #2145).
+/// See [`MongoBackend::write_next_version_in_bundle_transaction`].
+struct NextVersion<'a> {
+    resource_type: &'a str,
+    id: &'a str,
+    stored: &'a Document,
+    expected_version: &'a str,
+    deleted: bool,
+    fhir_version: helios_fhir::FhirVersion,
+    change: PendingSearchParameterChange,
+}
+
 /// What one attempt at a Bundle transaction has staged: the SearchParameter
 /// changes for after its commit, for a Bundle that defers its search rows
 /// (#1975) the resources it left without them, and the creates not yet sent to
@@ -5301,6 +5314,31 @@ impl MongoBackend {
                         let mut resource_with_id = resource;
                         resource_with_id["id"] = serde_json::json!(id);
 
+                        // A deleted id is restored, as the same PUT outside a
+                        // Bundle is; only an id never stored creates (#2145).
+                        if let Some(deleted) = self
+                            .read_deleted_in_bundle_transaction(
+                                db,
+                                session,
+                                tenant,
+                                &resource_type,
+                                &id,
+                            )
+                            .await?
+                        {
+                            let restored = self
+                                .restore_resource_in_bundle_transaction(
+                                    db,
+                                    session,
+                                    tenant,
+                                    &deleted,
+                                    resource_with_id,
+                                    writes,
+                                )
+                                .await?;
+                            return Ok(BundleEntryResult::created(restored));
+                        }
+
                         let created = self.create_resource_in_bundle_transaction(
                             index,
                             tenant,
@@ -5568,7 +5606,6 @@ impl MongoBackend {
         writes: &mut BundleWrites,
     ) -> StorageResult<StoredResource> {
         let resources = db.collection::<Document>(MongoBackend::RESOURCES_COLLECTION);
-        let history = db.collection::<Document>(MongoBackend::RESOURCE_HISTORY_COLLECTION);
         let tenant_id = tenant.tenant_id().as_str();
         let resource_type = current.resource_type();
         let id = current.id();
@@ -5608,15 +5645,131 @@ impl MongoBackend {
             ));
         }
 
-        let new_version = next_version(current.version_id())?;
+        self.write_next_version_in_bundle_transaction(
+            db,
+            session,
+            tenant,
+            NextVersion {
+                resource_type,
+                id,
+                stored: &existing_doc,
+                expected_version: current.version_id(),
+                deleted: false,
+                fhir_version: current.fhir_version(),
+                change: PendingSearchParameterChange::Update,
+            },
+            resource,
+            writes,
+        )
+        .await
+    }
+
+    /// The tombstone of a soft-deleted resource, read inside the Bundle's
+    /// session; `None` when the id is live or was never stored.
+    async fn read_deleted_in_bundle_transaction(
+        &self,
+        db: &mongodb::Database,
+        session: &mut ClientSession,
+        tenant: &TenantContext,
+        resource_type: &str,
+        id: &str,
+    ) -> StorageResult<Option<Document>> {
+        let resources = db.collection::<Document>(MongoBackend::RESOURCES_COLLECTION);
+        resources
+            .find_one(doc! {
+                "tenant_id": tenant.tenant_id().as_str(),
+                "resource_type": resource_type,
+                "id": id,
+                "is_deleted": true,
+            })
+            .session(&mut *session)
+            .await
+            .map_err(|e| internal_driver_error("Failed to read deleted resource in transaction", e))
+    }
+
+    /// A `PUT` to the id of a soft-deleted resource restores it inside the
+    /// Bundle's session and continues its version chain, as the same `PUT`
+    /// outside a Bundle does ([`Self::restore_attempt`], FHIR
+    /// http.html#delete; #2145). `deleted` is the tombstone
+    /// ([`Self::read_deleted_in_bundle_transaction`]).
+    ///
+    /// The tombstone keeps its document under the unique, non-partial
+    /// `idx_resources_identity`, so a create over it would be refused as
+    /// [`ResourceError::AlreadyExists`]; that refusal was the Bundle's answer
+    /// before this restore existed.
+    async fn restore_resource_in_bundle_transaction(
+        &self,
+        db: &mongodb::Database,
+        session: &mut ClientSession,
+        tenant: &TenantContext,
+        deleted: &Document,
+        resource: Value,
+        writes: &mut BundleWrites,
+    ) -> StorageResult<StoredResource> {
+        let field = |name: &str| {
+            deleted.get_str(name).map_err(|e| {
+                internal_error(format!(
+                    "Missing {name} of deleted resource in transaction: {e}"
+                ))
+            })
+        };
+        // The restore keeps the FHIR version the resource was stored under.
+        let fhir_version =
+            FhirVersion::from_storage(deleted.get_str("fhir_version").unwrap_or("4.0"))
+                .unwrap_or_else(helios_fhir::FhirVersion::default_enabled);
+
+        self.write_next_version_in_bundle_transaction(
+            db,
+            session,
+            tenant,
+            NextVersion {
+                resource_type: field("resource_type")?,
+                id: field("id")?,
+                stored: deleted,
+                expected_version: field("version_id")?,
+                deleted: true,
+                fhir_version,
+                change: PendingSearchParameterChange::Create,
+            },
+            resource,
+            writes,
+        )
+        .await
+    }
+
+    /// Writes the version after `expected_version` over the stored document
+    /// (live, or a tombstone when `deleted`) inside the Bundle's session: the
+    /// compare-and-swap on the current document, its history row and its
+    /// search rows. A lost compare-and-swap is a `VersionConflict`.
+    async fn write_next_version_in_bundle_transaction(
+        &self,
+        db: &mongodb::Database,
+        session: &mut ClientSession,
+        tenant: &TenantContext,
+        next: NextVersion<'_>,
+        resource: Value,
+        writes: &mut BundleWrites,
+    ) -> StorageResult<StoredResource> {
+        let NextVersion {
+            resource_type,
+            id,
+            stored: existing_doc,
+            expected_version,
+            deleted,
+            fhir_version,
+            change,
+        } = next;
+        let resources = db.collection::<Document>(MongoBackend::RESOURCES_COLLECTION);
+        let history = db.collection::<Document>(MongoBackend::RESOURCE_HISTORY_COLLECTION);
+        let tenant_id = tenant.tenant_id().as_str();
+        let new_version = next_version(expected_version)?;
         let mut resource = resource;
         ensure_resource_identity(resource_type, id, &mut resource);
         let search_documents = self.search_index_documents(tenant_id, resource_type, id, &resource);
         let payload = value_to_document(&resource)?;
 
         let now = Utc::now();
         let now_bson = chrono_to_bson(now);
-        let fhir_version = current.fhir_version();
         let fhir_version_str = fhir_version.as_mime_param().to_string();
 
         let mut set = doc! {
@@ -5637,8 +5790,8 @@ impl MongoBackend {
                     "tenant_id": tenant_id,
                     "resource_type": resource_type,
                     "id": id,
-                    "version_id": current.version_id(),
-                    "is_deleted": false,
+                    "version_id": expected_version,
+                    "is_deleted": deleted,
                 },
                 search_documents.resource_update(doc! { "$set": set }),
             )
@@ -5651,13 +5804,13 @@ impl MongoBackend {
                 ConcurrencyError::VersionConflict {
                     resource_type: resource_type.to_string(),
                     id: id.to_string(),
-                    expected_version: current.version_id().to_string(),
+                    expected_version: expected_version.to_string(),
                     actual_version: "unknown".to_string(),
                 },
             ));
         }
 
-        let created_at = extract_created_at(&existing_doc, now);
+        let created_at = extract_created_at(existing_doc, now);
 
         history
             .insert_one(doc! {
@@ -5688,9 +5841,7 @@ impl MongoBackend {
         .await?;
 
         if resource_type == "SearchParameter" {
-            writes
-                .search_parameter_changes
-                .push(PendingSearchParameterChange::Update);
+            writes.search_parameter_changes.push(change);
         }
 
         Ok(StoredResource::from_storage(

crates/persistence/src/backends/postgres/storage.rs

@@ -5221,9 +5221,18 @@ impl PostgresBackend {
                         Ok(BundleEntryResult::updated(updated))
                     }
                     None => {
-                        // Create new resource with specified ID
                         let mut resource_with_id = resource;
                         resource_with_id["id"] = serde_json::json!(id);
+                        // A deleted id is restored, as the same PUT outside a
+                        // Bundle is; only an id never stored creates (#2145).
+                        if let Some(deleted) = tx
+                            .read_including_deleted(&resource_type, &id)
+                            .await?
+                            .filter(|r| r.is_deleted())
+                        {
+                            let restored = tx.restore(&deleted, resource_with_id).await?;
+                            return Ok(BundleEntryResult::created(restored));
+                        }
                         let created = tx.create(&resource_type, resource_with_id).await?;
                         Ok(BundleEntryResult::created(created))
                     }

crates/persistence/src/backends/sqlite/storage.rs

@@ -3971,9 +3971,18 @@ impl SqliteBackend {
                         Ok(BundleEntryResult::updated(updated))
                     }
                     None => {
-                        // Create new resource with specified ID
                         let mut resource_with_id = resource;
                         resource_with_id["id"] = serde_json::json!(id);
+                        // A deleted id is restored, as the same PUT outside a
+                        // Bundle is; only an id never stored creates (#2145).
+                        if let Some(deleted) = tx
+                            .read_including_deleted(&resource_type, &id)
+                            .await?
+                            .filter(|r| r.is_deleted())
+                        {
+                            let restored = tx.restore(&deleted, resource_with_id).await?;
+                            return Ok(BundleEntryResult::created(restored));
+                        }
                         let created = tx.create(&resource_type, resource_with_id).await?;
                         Ok(BundleEntryResult::created(created))
                     }

crates/persistence/tests/mongodb/bundle_create_chunks.rs

@@ -870,19 +870,20 @@ async fn an_earlier_duplicate_is_reported_before_a_later_invalid_entry() {
     }
 }
 
-/// A PUT to the id of a deleted resource creates, and fails as the id is taken.
-/// This pins the answer the Bundle path has always given; a PUT outside a Bundle
-/// restores the resource instead.
+/// A PUT to the id of a deleted resource restores it, as the same PUT outside a
+/// Bundle does, instead of failing as the id is taken (#2145): the entry answers
+/// 201, the version chain continues (create 1, delete 2, restore 3) on the
+/// tombstone's document, and the restored body is searchable.
 #[tokio::test]
-async fn a_put_that_creates_over_a_deleted_id_fails_as_before() {
+async fn a_put_over_a_deleted_id_restores_it() {
     for mode in MODES {
         let name = format!("put_over_deleted_{}", mode.label());
         let Some(backend) = backend_or_skip(&name, mode).await else {
             return;
         };
         let tenant = create_tenant("chunks-reuse");
         seed_patient(&backend, &tenant, "dup-put", true).await;
-        let before = store_counts(&backend, tenant.tenant_id().as_str()).await;
+        let [resources, history, ..] = store_counts(&backend, tenant.tenant_id().as_str()).await;
 
         let mut entries = patient_posts(6);
         entries[2] = BundleEntry {
@@ -891,15 +892,40 @@ async fn a_put_that_creates_over_a_deleted_id_fails_as_before() {
             resource: Some(json!({
                 "resourceType": "Patient",
                 "id": "dup-put",
-                "name": [{"family": "Put"}],
+                "name": [{"family": "RestoredByPut"}],
             })),
             ..Default::default()
         };
-        let Some(failure) = assert_bundle_error(&backend, &tenant, entries, before, &name).await
+        let Some(result) = process_transaction_or_skip(&backend, &tenant, entries, &name).await
         else {
             return;
         };
-        assert_eq!(failure, (2, already_exists("dup-put")), "{name}");
+        assert_eq!(result.entries[2].status, 201, "{name}: the PUT restores");
+
+        let restored = backend
+            .read(&tenant, "Patient", "dup-put")
+            .await
+            .expect("the restored resource reads")
+            .expect("the restored resource is live");
+        assert_eq!(restored.version_id(), "3", "{name}: the chain continues");
+        assert_eq!(
+            restored.content()["name"][0]["family"],
+            "RestoredByPut",
+            "{name}"
+        );
+
+        // Five POSTs add five documents; the restore reuses the tombstone's.
+        // Every entry adds one history version.
+        let [resources_after, history_after, ..] =
+            store_counts(&backend, tenant.tenant_id().as_str()).await;
+        assert_eq!(resources_after, resources + 5, "{name}: resources");
+        assert_eq!(history_after, history + 6, "{name}: history");
+
+        assert_eq!(
+            patients_by_family(&backend, &tenant, "RestoredByPut").await,
+            vec!["dup-put".to_string()],
+            "{name}: the restored body is searchable"
+        );
     }
 }
 

crates/persistence/tests/mongodb_tests.rs

@@ -24152,6 +24152,10 @@ mongodb_bundle_identity_test!(
     mongodb_integration_changed_conditional_target_after_a_written_reference_rolls_back,
     changed_conditional_target_after_a_written_reference_rolls_back
 );
+mongodb_bundle_identity_test!(
+    mongodb_integration_put_over_a_deleted_id_restores_it,
+    put_over_a_deleted_id_restores_it
+);
 
 #[path = "mongodb/missing_index.rs"]
 mod missing_index;

crates/persistence/tests/postgres_tests.rs

@@ -31657,6 +31657,10 @@ mod postgres_integration {
         postgres_integration_changed_conditional_target_after_a_written_reference_rolls_back,
         changed_conditional_target_after_a_written_reference_rolls_back
     );
+    pg_bundle_identity_test!(
+        postgres_integration_put_over_a_deleted_id_restores_it,
+        put_over_a_deleted_id_restores_it
+    );
 
     pg_if_match_test!(
         postgres_integration_multi_valued_if_match_matches_any_member,