Skip to content

Commit 74fefc0

Browse files
authored
chore(rust/signed-doc): Proper DocumentRefs functional validation (#496)
* refactored ref validation, add a proper `doc_refs_check` function which performs all document references validation * wip
1 parent ad7f243 commit 74fefc0

File tree

7 files changed

+175
-155
lines changed

7 files changed

+175
-155
lines changed

rust/signed_doc/src/metadata/document_refs/mod.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
mod doc_locator;
44
mod doc_ref;
5-
use std::fmt::Display;
5+
use std::{fmt::Display, ops::Deref};
66

77
use catalyst_types::uuid::{CborContext, UuidV7};
88
use cbork_utils::{array::Array, decode_context::DecodeCtx};
@@ -17,6 +17,14 @@ use crate::CompatibilityPolicy;
1717
#[derive(Clone, Debug, PartialEq, Hash, Eq)]
1818
pub struct DocumentRefs(Vec<DocumentRef>);
1919

20+
impl Deref for DocumentRefs {
21+
type Target = Vec<DocumentRef>;
22+
23+
fn deref(&self) -> &Self::Target {
24+
&self.0
25+
}
26+
}
27+
2028
impl DocumentRefs {
2129
/// Returns true if provided `cbor` bytes is a valid old format.
2230
/// ```cddl
@@ -50,14 +58,6 @@ pub enum DocRefError {
5058
HexDecode(String),
5159
}
5260

53-
impl DocumentRefs {
54-
/// Get a list of document reference instance.
55-
#[must_use]
56-
pub fn doc_refs(&self) -> &Vec<DocumentRef> {
57-
&self.0
58-
}
59-
}
60-
6161
impl From<Vec<DocumentRef>> for DocumentRefs {
6262
fn from(value: Vec<DocumentRef>) -> Self {
6363
DocumentRefs(value)

rust/signed_doc/src/validator/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
pub(crate) mod json_schema;
44
pub(crate) mod rules;
5-
pub(crate) mod utils;
65

76
use std::{
87
collections::HashMap,

rust/signed_doc/src/validator/rules/doc_ref.rs

Lines changed: 98 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
use catalyst_types::problem_report::ProblemReport;
44

55
use crate::{
6-
providers::CatalystSignedDocumentProvider, validator::utils::validate_doc_refs,
7-
CatalystSignedDocument, DocType,
6+
providers::CatalystSignedDocumentProvider, CatalystSignedDocument, DocType, DocumentRef,
7+
DocumentRefs,
88
};
99

1010
/// `ref` field validation rule
@@ -36,11 +36,16 @@ impl RefRule {
3636
optional,
3737
} = self
3838
{
39-
if let Some(doc_ref) = doc.doc_meta().doc_ref() {
40-
let ref_validator = |ref_doc: CatalystSignedDocument| {
41-
referenced_doc_check(&ref_doc, exp_ref_types, "ref", doc.report())
42-
};
43-
return validate_doc_refs(doc_ref, provider, doc.report(), ref_validator).await;
39+
if let Some(doc_refs) = doc.doc_meta().doc_ref() {
40+
return doc_refs_check(
41+
doc_refs,
42+
exp_ref_types,
43+
"ref",
44+
provider,
45+
doc.report(),
46+
|_| true,
47+
)
48+
.await;
4449
} else if !optional {
4550
doc.report()
4651
.missing_field("ref", &format!("{context}, document must have ref field"));
@@ -62,15 +67,98 @@ impl RefRule {
6267
}
6368
}
6469

65-
/// A generic implementation of the referenced document validation.
66-
pub(crate) fn referenced_doc_check(
70+
/// Validate all the document references by the defined validation rules,
71+
/// plus conducting additional validations with the provided `validator`.
72+
/// Document all possible error in doc report (no fail fast)
73+
pub(crate) async fn doc_refs_check<Provider, Validator>(
74+
doc_refs: &DocumentRefs,
75+
exp_ref_types: &[DocType],
76+
field_name: &str,
77+
provider: &Provider,
78+
report: &ProblemReport,
79+
validator: Validator,
80+
) -> anyhow::Result<bool>
81+
where
82+
Provider: CatalystSignedDocumentProvider,
83+
Validator: Fn(&CatalystSignedDocument) -> bool,
84+
{
85+
let mut all_valid = true;
86+
87+
for dr in doc_refs.iter() {
88+
if let Some(ref ref_doc) = provider.try_get_doc(dr).await? {
89+
let is_valid = referenced_doc_type_check(ref_doc, exp_ref_types, field_name, report)
90+
&& referenced_doc_id_and_ver_check(ref_doc, dr, field_name, report)
91+
&& validator(ref_doc);
92+
93+
if !is_valid {
94+
all_valid = false;
95+
}
96+
} else {
97+
report.functional_validation(
98+
&format!("Cannot retrieve a document {dr}"),
99+
&format!("Referenced document validation for the `{field_name}` field"),
100+
);
101+
all_valid = false;
102+
}
103+
}
104+
Ok(all_valid)
105+
}
106+
107+
/// Validation check that the provided `ref_doc` is a correct referenced document found by
108+
/// `original_doc_ref`
109+
fn referenced_doc_id_and_ver_check(
110+
ref_doc: &CatalystSignedDocument,
111+
original_doc_ref: &DocumentRef,
112+
field_name: &str,
113+
report: &ProblemReport,
114+
) -> bool {
115+
let Ok(id) = ref_doc.doc_id() else {
116+
report.missing_field(
117+
"id",
118+
&format!("Referenced document validation for the `{field_name}` field"),
119+
);
120+
return false;
121+
};
122+
123+
let Ok(ver) = ref_doc.doc_ver() else {
124+
report.missing_field(
125+
"ver",
126+
&format!("Referenced document validation for the `{field_name}` field"),
127+
);
128+
return false;
129+
};
130+
131+
// id and version must match the values in ref doc
132+
if &id != original_doc_ref.id() && &ver != original_doc_ref.ver() {
133+
report.invalid_value(
134+
"id and version",
135+
&format!("id: {id}, ver: {ver}"),
136+
&format!(
137+
"id: {}, ver: {}",
138+
original_doc_ref.id(),
139+
original_doc_ref.ver()
140+
),
141+
&format!("Referenced document validation for the `{field_name}` field"),
142+
);
143+
return false;
144+
}
145+
146+
true
147+
}
148+
149+
/// Validation check that the provided `ref_doc` has an expected `type` field value from
150+
/// the allowed `exp_ref_types` list
151+
fn referenced_doc_type_check(
67152
ref_doc: &CatalystSignedDocument,
68153
exp_ref_types: &[DocType],
69154
field_name: &str,
70155
report: &ProblemReport,
71156
) -> bool {
72157
let Ok(ref_doc_type) = ref_doc.doc_type() else {
73-
report.missing_field("type", "Referenced document must have type field");
158+
report.missing_field(
159+
"type",
160+
&format!("Document reference validation for the `{field_name}` field"),
161+
);
74162
return false;
75163
};
76164

rust/signed_doc/src/validator/rules/parameters.rs

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ use catalyst_types::problem_report::ProblemReport;
44
use futures::FutureExt;
55

66
use crate::{
7-
providers::CatalystSignedDocumentProvider,
8-
validator::{rules::doc_ref::referenced_doc_check, utils::validate_doc_refs},
7+
providers::CatalystSignedDocumentProvider, validator::rules::doc_ref::doc_refs_check,
98
CatalystSignedDocument, DocType, DocumentRefs,
109
};
1110

@@ -41,12 +40,15 @@ impl ParametersRule {
4140
} = self
4241
{
4342
if let Some(parameters_ref) = doc.doc_meta().parameters() {
44-
let parameters_validator = |ref_doc: CatalystSignedDocument| {
45-
referenced_doc_check(&ref_doc, exp_parameters_type, "parameters", doc.report())
46-
};
47-
let parameters_check =
48-
validate_doc_refs(parameters_ref, provider, doc.report(), parameters_validator)
49-
.boxed();
43+
let parameters_check = doc_refs_check(
44+
parameters_ref,
45+
exp_parameters_type,
46+
"parameters",
47+
provider,
48+
doc.report(),
49+
|_| true,
50+
)
51+
.boxed();
5052

5153
let template_link_check = link_check(
5254
doc.doc_meta().template(),
@@ -125,32 +127,41 @@ where
125127
return Ok(true);
126128
};
127129

128-
let link_validator = |ref_doc: CatalystSignedDocument| {
129-
let Some(ref_doc_parameters) = ref_doc.doc_meta().parameters() else {
130-
report.missing_field(
131-
"parameters",
132-
&format!(
133-
"Referenced document via {field_name} must have `parameters` field. Referenced Document: {ref_doc}"
134-
),
135-
);
136-
return false;
137-
};
130+
let mut all_valid = true;
138131

139-
if exp_parameters != ref_doc_parameters {
140-
report.invalid_value(
141-
"parameters",
142-
&format!("Reference doc param: {ref_doc_parameters}",),
143-
&format!("Doc param: {exp_parameters}"),
144-
&format!(
145-
"Referenced document via {field_name} `parameters` field must match. Referenced Document: {ref_doc}"
146-
),
132+
for dr in ref_field.iter() {
133+
if let Some(ref ref_doc) = provider.try_get_doc(dr).await? {
134+
let Some(ref_doc_parameters) = ref_doc.doc_meta().parameters() else {
135+
report.missing_field(
136+
"parameters",
137+
&format!(
138+
"Referenced document via {field_name} must have `parameters` field. Referenced Document: {ref_doc}"
139+
),
140+
);
141+
all_valid = false;
142+
continue;
143+
};
144+
145+
if exp_parameters != ref_doc_parameters {
146+
report.invalid_value(
147+
"parameters",
148+
&format!("Reference doc param: {ref_doc_parameters}",),
149+
&format!("Doc param: {exp_parameters}"),
150+
&format!(
151+
"Referenced document via {field_name} `parameters` field must match. Referenced Document: {ref_doc}"
152+
),
153+
);
154+
all_valid = false;
155+
}
156+
} else {
157+
report.functional_validation(
158+
&format!("Cannot retrieve a document {dr}"),
159+
&format!("Referenced document link validation for the `{field_name}` field"),
147160
);
148-
return false;
161+
all_valid = false;
149162
}
150-
true
151-
};
152-
153-
validate_doc_refs(ref_field, provider, report, link_validator).await
163+
}
164+
Ok(all_valid)
154165
}
155166

156167
#[cfg(test)]

rust/signed_doc/src/validator/rules/reply.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//! `reply` rule type impl.
22
3-
use super::doc_ref::referenced_doc_check;
43
use crate::{
5-
providers::CatalystSignedDocumentProvider, validator::utils::validate_doc_refs,
4+
providers::CatalystSignedDocumentProvider, validator::rules::doc_ref::doc_refs_check,
65
CatalystSignedDocument, DocType,
76
};
87

@@ -37,17 +36,7 @@ impl ReplyRule {
3736
} = self
3837
{
3938
if let Some(reply_ref) = doc.doc_meta().reply() {
40-
let reply_validator = |ref_doc: CatalystSignedDocument| {
41-
// Validate type
42-
if !referenced_doc_check(
43-
&ref_doc,
44-
std::slice::from_ref(exp_reply_type),
45-
"reply",
46-
doc.report(),
47-
) {
48-
return false;
49-
}
50-
39+
let reply_validator = |ref_doc: &CatalystSignedDocument| {
5140
// Get `ref` from both the doc and the ref doc
5241
let Some(ref_doc_dr) = ref_doc.doc_meta().doc_ref() else {
5342
doc.report()
@@ -73,7 +62,16 @@ impl ReplyRule {
7362
}
7463
true
7564
};
76-
return validate_doc_refs(reply_ref, provider, doc.report(), reply_validator).await;
65+
66+
return doc_refs_check(
67+
reply_ref,
68+
std::slice::from_ref(exp_reply_type),
69+
"reply",
70+
provider,
71+
doc.report(),
72+
reply_validator,
73+
)
74+
.await;
7775
} else if !optional {
7876
doc.report().missing_field(
7977
"reply",

rust/signed_doc/src/validator/rules/template.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22
33
use std::fmt::Write;
44

5-
use super::doc_ref::referenced_doc_check;
65
use crate::{
76
metadata::ContentType,
87
providers::CatalystSignedDocumentProvider,
9-
validator::{json_schema, utils::validate_doc_refs},
8+
validator::{json_schema, rules::doc_ref::doc_refs_check},
109
CatalystSignedDocument, DocType,
1110
};
1211

@@ -49,15 +48,8 @@ impl ContentRule {
4948
.missing_field("template", &format!("{context}, doc"));
5049
return Ok(false);
5150
};
52-
let template_validator = |template_doc: CatalystSignedDocument| {
53-
if !referenced_doc_check(
54-
&template_doc,
55-
std::slice::from_ref(exp_template_type),
56-
"template",
57-
doc.report(),
58-
) {
59-
return false;
60-
}
51+
52+
let template_validator = |template_doc: &CatalystSignedDocument| {
6153
let Ok(template_content_type) = template_doc.doc_content_type() else {
6254
doc.report().missing_field(
6355
"content-type",
@@ -66,7 +58,7 @@ impl ContentRule {
6658
return false;
6759
};
6860
match template_content_type {
69-
ContentType::Json => templated_json_schema_check(doc, &template_doc),
61+
ContentType::Json => templated_json_schema_check(doc, template_doc),
7062
ContentType::Cddl
7163
| ContentType::Cbor
7264
| ContentType::JsonSchema
@@ -83,8 +75,16 @@ impl ContentRule {
8375
},
8476
}
8577
};
86-
return validate_doc_refs(template_ref, provider, doc.report(), template_validator)
87-
.await;
78+
79+
return doc_refs_check(
80+
template_ref,
81+
std::slice::from_ref(exp_template_type),
82+
"template",
83+
provider,
84+
doc.report(),
85+
template_validator,
86+
)
87+
.await;
8888
}
8989
if let Self::Static(content_schema) = self {
9090
if let Some(template) = doc.doc_meta().template() {

0 commit comments

Comments
 (0)