Don't error in SchemaStore when schema fields are missing (#192)

* Don't error in SchemaStore when schema fields are missing * Update CHANGELOG * Remove unused error * Add some more comments
p2panda · Jul 5, 2022 · 73369cc · 73369cc
1 parent 54e2aff
commit 73369cc
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Don't return errors from `SchemaStore` when a schema could not be constructed [#192](https://github.com/p2panda/aquadoggo/pull/192)
 - Filter out deleted documents in `get_documents_by_schema` SQL query [#193](https://github.com/p2panda/aquadoggo/pull/193)
 
 ## [0.3.0]

diff --git a/aquadoggo/src/db/errors.rs b/aquadoggo/src/db/errors.rs
@@ -43,12 +43,6 @@ pub enum SchemaStoreError {
     #[allow(dead_code)]
     Custom(String),
 
-    /// Error returned when no document view existed for the required schema field definition
-    #[error(
-        "No document view found for schema field definition with id: {0} which is required by schema definition {1}"
-    )]
-    MissingSchemaFieldDefinition(DocumentViewId, DocumentViewId),
-
     /// Error returned from converting p2panda-rs `DocumentView` into `SchemaView.
     #[error(transparent)]
     SystemSchemaError(#[from] SystemSchemaError),

diff --git a/aquadoggo/src/db/stores/schema.rs b/aquadoggo/src/db/stores/schema.rs
@@ -39,20 +39,18 @@ impl SchemaStore for SqlStorage {
             let scheme_field_view: SchemaFieldView =
                 match self.get_document_view_by_id(&field_id).await? {
                     Some(document_view) => document_view.try_into()?,
-                    None => {
-                        return Err(SchemaStoreError::MissingSchemaFieldDefinition(
-                            field_id,
-                            id.to_owned(),
-                        ))
-                    }
+                    None => return Ok(None),
                 };
 
             schema_fields.push(scheme_field_view);
         }
 
-        let schema = Schema::from_views(schema_view, schema_fields)?;
+        // We silently ignore errors as we are assuming views we retrieve from the database
+        // themselves are valid, meaning any error in constructing the schema must be because
+        // some of it's fields are simply missing from our database.
+        let schema = Schema::from_views(schema_view, schema_fields).ok();
 
-        Ok(Some(schema))
+        Ok(schema)
     }
 
     /// Get all Schema which have been published to this node.
@@ -87,10 +85,10 @@ impl SchemaStore for SqlStorage {
                 .map(|field| field.to_owned())
                 .collect();
 
-            all_schema.push(Schema::from_views(schema_view, schema_fields)?);
+            all_schema.push(Schema::from_views(schema_view, schema_fields).ok());
         }
 
-        Ok(all_schema)
+        Ok(all_schema.into_iter().flatten().collect())
     }
 }
 
@@ -280,7 +278,8 @@ mod tests {
         operation_fields(vec![
                          ("name", OperationValue::Text("venue".to_string())),
                          ("description", OperationValue::Text("My venue".to_string()))
-        ])
+        ]),
+        1
     )]
     #[case::does_not_work(
         operation_fields(vec![
@@ -289,15 +288,17 @@ mod tests {
         operation_fields(vec![
                          ("name", OperationValue::Text("venue".to_string())),
                          ("description", OperationValue::Text("My venue".to_string()))
-        ])
+        ]),
+        0
     )]
     fn get_all_schema(
         #[case] schema_field_definition: OperationFields,
         #[case] schema_definition: OperationFields,
+        #[case] expected_schema_count: usize,
         key_pair: KeyPair,
         #[from(test_db)] runner: TestDatabaseRunner,
     ) {
-        runner.with_db_teardown(|db: TestDatabase| async move {
+        runner.with_db_teardown(move |db: TestDatabase| async move {
             let document_view_id =
                 insert_schema_field_definition(&db.store, &key_pair, schema_field_definition).await;
 
@@ -306,14 +307,7 @@ mod tests {
 
             let schemas = db.store.get_all_schema().await;
 
-            if schemas.is_err() {
-                assert_eq!(
-                    schemas.unwrap_err().to_string(),
-                    "invalid fields found for this schema".to_string()
-                )
-            } else {
-                assert_eq!(schemas.unwrap().len(), 1);
-            }
+            assert_eq!(schemas.unwrap().len(), expected_schema_count);
         });
     }
 
@@ -331,21 +325,20 @@ mod tests {
         key_pair: KeyPair,
     ) {
         runner.with_db_teardown(|db: TestDatabase| async move {
-            let document_view_id =
-                insert_schema_definition(&db.store, &key_pair, &schema_fields_id, schema_definition)
-                    .await;
+            let document_view_id = insert_schema_definition(
+                &db.store,
+                &key_pair,
+                &schema_fields_id,
+                schema_definition,
+            )
+            .await;
 
             // Retrieve the schema by it's document_view_id.
-            let schema = db.store.get_schema_by_id(&document_view_id).await;
 
-            assert_eq!(
-                schema.unwrap_err().to_string(),
-                format!(
-                    "No document view found for schema field definition with id: {0} which is required by schema definition {1}",
-                    schema_fields_id,
-                    document_view_id
-                )
-            );
+            // We unwrap here as we expect an `Ok` result even though the schema could not be built.
+            let schema = db.store.get_schema_by_id(&document_view_id).await.unwrap();
+
+            assert!(schema.is_none());
         });
     }
 }