fix(cat-gateway): Publishing of existing document response issues (#3829)

Mr-Leshiy · web-flow · commit 625f6ac03995 · 2025-12-08T19:50:53.000+07:00
* check the document was already submitted before running validation

* fix

* fix tests
diff --git a/catalyst-gateway/bin/src/db/event/signed_docs/full_signed_doc.rs b/catalyst-gateway/bin/src/db/event/signed_docs/full_signed_doc.rs
@@ -1,10 +1,8 @@
 //! `FullSignedDoc` struct implementation.
 
-use anyhow::Context;
-
 use super::SignedDocBody;
 use crate::{
-    db::event::{EventDB, EventDBConnectionError},
+    db::event::EventDB,
     jinja::{JinjaTemplateSource, get_template},
 };
 
@@ -17,11 +15,6 @@ pub(crate) const SELECT_SIGNED_DOCS_TEMPLATE: JinjaTemplateSource = JinjaTemplat
     source: include_str!("./sql/select_signed_documents.sql.jinja"),
 };
 
-/// `FullSignedDoc::store` method error.
-#[derive(thiserror::Error, Debug)]
-#[error("Document with the same `id` and `ver` already exists")]
-pub(crate) struct StoreError;
-
 /// Full signed doc event db struct
 #[derive(Debug, Clone, PartialEq)]
 pub(crate) struct FullSignedDoc {
@@ -33,6 +26,45 @@ pub(crate) struct FullSignedDoc {
     raw: Vec<u8>,
 }
 
+impl TryFrom<&catalyst_signed_doc::CatalystSignedDocument> for FullSignedDoc {
+    type Error = anyhow::Error;
+
+    fn try_from(doc: &catalyst_signed_doc::CatalystSignedDocument) -> Result<Self, Self::Error> {
+        let authors = doc
+            .authors()
+            .iter()
+            .map(|v| v.as_short_id().to_string())
+            .collect();
+
+        let doc_meta_json = doc.doc_meta().to_json()?;
+
+        let payload = if matches!(
+            doc.doc_content_type(),
+            Some(catalyst_signed_doc::ContentType::Json)
+        ) {
+            match serde_json::from_slice(doc.decoded_content()?.as_slice()) {
+                Ok(payload) => Some(payload),
+                Err(e) => {
+                    anyhow::bail!("Invalid Document Content, not Json encoded: {e}");
+                },
+            }
+        } else {
+            None
+        };
+
+        let doc_body = SignedDocBody::new(
+            doc.doc_id()?.into(),
+            doc.doc_ver()?.into(),
+            doc.doc_type()?.uuid(),
+            authors,
+            Some(doc_meta_json),
+        );
+        let doc_bytes = doc.to_bytes()?;
+
+        Ok(FullSignedDoc::new(doc_body, payload, doc_bytes))
+    }
+}
+
 impl FullSignedDoc {
     /// Creates a  `FullSignedDoc` instance.
     pub(crate) fn new(
@@ -60,6 +92,7 @@ impl FullSignedDoc {
     }
 
     /// Returns the `SignedDocBody`.
+    #[allow(dead_code)]
     pub(crate) fn body(&self) -> &SignedDocBody {
         &self.body
     }
@@ -70,37 +103,25 @@ impl FullSignedDoc {
     }
 
     /// Uploads a `FullSignedDoc` to the event db.
-    /// Returns `true` if document was added into the db, `false` if it was already added
-    /// previously.
     ///
     /// Make an insert query into the `event-db` by adding data into the `signed_docs`
     /// table.
     ///
     /// * IF the record primary key (id,ver) does not exist, then add the new record.
     ///   Return success.
-    /// * IF the record does exist, but all values are the same as stored, return Success.
     /// * Otherwise return an error. (Can not over-write an existing record with new
     ///   data).
     ///
     /// # Arguments:
     ///  - `id` is a UUID v7
     ///  - `ver` is a UUID v7
     ///  - `doc_type` is a UUID v4
-    pub(crate) async fn store(&self) -> anyhow::Result<bool> {
+    pub(crate) async fn store(&self) -> anyhow::Result<()> {
         // Perform insert before checking if the document already exists.
         // This should prevent race condition.
         match EventDB::modify(INSERT_SIGNED_DOCS, &self.postgres_db_fields()).await {
             // Able to insert, no conflict
-            Ok(()) => Ok(true),
-            Err(e) if !e.is::<EventDBConnectionError>() => {
-                // Attempt to retrieve the document now that we failed to insert
-                let doc = Self::retrieve(self.id(), Some(self.ver()))
-                    .await
-                    .context("Cannot retrieve the document which has failed")?;
-                anyhow::ensure!(&doc == self, StoreError);
-                // Document already exists and matches, return false
-                Ok(false)
-            },
+            Ok(()) => Ok(()),
             Err(e) => Err(e),
         }
     }
diff --git a/catalyst-gateway/bin/src/db/event/signed_docs/mod.rs b/catalyst-gateway/bin/src/db/event/signed_docs/mod.rs
@@ -8,7 +8,7 @@ mod signed_doc_body;
 mod tests;
 
 pub(crate) use doc_ref::DocumentRef;
-pub(crate) use full_signed_doc::{FullSignedDoc, SELECT_SIGNED_DOCS_TEMPLATE, StoreError};
+pub(crate) use full_signed_doc::{FullSignedDoc, SELECT_SIGNED_DOCS_TEMPLATE};
 pub(crate) use query_filter::DocsQueryFilter;
 pub(crate) use signed_doc_body::{
     FILTERED_COUNT_SIGNED_DOCS_TEMPLATE, FILTERED_SELECT_SIGNED_DOCS_TEMPLATE, SignedDocBody,
diff --git a/catalyst-gateway/bin/src/db/event/signed_docs/signed_doc_body.rs b/catalyst-gateway/bin/src/db/event/signed_docs/signed_doc_body.rs
@@ -53,6 +53,7 @@ impl SignedDocBody {
     }
 
     /// Returns the document authors.
+    #[allow(dead_code)]
     pub(crate) fn authors(&self) -> &Vec<String> {
         &self.authors
     }
diff --git a/catalyst-gateway/bin/src/db/event/signed_docs/tests/mod.rs b/catalyst-gateway/bin/src/db/event/signed_docs/tests/mod.rs
@@ -94,9 +94,9 @@ async fn store_full_signed_doc(
     doc: &FullSignedDoc,
     doc_type: uuid::Uuid,
 ) {
-    assert!(doc.store().await.unwrap());
+    assert!(doc.store().await.is_ok());
     // try to insert the same data again
-    assert!(!doc.store().await.unwrap());
+    assert!(doc.store().await.is_err());
     // try another doc with the same `id` and `ver` and with different other fields
     let another_doc = FullSignedDoc::new(
         SignedDocBody::new(
diff --git a/catalyst-gateway/bin/src/service/api/documents/put_document/mod.rs b/catalyst-gateway/bin/src/service/api/documents/put_document/mod.rs
@@ -1,18 +1,12 @@
 //! Implementation of the PUT `/document` endpoint
 
-use std::{collections::HashSet, str::FromStr};
-
-use catalyst_signed_doc::CatalystSignedDocument;
 use poem_openapi::{ApiResponse, payload::Json};
 use unprocessable_content_request::PutDocumentUnprocessableContent;
 
 use super::common::{DocProvider, VerifyingKeyProvider};
 use crate::{
     db::{
-        event::{
-            error,
-            signed_docs::{FullSignedDoc, SignedDocBody, StoreError},
-        },
+        event::{error::NotFoundError, signed_docs::FullSignedDoc},
         index::session::CassandraSessionError,
     },
     service::{
@@ -74,20 +68,43 @@ pub(crate) async fn endpoint(
         .into();
     };
 
-    // validate document signatures
-    let verifying_key_provider =
-        match VerifyingKeyProvider::try_new(&mut token, &doc.authors()).await {
-            Ok(value) => value,
-            Err(err) if err.is::<CassandraSessionError>() => {
-                return AllResponses::service_unavailable(&err, RetryAfterOption::Default);
-            },
-            Err(err) => {
-                return Responses::UnprocessableContent(Json(
-                    PutDocumentUnprocessableContent::new(&err, None),
-                ))
-                .into();
-            },
-        };
+    let db_doc = match FullSignedDoc::try_from(&doc) {
+        Ok(doc) => doc,
+        Err(err) => return AllResponses::handle_error(&err),
+    };
+
+    // TODO: use the `ValidationProvider::try_get_doc` method and verify the returned
+    // document cid values.
+    // making sure that the document was already submitted before running validation,
+    // otherwise validation would fail, because of the assumption that this document wasn't
+    // published yet.
+    match FullSignedDoc::retrieve(db_doc.id(), Some(db_doc.ver())).await {
+        Ok(retrieved) if db_doc != retrieved => {
+            return Responses::UnprocessableContent(Json(PutDocumentUnprocessableContent::new(
+                "Document with the same `id` and `ver` already exists",
+                Some(doc.problem_report()),
+            )))
+            .into();
+        },
+        Ok(_) => return Responses::NoContent.into(),
+        Err(err) if err.is::<NotFoundError>() => (),
+        Err(err) => return AllResponses::handle_error(&err),
+    }
+
+    // validation provider
+    let validation_provider = match VerifyingKeyProvider::try_new(&mut token, &doc.authors()).await
+    {
+        Ok(value) => ValidationProvider::new(DocProvider, value),
+        Err(err) if err.is::<CassandraSessionError>() => {
+            return AllResponses::service_unavailable(&err, RetryAfterOption::Default);
+        },
+        Err(err) => {
+            return Responses::UnprocessableContent(Json(PutDocumentUnprocessableContent::new(
+                &err, None,
+            )))
+            .into();
+        },
+    };
 
     match doc.is_deprecated() {
         // apply older validation rule
@@ -113,8 +130,6 @@ pub(crate) async fn endpoint(
         },
         // apply newest validation rules
         Ok(false) => {
-            let validation_provider = ValidationProvider::new(DocProvider, verifying_key_provider);
-
             match catalyst_signed_doc::validator::validate(&doc, &validation_provider).await {
                 Ok(true) => (),
                 Ok(false) => {
@@ -143,88 +158,8 @@ pub(crate) async fn endpoint(
         .into();
     }
 
-    match validate_against_original_doc(&doc).await {
-        Ok(true) => (),
-        Ok(false) => {
-            return Responses::UnprocessableContent(Json(
-                PutDocumentUnprocessableContent::new("Failed validating document: catalyst-id or role does not match the current version.", None),
-            ))
-            .into();
-        },
-        Err(err) => return AllResponses::handle_error(&err),
-    }
-
-    // update the document storing in the db
-    match store_document_in_db(&doc, doc_bytes).await {
-        Ok(true) => Responses::Created.into(),
-        Ok(false) => Responses::NoContent.into(),
-        Err(err) if err.is::<StoreError>() => {
-            Responses::UnprocessableContent(Json(PutDocumentUnprocessableContent::new(
-                "Document with the same `id` and `ver` already exists",
-                Some(doc.problem_report()),
-            )))
-            .into()
-        },
+    match db_doc.store().await {
+        Ok(()) => Responses::Created.into(),
         Err(err) => AllResponses::handle_error(&err),
     }
 }
-
-/// Fetch the latest version and ensure its catalyst-id match those in the newer version.
-async fn validate_against_original_doc(doc: &CatalystSignedDocument) -> anyhow::Result<bool> {
-    let original_doc = match FullSignedDoc::retrieve(&doc.doc_id()?.uuid(), None).await {
-        Ok(doc) => doc,
-        Err(e) if e.is::<error::NotFoundError>() => return Ok(true),
-        Err(e) => return Err(e),
-    };
-
-    let original_authors = original_doc
-        .body()
-        .authors()
-        .iter()
-        .map(|author| catalyst_signed_doc::CatalystId::from_str(author))
-        .collect::<Result<HashSet<_>, _>>()?;
-    let authors: HashSet<_> = doc.authors().into_iter().collect();
-    Ok(authors == original_authors)
-}
-
-/// Store a provided and validated document inside the db.
-/// Returns `true` if its a new document.
-/// Returns `false` if the same document already exists.
-async fn store_document_in_db(
-    doc: &catalyst_signed_doc::CatalystSignedDocument,
-    doc_bytes: Vec<u8>,
-) -> anyhow::Result<bool> {
-    let authors = doc
-        .authors()
-        .iter()
-        .map(|v| v.as_short_id().to_string())
-        .collect();
-
-    let doc_meta_json = doc.doc_meta().to_json()?;
-
-    let payload = if matches!(
-        doc.doc_content_type(),
-        Some(catalyst_signed_doc::ContentType::Json)
-    ) {
-        match serde_json::from_slice(doc.decoded_content()?.as_slice()) {
-            Ok(payload) => Some(payload),
-            Err(e) => {
-                anyhow::bail!("Invalid Document Content, not Json encoded: {e}");
-            },
-        }
-    } else {
-        None
-    };
-
-    let doc_body = SignedDocBody::new(
-        doc.doc_id()?.into(),
-        doc.doc_ver()?.into(),
-        doc.doc_type()?.uuid(),
-        authors,
-        Some(doc_meta_json),
-    );
-
-    FullSignedDoc::new(doc_body, payload, doc_bytes)
-        .store()
-        .await
-}
diff --git a/catalyst-gateway/tests/api_tests/integration/signed_doc/test_signed_doc.py b/catalyst-gateway/tests/api_tests/integration/signed_doc/test_signed_doc.py
@@ -48,8 +48,7 @@ def test_document_put_and_get_endpoints(proposal_doc_factory, rbac_chain_factory
         data=new_doc_cbor,
         token=rbac_chain.auth_token(),
     )
-    # TODO: fix it after fully integrating the latest changes of the 'catalyst-signed-doc' crate
-    assert resp.status_code == 422, (
+    assert resp.status_code == 204, (
         f"Failed to publish document: {resp.status_code} - {resp.text}"
     )
 

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,7 @@ impl SignedDocBody {`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`/// Returns the document authors.`
	`56`	`+ #[allow(dead_code)]`
`56`	`57`	`pub(crate) fn authors(&self) -> &Vec<String> {`
`57`	`58`	`&self.authors`
`58`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -48,8 +48,7 @@ def test_document_put_and_get_endpoints(proposal_doc_factory, rbac_chain_factory`
`48`	`48`	`data=new_doc_cbor,`
`49`	`49`	`token=rbac_chain.auth_token(),`
`50`	`50`	`)`
`51`		`- # TODO: fix it after fully integrating the latest changes of the 'catalyst-signed-doc' crate`
`52`		`- assert resp.status_code == 422, (`
	`51`	`+ assert resp.status_code == 204, (`
`53`	`52`	`f"Failed to publish document: {resp.status_code} - {resp.text}"`
`54`	`53`	`)`
`55`	`54`