Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 0 additions & 6 deletions aquadoggo/src/db/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
59 changes: 26 additions & 33 deletions aquadoggo/src/db/stores/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
sandreae marked this conversation as resolved.
Show resolved Hide resolved

Ok(Some(schema))
Ok(schema)
}

/// Get all Schema which have been published to this node.
Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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![
Expand All @@ -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;

Expand All @@ -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);
});
}

Expand All @@ -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();
sandreae marked this conversation as resolved.
Show resolved Hide resolved

assert!(schema.is_none());
});
}
}