From 68675f0fdead31cc81d939ac4b7dce69298d2dc4 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Thu, 27 Oct 2022 15:36:58 +0200 Subject: [PATCH 01/18] Added test --- identity_did/src/document/builder.rs | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/identity_did/src/document/builder.rs b/identity_did/src/document/builder.rs index 7a29c3306b..bc774dc988 100644 --- a/identity_did/src/document/builder.rs +++ b/identity_did/src/document/builder.rs @@ -142,6 +142,8 @@ where #[cfg(test)] mod tests { use super::*; + use crate::verification::MethodData; + use crate::verification::MethodType; use crate::Error; #[test] @@ -149,4 +151,34 @@ mod tests { let result: Result = DocumentBuilder::default().build(); assert!(matches!(result.unwrap_err(), Error::InvalidDocument(_, None))); } + + #[test] + fn duplicate_id_different_scopes() { + let did: CoreDID = "did:example:1234".parse().unwrap(); + let fragment = "#key1"; + let id = did.clone().to_url().join(fragment).unwrap(); + + let method1: VerificationMethod = VerificationMethod::builder(Default::default()) + .id(id.clone()) + .controller(did.clone()) + .type_(MethodType::Ed25519VerificationKey2018) + .data(MethodData::new_multibase("test")) + .build() + .unwrap(); + + let method2: VerificationMethod = VerificationMethod::builder(Default::default()) + .id(id.clone()) + .controller(did.clone()) + .type_(MethodType::X25519KeyAgreementKey2019) + .data(MethodData::new_multibase("test")) + .build() + .unwrap(); + + let result: Result = DocumentBuilder::default() + .id(did) + .verification_method(method1) + .key_agreement(method2) + .build(); + assert!(result.is_err()); + } } From ebc7e1ccc5a45d964e0bad90e436fbb25a4774e6 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Fri, 28 Oct 2022 11:27:05 +0200 Subject: [PATCH 02/18] Added more tests --- identity_did/src/document/builder.rs | 18 ++++- identity_did/src/document/core_document.rs | 82 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/identity_did/src/document/builder.rs b/identity_did/src/document/builder.rs index bc774dc988..e9bedbb0c1 100644 --- a/identity_did/src/document/builder.rs +++ b/identity_did/src/document/builder.rs @@ -162,7 +162,9 @@ mod tests { .id(id.clone()) .controller(did.clone()) .type_(MethodType::Ed25519VerificationKey2018) - .data(MethodData::new_multibase("test")) + .data(MethodData::PublicKeyBase58( + "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J".into(), + )) .build() .unwrap(); @@ -170,7 +172,9 @@ mod tests { .id(id.clone()) .controller(did.clone()) .type_(MethodType::X25519KeyAgreementKey2019) - .data(MethodData::new_multibase("test")) + .data(MethodData::PublicKeyBase58( + "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x".into(), + )) .build() .unwrap(); @@ -179,6 +183,16 @@ mod tests { .verification_method(method1) .key_agreement(method2) .build(); + // TODO: Remove match once this test passes + match result { + Ok(ref doc) => { + println!( + "{}", + ::to_json_pretty(doc).unwrap() + ); + } + _ => (), + }; assert!(result.is_err()); } } diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index 8189fb3771..d2caa15007 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -934,6 +934,8 @@ where #[cfg(test)] mod tests { + use identity_core::convert::FromJson; + use crate::verification::MethodData; use super::*; @@ -1248,6 +1250,37 @@ mod tests { .is_err()); } + #[test] + fn deserialize_duplicate_method_different_scopes() { + const JSON_VERIFICATION_METHOD_KEY_AGREEMENT_DUPLICATE: &str = r#"{ + "id": "did:example:1234", + "verificationMethod": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ], + "keyAgreement": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + let verifier = |json: &str| { + let result: std::result::Result> = + CoreDocument::from_json(json).map_err(Into::into); + assert!(result.is_err()); + }; + + verifier(JSON_VERIFICATION_METHOD_KEY_AGREEMENT_DUPLICATE); + } + #[test] fn test_method_remove_existence() { let mut document: CoreDocument = document(); @@ -1333,4 +1366,53 @@ mod tests { assert!(!decoded_bitmap.is_revoked(index)); } } + + #[test] + fn deserialize_valid() { + // The verification method types here are really Ed25519VerificationKey2020, changed to be compatible + // with the current version of this library. + const JSON_DOCUMENT: &str = r#"{ + "@context": [ + "https://www.w3.org/ns/did/v1", + "https://w3id.org/security/suites/ed25519-2020/v1" + ], + "id": "did:example:123", + "authentication": [ + { + "id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf" + } + ], + "capabilityInvocation": [ + { + "id": "did:example:123#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN" + } + ], + "capabilityDelegation": [ + { + "id": "did:example:123#z6Mkw94ByR26zMSkNdCUi6FNRsWnc2DFEeDXyBGJ5KTzSWyi", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zHgo9PAmfeoxHG8Mn2XHXamxnnSwPpkyBHAMNF3VyXJCL" + } + ], + "assertionMethod": [ + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomY", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + } + ] + }"#; + let doc: std::result::Result> = + CoreDocument::from_json(JSON_DOCUMENT).map_err(Into::into); + dbg!(&doc); + assert!(doc.is_ok()); + } } From f735a7edff66bcf16aa7bf983f092ca3aa07c234 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Fri, 28 Oct 2022 13:58:10 +0200 Subject: [PATCH 03/18] Added one more test --- identity_did/src/document/core_document.rs | 158 ++++++++++++++++----- 1 file changed, 124 insertions(+), 34 deletions(-) diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index d2caa15007..4a66d4d9f2 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -1250,37 +1250,6 @@ mod tests { .is_err()); } - #[test] - fn deserialize_duplicate_method_different_scopes() { - const JSON_VERIFICATION_METHOD_KEY_AGREEMENT_DUPLICATE: &str = r#"{ - "id": "did:example:1234", - "verificationMethod": [ - { - "id": "did:example:1234#key1", - "controller": "did:example:1234", - "type": "Ed25519VerificationKey2018", - "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" - } - ], - "keyAgreement": [ - { - "id": "did:example:1234#key1", - "controller": "did:example:1234", - "type": "X25519KeyAgreementKey2019", - "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" - } - ] - }"#; - - let verifier = |json: &str| { - let result: std::result::Result> = - CoreDocument::from_json(json).map_err(Into::into); - assert!(result.is_err()); - }; - - verifier(JSON_VERIFICATION_METHOD_KEY_AGREEMENT_DUPLICATE); - } - #[test] fn test_method_remove_existence() { let mut document: CoreDocument = document(); @@ -1369,8 +1338,8 @@ mod tests { #[test] fn deserialize_valid() { - // The verification method types here are really Ed25519VerificationKey2020, changed to be compatible - // with the current version of this library. + // The verification method types here are really Ed25519VerificationKey2020, changed to be compatible + // with the current version of this library. const JSON_DOCUMENT: &str = r#"{ "@context": [ "https://www.w3.org/ns/did/v1", @@ -1412,7 +1381,128 @@ mod tests { }"#; let doc: std::result::Result> = CoreDocument::from_json(JSON_DOCUMENT).map_err(Into::into); - dbg!(&doc); + dbg!(&doc); assert!(doc.is_ok()); } + + #[test] + fn deserialize_duplicate_method_different_scopes() { + const JSON_VERIFICATION_METHOD_KEY_AGREEMENT: &str = r#"{ + "id": "did:example:1234", + "verificationMethod": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ], + "keyAgreement": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + const JSON_KEY_AGREEMENT_CAPABILITY_INVOCATION: &str = r#"{ + "id": "did:example:1234", + "capabilityInvocation": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ], + "keyAgreement": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + const JSON_ASSERTION_METHOD_CAPABILITY_INVOCATION: &str = r#"{ + "id": "did:example:1234", + "assertionMethod": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ], + "capabilityInvocation": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ] + }"#; + + const JSON_VERIFICATION_METHOD_AUTHENTICATION: &str = r#"{ + "id": "did:example:1234", + "verificationMethod": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ], + "authentication": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + const JSON_CAPABILITY_DELEGATION_ASSERTION_METHOD: &str = r#"{ + "id": "did:example:1234", + "capabilityDelegation": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ], + "assertionMethod": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + let verifier = |json: &str| { + let result: std::result::Result> = + CoreDocument::from_json(json).map_err(Into::into); + // Print the json if the test fails to aid debugging. + println!("the following non-spec compliant document was deserialized: \n {json}"); + assert!(result.is_err()); + }; + + for json in [ + JSON_VERIFICATION_METHOD_KEY_AGREEMENT, + JSON_KEY_AGREEMENT_CAPABILITY_INVOCATION, + JSON_ASSERTION_METHOD_CAPABILITY_INVOCATION, + JSON_VERIFICATION_METHOD_AUTHENTICATION, + JSON_CAPABILITY_DELEGATION_ASSERTION_METHOD + ] { + verifier(json); + } + } } From fa8b4e4f8c0becba7a3d597c22a931c39d298de6 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Fri, 28 Oct 2022 14:38:09 +0200 Subject: [PATCH 04/18] added invalid method reference test --- identity_did/src/document/core_document.rs | 101 +++++++++++++++++++-- 1 file changed, 94 insertions(+), 7 deletions(-) diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index 4a66d4d9f2..f3668c10d0 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -1381,6 +1381,7 @@ mod tests { }"#; let doc: std::result::Result> = CoreDocument::from_json(JSON_DOCUMENT).map_err(Into::into); + // Print debug representation if the test fails. dbg!(&doc); assert!(doc.is_ok()); } @@ -1433,8 +1434,8 @@ mod tests { { "id": "did:example:1234#key1", "controller": "did:example:1234", - "type": "X25519KeyAgreementKey2019", - "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV" } ], "capabilityInvocation": [ @@ -1461,8 +1462,8 @@ mod tests { { "id": "did:example:1234#key1", "controller": "did:example:1234", - "type": "X25519KeyAgreementKey2019", - "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV" } ] }"#; @@ -1481,8 +1482,8 @@ mod tests { { "id": "did:example:1234#key1", "controller": "did:example:1234", - "type": "X25519KeyAgreementKey2019", - "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV" } ] }"#; @@ -1500,7 +1501,93 @@ mod tests { JSON_KEY_AGREEMENT_CAPABILITY_INVOCATION, JSON_ASSERTION_METHOD_CAPABILITY_INVOCATION, JSON_VERIFICATION_METHOD_AUTHENTICATION, - JSON_CAPABILITY_DELEGATION_ASSERTION_METHOD + JSON_CAPABILITY_DELEGATION_ASSERTION_METHOD, + ] { + verifier(json); + } + } + + #[test] + fn deserialize_invalid_id_references() { + const JSON_KEY_AGREEMENT_CAPABILITY_INVOCATION: &str = r#"{ + "id": "did:example:1234", + "capabilityInvocation": [ + "did:example:1234#key1" + ], + "keyAgreement": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + const JSON_ASSERTION_METHOD_CAPABILITY_INVOCATION: &str = r#"{ + "id": "did:example:1234", + "assertionMethod": [ + "did:example:1234#key1", + { + "id": "did:example:1234#key2", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "H3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV" + } + ], + "capabilityInvocation": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ] + }"#; + + const JSON_AUTHENTICATION_KEY_AGREEMENT: &str = r#"{ + "id": "did:example:1234", + "keyAgreement": [ + "did:example:1234#key1" + ], + "authentication": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyMultibase": "zH3C2AVvLMv6gmMNam3uVAjZpfkcJCwDwnZn6z3wXmqPV" + } + ] + }"#; + + const JSON_CAPABILITY_DELEGATION_ASSERTION_METHOD: &str = r#"{ + "id": "did:example:1234", + "capabilityDelegation": [ + "did:example:1234#key1" + ], + "assertionMethod": [ + { + "id": "did:example:1234#key1", + "controller": "did:example:1234", + "type": "X25519KeyAgreementKey2019", + "publicKeyBase58": "FbQWLPRhTH95MCkQUeFYdiSoQt8zMwetqfWoxqPgaq7x" + } + ] + }"#; + + let verifier = |json: &str| { + let result: std::result::Result> = + CoreDocument::from_json(json).map_err(Into::into); + // Print the json if the test fails to aid debugging. + println!("the following non-spec compliant document was deserialized: \n {json}"); + assert!(result.is_err()); + }; + + for json in [ + JSON_KEY_AGREEMENT_CAPABILITY_INVOCATION, + JSON_ASSERTION_METHOD_CAPABILITY_INVOCATION, + JSON_AUTHENTICATION_KEY_AGREEMENT, + JSON_CAPABILITY_DELEGATION_ASSERTION_METHOD, ] { verifier(json); } From 956cc5bd887fd125780b027e53236873c63f8948 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Mon, 31 Oct 2022 12:19:48 +0100 Subject: [PATCH 05/18] refactored CoreDocument --- identity_did/src/diff/diff_document.rs | 103 ++--- identity_did/src/document/core_document.rs | 468 ++++++++++++--------- identity_did/src/document/mod.rs | 1 + 3 files changed, 333 insertions(+), 239 deletions(-) diff --git a/identity_did/src/diff/diff_document.rs b/identity_did/src/diff/diff_document.rs index 5adfc9236a..7f30717ecd 100644 --- a/identity_did/src/diff/diff_document.rs +++ b/identity_did/src/diff/diff_document.rs @@ -17,6 +17,7 @@ use identity_core::diff::Result; use crate::did::CoreDID; use crate::did::DID; use crate::document::CoreDocument; +use crate::document::CoreDocumentInner; use crate::service::Service; use crate::verification::MethodRef; use crate::verification::VerificationMethod; @@ -201,17 +202,19 @@ where .unwrap_or_else(|| self.properties().clone()); Ok(CoreDocument { - id, - controller, - also_known_as, - verification_method, - authentication, - assertion_method, - key_agreement, - capability_delegation, - capability_invocation, - service, - properties, + inner: CoreDocumentInner { + id, + controller, + also_known_as, + verification_method, + authentication, + assertion_method, + key_agreement, + capability_delegation, + capability_invocation, + service, + properties, + }, }) } @@ -282,36 +285,40 @@ where let properties: T = diff.properties.map(T::from_diff).transpose()?.unwrap_or_default(); Ok(CoreDocument { - id, - controller, - also_known_as, - verification_method, - authentication, - assertion_method, - key_agreement, - capability_delegation, - capability_invocation, - service, - properties, + inner: CoreDocumentInner { + id, + controller, + also_known_as, + verification_method, + authentication, + assertion_method, + key_agreement, + capability_delegation, + capability_invocation, + service, + properties, + }, }) } fn into_diff(self) -> Result { + let inner = self.inner; + Ok(DiffDocument { - id: Some(self.id.into_diff()?), - controller: Some(self.controller.map(|value| value.into_diff()).transpose()?), - also_known_as: Some(self.also_known_as.into_diff()?), - verification_method: Some(self.verification_method.into_diff()?), - authentication: Some(self.authentication.into_diff()?), - assertion_method: Some(self.assertion_method.into_diff()?), - key_agreement: Some(self.key_agreement.into_diff()?), - capability_delegation: Some(self.capability_delegation.into_diff()?), - capability_invocation: Some(self.capability_invocation.into_diff()?), - service: Some(self.service.into_diff()?), - properties: if self.properties == Default::default() { + id: Some(inner.id.into_diff()?), + controller: Some(inner.controller.map(|value| value.into_diff()).transpose()?), + also_known_as: Some(inner.also_known_as.into_diff()?), + verification_method: Some(inner.verification_method.into_diff()?), + authentication: Some(inner.authentication.into_diff()?), + assertion_method: Some(inner.assertion_method.into_diff()?), + key_agreement: Some(inner.key_agreement.into_diff()?), + capability_delegation: Some(inner.capability_delegation.into_diff()?), + capability_invocation: Some(inner.capability_invocation.into_diff()?), + service: Some(inner.service.into_diff()?), + properties: if inner.properties == Default::default() { None } else { - Some(self.properties.into_diff()?) + Some(inner.properties.into_diff()?) }, }) } @@ -455,7 +462,7 @@ mod test { let mut new = doc.clone(); // add new method - assert!(new.verification_method_mut().append(method(&doc.id, "#key-diff"))); + assert!(new.verification_method_mut().append(method(&doc.inner.id, "#key-diff"))); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -498,7 +505,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); assert!(new.authentication_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -512,7 +519,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); let first = new.authentication().first().unwrap().clone(); new.authentication_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -541,7 +548,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); assert!(new.assertion_method_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -555,7 +562,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); let first = new.assertion_method().first().unwrap().clone(); new.assertion_method_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -584,7 +591,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); assert!(new.key_agreement_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -598,7 +605,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); let first = new.key_agreement().first().unwrap().clone(); new.key_agreement_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -627,7 +634,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); assert!(new.capability_delegation_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -641,7 +648,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); let first = new.capability_delegation().first().unwrap().clone(); new.capability_delegation_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -670,7 +677,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); assert!(new.capability_invocation_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -684,7 +691,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); let first = new.capability_invocation().first().unwrap().clone(); new.capability_invocation_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -713,7 +720,7 @@ mod test { let mut new = doc.clone(); // Add new service - let service = service(doc.id.to_url().join("#key-diff").unwrap()); + let service = service(doc.inner.id.to_url().join("#key-diff").unwrap()); assert!(new.service_mut().append(service)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -727,7 +734,7 @@ mod test { let mut new = doc.clone(); // add new service - let service = service(doc.id.to_url().join("#key-diff").unwrap()); + let service = service(doc.inner.id.to_url().join("#key-diff").unwrap()); let first = new.service().first().unwrap().clone(); new.service_mut().replace(&first, service); assert_ne!(doc, new); @@ -806,7 +813,7 @@ mod test { let method_ref: MethodRef = MethodBuilder::default() .id(first) - .controller(new.id.clone()) + .controller(new.inner.id.clone()) .type_(MethodType::Ed25519VerificationKey2018) .data(MethodData::new_multibase(b"key_material")) .build() diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index f3668c10d0..761e641642 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -4,6 +4,7 @@ use core::convert::TryInto as _; use core::fmt::Display; use core::fmt::Formatter; +use std::collections::HashSet; use serde::Serialize; @@ -42,12 +43,9 @@ use crate::verification::MethodUriType; use crate::verification::TryMethod; use crate::verification::VerificationMethod; -/// A DID Document. -/// -/// [Specification](https://www.w3.org/TR/did-core/#did-document-properties) #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[rustfmt::skip] -pub struct CoreDocument +pub(crate) struct CoreDocumentInner where D: DID + KeyComparable { @@ -74,12 +72,25 @@ pub struct CoreDocument pub(crate) properties: T, } +/// A DID Document. +/// +/// [Specification](https://www.w3.org/TR/did-core/#did-document-properties) +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[rustfmt::skip] +#[serde(transparent)] +pub struct CoreDocument + where + D: DID + KeyComparable +{ + pub(crate) inner: CoreDocumentInner, +} + // Workaround for lifetime issues with a mutable reference to self preventing closures from being used. macro_rules! method_ref_mut_helper { ($doc:ident, $method: ident, $query: ident) => { - match $doc.$method.query_mut($query.into())? { + match $doc.inner.$method.query_mut($query.into())? { MethodRef::Embed(method) => Some(method), - MethodRef::Refer(ref did) => $doc.verification_method.query_mut(did), + MethodRef::Refer(ref did) => $doc.inner.verification_method.query_mut(did), } }; } @@ -98,156 +109,158 @@ where /// Returns a new `CoreDocument` based on the [`DocumentBuilder`] configuration. pub fn from_builder(builder: DocumentBuilder) -> Result { Ok(Self { - id: builder.id.ok_or(Error::InvalidDocument("missing id", None))?, - controller: Some(builder.controller) - .filter(|controllers| !controllers.is_empty()) - .map(TryFrom::try_from) - .transpose() - .map_err(|err| Error::InvalidDocument("controller", Some(err)))?, - also_known_as: builder - .also_known_as - .try_into() - .map_err(|err| Error::InvalidDocument("also_known_as", Some(err)))?, - verification_method: builder - .verification_method - .try_into() - .map_err(|err| Error::InvalidDocument("verification_method", Some(err)))?, - authentication: builder - .authentication - .try_into() - .map_err(|err| Error::InvalidDocument("authentication", Some(err)))?, - assertion_method: builder - .assertion_method - .try_into() - .map_err(|err| Error::InvalidDocument("assertion_method", Some(err)))?, - key_agreement: builder - .key_agreement - .try_into() - .map_err(|err| Error::InvalidDocument("key_agreement", Some(err)))?, - capability_delegation: builder - .capability_delegation - .try_into() - .map_err(|err| Error::InvalidDocument("capability_delegation", Some(err)))?, - capability_invocation: builder - .capability_invocation - .try_into() - .map_err(|err| Error::InvalidDocument("capability_invocation", Some(err)))?, - service: builder - .service - .try_into() - .map_err(|err| Error::InvalidDocument("service", Some(err)))?, - properties: builder.properties, + inner: CoreDocumentInner { + id: builder.id.ok_or(Error::InvalidDocument("missing id", None))?, + controller: Some(builder.controller) + .filter(|controllers| !controllers.is_empty()) + .map(TryFrom::try_from) + .transpose() + .map_err(|err| Error::InvalidDocument("controller", Some(err)))?, + also_known_as: builder + .also_known_as + .try_into() + .map_err(|err| Error::InvalidDocument("also_known_as", Some(err)))?, + verification_method: builder + .verification_method + .try_into() + .map_err(|err| Error::InvalidDocument("verification_method", Some(err)))?, + authentication: builder + .authentication + .try_into() + .map_err(|err| Error::InvalidDocument("authentication", Some(err)))?, + assertion_method: builder + .assertion_method + .try_into() + .map_err(|err| Error::InvalidDocument("assertion_method", Some(err)))?, + key_agreement: builder + .key_agreement + .try_into() + .map_err(|err| Error::InvalidDocument("key_agreement", Some(err)))?, + capability_delegation: builder + .capability_delegation + .try_into() + .map_err(|err| Error::InvalidDocument("capability_delegation", Some(err)))?, + capability_invocation: builder + .capability_invocation + .try_into() + .map_err(|err| Error::InvalidDocument("capability_invocation", Some(err)))?, + service: builder + .service + .try_into() + .map_err(|err| Error::InvalidDocument("service", Some(err)))?, + properties: builder.properties, + }, }) } /// Returns a reference to the `CoreDocument` id. pub fn id(&self) -> &D { - &self.id + &self.inner.id } /// Returns a mutable reference to the `CoreDocument` id. pub fn id_mut(&mut self) -> &mut D { - &mut self.id + &mut self.inner.id } /// Returns a reference to the `CoreDocument` controller. pub fn controller(&self) -> Option<&OneOrSet> { - self.controller.as_ref() + self.inner.controller.as_ref() } /// Returns a mutable reference to the `CoreDocument` controller. pub fn controller_mut(&mut self) -> &mut Option> { - &mut self.controller + &mut self.inner.controller } /// Returns a reference to the `CoreDocument` alsoKnownAs set. pub fn also_known_as(&self) -> &OrderedSet { - &self.also_known_as + &self.inner.also_known_as } /// Returns a mutable reference to the `CoreDocument` alsoKnownAs set. pub fn also_known_as_mut(&mut self) -> &mut OrderedSet { - &mut self.also_known_as + &mut self.inner.also_known_as } /// Returns a reference to the `CoreDocument` verificationMethod set. pub fn verification_method(&self) -> &OrderedSet> { - &self.verification_method + &self.inner.verification_method } /// Returns a mutable reference to the `CoreDocument` verificationMethod set. pub fn verification_method_mut(&mut self) -> &mut OrderedSet> { - &mut self.verification_method + &mut self.inner.verification_method } /// Returns a reference to the `CoreDocument` authentication set. pub fn authentication(&self) -> &OrderedSet> { - &self.authentication + &self.inner.authentication } /// Returns a mutable reference to the `CoreDocument` authentication set. pub fn authentication_mut(&mut self) -> &mut OrderedSet> { - &mut self.authentication + &mut self.inner.authentication } /// Returns a reference to the `CoreDocument` assertionMethod set. pub fn assertion_method(&self) -> &OrderedSet> { - &self.assertion_method + &self.inner.assertion_method } /// Returns a mutable reference to the `CoreDocument` assertionMethod set. pub fn assertion_method_mut(&mut self) -> &mut OrderedSet> { - &mut self.assertion_method + &mut self.inner.assertion_method } /// Returns a reference to the `CoreDocument` keyAgreement set. pub fn key_agreement(&self) -> &OrderedSet> { - &self.key_agreement + &self.inner.key_agreement } /// Returns a mutable reference to the `CoreDocument` keyAgreement set. pub fn key_agreement_mut(&mut self) -> &mut OrderedSet> { - &mut self.key_agreement + &mut self.inner.key_agreement } /// Returns a reference to the `CoreDocument` capabilityDelegation set. pub fn capability_delegation(&self) -> &OrderedSet> { - &self.capability_delegation + &self.inner.capability_delegation } /// Returns a mutable reference to the `CoreDocument` capabilityDelegation set. pub fn capability_delegation_mut(&mut self) -> &mut OrderedSet> { - &mut self.capability_delegation + &mut self.inner.capability_delegation } /// Returns a reference to the `CoreDocument` capabilityInvocation set. pub fn capability_invocation(&self) -> &OrderedSet> { - &self.capability_invocation + &self.inner.capability_invocation } /// Returns a mutable reference to the `CoreDocument` capabilityInvocation set. pub fn capability_invocation_mut(&mut self) -> &mut OrderedSet> { - &mut self.capability_invocation + &mut self.inner.capability_invocation } /// Returns a reference to the `CoreDocument` service set. pub fn service(&self) -> &OrderedSet> { - &self.service + &self.inner.service } /// Returns a mutable reference to the `CoreDocument` service set. pub fn service_mut(&mut self) -> &mut OrderedSet> { - &mut self.service + &mut self.inner.service } /// Returns a reference to the custom `CoreDocument` properties. pub fn properties(&self) -> &T { - &self.properties + &self.inner.properties } /// Returns a mutable reference to the custom `CoreDocument` properties. pub fn properties_mut(&mut self) -> &mut T { - &mut self.properties + &mut self.inner.properties } /// Maps `CoreDocument` to `CoreDocument` by applying a function `f` to all [`DID`] fields @@ -258,42 +271,51 @@ where F: FnMut(D) -> C, G: FnOnce(T) -> S, { + let current_inner = self.inner; CoreDocument { - id: f(self.id), - controller: self.controller.map(|controller_set| controller_set.map(&mut f)), - also_known_as: self.also_known_as, - verification_method: self - .verification_method - .into_iter() - .map(|method| method.map(&mut f)) - .collect(), - authentication: self - .authentication - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - assertion_method: self - .assertion_method - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - key_agreement: self - .key_agreement - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - capability_delegation: self - .capability_delegation - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - capability_invocation: self - .capability_invocation - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - service: self.service.into_iter().map(|service| service.map(&mut f)).collect(), - properties: g(self.properties), + inner: CoreDocumentInner { + id: f(current_inner.id), + controller: current_inner + .controller + .map(|controller_set| controller_set.map(&mut f)), + also_known_as: current_inner.also_known_as, + verification_method: current_inner + .verification_method + .into_iter() + .map(|method| method.map(&mut f)) + .collect(), + authentication: current_inner + .authentication + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + assertion_method: current_inner + .assertion_method + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + key_agreement: current_inner + .key_agreement + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + capability_delegation: current_inner + .capability_delegation + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + capability_invocation: current_inner + .capability_invocation + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + service: current_inner + .service + .into_iter() + .map(|service| service.map(&mut f)) + .collect(), + properties: g(current_inner.properties), + }, } } @@ -308,49 +330,52 @@ where F: FnMut(D) -> Result, G: FnOnce(T) -> Result, { + let current_inner = self.inner; Ok(CoreDocument { - id: f(self.id)?, - controller: self - .controller - .map(|controller_set| controller_set.try_map(&mut f)) - .transpose()?, - also_known_as: self.also_known_as, - verification_method: self - .verification_method - .into_iter() - .map(|method| method.try_map(&mut f)) - .collect::>()?, - authentication: self - .authentication - .into_iter() - .map(|method_ref| method_ref.try_map(&mut f)) - .collect::>()?, - assertion_method: self - .assertion_method - .into_iter() - .map(|method_ref| method_ref.try_map(&mut f)) - .collect::>()?, - key_agreement: self - .key_agreement - .into_iter() - .map(|method_ref| method_ref.try_map(&mut f)) - .collect::>()?, - capability_delegation: self - .capability_delegation - .into_iter() - .map(|method_ref| method_ref.try_map(&mut f)) - .collect::>()?, - capability_invocation: self - .capability_invocation - .into_iter() - .map(|method_ref| method_ref.try_map(&mut f)) - .collect::>()?, - service: self - .service - .into_iter() - .map(|service| service.try_map(&mut f)) - .collect::>()?, - properties: g(self.properties)?, + inner: CoreDocumentInner { + id: f(current_inner.id)?, + controller: current_inner + .controller + .map(|controller_set| controller_set.try_map(&mut f)) + .transpose()?, + also_known_as: current_inner.also_known_as, + verification_method: current_inner + .verification_method + .into_iter() + .map(|method| method.try_map(&mut f)) + .collect::>()?, + authentication: current_inner + .authentication + .into_iter() + .map(|method_ref| method_ref.try_map(&mut f)) + .collect::>()?, + assertion_method: current_inner + .assertion_method + .into_iter() + .map(|method_ref| method_ref.try_map(&mut f)) + .collect::>()?, + key_agreement: current_inner + .key_agreement + .into_iter() + .map(|method_ref| method_ref.try_map(&mut f)) + .collect::>()?, + capability_delegation: current_inner + .capability_delegation + .into_iter() + .map(|method_ref| method_ref.try_map(&mut f)) + .collect::>()?, + capability_invocation: current_inner + .capability_invocation + .into_iter() + .map(|method_ref| method_ref.try_map(&mut f)) + .collect::>()?, + service: current_inner + .service + .into_iter() + .map(|service| service.try_map(&mut f)) + .collect::>()?, + properties: g(current_inner.properties)?, + }, }) } @@ -365,21 +390,21 @@ where } match scope { - MethodScope::VerificationMethod => self.verification_method.append(method), + MethodScope::VerificationMethod => self.inner.verification_method.append(method), MethodScope::VerificationRelationship(MethodRelationship::Authentication) => { - self.authentication.append(MethodRef::Embed(method)) + self.inner.authentication.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::AssertionMethod) => { - self.assertion_method.append(MethodRef::Embed(method)) + self.inner.assertion_method.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::KeyAgreement) => { - self.key_agreement.append(MethodRef::Embed(method)) + self.inner.key_agreement.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::CapabilityDelegation) => { - self.capability_delegation.append(MethodRef::Embed(method)) + self.inner.capability_delegation.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::CapabilityInvocation) => { - self.capability_invocation.append(MethodRef::Embed(method)) + self.inner.capability_invocation.append(MethodRef::Embed(method)) } }; @@ -393,12 +418,12 @@ where /// Returns an error if the method does not exist. pub fn remove_method(&mut self, did: &DIDUrl) -> Result<()> { let was_removed: bool = [ - self.authentication.remove(did), - self.assertion_method.remove(did), - self.key_agreement.remove(did), - self.capability_delegation.remove(did), - self.capability_invocation.remove(did), - self.verification_method.remove(did), + self.inner.authentication.remove(did), + self.inner.assertion_method.remove(did), + self.inner.key_agreement.remove(did), + self.inner.capability_delegation.remove(did), + self.inner.capability_invocation.remove(did), + self.inner.verification_method.remove(did), ] .contains(&true); @@ -539,13 +564,14 @@ where } self + .inner .verification_method .iter() - .chain(self.authentication.iter().filter_map(__filter_ref)) - .chain(self.assertion_method.iter().filter_map(__filter_ref)) - .chain(self.key_agreement.iter().filter_map(__filter_ref)) - .chain(self.capability_delegation.iter().filter_map(__filter_ref)) - .chain(self.capability_invocation.iter().filter_map(__filter_ref)) + .chain(self.inner.authentication.iter().filter_map(__filter_ref)) + .chain(self.inner.assertion_method.iter().filter_map(__filter_ref)) + .chain(self.inner.key_agreement.iter().filter_map(__filter_ref)) + .chain(self.inner.capability_delegation.iter().filter_map(__filter_ref)) + .chain(self.inner.capability_invocation.iter().filter_map(__filter_ref)) } /// Returns an iterator over all verification relationships. @@ -553,12 +579,13 @@ where /// This includes embedded and referenced [`VerificationMethods`](VerificationMethod). pub fn verification_relationships(&self) -> impl Iterator> { self + .inner .authentication .iter() - .chain(self.assertion_method.iter()) - .chain(self.key_agreement.iter()) - .chain(self.capability_delegation.iter()) - .chain(self.capability_invocation.iter()) + .chain(self.inner.assertion_method.iter()) + .chain(self.inner.key_agreement.iter()) + .chain(self.inner.capability_delegation.iter()) + .chain(self.inner.capability_invocation.iter()) } /// Returns the first [`VerificationMethod`] with an `id` property matching the @@ -576,21 +603,29 @@ where let resolve_ref_helper = |method_ref: &'me MethodRef| self.resolve_method_ref(method_ref); match scope { - MethodScope::VerificationMethod => self.verification_method.query(query.into()), - MethodScope::VerificationRelationship(MethodRelationship::Authentication) => { - self.authentication.query(query.into()).and_then(resolve_ref_helper) - } - MethodScope::VerificationRelationship(MethodRelationship::AssertionMethod) => { - self.assertion_method.query(query.into()).and_then(resolve_ref_helper) - } - MethodScope::VerificationRelationship(MethodRelationship::KeyAgreement) => { - self.key_agreement.query(query.into()).and_then(resolve_ref_helper) - } + MethodScope::VerificationMethod => self.inner.verification_method.query(query.into()), + MethodScope::VerificationRelationship(MethodRelationship::Authentication) => self + .inner + .authentication + .query(query.into()) + .and_then(resolve_ref_helper), + MethodScope::VerificationRelationship(MethodRelationship::AssertionMethod) => self + .inner + .assertion_method + .query(query.into()) + .and_then(resolve_ref_helper), + MethodScope::VerificationRelationship(MethodRelationship::KeyAgreement) => self + .inner + .key_agreement + .query(query.into()) + .and_then(resolve_ref_helper), MethodScope::VerificationRelationship(MethodRelationship::CapabilityDelegation) => self + .inner .capability_delegation .query(query.into()) .and_then(resolve_ref_helper), MethodScope::VerificationRelationship(MethodRelationship::CapabilityInvocation) => self + .inner .capability_invocation .query(query.into()) .and_then(resolve_ref_helper), @@ -612,7 +647,7 @@ where { match scope { Some(scope) => match scope { - MethodScope::VerificationMethod => self.verification_method.query_mut(query.into()), + MethodScope::VerificationMethod => self.inner.verification_method.query_mut(query.into()), MethodScope::VerificationRelationship(MethodRelationship::Authentication) => { method_ref_mut_helper!(self, authentication, query) } @@ -637,7 +672,7 @@ where pub fn resolve_method_ref<'a>(&'a self, method_ref: &'a MethodRef) -> Option<&'a VerificationMethod> { match method_ref { MethodRef::Embed(method) => Some(method), - MethodRef::Refer(did) => self.verification_method.query(did), + MethodRef::Refer(did) => self.inner.verification_method.query(did), } } @@ -645,29 +680,29 @@ where let mut method: Option<&MethodRef> = None; if method.is_none() { - method = self.authentication.query(query.clone()); + method = self.inner.authentication.query(query.clone()); } if method.is_none() { - method = self.assertion_method.query(query.clone()); + method = self.inner.assertion_method.query(query.clone()); } if method.is_none() { - method = self.key_agreement.query(query.clone()); + method = self.inner.key_agreement.query(query.clone()); } if method.is_none() { - method = self.capability_delegation.query(query.clone()); + method = self.inner.capability_delegation.query(query.clone()); } if method.is_none() { - method = self.capability_invocation.query(query.clone()); + method = self.inner.capability_invocation.query(query.clone()); } match method { Some(MethodRef::Embed(method)) => Some(method), - Some(MethodRef::Refer(did)) => self.verification_method.query(&did.to_string()), - None => self.verification_method.query(query), + Some(MethodRef::Refer(did)) => self.inner.verification_method.query(&did.to_string()), + None => self.inner.verification_method.query(query), } } @@ -675,31 +710,82 @@ where let mut method: Option<&mut MethodRef> = None; if method.is_none() { - method = self.authentication.query_mut(query.clone()); + method = self.inner.authentication.query_mut(query.clone()); } if method.is_none() { - method = self.assertion_method.query_mut(query.clone()); + method = self.inner.assertion_method.query_mut(query.clone()); } if method.is_none() { - method = self.key_agreement.query_mut(query.clone()); + method = self.inner.key_agreement.query_mut(query.clone()); } if method.is_none() { - method = self.capability_delegation.query_mut(query.clone()); + method = self.inner.capability_delegation.query_mut(query.clone()); } if method.is_none() { - method = self.capability_invocation.query_mut(query.clone()); + method = self.inner.capability_invocation.query_mut(query.clone()); } match method { Some(MethodRef::Embed(method)) => Some(method), - Some(MethodRef::Refer(did)) => self.verification_method.query_mut(&did.to_string()), - None => self.verification_method.query_mut(query), + Some(MethodRef::Refer(did)) => self.inner.verification_method.query_mut(&did.to_string()), + None => self.inner.verification_method.query_mut(query), } } + + fn check_id_constraints(&self) -> Result<()> { + let mut id_references: HashSet<&DIDUrl> = HashSet::new(); + let mut scoped_method_ids: HashSet<&DIDUrl> = HashSet::new(); + for method_ref in self + .inner + .authentication + .iter() + .chain(self.inner.assertion_method.iter()) + .chain(self.inner.key_agreement.iter()) + .chain(self.inner.capability_delegation.iter()) + .chain(self.inner.capability_invocation.iter()) + { + match method_ref { + MethodRef::Embed(method) => { + if !scoped_method_ids.insert(method.id()) { + //TODO: Consider renaming error to DuplicateMethodAttempt + return Err(Error::MethodAlreadyExists); + } + } + MethodRef::Refer(id) => { + id_references.insert(id); + } + } + } + if !scoped_method_ids.is_disjoint(&id_references) { + // TODO: Consider renaming InvalidMethodEmbedded or create a new error variant + return Err(Error::InvalidMethodEmbedded); + } + + let num_scoped = scoped_method_ids.len(); + let num_non_scoped = self.inner.verification_method.len(); + let mut method_ids = { + scoped_method_ids.extend(self.inner.verification_method.iter().map(|method| method.id())); + scoped_method_ids + }; + if method_ids.len() < num_scoped + num_non_scoped { + return Err(Error::MethodAlreadyExists); + } + method_ids.extend(id_references); + + for service_id in self.inner.service.iter().map(|service| service.id()) { + if method_ids.contains(service_id) { + return Err(Error::InvalidService( + "the service id is shared with a verification method", + )); + } + } + + Ok(()) + } } impl CoreDocument diff --git a/identity_did/src/document/mod.rs b/identity_did/src/document/mod.rs index c3de65a652..20ed597c02 100644 --- a/identity_did/src/document/mod.rs +++ b/identity_did/src/document/mod.rs @@ -9,6 +9,7 @@ pub use self::builder::DocumentBuilder; pub use self::core_document::CoreDocument; pub use self::traits::Document; +pub(crate) use core_document::CoreDocumentInner; mod builder; mod core_document; mod traits; From c1b4b2cb0f6895594f51732e279b3ab87314f515 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Mon, 31 Oct 2022 18:42:20 +0100 Subject: [PATCH 06/18] fixed deserialization --- identity_did/Cargo.toml | 5 + identity_did/benches/deserialize_document.rs | 235 ++++++++++++ identity_did/src/diff/diff_document.rs | 6 +- identity_did/src/document/core_document.rs | 344 ++++++++++-------- identity_did/src/document/mod.rs | 2 +- .../src/state_metadata/document.rs | 4 +- 6 files changed, 443 insertions(+), 153 deletions(-) create mode 100644 identity_did/benches/deserialize_document.rs diff --git a/identity_did/Cargo.toml b/identity_did/Cargo.toml index 500439250c..815260a1d5 100644 --- a/identity_did/Cargo.toml +++ b/identity_did/Cargo.toml @@ -23,6 +23,7 @@ strum = { version = "0.24.0", default-features = false, features = ["std", "deri thiserror = { version = "1.0", default-features = false } [dev-dependencies] +criterion = { version = "0.4.0", default-features = false, features = ["cargo_bench_support"] } proptest = { version = "1.0" } serde_json = { version = "1.0", default-features = false } @@ -35,3 +36,7 @@ rustdoc-args = ["--cfg", "docsrs"] [features] default = ["revocation-bitmap"] revocation-bitmap = ["dataurl", "flate2", "roaring"] + +[[bench]] +name = "deserialize_document" +harness = false diff --git a/identity_did/benches/deserialize_document.rs b/identity_did/benches/deserialize_document.rs new file mode 100644 index 0000000000..e171a0c73f --- /dev/null +++ b/identity_did/benches/deserialize_document.rs @@ -0,0 +1,235 @@ +// Copyright 2020-2022 IOTA Stiftung +// SPDX-License-Identifier: Apache-2.0 + +use criterion::Throughput; +use identity_core::convert::FromJson; +// This is a benchmark measuring the time it takes to deserialize a DID document from JSON. +use criterion::criterion_group; +use criterion::criterion_main; +use criterion::BenchmarkId; +use criterion::Criterion; +use identity_did::document::CoreDocument; + +const JSON_DOC_SHORT: &str = r#" + { + "id": "did:iota:0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "verificationMethod": [ + { + "id": "did:iota:0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa#issuerKey", + "controller": "did:iota:0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "type": "Ed25519VerificationKey2018", + "publicKeyMultibase": "zFVen3X669xLzsi6N2V91DoiyzHzg1uAgqiT8jZ9nS96Z" + } + ] + }"#; + +const JSON_DOC_DID_KEY: &str = r#"{ + "@context": [ + "https://www.w3.org/ns/did/v1", + "https://w3id.org/security/suites/ed25519-2020/v1", + "https://w3id.org/security/suites/x25519-2020/v1" + ], + "id": "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK", + "verificationMethod": [{ + "id": "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK#z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK", + "type": "Ed25519VerificationKey2018", + "controller": "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK", + "publicKeyMultibase": "z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK" + }], + "authentication": [ + "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK#z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK" + ], + "assertionMethod": [ + "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK#z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK" + ], + "capabilityDelegation": [ + "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK#z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK" + ], + "capabilityInvocation": [ + "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK#z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK" + ], + "keyAgreement": [{ + "id": "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK#z6LSj72tK8brWgZja8NLRwPigth2T9QRiG1uH9oKZuKjdh9p", + "type": "X25519KeyAgreementKey2019", + "controller": "did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK", + "publicKeyMultibase": "z6LSj72tK8brWgZja8NLRwPigth2T9QRiG1uH9oKZuKjdh9p" + }] + }"#; + +// This is not a realistic document in any way. +const JSON_DOCUMENT_LARGE: &str = r#"{ + "@context": [ + "https://www.w3.org/ns/did/v1", + "https://w3id.org/security/suites/ed25519-2020/v1" + ], + "id": "did:example:123", + "verificationMethod": [ + { + "id": "did:example:123#key1", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + }, + { + "id": "did:example:123#key2", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + }, + { + "id": "did:example:123#key3", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + }, + { + "id": "did:example:123#key4", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + }, + { + "id": "did:example:123#key5", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + }, + { + "id": "did:example:123#key6", + "controller": "did:example:1234", + "type": "Ed25519VerificationKey2018", + "publicKeyBase58": "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J" + } + ], + "authentication": [ + { + "id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3EMB1", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf" + }, + "did:example:123#key1", + { + "id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3EMB2", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf" + }, + "did:example:123#key4", + "did:example:123#key2", + { + "id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3EMB3", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf" + }, + { + "id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3EMB4", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf" + } + ], + "capabilityInvocation": [ + { + "id": "did:example:123#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN" + }, + "did:example:123#key1", + { + "id": "did:example:123#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39kEMB2", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN" + }, + { + "id": "did:example:123#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39kEMB3", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN" + }, + "did:example:123#key5" + ], + "capabilityDelegation": [ + { + "id": "did:example:123#z6Mkw94ByR26zMSkNdCUi6FNRsWnc2DFEeDXyBGJ5KTzSWyi", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "zHgo9PAmfeoxHG8Mn2XHXamxnnSwPpkyBHAMNF3VyXJCL" + }, + "did:example:123#key6" + ], + "assertionMethod": [ + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomY", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + }, + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomYEMB2", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + }, + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomYEMB3", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + }, + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomYEMB4", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + }, + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomYEMB5", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + }, + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomYEMB6", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + }, + "did:example:123#key4", + { + "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomYEMB7", + "type": "Ed25519VerificationKey2018", + "controller": "did:example:123", + "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA" + } + ], + "service": [{ + "id":"did:example:123#linked-domain", + "type": "LinkedDomains", + "serviceEndpoint": "https://bar.example.com" + }] +}"#; + +fn deserialize_json_document(c: &mut Criterion) { + let mut group = c.benchmark_group("deserialize_json_document"); + for (json, name) in [ + (JSON_DOC_SHORT, "short document"), + (JSON_DOC_DID_KEY, "did:key document"), + (JSON_DOCUMENT_LARGE, "large document"), + ] { + group.throughput(Throughput::Bytes(json.as_bytes().len() as u64)); + group.bench_with_input(BenchmarkId::from_parameter(name), json, |b, json| { + b.iter(|| { + let doc: Result = CoreDocument::from_json(json); + assert!(doc.is_ok(), "bench {name} failed: {:#?}", doc); + }) + }); + } + group.finish(); +} + +criterion_group!(benches, deserialize_json_document); +criterion_main!(benches); diff --git a/identity_did/src/diff/diff_document.rs b/identity_did/src/diff/diff_document.rs index 7f30717ecd..fdaa1cf29a 100644 --- a/identity_did/src/diff/diff_document.rs +++ b/identity_did/src/diff/diff_document.rs @@ -17,7 +17,7 @@ use identity_core::diff::Result; use crate::did::CoreDID; use crate::did::DID; use crate::document::CoreDocument; -use crate::document::CoreDocumentInner; +use crate::document::CoreDocumentData; use crate::service::Service; use crate::verification::MethodRef; use crate::verification::VerificationMethod; @@ -202,7 +202,7 @@ where .unwrap_or_else(|| self.properties().clone()); Ok(CoreDocument { - inner: CoreDocumentInner { + inner: CoreDocumentData { id, controller, also_known_as, @@ -285,7 +285,7 @@ where let properties: T = diff.properties.map(T::from_diff).transpose()?.unwrap_or_default(); Ok(CoreDocument { - inner: CoreDocumentInner { + inner: CoreDocumentData { id, controller, also_known_as, diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index 761e641642..d2c65f3ea3 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -22,6 +22,7 @@ use identity_core::crypto::PrivateKey; use identity_core::crypto::Proof; use identity_core::crypto::ProofPurpose; use identity_core::crypto::Verifier; +use serde::Serializer; use crate::did::CoreDID; use crate::did::DIDUrl; @@ -45,7 +46,7 @@ use crate::verification::VerificationMethod; #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[rustfmt::skip] -pub(crate) struct CoreDocumentInner +pub(crate) struct CoreDocumentData where D: DID + KeyComparable { @@ -72,17 +73,85 @@ pub(crate) struct CoreDocumentInner CoreDocumentData { + fn check_id_constraints(&self) -> Result<()> { + let mut id_references: HashSet<&DIDUrl> = HashSet::new(); + let mut scoped_method_ids: HashSet<&DIDUrl> = HashSet::new(); + for method_ref in self + .authentication + .iter() + .chain(self.assertion_method.iter()) + .chain(self.key_agreement.iter()) + .chain(self.capability_delegation.iter()) + .chain(self.capability_invocation.iter()) + { + match method_ref { + MethodRef::Embed(method) => { + if !scoped_method_ids.insert(method.id()) { + //TODO: Consider renaming error to DuplicateMethodAttempt + return Err(Error::MethodAlreadyExists); + } + } + MethodRef::Refer(id) => { + id_references.insert(id); + } + } + } + if !scoped_method_ids.is_disjoint(&id_references) { + // TODO: Consider renaming InvalidMethodEmbedded or create a new error variant + return Err(Error::InvalidMethodEmbedded); + } + + let num_scoped = scoped_method_ids.len(); + let num_non_scoped = self.verification_method.len(); + let mut method_ids = { + scoped_method_ids.extend(self.verification_method.iter().map(|method| method.id())); + scoped_method_ids + }; + if method_ids.len() < num_scoped + num_non_scoped { + return Err(Error::MethodAlreadyExists); + } + method_ids.extend(id_references); + + for service_id in self.service.iter().map(|service| service.id()) { + if method_ids.contains(service_id) { + return Err(Error::InvalidService( + "the service id is shared with a verification method", + )); + } + } + + Ok(()) + } +} + /// A DID Document. /// /// [Specification](https://www.w3.org/TR/did-core/#did-document-properties) -#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[derive(Clone, Debug, PartialEq, Eq, Deserialize)] #[rustfmt::skip] -#[serde(transparent)] +#[serde(try_from = "CoreDocumentData")] pub struct CoreDocument where D: DID + KeyComparable { - pub(crate) inner: CoreDocumentInner, + pub(crate) inner: CoreDocumentData, +} + +//Forward serialization to inner +impl Serialize for CoreDocument +where + D: DID + KeyComparable + Serialize, + T: Serialize, + U: Serialize, + V: Serialize, +{ + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.inner.serialize(serializer) + } } // Workaround for lifetime issues with a mutable reference to self preventing closures from being used. @@ -108,48 +177,46 @@ where /// Returns a new `CoreDocument` based on the [`DocumentBuilder`] configuration. pub fn from_builder(builder: DocumentBuilder) -> Result { - Ok(Self { - inner: CoreDocumentInner { - id: builder.id.ok_or(Error::InvalidDocument("missing id", None))?, - controller: Some(builder.controller) - .filter(|controllers| !controllers.is_empty()) - .map(TryFrom::try_from) - .transpose() - .map_err(|err| Error::InvalidDocument("controller", Some(err)))?, - also_known_as: builder - .also_known_as - .try_into() - .map_err(|err| Error::InvalidDocument("also_known_as", Some(err)))?, - verification_method: builder - .verification_method - .try_into() - .map_err(|err| Error::InvalidDocument("verification_method", Some(err)))?, - authentication: builder - .authentication - .try_into() - .map_err(|err| Error::InvalidDocument("authentication", Some(err)))?, - assertion_method: builder - .assertion_method - .try_into() - .map_err(|err| Error::InvalidDocument("assertion_method", Some(err)))?, - key_agreement: builder - .key_agreement - .try_into() - .map_err(|err| Error::InvalidDocument("key_agreement", Some(err)))?, - capability_delegation: builder - .capability_delegation - .try_into() - .map_err(|err| Error::InvalidDocument("capability_delegation", Some(err)))?, - capability_invocation: builder - .capability_invocation - .try_into() - .map_err(|err| Error::InvalidDocument("capability_invocation", Some(err)))?, - service: builder - .service - .try_into() - .map_err(|err| Error::InvalidDocument("service", Some(err)))?, - properties: builder.properties, - }, + Self::try_from(CoreDocumentData { + id: builder.id.ok_or(Error::InvalidDocument("missing id", None))?, + controller: Some(builder.controller) + .filter(|controllers| !controllers.is_empty()) + .map(TryFrom::try_from) + .transpose() + .map_err(|err| Error::InvalidDocument("controller", Some(err)))?, + also_known_as: builder + .also_known_as + .try_into() + .map_err(|err| Error::InvalidDocument("also_known_as", Some(err)))?, + verification_method: builder + .verification_method + .try_into() + .map_err(|err| Error::InvalidDocument("verification_method", Some(err)))?, + authentication: builder + .authentication + .try_into() + .map_err(|err| Error::InvalidDocument("authentication", Some(err)))?, + assertion_method: builder + .assertion_method + .try_into() + .map_err(|err| Error::InvalidDocument("assertion_method", Some(err)))?, + key_agreement: builder + .key_agreement + .try_into() + .map_err(|err| Error::InvalidDocument("key_agreement", Some(err)))?, + capability_delegation: builder + .capability_delegation + .try_into() + .map_err(|err| Error::InvalidDocument("capability_delegation", Some(err)))?, + capability_invocation: builder + .capability_invocation + .try_into() + .map_err(|err| Error::InvalidDocument("capability_invocation", Some(err)))?, + service: builder + .service + .try_into() + .map_err(|err| Error::InvalidDocument("service", Some(err)))?, + properties: builder.properties, }) } @@ -265,6 +332,10 @@ where /// Maps `CoreDocument` to `CoreDocument` by applying a function `f` to all [`DID`] fields /// and another function `g` to the custom properties. + /// + /// # Panics + /// Panics if the mapping `f` introduces methods referencing embedded method identifiers, + /// or services with identifiers matching method identifiers. pub fn map(self, mut f: F, g: G) -> CoreDocument where C: DID + KeyComparable, @@ -272,67 +343,68 @@ where G: FnOnce(T) -> S, { let current_inner = self.inner; - CoreDocument { - inner: CoreDocumentInner { - id: f(current_inner.id), - controller: current_inner - .controller - .map(|controller_set| controller_set.map(&mut f)), - also_known_as: current_inner.also_known_as, - verification_method: current_inner - .verification_method - .into_iter() - .map(|method| method.map(&mut f)) - .collect(), - authentication: current_inner - .authentication - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - assertion_method: current_inner - .assertion_method - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - key_agreement: current_inner - .key_agreement - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - capability_delegation: current_inner - .capability_delegation - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - capability_invocation: current_inner - .capability_invocation - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - service: current_inner - .service - .into_iter() - .map(|service| service.map(&mut f)) - .collect(), - properties: g(current_inner.properties), - }, - } + CoreDocument::try_from(CoreDocumentData { + id: f(current_inner.id), + controller: current_inner + .controller + .map(|controller_set| controller_set.map(&mut f)), + also_known_as: current_inner.also_known_as, + verification_method: current_inner + .verification_method + .into_iter() + .map(|method| method.map(&mut f)) + .collect(), + authentication: current_inner + .authentication + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + assertion_method: current_inner + .assertion_method + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + key_agreement: current_inner + .key_agreement + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + capability_delegation: current_inner + .capability_delegation + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + capability_invocation: current_inner + .capability_invocation + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + service: current_inner + .service + .into_iter() + .map(|service| service.map(&mut f)) + .collect(), + properties: g(current_inner.properties), + }) + .unwrap() } /// Fallible version of [`CoreDocument::map`]. /// /// # Errors /// - /// `try_map` can fail if either of the provided functions fail. - pub fn try_map(self, mut f: F, g: G) -> Result, E> + /// `try_map` can fail if either of the provided functions fail or if the mapping `f` + /// introduces methods referencing embedded method identifiers, or services with identifiers matching method + /// identifiers.. + pub fn try_map(self, mut f: F, g: G) -> Result, Error>, E> where C: DID + KeyComparable, F: FnMut(D) -> Result, G: FnOnce(T) -> Result, { let current_inner = self.inner; - Ok(CoreDocument { - inner: CoreDocumentInner { + let helper = || -> Result, E> { + Ok(CoreDocumentData { id: f(current_inner.id)?, controller: current_inner .controller @@ -375,8 +447,9 @@ where .map(|service| service.try_map(&mut f)) .collect::>()?, properties: g(current_inner.properties)?, - }, - }) + }) + }; + helper().map(|data| CoreDocument::try_from(data)) } /// Adds a new [`VerificationMethod`] to the document in the given [`MethodScope`]. @@ -735,57 +808,6 @@ where None => self.inner.verification_method.query_mut(query), } } - - fn check_id_constraints(&self) -> Result<()> { - let mut id_references: HashSet<&DIDUrl> = HashSet::new(); - let mut scoped_method_ids: HashSet<&DIDUrl> = HashSet::new(); - for method_ref in self - .inner - .authentication - .iter() - .chain(self.inner.assertion_method.iter()) - .chain(self.inner.key_agreement.iter()) - .chain(self.inner.capability_delegation.iter()) - .chain(self.inner.capability_invocation.iter()) - { - match method_ref { - MethodRef::Embed(method) => { - if !scoped_method_ids.insert(method.id()) { - //TODO: Consider renaming error to DuplicateMethodAttempt - return Err(Error::MethodAlreadyExists); - } - } - MethodRef::Refer(id) => { - id_references.insert(id); - } - } - } - if !scoped_method_ids.is_disjoint(&id_references) { - // TODO: Consider renaming InvalidMethodEmbedded or create a new error variant - return Err(Error::InvalidMethodEmbedded); - } - - let num_scoped = scoped_method_ids.len(); - let num_non_scoped = self.inner.verification_method.len(); - let mut method_ids = { - scoped_method_ids.extend(self.inner.verification_method.iter().map(|method| method.id())); - scoped_method_ids - }; - if method_ids.len() < num_scoped + num_non_scoped { - return Err(Error::MethodAlreadyExists); - } - method_ids.extend(id_references); - - for service_id in self.inner.service.iter().map(|service| service.id()) { - if method_ids.contains(service_id) { - return Err(Error::InvalidService( - "the service id is shared with a verification method", - )); - } - } - - Ok(()) - } } impl CoreDocument @@ -920,6 +942,19 @@ where } } +impl TryFrom> for CoreDocument +where + D: DID + KeyComparable, +{ + type Error = crate::Error; + fn try_from(value: CoreDocumentData) -> Result { + match value.check_id_constraints() { + Ok(_) => Ok(Self { inner: value }), + Err(err) => Err(err), + } + } +} + #[cfg(feature = "revocation-bitmap")] mod core_document_revocation { use identity_core::common::KeyComparable; @@ -1021,6 +1056,7 @@ where #[cfg(test)] mod tests { use identity_core::convert::FromJson; + use identity_core::convert::ToJson; use crate::verification::MethodData; @@ -1422,6 +1458,18 @@ mod tests { } } + #[test] + fn serialize_deserialize_roundtrip() { + let document: CoreDocument = document(); + let doc_json: String = document.to_json().unwrap(); + let doc_json_value: serde_json::Value = document.to_json_value().unwrap(); + let doc_json_vec: Vec = document.to_json_vec().unwrap(); + assert_eq!(document, CoreDocument::from_json(&doc_json).unwrap()); + assert_eq!(document, CoreDocument::from_json_value(doc_json_value).unwrap()); + + assert_eq!(document, CoreDocument::from_json_slice(&doc_json_vec).unwrap()); + } + #[test] fn deserialize_valid() { // The verification method types here are really Ed25519VerificationKey2020, changed to be compatible diff --git a/identity_did/src/document/mod.rs b/identity_did/src/document/mod.rs index 20ed597c02..5571f327e1 100644 --- a/identity_did/src/document/mod.rs +++ b/identity_did/src/document/mod.rs @@ -9,7 +9,7 @@ pub use self::builder::DocumentBuilder; pub use self::core_document::CoreDocument; pub use self::traits::Document; -pub(crate) use core_document::CoreDocumentInner; +pub(crate) use core_document::CoreDocumentData; mod builder; mod core_document; mod traits; diff --git a/identity_iota_core/src/state_metadata/document.rs b/identity_iota_core/src/state_metadata/document.rs index b0e2c66f37..fd463c1feb 100644 --- a/identity_iota_core/src/state_metadata/document.rs +++ b/identity_iota_core/src/state_metadata/document.rs @@ -40,7 +40,7 @@ impl StateMetadataDocument { /// Transforms the document into a [`IotaDocument`] by replacing all placeholders with `original_did`. pub fn into_iota_document(self, original_did: &IotaDID) -> Result { let Self { document, metadata } = self; - let core_document: IotaCoreDocument = document.try_map( + let core_document_result: Result = document.try_map( // Replace placeholder identifiers. |did| { if did == PLACEHOLDER_DID.as_ref() { @@ -53,6 +53,8 @@ impl StateMetadataDocument { // Do not modify properties. Ok, )?; + let core_document: IotaCoreDocument = core_document_result.map_err(|error| crate::Error::InvalidDoc(error))?; + Ok(IotaDocument::from((core_document, metadata))) } From 6b855e5e0da10f5148cda96931a4f85c84237937 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Mon, 31 Oct 2022 20:34:44 +0100 Subject: [PATCH 07/18] documented check --- identity_did/src/document/core_document.rs | 33 ++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index d2c65f3ea3..e08c17c522 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -75,8 +75,21 @@ pub(crate) struct CoreDocumentData CoreDocumentData { fn check_id_constraints(&self) -> Result<()> { + // Algorithm: + // 1: Create two empty sets: `embedded_method_ids` and `id_references`. + // 2: Loop through all the scoped verification methods and push the ids of the embedded methods and references into + // `embedded_method_ids` and `id_references` respectively. If a duplicate embedded method is encountered (that + // is an embedded method, identified by id, exists in two different scopes) then immediately throw an error. + // 3: Ensure that the two constructed sets have an empty intersection, otherwise an embedded method is being + // referenced and an error needs to be thrown. 4: Create a new set that is the union of the + // `embedded_method_ids` and the set of ids of the unscoped methods (the remaining methods). If the two + // aforementioned sets have a non trivial intersection there is a duplicate method and an error must be thrown. + // 5: Create a set of all ids (embedded, references, and unscoped). + // 6: Loop through all services and check that their ids are not contained in the set constructed in the previous + // step. + + let mut embedded_method_ids: HashSet<&DIDUrl> = HashSet::new(); let mut id_references: HashSet<&DIDUrl> = HashSet::new(); - let mut scoped_method_ids: HashSet<&DIDUrl> = HashSet::new(); for method_ref in self .authentication .iter() @@ -87,8 +100,8 @@ impl CoreDocumentData { { match method_ref { MethodRef::Embed(method) => { - if !scoped_method_ids.insert(method.id()) { - //TODO: Consider renaming error to DuplicateMethodAttempt + if !embedded_method_ids.insert(method.id()) { + //TODO: Consider renaming error or adding a MethodDuplicationAttempt error variant, return Err(Error::MethodAlreadyExists); } } @@ -97,24 +110,28 @@ impl CoreDocumentData { } } } - if !scoped_method_ids.is_disjoint(&id_references) { + if !embedded_method_ids.is_disjoint(&id_references) { // TODO: Consider renaming InvalidMethodEmbedded or create a new error variant return Err(Error::InvalidMethodEmbedded); } - let num_scoped = scoped_method_ids.len(); + // Create the union of method ids that belong to embedded methods and those that belong to an unscoped verification + // method. If the number of elements in the union is not equal to the sum of the number of elements in each of + // the two aforementioned sets there is a non trivial intersection. + let num_embedded = embedded_method_ids.len(); let num_non_scoped = self.verification_method.len(); let mut method_ids = { - scoped_method_ids.extend(self.verification_method.iter().map(|method| method.id())); - scoped_method_ids + embedded_method_ids.extend(self.verification_method.iter().map(|method| method.id())); + embedded_method_ids }; - if method_ids.len() < num_scoped + num_non_scoped { + if method_ids.len() < num_embedded + num_non_scoped { return Err(Error::MethodAlreadyExists); } method_ids.extend(id_references); for service_id in self.service.iter().map(|service| service.id()) { if method_ids.contains(service_id) { + // TODO: consider using another error variant return Err(Error::InvalidService( "the service id is shared with a verification method", )); From cbf6e0e69e08a4d5ffb113b2b196c31eb7591637 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 1 Nov 2022 16:12:38 +0100 Subject: [PATCH 08/18] added checks for service --- identity_did/benches/deserialize_document.rs | 16 +- identity_did/src/document/core_document.rs | 203 ++++++++++++++----- identity_did/src/error.rs | 17 +- 3 files changed, 177 insertions(+), 59 deletions(-) diff --git a/identity_did/benches/deserialize_document.rs b/identity_did/benches/deserialize_document.rs index e171a0c73f..ba92a50350 100644 --- a/identity_did/benches/deserialize_document.rs +++ b/identity_did/benches/deserialize_document.rs @@ -221,12 +221,16 @@ fn deserialize_json_document(c: &mut Criterion) { (JSON_DOCUMENT_LARGE, "large document"), ] { group.throughput(Throughput::Bytes(json.as_bytes().len() as u64)); - group.bench_with_input(BenchmarkId::from_parameter(name), json, |b, json| { - b.iter(|| { - let doc: Result = CoreDocument::from_json(json); - assert!(doc.is_ok(), "bench {name} failed: {:#?}", doc); - }) - }); + group.bench_with_input( + BenchmarkId::from_parameter(format!("{name}, document size: {} bytes", json.as_bytes().len())), + json, + |b, json| { + b.iter(|| { + let doc: Result = CoreDocument::from_json(json); + assert!(doc.is_ok(), "bench {name} failed: {:#?}", doc); + }) + }, + ); } group.finish(); } diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index e08c17c522..f204aaa4c4 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -4,7 +4,7 @@ use core::convert::TryInto as _; use core::fmt::Display; use core::fmt::Formatter; -use std::collections::HashSet; +use std::collections::HashMap; use serde::Serialize; @@ -75,65 +75,65 @@ pub(crate) struct CoreDocumentData CoreDocumentData { fn check_id_constraints(&self) -> Result<()> { - // Algorithm: - // 1: Create two empty sets: `embedded_method_ids` and `id_references`. - // 2: Loop through all the scoped verification methods and push the ids of the embedded methods and references into - // `embedded_method_ids` and `id_references` respectively. If a duplicate embedded method is encountered (that - // is an embedded method, identified by id, exists in two different scopes) then immediately throw an error. - // 3: Ensure that the two constructed sets have an empty intersection, otherwise an embedded method is being - // referenced and an error needs to be thrown. 4: Create a new set that is the union of the - // `embedded_method_ids` and the set of ids of the unscoped methods (the remaining methods). If the two - // aforementioned sets have a non trivial intersection there is a duplicate method and an error must be thrown. - // 5: Create a set of all ids (embedded, references, and unscoped). - // 6: Loop through all services and check that their ids are not contained in the set constructed in the previous - // step. - - let mut embedded_method_ids: HashSet<&DIDUrl> = HashSet::new(); - let mut id_references: HashSet<&DIDUrl> = HashSet::new(); - for method_ref in self + let max_unique_method_ids = self.verification_method.len() + + self.authentication.len() + + self.assertion_method.len() + + self.key_agreement.len() + + self.capability_delegation.len() + + self.capability_invocation.len(); + + // Value = true => the identifier belongs to an embedded method, false means it belongs to a method reference or a + // general purpose verification method + let mut method_identifiers: HashMap<&DIDUrl, bool> = HashMap::with_capacity(max_unique_method_ids); + + for (id, is_embedded) in self .authentication .iter() .chain(self.assertion_method.iter()) .chain(self.key_agreement.iter()) .chain(self.capability_delegation.iter()) .chain(self.capability_invocation.iter()) + .map(|method_ref| match method_ref { + MethodRef::Embed(_) => (method_ref.id(), true), + MethodRef::Refer(_) => (method_ref.id(), false), + }) { - match method_ref { - MethodRef::Embed(method) => { - if !embedded_method_ids.insert(method.id()) { - //TODO: Consider renaming error or adding a MethodDuplicationAttempt error variant, - return Err(Error::MethodAlreadyExists); + if let Some(previous) = method_identifiers.insert(id, is_embedded) { + match previous { + // An embedded method with the same id has previously been encountered + true => { + return Err(Error::InvalidDocument( + "attempted to construct document with a duplicated or aliased embedded method", + None, + )); + } + // A method reference to the identifier has previously been encountered + false => { + if is_embedded { + return Err(Error::InvalidDocument( + "attempted to construct document with an aliased embedded method", + None, + )); + } } - } - MethodRef::Refer(id) => { - id_references.insert(id); } } } - if !embedded_method_ids.is_disjoint(&id_references) { - // TODO: Consider renaming InvalidMethodEmbedded or create a new error variant - return Err(Error::InvalidMethodEmbedded); - } - // Create the union of method ids that belong to embedded methods and those that belong to an unscoped verification - // method. If the number of elements in the union is not equal to the sum of the number of elements in each of - // the two aforementioned sets there is a non trivial intersection. - let num_embedded = embedded_method_ids.len(); - let num_non_scoped = self.verification_method.len(); - let mut method_ids = { - embedded_method_ids.extend(self.verification_method.iter().map(|method| method.id())); - embedded_method_ids - }; - if method_ids.len() < num_embedded + num_non_scoped { - return Err(Error::MethodAlreadyExists); + for method_id in self.verification_method.iter().map(|method| method.id()) { + if let Some(_) = method_identifiers.insert(method_id, false).filter(|value| *value) { + return Err(Error::InvalidDocument( + "attempted to construct document with a duplicated embedded method", + None, + )); + } } - method_ids.extend(id_references); for service_id in self.service.iter().map(|service| service.id()) { - if method_ids.contains(service_id) { - // TODO: consider using another error variant - return Err(Error::InvalidService( - "the service id is shared with a verification method", + if method_identifiers.contains_key(service_id) { + return Err(Error::InvalidDocument( + "attempted to construct document with a service identifier shared with a verification method", + None, )); } } @@ -473,12 +473,12 @@ where /// /// # Errors /// - /// Returns an error if a method with the same fragment already exists. + /// Returns an error if a method or service with the same fragment already exists. pub fn insert_method(&mut self, method: VerificationMethod, scope: MethodScope) -> Result<()> { - if self.resolve_method(method.id(), None).is_some() { - return Err(Error::MethodAlreadyExists); + // check that the method identifier is not already in use by an existing method or service. + if self.resolve_method(method.id(), None).is_some() || self.service().query(method.id()).is_some() { + return Err(Error::MethodInsertionError); } - match scope { MethodScope::VerificationMethod => self.inner.verification_method.append(method), MethodScope::VerificationRelationship(MethodRelationship::Authentication) => { @@ -524,6 +524,36 @@ where } } + /// Adds a new [`Service`] to the document. + /// + /// # Errors + /// + /// Returns an error if there already exists a service or (verification) method with the same identifier. + pub fn insert_service(&mut self, service: Service) -> Result<()> { + let service_id = service.id(); + let id_shared_with_method = self + .verification_relationships() + .map(|method_ref| method_ref.id()) + .chain(self.verification_method().iter().map(|method| method.id())) + .any(|id| id == service_id); + + ((!id_shared_with_method) && self.service_mut().append(service)) + .then_some(()) + .ok_or(Error::InvalidServiceInsertion) + } + + /// Removes a [`Service`] from the document. + /// + /// # Errors + /// + /// Returns an error if the service does not exist. + pub fn remove_service(&mut self, id: &DIDUrl) -> Result<()> { + self + .service_mut() + .remove(id) + .then_some(()) + .ok_or(Error::ServiceNotFound) + } /// Attaches the relationship to the method resolved by `method_query`. /// /// # Errors @@ -1075,6 +1105,8 @@ mod tests { use identity_core::convert::FromJson; use identity_core::convert::ToJson; + use crate::service::ServiceBuilder; + use crate::verification::MethodBuilder; use crate::verification::MethodData; use super::*; @@ -1475,6 +1507,79 @@ mod tests { } } + #[test] + fn test_service_updates() { + let mut document = document(); + let service_id = document.id().to_url().join("#service-update-test").unwrap(); + let service_type = "test"; + let service_endpoint = Url::parse("https://example.com").unwrap(); + + let service: Service = ServiceBuilder::default() + .id(service_id.clone()) + .type_(service_type) + .service_endpoint(service_endpoint) + .build() + .unwrap(); + // inserting a service with an identifier not present in the document should be Ok. + assert!(document.insert_service(service.clone()).is_ok()); + // inserting a service with the same identifier as an already existing service should fail. + let mut service_clone = service.clone(); + *service_clone.service_endpoint_mut() = Url::parse("https://other-example.com").unwrap().into(); + assert!(document.insert_service(service_clone).is_err()); + // removing an existing service should succeed + assert!(document.remove_service(service.id()).is_ok()); + // it should now be possible to insert the service again + assert!(document.insert_service(service.clone()).is_ok()); + + // inserting a method with the same identifier as an existing service should fail + let method: VerificationMethod = MethodBuilder::default() + .type_(MethodType::Ed25519VerificationKey2018) + .data(MethodData::PublicKeyBase58( + "3M5RCDjPTWPkKSN3sxUmmMqHbmRPegYP1tjcKyrDbt9J".into(), + )) + .id(service.id().clone()) + .controller(document.id().clone()) + .build() + .unwrap(); + + let method_scopes = [ + MethodScope::VerificationMethod, + MethodScope::assertion_method(), + MethodScope::authentication(), + MethodScope::key_agreement(), + MethodScope::capability_delegation(), + MethodScope::capability_invocation(), + ]; + for scope in method_scopes { + let mut document_clone = document.clone(); + assert!(document_clone.insert_method(method.clone(), scope).is_err()); + // should succeed after removing the service + assert!(document_clone.remove_service(service.id()).is_ok()); + assert!(document_clone.insert_method(method.clone(), scope).is_ok()); + } + + // inserting a service with the same identifier as a method should fail + for scope in method_scopes { + let mut doc_clone = document.clone(); + let valid_method_id = document.id().to_url().join("#valid-method-identifier").unwrap(); + let mut valid_method = method.clone(); + valid_method.set_id(valid_method_id.clone()).unwrap(); + // make sure that the method actually gets inserted + assert!(doc_clone.insert_method(valid_method.clone(), scope).is_ok()); + let mut service_clone = service.clone(); + service_clone.set_id(valid_method_id).unwrap(); + assert!(doc_clone.insert_service(service_clone.clone()).is_err()); + // but should work after the method has been removed + assert!(doc_clone.remove_method(valid_method.id()).is_ok()); + assert!(doc_clone.insert_service(service_clone).is_ok()); + } + + //removing a service that does not exist should fail + assert!(document + .remove_service(&service.id().join("#service-does-not-exist").unwrap()) + .is_err()); + } + #[test] fn serialize_deserialize_roundtrip() { let document: CoreDocument = document(); diff --git a/identity_did/src/error.rs b/identity_did/src/error.rs index 076f199621..46cc0dcd93 100644 --- a/identity_did/src/error.rs +++ b/identity_did/src/error.rs @@ -33,14 +33,23 @@ pub enum Error { MissingIdFragment, #[error("Invalid Verification Method Type")] InvalidMethodType, - /// Caused by attempting to add a verification method to a document, where a method with the same fragment already - /// exists. - #[error("verification method already exists")] - MethodAlreadyExists, + /// Caused by attempting to add a verification method to a document, where a method or service with the same fragment + /// already exists. + #[error("unable to insert method: the id is already in use")] + MethodInsertionError, /// Caused by attempting to attach or detach a relationship on an embedded method. #[error("unable to modify relationships on embedded methods, use insert or remove instead")] InvalidMethodEmbedded, + /// Caused by attempting to insert a service whose id overlaps with a (verification) method or an already existing + /// service. + #[error("unable to insert service: the id is already in use")] + InvalidServiceInsertion, + + /// Caused by attempting to remove a service that cannot be found in the document. + #[error("service not found")] + ServiceNotFound, + #[error("Unknown Method Scope")] UnknownMethodScope, #[error("Unknown Method Type")] From 3c56b233bdf1be844af25072b0807e71f29597c0 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 1 Nov 2022 16:13:44 +0100 Subject: [PATCH 09/18] renamed inner --- identity_did/src/diff/diff_document.rs | 34 ++--- identity_did/src/document/core_document.rs | 154 ++++++++++----------- 2 files changed, 93 insertions(+), 95 deletions(-) diff --git a/identity_did/src/diff/diff_document.rs b/identity_did/src/diff/diff_document.rs index fdaa1cf29a..a8091db350 100644 --- a/identity_did/src/diff/diff_document.rs +++ b/identity_did/src/diff/diff_document.rs @@ -202,7 +202,7 @@ where .unwrap_or_else(|| self.properties().clone()); Ok(CoreDocument { - inner: CoreDocumentData { + data: CoreDocumentData { id, controller, also_known_as, @@ -285,7 +285,7 @@ where let properties: T = diff.properties.map(T::from_diff).transpose()?.unwrap_or_default(); Ok(CoreDocument { - inner: CoreDocumentData { + data: CoreDocumentData { id, controller, also_known_as, @@ -302,7 +302,7 @@ where } fn into_diff(self) -> Result { - let inner = self.inner; + let inner = self.data; Ok(DiffDocument { id: Some(inner.id.into_diff()?), @@ -462,7 +462,7 @@ mod test { let mut new = doc.clone(); // add new method - assert!(new.verification_method_mut().append(method(&doc.inner.id, "#key-diff"))); + assert!(new.verification_method_mut().append(method(&doc.data.id, "#key-diff"))); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -505,7 +505,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); assert!(new.authentication_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -519,7 +519,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.authentication().first().unwrap().clone(); new.authentication_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -548,7 +548,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); assert!(new.assertion_method_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -562,7 +562,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.assertion_method().first().unwrap().clone(); new.assertion_method_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -591,7 +591,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); assert!(new.key_agreement_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -605,7 +605,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.key_agreement().first().unwrap().clone(); new.key_agreement_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -634,7 +634,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); assert!(new.capability_delegation_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -648,7 +648,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.capability_delegation().first().unwrap().clone(); new.capability_delegation_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -677,7 +677,7 @@ mod test { let mut new = doc.clone(); // add new method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); assert!(new.capability_invocation_mut().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -691,7 +691,7 @@ mod test { let mut new = doc.clone(); // update method - let method_ref: MethodRef = method(&doc.inner.id, "#key-diff").into(); + let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.capability_invocation().first().unwrap().clone(); new.capability_invocation_mut().replace(&first, method_ref); assert_ne!(doc, new); @@ -720,7 +720,7 @@ mod test { let mut new = doc.clone(); // Add new service - let service = service(doc.inner.id.to_url().join("#key-diff").unwrap()); + let service = service(doc.data.id.to_url().join("#key-diff").unwrap()); assert!(new.service_mut().append(service)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -734,7 +734,7 @@ mod test { let mut new = doc.clone(); // add new service - let service = service(doc.inner.id.to_url().join("#key-diff").unwrap()); + let service = service(doc.data.id.to_url().join("#key-diff").unwrap()); let first = new.service().first().unwrap().clone(); new.service_mut().replace(&first, service); assert_ne!(doc, new); @@ -813,7 +813,7 @@ mod test { let method_ref: MethodRef = MethodBuilder::default() .id(first) - .controller(new.inner.id.clone()) + .controller(new.data.id.clone()) .type_(MethodType::Ed25519VerificationKey2018) .data(MethodData::new_multibase(b"key_material")) .build() diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index f204aaa4c4..8cad45b9ec 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -152,7 +152,7 @@ pub struct CoreDocument where D: DID + KeyComparable { - pub(crate) inner: CoreDocumentData, + pub(crate) data: CoreDocumentData, } //Forward serialization to inner @@ -167,16 +167,16 @@ where where S: Serializer, { - self.inner.serialize(serializer) + self.data.serialize(serializer) } } // Workaround for lifetime issues with a mutable reference to self preventing closures from being used. macro_rules! method_ref_mut_helper { ($doc:ident, $method: ident, $query: ident) => { - match $doc.inner.$method.query_mut($query.into())? { + match $doc.data.$method.query_mut($query.into())? { MethodRef::Embed(method) => Some(method), - MethodRef::Refer(ref did) => $doc.inner.verification_method.query_mut(did), + MethodRef::Refer(ref did) => $doc.data.verification_method.query_mut(did), } }; } @@ -239,112 +239,112 @@ where /// Returns a reference to the `CoreDocument` id. pub fn id(&self) -> &D { - &self.inner.id + &self.data.id } /// Returns a mutable reference to the `CoreDocument` id. pub fn id_mut(&mut self) -> &mut D { - &mut self.inner.id + &mut self.data.id } /// Returns a reference to the `CoreDocument` controller. pub fn controller(&self) -> Option<&OneOrSet> { - self.inner.controller.as_ref() + self.data.controller.as_ref() } /// Returns a mutable reference to the `CoreDocument` controller. pub fn controller_mut(&mut self) -> &mut Option> { - &mut self.inner.controller + &mut self.data.controller } /// Returns a reference to the `CoreDocument` alsoKnownAs set. pub fn also_known_as(&self) -> &OrderedSet { - &self.inner.also_known_as + &self.data.also_known_as } /// Returns a mutable reference to the `CoreDocument` alsoKnownAs set. pub fn also_known_as_mut(&mut self) -> &mut OrderedSet { - &mut self.inner.also_known_as + &mut self.data.also_known_as } /// Returns a reference to the `CoreDocument` verificationMethod set. pub fn verification_method(&self) -> &OrderedSet> { - &self.inner.verification_method + &self.data.verification_method } /// Returns a mutable reference to the `CoreDocument` verificationMethod set. pub fn verification_method_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.verification_method + &mut self.data.verification_method } /// Returns a reference to the `CoreDocument` authentication set. pub fn authentication(&self) -> &OrderedSet> { - &self.inner.authentication + &self.data.authentication } /// Returns a mutable reference to the `CoreDocument` authentication set. pub fn authentication_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.authentication + &mut self.data.authentication } /// Returns a reference to the `CoreDocument` assertionMethod set. pub fn assertion_method(&self) -> &OrderedSet> { - &self.inner.assertion_method + &self.data.assertion_method } /// Returns a mutable reference to the `CoreDocument` assertionMethod set. pub fn assertion_method_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.assertion_method + &mut self.data.assertion_method } /// Returns a reference to the `CoreDocument` keyAgreement set. pub fn key_agreement(&self) -> &OrderedSet> { - &self.inner.key_agreement + &self.data.key_agreement } /// Returns a mutable reference to the `CoreDocument` keyAgreement set. pub fn key_agreement_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.key_agreement + &mut self.data.key_agreement } /// Returns a reference to the `CoreDocument` capabilityDelegation set. pub fn capability_delegation(&self) -> &OrderedSet> { - &self.inner.capability_delegation + &self.data.capability_delegation } /// Returns a mutable reference to the `CoreDocument` capabilityDelegation set. pub fn capability_delegation_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.capability_delegation + &mut self.data.capability_delegation } /// Returns a reference to the `CoreDocument` capabilityInvocation set. pub fn capability_invocation(&self) -> &OrderedSet> { - &self.inner.capability_invocation + &self.data.capability_invocation } /// Returns a mutable reference to the `CoreDocument` capabilityInvocation set. pub fn capability_invocation_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.capability_invocation + &mut self.data.capability_invocation } /// Returns a reference to the `CoreDocument` service set. pub fn service(&self) -> &OrderedSet> { - &self.inner.service + &self.data.service } /// Returns a mutable reference to the `CoreDocument` service set. pub fn service_mut(&mut self) -> &mut OrderedSet> { - &mut self.inner.service + &mut self.data.service } /// Returns a reference to the custom `CoreDocument` properties. pub fn properties(&self) -> &T { - &self.inner.properties + &self.data.properties } /// Returns a mutable reference to the custom `CoreDocument` properties. pub fn properties_mut(&mut self) -> &mut T { - &mut self.inner.properties + &mut self.data.properties } /// Maps `CoreDocument` to `CoreDocument` by applying a function `f` to all [`DID`] fields @@ -359,7 +359,7 @@ where F: FnMut(D) -> C, G: FnOnce(T) -> S, { - let current_inner = self.inner; + let current_inner = self.data; CoreDocument::try_from(CoreDocumentData { id: f(current_inner.id), controller: current_inner @@ -419,7 +419,7 @@ where F: FnMut(D) -> Result, G: FnOnce(T) -> Result, { - let current_inner = self.inner; + let current_inner = self.data; let helper = || -> Result, E> { Ok(CoreDocumentData { id: f(current_inner.id)?, @@ -480,21 +480,21 @@ where return Err(Error::MethodInsertionError); } match scope { - MethodScope::VerificationMethod => self.inner.verification_method.append(method), + MethodScope::VerificationMethod => self.data.verification_method.append(method), MethodScope::VerificationRelationship(MethodRelationship::Authentication) => { - self.inner.authentication.append(MethodRef::Embed(method)) + self.data.authentication.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::AssertionMethod) => { - self.inner.assertion_method.append(MethodRef::Embed(method)) + self.data.assertion_method.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::KeyAgreement) => { - self.inner.key_agreement.append(MethodRef::Embed(method)) + self.data.key_agreement.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::CapabilityDelegation) => { - self.inner.capability_delegation.append(MethodRef::Embed(method)) + self.data.capability_delegation.append(MethodRef::Embed(method)) } MethodScope::VerificationRelationship(MethodRelationship::CapabilityInvocation) => { - self.inner.capability_invocation.append(MethodRef::Embed(method)) + self.data.capability_invocation.append(MethodRef::Embed(method)) } }; @@ -508,12 +508,12 @@ where /// Returns an error if the method does not exist. pub fn remove_method(&mut self, did: &DIDUrl) -> Result<()> { let was_removed: bool = [ - self.inner.authentication.remove(did), - self.inner.assertion_method.remove(did), - self.inner.key_agreement.remove(did), - self.inner.capability_delegation.remove(did), - self.inner.capability_invocation.remove(did), - self.inner.verification_method.remove(did), + self.data.authentication.remove(did), + self.data.assertion_method.remove(did), + self.data.key_agreement.remove(did), + self.data.capability_delegation.remove(did), + self.data.capability_invocation.remove(did), + self.data.verification_method.remove(did), ] .contains(&true); @@ -684,14 +684,14 @@ where } self - .inner + .data .verification_method .iter() - .chain(self.inner.authentication.iter().filter_map(__filter_ref)) - .chain(self.inner.assertion_method.iter().filter_map(__filter_ref)) - .chain(self.inner.key_agreement.iter().filter_map(__filter_ref)) - .chain(self.inner.capability_delegation.iter().filter_map(__filter_ref)) - .chain(self.inner.capability_invocation.iter().filter_map(__filter_ref)) + .chain(self.data.authentication.iter().filter_map(__filter_ref)) + .chain(self.data.assertion_method.iter().filter_map(__filter_ref)) + .chain(self.data.key_agreement.iter().filter_map(__filter_ref)) + .chain(self.data.capability_delegation.iter().filter_map(__filter_ref)) + .chain(self.data.capability_invocation.iter().filter_map(__filter_ref)) } /// Returns an iterator over all verification relationships. @@ -699,13 +699,13 @@ where /// This includes embedded and referenced [`VerificationMethods`](VerificationMethod). pub fn verification_relationships(&self) -> impl Iterator> { self - .inner + .data .authentication .iter() - .chain(self.inner.assertion_method.iter()) - .chain(self.inner.key_agreement.iter()) - .chain(self.inner.capability_delegation.iter()) - .chain(self.inner.capability_invocation.iter()) + .chain(self.data.assertion_method.iter()) + .chain(self.data.key_agreement.iter()) + .chain(self.data.capability_delegation.iter()) + .chain(self.data.capability_invocation.iter()) } /// Returns the first [`VerificationMethod`] with an `id` property matching the @@ -723,29 +723,27 @@ where let resolve_ref_helper = |method_ref: &'me MethodRef| self.resolve_method_ref(method_ref); match scope { - MethodScope::VerificationMethod => self.inner.verification_method.query(query.into()), + MethodScope::VerificationMethod => self.data.verification_method.query(query.into()), MethodScope::VerificationRelationship(MethodRelationship::Authentication) => self - .inner + .data .authentication .query(query.into()) .and_then(resolve_ref_helper), MethodScope::VerificationRelationship(MethodRelationship::AssertionMethod) => self - .inner + .data .assertion_method .query(query.into()) .and_then(resolve_ref_helper), - MethodScope::VerificationRelationship(MethodRelationship::KeyAgreement) => self - .inner - .key_agreement - .query(query.into()) - .and_then(resolve_ref_helper), + MethodScope::VerificationRelationship(MethodRelationship::KeyAgreement) => { + self.data.key_agreement.query(query.into()).and_then(resolve_ref_helper) + } MethodScope::VerificationRelationship(MethodRelationship::CapabilityDelegation) => self - .inner + .data .capability_delegation .query(query.into()) .and_then(resolve_ref_helper), MethodScope::VerificationRelationship(MethodRelationship::CapabilityInvocation) => self - .inner + .data .capability_invocation .query(query.into()) .and_then(resolve_ref_helper), @@ -767,7 +765,7 @@ where { match scope { Some(scope) => match scope { - MethodScope::VerificationMethod => self.inner.verification_method.query_mut(query.into()), + MethodScope::VerificationMethod => self.data.verification_method.query_mut(query.into()), MethodScope::VerificationRelationship(MethodRelationship::Authentication) => { method_ref_mut_helper!(self, authentication, query) } @@ -792,7 +790,7 @@ where pub fn resolve_method_ref<'a>(&'a self, method_ref: &'a MethodRef) -> Option<&'a VerificationMethod> { match method_ref { MethodRef::Embed(method) => Some(method), - MethodRef::Refer(did) => self.inner.verification_method.query(did), + MethodRef::Refer(did) => self.data.verification_method.query(did), } } @@ -800,29 +798,29 @@ where let mut method: Option<&MethodRef> = None; if method.is_none() { - method = self.inner.authentication.query(query.clone()); + method = self.data.authentication.query(query.clone()); } if method.is_none() { - method = self.inner.assertion_method.query(query.clone()); + method = self.data.assertion_method.query(query.clone()); } if method.is_none() { - method = self.inner.key_agreement.query(query.clone()); + method = self.data.key_agreement.query(query.clone()); } if method.is_none() { - method = self.inner.capability_delegation.query(query.clone()); + method = self.data.capability_delegation.query(query.clone()); } if method.is_none() { - method = self.inner.capability_invocation.query(query.clone()); + method = self.data.capability_invocation.query(query.clone()); } match method { Some(MethodRef::Embed(method)) => Some(method), - Some(MethodRef::Refer(did)) => self.inner.verification_method.query(&did.to_string()), - None => self.inner.verification_method.query(query), + Some(MethodRef::Refer(did)) => self.data.verification_method.query(&did.to_string()), + None => self.data.verification_method.query(query), } } @@ -830,29 +828,29 @@ where let mut method: Option<&mut MethodRef> = None; if method.is_none() { - method = self.inner.authentication.query_mut(query.clone()); + method = self.data.authentication.query_mut(query.clone()); } if method.is_none() { - method = self.inner.assertion_method.query_mut(query.clone()); + method = self.data.assertion_method.query_mut(query.clone()); } if method.is_none() { - method = self.inner.key_agreement.query_mut(query.clone()); + method = self.data.key_agreement.query_mut(query.clone()); } if method.is_none() { - method = self.inner.capability_delegation.query_mut(query.clone()); + method = self.data.capability_delegation.query_mut(query.clone()); } if method.is_none() { - method = self.inner.capability_invocation.query_mut(query.clone()); + method = self.data.capability_invocation.query_mut(query.clone()); } match method { Some(MethodRef::Embed(method)) => Some(method), - Some(MethodRef::Refer(did)) => self.inner.verification_method.query_mut(&did.to_string()), - None => self.inner.verification_method.query_mut(query), + Some(MethodRef::Refer(did)) => self.data.verification_method.query_mut(&did.to_string()), + None => self.data.verification_method.query_mut(query), } } } @@ -996,7 +994,7 @@ where type Error = crate::Error; fn try_from(value: CoreDocumentData) -> Result { match value.check_id_constraints() { - Ok(_) => Ok(Self { inner: value }), + Ok(_) => Ok(Self { data: value }), Err(err) => Err(err), } } From f1c0f7dad86c75d912d675fd44fe8d4e89e1a4db Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 1 Nov 2022 16:28:39 +0100 Subject: [PATCH 10/18] clippy lints --- identity_core/src/common/one_or_many.rs | 6 +++--- identity_core/src/common/one_or_set.rs | 2 +- identity_core/src/common/url.rs | 4 ++-- identity_core/src/crypto/proof/proof.rs | 6 +++--- identity_did/src/did/did.rs | 4 ++-- identity_did/src/did/did_url.rs | 4 ++-- identity_did/src/document/core_document.rs | 8 ++++++-- identity_did/src/revocation/bitmap.rs | 2 +- 8 files changed, 20 insertions(+), 16 deletions(-) diff --git a/identity_core/src/common/one_or_many.rs b/identity_core/src/common/one_or_many.rs index a70c4f075d..21f30c0226 100644 --- a/identity_core/src/common/one_or_many.rs +++ b/identity_core/src/common/one_or_many.rs @@ -93,7 +93,7 @@ impl OneOrMany { /// Returns a reference to the contents as a slice. pub fn as_slice(&self) -> &[T] { - &**self + self } /// Consumes the [`OneOrMany`] and returns the contents as a [`Vec`]. @@ -123,14 +123,14 @@ impl Deref for OneOrMany { fn deref(&self) -> &Self::Target { match self { Self::One(inner) => from_ref(inner), - Self::Many(inner) => &**inner, + Self::Many(inner) => inner, } } } impl AsRef<[T]> for OneOrMany { fn as_ref(&self) -> &[T] { - &**self + self } } diff --git a/identity_core/src/common/one_or_set.rs b/identity_core/src/common/one_or_set.rs index af3b012b1c..21021603e2 100644 --- a/identity_core/src/common/one_or_set.rs +++ b/identity_core/src/common/one_or_set.rs @@ -186,7 +186,7 @@ where /// Returns a reference to the contents as a slice. pub fn as_slice(&self) -> &[T] { - &**self + self } /// Consumes the [`OneOrSet`] and returns the contents as a [`Vec`]. diff --git a/identity_core/src/common/url.rs b/identity_core/src/common/url.rs index 094f6bd6b4..fa42ed10bd 100644 --- a/identity_core/src/common/url.rs +++ b/identity_core/src/common/url.rs @@ -102,11 +102,11 @@ impl Diff for Url { self .to_string() .merge(diff) - .and_then(|this| Self::parse(&this).map_err(diff::Error::merge)) + .and_then(|this| Self::parse(this).map_err(diff::Error::merge)) } fn from_diff(diff: Self::Type) -> diff::Result { - String::from_diff(diff).and_then(|this| Self::parse(&this).map_err(diff::Error::convert)) + String::from_diff(diff).and_then(|this| Self::parse(this).map_err(diff::Error::convert)) } fn into_diff(self) -> diff::Result { diff --git a/identity_core/src/crypto/proof/proof.rs b/identity_core/src/crypto/proof/proof.rs index f65f42fd17..baf8550607 100644 --- a/identity_core/src/crypto/proof/proof.rs +++ b/identity_core/src/crypto/proof/proof.rs @@ -75,12 +75,12 @@ impl Proof { /// Returns the `type` property of the proof. pub fn type_(&self) -> &str { - &*self.type_ + &self.type_ } /// Returns the identifier of the DID method used to create this proof. pub fn verification_method(&self) -> &str { - &*self.method + &self.method } /// Returns a reference to the proof `value`. @@ -145,7 +145,7 @@ impl Serialize for Proof { } else { 3 // type + method + value }; - count_fields += if self.created.is_some() { 1 } else { 0 }; + count_fields += usize::from(self.created.is_some()); count_fields += if self.expires.is_some() { 1 } else { 0 }; count_fields += if self.challenge.is_some() { 1 } else { 0 }; count_fields += if self.domain.is_some() { 1 } else { 0 }; diff --git a/identity_did/src/did/did.rs b/identity_did/src/did/did.rs index 021db62863..2954e78336 100644 --- a/identity_did/src/did/did.rs +++ b/identity_did/src/did/did.rs @@ -253,11 +253,11 @@ impl Diff for CoreDID { self .to_string() .merge(diff) - .and_then(|this| Self::parse(&this).map_err(identity_core::diff::Error::merge)) + .and_then(|this| Self::parse(this).map_err(identity_core::diff::Error::merge)) } fn from_diff(diff: Self::Type) -> identity_core::diff::Result { - String::from_diff(diff).and_then(|this| Self::parse(&this).map_err(identity_core::diff::Error::convert)) + String::from_diff(diff).and_then(|this| Self::parse(this).map_err(identity_core::diff::Error::convert)) } fn into_diff(self) -> identity_core::diff::Result { diff --git a/identity_did/src/did/did_url.rs b/identity_did/src/did/did_url.rs index 61e0067b1d..f8a2242a88 100644 --- a/identity_did/src/did/did_url.rs +++ b/identity_did/src/did/did_url.rs @@ -596,11 +596,11 @@ where self .to_string() .merge(diff) - .and_then(|this| Self::parse(&this).map_err(identity_core::diff::Error::merge)) + .and_then(|this| Self::parse(this).map_err(identity_core::diff::Error::merge)) } fn from_diff(diff: Self::Type) -> identity_core::diff::Result { - String::from_diff(diff).and_then(|this| Self::parse(&this).map_err(identity_core::diff::Error::convert)) + String::from_diff(diff).and_then(|this| Self::parse(this).map_err(identity_core::diff::Error::convert)) } fn into_diff(self) -> identity_core::diff::Result { diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index 8cad45b9ec..da19ad71dc 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -121,7 +121,11 @@ impl CoreDocumentData { } for method_id in self.verification_method.iter().map(|method| method.id()) { - if let Some(_) = method_identifiers.insert(method_id, false).filter(|value| *value) { + if method_identifiers + .insert(method_id, false) + .filter(|value| *value) + .is_some() + { return Err(Error::InvalidDocument( "attempted to construct document with a duplicated embedded method", None, @@ -466,7 +470,7 @@ where properties: g(current_inner.properties)?, }) }; - helper().map(|data| CoreDocument::try_from(data)) + helper().map(CoreDocument::try_from) } /// Adds a new [`VerificationMethod`] to the document in the given [`MethodScope`]. diff --git a/identity_did/src/revocation/bitmap.rs b/identity_did/src/revocation/bitmap.rs index eab516955a..984274de63 100644 --- a/identity_did/src/revocation/bitmap.rs +++ b/identity_did/src/revocation/bitmap.rs @@ -109,7 +109,7 @@ impl RevocationBitmap { /// Serializes and compressess [`RevocationBitmap`] as a base64-encoded `String`. pub(crate) fn serialize_compressed_base64(&self) -> Result { let serialized_data: Vec = self.serialize_vec()?; - Self::compress_zlib(&serialized_data).map(|data| BaseEncoding::encode(&data, Base::Base64Url)) + Self::compress_zlib(serialized_data).map(|data| BaseEncoding::encode(&data, Base::Base64Url)) } /// Deserializes [`RevocationBitmap`] from a slice of bytes. From 32470e6c0360aa7adab8f4cf772e84d9119452df Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 1 Nov 2022 16:31:26 +0100 Subject: [PATCH 11/18] fixed clippy lint --- identity_core/src/crypto/proof/proof.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/identity_core/src/crypto/proof/proof.rs b/identity_core/src/crypto/proof/proof.rs index baf8550607..69f6d0ddfc 100644 --- a/identity_core/src/crypto/proof/proof.rs +++ b/identity_core/src/crypto/proof/proof.rs @@ -146,10 +146,10 @@ impl Serialize for Proof { 3 // type + method + value }; count_fields += usize::from(self.created.is_some()); - count_fields += if self.expires.is_some() { 1 } else { 0 }; - count_fields += if self.challenge.is_some() { 1 } else { 0 }; - count_fields += if self.domain.is_some() { 1 } else { 0 }; - count_fields += if self.purpose.is_some() { 1 } else { 0 }; + count_fields += usize::from(self.expires.is_some()); + count_fields += usize::from(self.challenge.is_some()); + count_fields += usize::from(self.domain.is_some()); + count_fields += usize::from(self.purpose.is_some()); let mut state: S::SerializeMap = serializer.serialize_map(Some(count_fields))?; state.serialize_entry("type", &self.type_)?; From 6ea02996d6bb1da167282a6f6d367d69f135883a Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 1 Nov 2022 16:33:13 +0100 Subject: [PATCH 12/18] clippy lints --- identity_core/src/crypto/proof/proof_value.rs | 6 +++--- identity_core/src/utils/base_encoding.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/identity_core/src/crypto/proof/proof_value.rs b/identity_core/src/crypto/proof/proof_value.rs index ce329ca424..7e50e60d9b 100644 --- a/identity_core/src/crypto/proof/proof_value.rs +++ b/identity_core/src/crypto/proof/proof_value.rs @@ -51,9 +51,9 @@ impl ProofValue { pub fn as_str(&self) -> &str { match self { Self::None => "", - Self::Jws(inner) => &**inner, - Self::Proof(inner) => &**inner, - Self::Signature(inner) => &**inner, + Self::Jws(inner) => inner, + Self::Proof(inner) => inner, + Self::Signature(inner) => inner, } } diff --git a/identity_core/src/utils/base_encoding.rs b/identity_core/src/utils/base_encoding.rs index 469b755ca6..082bd92a2e 100644 --- a/identity_core/src/utils/base_encoding.rs +++ b/identity_core/src/utils/base_encoding.rs @@ -134,7 +134,7 @@ impl BaseEncoding { if data.as_ref().is_empty() { return Ok(Vec::new()); } - multibase::decode(&data) + multibase::decode(data) .map(|(_base, output)| output) .map_err(Error::DecodeMultibase) } From 48768cd158c4c1da2505ccc249c767ee8bab13c4 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 1 Nov 2022 17:37:58 +0100 Subject: [PATCH 13/18] fixed compilation error --- identity_did/src/document/core_document.rs | 10 ++++++---- identity_iota_core/src/error.rs | 2 +- identity_iota_core/src/state_metadata/document.rs | 9 +++++---- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index da19ad71dc..e4096a4be3 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -416,12 +416,14 @@ where /// /// `try_map` can fail if either of the provided functions fail or if the mapping `f` /// introduces methods referencing embedded method identifiers, or services with identifiers matching method - /// identifiers.. - pub fn try_map(self, mut f: F, g: G) -> Result, Error>, E> + /// identifiers. In the case of the latter the provided function `h` will be called to construct an error. + pub fn try_map(self, mut f: F, g: G, h: H) -> std::result::Result, E> where C: DID + KeyComparable, F: FnMut(D) -> Result, G: FnOnce(T) -> Result, + H: FnOnce(crate::error::Error) -> E, + E: std::error::Error, { let current_inner = self.data; let helper = || -> Result, E> { @@ -470,7 +472,7 @@ where properties: g(current_inner.properties)?, }) }; - helper().map(CoreDocument::try_from) + helper().and_then(|data| CoreDocument::try_from(data).map_err(h)) } /// Adds a new [`VerificationMethod`] to the document in the given [`MethodScope`]. @@ -995,7 +997,7 @@ impl TryFrom> for CoreDocument) -> Result { match value.check_id_constraints() { Ok(_) => Ok(Self { data: value }), diff --git a/identity_iota_core/src/error.rs b/identity_iota_core/src/error.rs index b125bad487..b3069018b2 100644 --- a/identity_iota_core/src/error.rs +++ b/identity_iota_core/src/error.rs @@ -11,7 +11,7 @@ pub enum Error { #[error("{0}")] DIDSyntaxError(#[from] identity_did::did::DIDError), #[error("{0}")] - InvalidDoc(#[from] identity_did::Error), + InvalidDoc(#[from] identity_did::error::Error), #[cfg(feature = "iota-client")] #[error("DID update: {0}")] DIDUpdateError(&'static str, #[source] Option), diff --git a/identity_iota_core/src/state_metadata/document.rs b/identity_iota_core/src/state_metadata/document.rs index fd463c1feb..94bcd57316 100644 --- a/identity_iota_core/src/state_metadata/document.rs +++ b/identity_iota_core/src/state_metadata/document.rs @@ -1,6 +1,7 @@ // Copyright 2020-2022 IOTA Stiftung // SPDX-License-Identifier: Apache-2.0 +use identity_core::common::Object; use identity_core::convert::FromJson; use identity_core::convert::ToJson; use identity_did::did::CoreDID; @@ -40,20 +41,20 @@ impl StateMetadataDocument { /// Transforms the document into a [`IotaDocument`] by replacing all placeholders with `original_did`. pub fn into_iota_document(self, original_did: &IotaDID) -> Result { let Self { document, metadata } = self; - let core_document_result: Result = document.try_map( + let core_document: IotaCoreDocument = document.try_map( // Replace placeholder identifiers. |did| { if did == PLACEHOLDER_DID.as_ref() { Ok(original_did.clone()) } else { // TODO: wrap error? - IotaDID::try_from_core(did) + IotaDID::try_from_core(did).map_err(crate::error::Error::DIDSyntaxError) } }, // Do not modify properties. - Ok, + Result::::Ok, + crate::error::Error::InvalidDoc, )?; - let core_document: IotaCoreDocument = core_document_result.map_err(|error| crate::Error::InvalidDoc(error))?; Ok(IotaDocument::from((core_document, metadata))) } From b5fdb0dbf70e27c2bdbbaec0b9363838f26637fb Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 8 Nov 2022 09:19:22 +0100 Subject: [PATCH 14/18] started refactoring OrderedSet::remove --- identity_core/src/common/ordered_set.rs | 27 ++++- .../src/validator/credential_validator.rs | 2 +- identity_did/src/diff/diff_document.rs | 60 +++++------ identity_did/src/document/core_document.rs | 102 +++++++++++------- .../src/document/iota_document.rs | 12 +-- .../src/state_metadata/document.rs | 8 +- 6 files changed, 131 insertions(+), 80 deletions(-) diff --git a/identity_core/src/common/ordered_set.rs b/identity_core/src/common/ordered_set.rs index 9cccca1288..776b8aeaee 100644 --- a/identity_core/src/common/ordered_set.rs +++ b/identity_core/src/common/ordered_set.rs @@ -174,19 +174,26 @@ impl OrderedSet { self.change(update, |item, update| item.key() == update.key()) } - /// Removes all matching items from the set. + /// Removes and returns the matching item from the set, if it exists. #[inline] - pub fn remove(&mut self, item: &U) -> bool + pub fn remove(&mut self, item: &U) -> Option where T: KeyComparable, U: KeyComparable, { + self.iter().enumerate().find(|(_, entry)| entry.key() == item.key()).map(|(idx,_)| idx).map(|idx| self.0.remove(idx)) + //Some(self.0.remove(idx)) + + + /* if self.contains(item) { self.0.retain(|this| this.borrow().key() != item.key()); true } else { false } + */ + } fn change(&mut self, data: T, f: F) -> bool @@ -312,6 +319,9 @@ where #[cfg(test)] mod tests { use super::*; + use proptest::prelude::Rng; +use proptest::strategy::Strategy; + use proptest::*; #[test] fn test_ordered_set_works() { @@ -456,4 +466,17 @@ mod tests { assert_eq!(set.head().unwrap().key, cs2.key); assert_eq!(set.head().unwrap().value, cs2.value); } + + + + // + fn set_with_elements() -> impl Strategy, T, T)> { + proptest::prelude::any::>().prop_map(|init|init.into_iter().collect::>()) + } + + #[test] + fn append_preserves_invariant() { + + } } + diff --git a/identity_credential/src/validator/credential_validator.rs b/identity_credential/src/validator/credential_validator.rs index c65b410119..1cce14f03d 100644 --- a/identity_credential/src/validator/credential_validator.rs +++ b/identity_credential/src/validator/credential_validator.rs @@ -774,7 +774,7 @@ mod tests { // Add a RevocationBitmap service to the issuer. let bitmap: RevocationBitmap = RevocationBitmap::new(); - assert!(issuer_doc.service_mut().append( + assert!(issuer_doc.service_mut_unchecked().append( Service::builder(Object::new()) .id(service_url.clone()) .type_(RevocationBitmap::TYPE) diff --git a/identity_did/src/diff/diff_document.rs b/identity_did/src/diff/diff_document.rs index a8091db350..d02c15f21a 100644 --- a/identity_did/src/diff/diff_document.rs +++ b/identity_did/src/diff/diff_document.rs @@ -393,7 +393,7 @@ mod test { let doc = document(); let mut new = doc.clone(); let new_did = "did:diff:1234"; - *new.id_mut() = new_did.parse().unwrap(); + *new.id_mut_unchecked() = new_did.parse().unwrap(); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -407,7 +407,7 @@ mod test { let doc: CoreDocument = document(); let mut new: CoreDocument = doc.clone(); let new_controller: CoreDID = "did:diff:1234".parse().unwrap(); - *new.controller_mut() = Some(OneOrSet::new_one(new_controller)); + *new.controller_mut_unchecked() = Some(OneOrSet::new_one(new_controller)); assert_ne!(doc, new); let diff: DiffDocument = doc.diff(&new).unwrap(); @@ -424,7 +424,7 @@ mod test { "did:diff:5678".parse().unwrap(), "did:diff:9012".parse().unwrap(), ]; - *new.controller_mut() = Some(new_controllers.try_into().unwrap()); + *new.controller_mut_unchecked() = Some(new_controllers.try_into().unwrap()); assert_ne!(doc, new); let diff: DiffDocument = doc.diff(&new).unwrap(); @@ -436,7 +436,7 @@ mod test { fn test_controller_unset() { let doc: CoreDocument = document(); let mut new: CoreDocument = doc.clone(); - *new.controller_mut() = None; + *new.controller_mut_unchecked() = None; assert_ne!(doc, new); let diff: DiffDocument = doc.diff(&new).unwrap(); @@ -448,7 +448,7 @@ mod test { fn test_also_known_as() { let doc = document(); let mut new = doc.clone(); - new.also_known_as_mut().append("diff:diff:1234".parse().unwrap()); + new.also_known_as_mut_unchecked().append("diff:diff:1234".parse().unwrap()); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -462,7 +462,7 @@ mod test { let mut new = doc.clone(); // add new method - assert!(new.verification_method_mut().append(method(&doc.data.id, "#key-diff"))); + assert!(new.verification_method_mut_unchecked().append(method(&doc.data.id, "#key-diff"))); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -477,7 +477,7 @@ mod test { // update method let first = new.verification_method().first().unwrap().clone(); new - .verification_method_mut() + .verification_method_mut_unchecked() .replace(&first, method(&"did:diff:1234".parse().unwrap(), "#key-diff")); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -492,7 +492,7 @@ mod test { // remove method let first = new.verification_method().first().unwrap().clone(); - new.verification_method_mut().remove(&first); + new.verification_method_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -506,7 +506,7 @@ mod test { // add new method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); - assert!(new.authentication_mut().append(method_ref)); + assert!(new.authentication_mut_unchecked().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -521,7 +521,7 @@ mod test { // update method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.authentication().first().unwrap().clone(); - new.authentication_mut().replace(&first, method_ref); + new.authentication_mut_unchecked().replace(&first, method_ref); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -535,7 +535,7 @@ mod test { // remove method let first = new.authentication().first().unwrap().clone(); - new.authentication_mut().remove(&first); + new.authentication_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -549,7 +549,7 @@ mod test { // add new method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); - assert!(new.assertion_method_mut().append(method_ref)); + assert!(new.assertion_method_mut_unchecked().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -564,7 +564,7 @@ mod test { // update method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.assertion_method().first().unwrap().clone(); - new.assertion_method_mut().replace(&first, method_ref); + new.assertion_method_mut_unchecked().replace(&first, method_ref); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -578,7 +578,7 @@ mod test { // remove method let first = new.assertion_method().first().unwrap().clone(); - new.assertion_method_mut().remove(&first); + new.assertion_method_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -592,7 +592,7 @@ mod test { // add new method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); - assert!(new.key_agreement_mut().append(method_ref)); + assert!(new.key_agreement_mut_unchecked().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -607,7 +607,7 @@ mod test { // update method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.key_agreement().first().unwrap().clone(); - new.key_agreement_mut().replace(&first, method_ref); + new.key_agreement_mut_unchecked().replace(&first, method_ref); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -621,7 +621,7 @@ mod test { // remove method let first = new.key_agreement().first().unwrap().clone(); - new.key_agreement_mut().remove(&first); + new.key_agreement_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -635,7 +635,7 @@ mod test { // add new method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); - assert!(new.capability_delegation_mut().append(method_ref)); + assert!(new.capability_delegation_mut_unchecked().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -650,7 +650,7 @@ mod test { // update method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.capability_delegation().first().unwrap().clone(); - new.capability_delegation_mut().replace(&first, method_ref); + new.capability_delegation_mut_unchecked().replace(&first, method_ref); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -664,7 +664,7 @@ mod test { // remove method let first = new.capability_delegation().first().unwrap().clone(); - new.capability_delegation_mut().remove(&first); + new.capability_delegation_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -678,7 +678,7 @@ mod test { // add new method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); - assert!(new.capability_invocation_mut().append(method_ref)); + assert!(new.capability_invocation_mut_unchecked().append(method_ref)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -693,7 +693,7 @@ mod test { // update method let method_ref: MethodRef = method(&doc.data.id, "#key-diff").into(); let first = new.capability_invocation().first().unwrap().clone(); - new.capability_invocation_mut().replace(&first, method_ref); + new.capability_invocation_mut_unchecked().replace(&first, method_ref); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -707,7 +707,7 @@ mod test { // remove method let first = new.capability_invocation().first().unwrap().clone(); - new.capability_invocation_mut().remove(&first); + new.capability_invocation_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -721,7 +721,7 @@ mod test { // Add new service let service = service(doc.data.id.to_url().join("#key-diff").unwrap()); - assert!(new.service_mut().append(service)); + assert!(new.service_mut_unchecked().append(service)); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -736,7 +736,7 @@ mod test { // add new service let service = service(doc.data.id.to_url().join("#key-diff").unwrap()); let first = new.service().first().unwrap().clone(); - new.service_mut().replace(&first, service); + new.service_mut_unchecked().replace(&first, service); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -750,7 +750,7 @@ mod test { // remove method let first = new.service().first().unwrap().clone(); - new.service_mut().remove(&first); + new.service_mut_unchecked().remove(&first); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); @@ -763,7 +763,7 @@ mod test { let mut new = doc.clone(); // update properties - *new.properties_mut() = BTreeMap::default(); + *new.properties_mut_unchecked() = BTreeMap::default(); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -778,7 +778,7 @@ mod test { // update properties assert!(new - .properties_mut() + .properties_mut_unchecked() .insert("key2".to_string(), "value2".into()) .is_none()); @@ -809,7 +809,7 @@ mod test { let mut new = doc.clone(); let first: CoreDIDUrl = new.capability_invocation().first().unwrap().as_ref().clone(); - new.capability_invocation_mut().remove(&first); + new.capability_invocation_mut_unchecked().remove(&first); let method_ref: MethodRef = MethodBuilder::default() .id(first) @@ -820,7 +820,7 @@ mod test { .unwrap() .into(); - assert!(new.capability_invocation_mut().append(method_ref)); + assert!(new.capability_invocation_mut_unchecked().append(method_ref)); assert_ne!(doc, new); diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index e4096a4be3..ebc4ec162e 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -247,7 +247,10 @@ where } /// Returns a mutable reference to the `CoreDocument` id. - pub fn id_mut(&mut self) -> &mut D { + /// + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn id_mut_unchecked(&mut self) -> &mut D { &mut self.data.id } @@ -257,7 +260,10 @@ where } /// Returns a mutable reference to the `CoreDocument` controller. - pub fn controller_mut(&mut self) -> &mut Option> { + /// + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn controller_mut_unchecked(&mut self) -> &mut Option> { &mut self.data.controller } @@ -267,7 +273,9 @@ where } /// Returns a mutable reference to the `CoreDocument` alsoKnownAs set. - pub fn also_known_as_mut(&mut self) -> &mut OrderedSet { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn also_known_as_mut_unchecked(&mut self) -> &mut OrderedSet { &mut self.data.also_known_as } @@ -277,7 +285,9 @@ where } /// Returns a mutable reference to the `CoreDocument` verificationMethod set. - pub fn verification_method_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn verification_method_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.verification_method } @@ -287,7 +297,9 @@ where } /// Returns a mutable reference to the `CoreDocument` authentication set. - pub fn authentication_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn authentication_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.authentication } @@ -297,7 +309,9 @@ where } /// Returns a mutable reference to the `CoreDocument` assertionMethod set. - pub fn assertion_method_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn assertion_method_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.assertion_method } @@ -307,7 +321,9 @@ where } /// Returns a mutable reference to the `CoreDocument` keyAgreement set. - pub fn key_agreement_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn key_agreement_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.key_agreement } @@ -317,7 +333,9 @@ where } /// Returns a mutable reference to the `CoreDocument` capabilityDelegation set. - pub fn capability_delegation_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn capability_delegation_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.capability_delegation } @@ -327,7 +345,9 @@ where } /// Returns a mutable reference to the `CoreDocument` capabilityInvocation set. - pub fn capability_invocation_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn capability_invocation_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.capability_invocation } @@ -337,7 +357,10 @@ where } /// Returns a mutable reference to the `CoreDocument` service set. - pub fn service_mut(&mut self) -> &mut OrderedSet> { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. Consider using the safer [`Self::insert_service`](CoreDocument::insert_service())/ [`Self::remove_service`](CoreDocument::remove_service) + /// API(s) instead. + pub fn service_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.service } @@ -347,24 +370,26 @@ where } /// Returns a mutable reference to the custom `CoreDocument` properties. - pub fn properties_mut(&mut self) -> &mut T { + /// # Warning + /// Incorrect use of this method can lead to broken invariants. + pub fn properties_mut_unchecked(&mut self) -> &mut T { &mut self.data.properties } /// Maps `CoreDocument` to `CoreDocument` by applying a function `f` to all [`DID`] fields /// and another function `g` to the custom properties. /// - /// # Panics - /// Panics if the mapping `f` introduces methods referencing embedded method identifiers, - /// or services with identifiers matching method identifiers. - pub fn map(self, mut f: F, g: G) -> CoreDocument + /// # Warning + /// Can lead to broken invariants if used incorrectly. See [`Self::try_map`](CoreDocument::try_map()) for a fallible version with additional built-in checks. + pub fn map_unchecked(self, mut f: F, g: G) -> CoreDocument where C: DID + KeyComparable, F: FnMut(D) -> C, G: FnOnce(T) -> S, { let current_inner = self.data; - CoreDocument::try_from(CoreDocumentData { + CoreDocument::{ + data: CoreDocumentData { id: f(current_inner.id), controller: current_inner .controller @@ -406,8 +431,8 @@ where .map(|service| service.map(&mut f)) .collect(), properties: g(current_inner.properties), - }) - .unwrap() + } + } } /// Fallible version of [`CoreDocument::map`]. @@ -416,7 +441,7 @@ where /// /// `try_map` can fail if either of the provided functions fail or if the mapping `f` /// introduces methods referencing embedded method identifiers, or services with identifiers matching method - /// identifiers. In the case of the latter the provided function `h` will be called to construct an error. + /// identifiers. In the case of the latter the provided function `h` will be called to construct a compatible error. pub fn try_map(self, mut f: F, g: G, h: H) -> std::result::Result, E> where C: DID + KeyComparable, @@ -543,7 +568,7 @@ where .chain(self.verification_method().iter().map(|method| method.id())) .any(|id| id == service_id); - ((!id_shared_with_method) && self.service_mut().append(service)) + ((!id_shared_with_method) && self.service_mut_unchecked().append(service)) .then_some(()) .ok_or(Error::InvalidServiceInsertion) } @@ -555,7 +580,7 @@ where /// Returns an error if the service does not exist. pub fn remove_service(&mut self, id: &DIDUrl) -> Result<()> { self - .service_mut() + .service_mut_unchecked() .remove(id) .then_some(()) .ok_or(Error::ServiceNotFound) @@ -586,11 +611,11 @@ where let method_ref = MethodRef::Refer(method.id().clone()); let was_attached = match relationship { - MethodRelationship::Authentication => self.authentication_mut().append(method_ref), - MethodRelationship::AssertionMethod => self.assertion_method_mut().append(method_ref), - MethodRelationship::KeyAgreement => self.key_agreement_mut().append(method_ref), - MethodRelationship::CapabilityDelegation => self.capability_delegation_mut().append(method_ref), - MethodRelationship::CapabilityInvocation => self.capability_invocation_mut().append(method_ref), + MethodRelationship::Authentication => self.authentication_mut_unchecked().append(method_ref), + MethodRelationship::AssertionMethod => self.assertion_method_mut_unchecked().append(method_ref), + MethodRelationship::KeyAgreement => self.key_agreement_mut_unchecked().append(method_ref), + MethodRelationship::CapabilityDelegation => self.capability_delegation_mut_unchecked().append(method_ref), + MethodRelationship::CapabilityInvocation => self.capability_invocation_mut_unchecked().append(method_ref), }; Ok(was_attached) @@ -622,11 +647,11 @@ where let did_url: DIDUrl = method.id().clone(); let was_detached = match relationship { - MethodRelationship::Authentication => self.authentication_mut().remove(&did_url), - MethodRelationship::AssertionMethod => self.assertion_method_mut().remove(&did_url), - MethodRelationship::KeyAgreement => self.key_agreement_mut().remove(&did_url), - MethodRelationship::CapabilityDelegation => self.capability_delegation_mut().remove(&did_url), - MethodRelationship::CapabilityInvocation => self.capability_invocation_mut().remove(&did_url), + MethodRelationship::Authentication => self.authentication_mut_unchecked().remove(&did_url), + MethodRelationship::AssertionMethod => self.assertion_method_mut_unchecked().remove(&did_url), + MethodRelationship::KeyAgreement => self.key_agreement_mut_unchecked().remove(&did_url), + MethodRelationship::CapabilityDelegation => self.capability_delegation_mut_unchecked().remove(&did_url), + MethodRelationship::CapabilityInvocation => self.capability_invocation_mut_unchecked().remove(&did_url), }; Ok(was_detached) @@ -761,6 +786,9 @@ where /// Returns a mutable reference to the first [`VerificationMethod`] with an `id` property /// matching the provided `query`. + /// + /// # Warning + /// Incorrect use of this method can lead to broken invariants. pub fn resolve_method_mut<'query, 'me, Q>( &'me mut self, query: Q, @@ -1056,7 +1084,7 @@ mod core_document_revocation { Q: Into>, { let service: &mut Service = self - .service_mut() + .service_mut_unchecked() .query_mut(service_query) .ok_or(Error::InvalidService("invalid id - service not found"))?; @@ -1150,10 +1178,10 @@ mod tests { { let mut document: CoreDocument = document(); let expected: CoreDID = CoreDID::parse("did:example:one1234").unwrap(); - *document.controller_mut() = Some(OneOrSet::new_one(expected.clone())); + *document.controller_mut_unchecked() = Some(OneOrSet::new_one(expected.clone())); assert_eq!(document.controller().unwrap().as_slice(), &[expected]); // Unset. - *document.controller_mut() = None; + *document.controller_mut_unchecked() = None; assert!(document.controller().is_none()); } @@ -1165,10 +1193,10 @@ mod tests { CoreDID::parse("did:example:many4567").unwrap(), CoreDID::parse("did:example:many8910").unwrap(), ]; - *document.controller_mut() = Some(expected_controllers.clone().try_into().unwrap()); + *document.controller_mut_unchecked() = Some(expected_controllers.clone().try_into().unwrap()); assert_eq!(document.controller().unwrap().as_slice(), &expected_controllers); // Unset. - *document.controller_mut() = None; + *document.controller_mut_unchecked() = None; assert!(document.controller().is_none()); } } @@ -1477,7 +1505,7 @@ mod tests { for index in indices_1.iter() { bitmap.revoke(*index); } - assert!(document.service_mut().append( + assert!(document.service_mut_unchecked().append( Service::builder(Object::new()) .id(service_id.clone()) .type_(crate::revocation::RevocationBitmap::TYPE) diff --git a/identity_iota_core/src/document/iota_document.rs b/identity_iota_core/src/document/iota_document.rs index f4932b628a..24c2407743 100644 --- a/identity_iota_core/src/document/iota_document.rs +++ b/identity_iota_core/src/document/iota_document.rs @@ -105,7 +105,7 @@ impl IotaDocument { /// Returns a mutable reference to the `alsoKnownAs` set. pub fn also_known_as_mut(&mut self) -> &mut OrderedSet { - self.document.also_known_as_mut() + self.document.also_known_as_mut_unchecked() } /// Returns a reference to the underlying [`IotaCoreDocument`]. @@ -128,7 +128,7 @@ impl IotaDocument { /// Returns a mutable reference to the custom DID Document properties. pub fn properties_mut(&mut self) -> &mut Object { - self.document.properties_mut() + self.document.properties_mut_unchecked() } // =========================================================================== @@ -147,7 +147,7 @@ impl IotaDocument { if service.id().fragment().is_none() { false } else { - self.core_document_mut().service_mut().append(service) + self.core_document_mut().service_mut_unchecked().append(service) } } @@ -155,7 +155,7 @@ impl IotaDocument { /// /// Returns `true` if a service was removed. pub fn remove_service(&mut self, did_url: &IotaDIDUrl) -> bool { - self.core_document_mut().service_mut().remove(did_url) + self.core_document_mut().service_mut_unchecked().remove(did_url) } // =========================================================================== @@ -334,7 +334,7 @@ mod client_document { Address::Alias(alias_address) => Some(IotaDID::new(alias_address.alias_id(), network_name)), _ => None, }; - *self.core_document_mut().controller_mut() = controller_did.map(OneOrSet::new_one); + *self.core_document_mut().controller_mut_unchecked() = controller_did.map(OneOrSet::new_one); } /// Returns all DID documents of the Alias Outputs contained in the block's transaction payload @@ -453,7 +453,7 @@ impl From for IotaCoreDocument { impl From for CoreDocument { fn from(document: IotaDocument) -> Self { - document.document.map(Into::into, |id| id) + document.document.map_unchecked(Into::into, |id| id) } } diff --git a/identity_iota_core/src/state_metadata/document.rs b/identity_iota_core/src/state_metadata/document.rs index 94bcd57316..76113a5799 100644 --- a/identity_iota_core/src/state_metadata/document.rs +++ b/identity_iota_core/src/state_metadata/document.rs @@ -65,7 +65,7 @@ impl StateMetadataDocument { // Unset Governor and State Controller Addresses to avoid bloating the payload self.metadata.governor_address = None; self.metadata.state_controller_address = None; - *self.document.controller_mut() = None; + *self.document.controller_mut_unchecked() = None; let encoded_message_data: Vec = match encoding { StateMetadataEncoding::Json => self @@ -158,7 +158,7 @@ impl From for StateMetadataDocument { fn from(document: IotaDocument) -> Self { let IotaDocument { document, metadata } = document; let id: IotaDID = document.id().clone(); - let core_document: CoreDocument = document.map( + let core_document: CoreDocument = document.map_unchecked( // Replace self-referential identifiers with a placeholder, but not others. |did| { if did == id { @@ -258,7 +258,7 @@ mod tests { .append(Url::parse("did:example:xyz").unwrap()); let controllers = OneOrSet::try_from(vec![did_foreign.clone(), did_self.clone()]).unwrap(); - *document.core_document_mut().controller_mut() = Some(controllers); + *document.core_document_mut().controller_mut_unchecked() = Some(controllers); TestSetup { document, @@ -384,7 +384,7 @@ mod tests { let mut state_metadata_doc: StateMetadataDocument = StateMetadataDocument::from(document); let packed: Vec = state_metadata_doc.clone().pack(StateMetadataEncoding::Json).unwrap(); // Controller and State Controller are set to None when packing - *state_metadata_doc.document.controller_mut() = None; + *state_metadata_doc.document.controller_mut_unchecked() = None; state_metadata_doc.metadata.governor_address = None; state_metadata_doc.metadata.state_controller_address = None; let expected_payload: String = format!( From b52c9dbcd1b7cf06d3829e80f35d6a6a31961e24 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 8 Nov 2022 10:05:52 +0100 Subject: [PATCH 15/18] temporarily revert remove --- identity_core/src/common/ordered_set.rs | 26 ++--- identity_did/src/diff/diff_document.rs | 8 +- identity_did/src/document/core_document.rs | 122 +++++++++++---------- 3 files changed, 77 insertions(+), 79 deletions(-) diff --git a/identity_core/src/common/ordered_set.rs b/identity_core/src/common/ordered_set.rs index 776b8aeaee..f43d59cbe3 100644 --- a/identity_core/src/common/ordered_set.rs +++ b/identity_core/src/common/ordered_set.rs @@ -176,24 +176,20 @@ impl OrderedSet { /// Removes and returns the matching item from the set, if it exists. #[inline] - pub fn remove(&mut self, item: &U) -> Option + pub fn remove(&mut self, item: &U) -> bool where T: KeyComparable, U: KeyComparable, { - self.iter().enumerate().find(|(_, entry)| entry.key() == item.key()).map(|(idx,_)| idx).map(|idx| self.0.remove(idx)) - //Some(self.0.remove(idx)) + // self.iter().enumerate().find(|(_, entry)| entry.key() == item.key()).map(|(idx,_)| idx).map(|idx| + // self.0.remove(idx)) - - /* if self.contains(item) { self.0.retain(|this| this.borrow().key() != item.key()); true } else { false } - */ - } fn change(&mut self, data: T, f: F) -> bool @@ -320,7 +316,7 @@ where mod tests { use super::*; use proptest::prelude::Rng; -use proptest::strategy::Strategy; + use proptest::strategy::Strategy; use proptest::*; #[test] @@ -467,16 +463,12 @@ use proptest::strategy::Strategy; assert_eq!(set.head().unwrap().value, cs2.value); } - - - // - fn set_with_elements() -> impl Strategy, T, T)> { - proptest::prelude::any::>().prop_map(|init|init.into_iter().collect::>()) + // + fn set_with_elements( + ) -> impl Strategy> { + proptest::prelude::any::>().prop_map(|init| init.into_iter().collect::>()) } #[test] - fn append_preserves_invariant() { - - } + fn append_preserves_invariant() {} } - diff --git a/identity_did/src/diff/diff_document.rs b/identity_did/src/diff/diff_document.rs index d02c15f21a..4b3c65f3dd 100644 --- a/identity_did/src/diff/diff_document.rs +++ b/identity_did/src/diff/diff_document.rs @@ -448,7 +448,9 @@ mod test { fn test_also_known_as() { let doc = document(); let mut new = doc.clone(); - new.also_known_as_mut_unchecked().append("diff:diff:1234".parse().unwrap()); + new + .also_known_as_mut_unchecked() + .append("diff:diff:1234".parse().unwrap()); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); @@ -462,7 +464,9 @@ mod test { let mut new = doc.clone(); // add new method - assert!(new.verification_method_mut_unchecked().append(method(&doc.data.id, "#key-diff"))); + assert!(new + .verification_method_mut_unchecked() + .append(method(&doc.data.id, "#key-diff"))); assert_ne!(doc, new); let diff = doc.diff(&new).unwrap(); let merge = doc.merge(diff).unwrap(); diff --git a/identity_did/src/document/core_document.rs b/identity_did/src/document/core_document.rs index ebc4ec162e..682c9605ca 100644 --- a/identity_did/src/document/core_document.rs +++ b/identity_did/src/document/core_document.rs @@ -247,9 +247,9 @@ where } /// Returns a mutable reference to the `CoreDocument` id. - /// + /// /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn id_mut_unchecked(&mut self) -> &mut D { &mut self.data.id } @@ -260,9 +260,9 @@ where } /// Returns a mutable reference to the `CoreDocument` controller. - /// + /// /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn controller_mut_unchecked(&mut self) -> &mut Option> { &mut self.data.controller } @@ -274,7 +274,7 @@ where /// Returns a mutable reference to the `CoreDocument` alsoKnownAs set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn also_known_as_mut_unchecked(&mut self) -> &mut OrderedSet { &mut self.data.also_known_as } @@ -286,7 +286,7 @@ where /// Returns a mutable reference to the `CoreDocument` verificationMethod set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn verification_method_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.verification_method } @@ -298,7 +298,7 @@ where /// Returns a mutable reference to the `CoreDocument` authentication set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn authentication_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.authentication } @@ -310,7 +310,7 @@ where /// Returns a mutable reference to the `CoreDocument` assertionMethod set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn assertion_method_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.assertion_method } @@ -322,7 +322,7 @@ where /// Returns a mutable reference to the `CoreDocument` keyAgreement set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn key_agreement_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.key_agreement } @@ -334,7 +334,7 @@ where /// Returns a mutable reference to the `CoreDocument` capabilityDelegation set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn capability_delegation_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.capability_delegation } @@ -346,7 +346,7 @@ where /// Returns a mutable reference to the `CoreDocument` capabilityInvocation set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn capability_invocation_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.capability_invocation } @@ -358,8 +358,9 @@ where /// Returns a mutable reference to the `CoreDocument` service set. /// # Warning - /// Incorrect use of this method can lead to broken invariants. Consider using the safer [`Self::insert_service`](CoreDocument::insert_service())/ [`Self::remove_service`](CoreDocument::remove_service) - /// API(s) instead. + /// Incorrect use of this method can lead to broken invariants. Consider using the safer + /// [`Self::insert_service`](CoreDocument::insert_service())/ [`Self::remove_service`](CoreDocument::remove_service) + /// API(s) instead. pub fn service_mut_unchecked(&mut self) -> &mut OrderedSet> { &mut self.data.service } @@ -371,7 +372,7 @@ where /// Returns a mutable reference to the custom `CoreDocument` properties. /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn properties_mut_unchecked(&mut self) -> &mut T { &mut self.data.properties } @@ -380,7 +381,8 @@ where /// and another function `g` to the custom properties. /// /// # Warning - /// Can lead to broken invariants if used incorrectly. See [`Self::try_map`](CoreDocument::try_map()) for a fallible version with additional built-in checks. + /// Can lead to broken invariants if used incorrectly. See [`Self::try_map`](CoreDocument::try_map()) for a fallible + /// version with additional built-in checks. pub fn map_unchecked(self, mut f: F, g: G) -> CoreDocument where C: DID + KeyComparable, @@ -388,52 +390,52 @@ where G: FnOnce(T) -> S, { let current_inner = self.data; - CoreDocument::{ + CoreDocument:: { data: CoreDocumentData { - id: f(current_inner.id), - controller: current_inner - .controller - .map(|controller_set| controller_set.map(&mut f)), - also_known_as: current_inner.also_known_as, - verification_method: current_inner - .verification_method - .into_iter() - .map(|method| method.map(&mut f)) - .collect(), - authentication: current_inner - .authentication - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - assertion_method: current_inner - .assertion_method - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - key_agreement: current_inner - .key_agreement - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - capability_delegation: current_inner - .capability_delegation - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - capability_invocation: current_inner - .capability_invocation - .into_iter() - .map(|method_ref| method_ref.map(&mut f)) - .collect(), - service: current_inner - .service - .into_iter() - .map(|service| service.map(&mut f)) - .collect(), - properties: g(current_inner.properties), + id: f(current_inner.id), + controller: current_inner + .controller + .map(|controller_set| controller_set.map(&mut f)), + also_known_as: current_inner.also_known_as, + verification_method: current_inner + .verification_method + .into_iter() + .map(|method| method.map(&mut f)) + .collect(), + authentication: current_inner + .authentication + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + assertion_method: current_inner + .assertion_method + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + key_agreement: current_inner + .key_agreement + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + capability_delegation: current_inner + .capability_delegation + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + capability_invocation: current_inner + .capability_invocation + .into_iter() + .map(|method_ref| method_ref.map(&mut f)) + .collect(), + service: current_inner + .service + .into_iter() + .map(|service| service.map(&mut f)) + .collect(), + properties: g(current_inner.properties), + }, } } - } /// Fallible version of [`CoreDocument::map`]. /// @@ -786,9 +788,9 @@ where /// Returns a mutable reference to the first [`VerificationMethod`] with an `id` property /// matching the provided `query`. - /// + /// /// # Warning - /// Incorrect use of this method can lead to broken invariants. + /// Incorrect use of this method can lead to broken invariants. pub fn resolve_method_mut<'query, 'me, Q>( &'me mut self, query: Q, From 3a0ca862289f68db64cf08bc01b4c642d75dcbad Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 8 Nov 2022 10:52:39 +0100 Subject: [PATCH 16/18] arbitrary constructor --- identity_core/src/common/ordered_set.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/identity_core/src/common/ordered_set.rs b/identity_core/src/common/ordered_set.rs index f43d59cbe3..02e6d7beb0 100644 --- a/identity_core/src/common/ordered_set.rs +++ b/identity_core/src/common/ordered_set.rs @@ -389,7 +389,7 @@ mod tests { assert!(!set.contains(&cs4)); } - #[derive(Clone, Copy, PartialEq, Eq)] + #[derive(Clone, Copy, PartialEq, Eq, Debug)] struct ComparableStruct { key: u8, value: i32, @@ -463,12 +463,11 @@ mod tests { assert_eq!(set.head().unwrap().value, cs2.value); } - // - fn set_with_elements( - ) -> impl Strategy> { - proptest::prelude::any::>().prop_map(|init| init.into_iter().collect::>()) + fn arbitrary_set_comparable_struct() -> impl Strategy> { + proptest::arbitrary::any::>().prop_map(|values| values.into_iter().map(|(key, value)| ComparableStruct{key, value}).collect()) } - + + #[test] fn append_preserves_invariant() {} } From 747e51d98df42f5a5f179bf2302bdacb0c5516a3 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 8 Nov 2022 14:07:37 +0100 Subject: [PATCH 17/18] new generator --- identity_core/src/common/ordered_set.rs | 36 ++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/identity_core/src/common/ordered_set.rs b/identity_core/src/common/ordered_set.rs index 02e6d7beb0..8dee280f6a 100644 --- a/identity_core/src/common/ordered_set.rs +++ b/identity_core/src/common/ordered_set.rs @@ -389,7 +389,7 @@ mod tests { assert!(!set.contains(&cs4)); } - #[derive(Clone, Copy, PartialEq, Eq, Debug)] + #[derive(Clone, Copy, PartialEq, Eq, Debug, Default)] struct ComparableStruct { key: u8, value: i32, @@ -464,10 +464,34 @@ mod tests { } fn arbitrary_set_comparable_struct() -> impl Strategy> { - proptest::arbitrary::any::>().prop_map(|values| values.into_iter().map(|(key, value)| ComparableStruct{key, value}).collect()) + proptest::arbitrary::any::>().prop_map(|values| { + values + .into_iter() + .map(|(key, value)| ComparableStruct { key, value }) + .collect() + }) } - - - #[test] - fn append_preserves_invariant() {} + + fn arbitrary_set_u128() -> impl Strategy> { + proptest::arbitrary::any::>().prop_map(|vector| vector.into_iter().collect()) + } + + fn set_with_values() -> impl Strategy, ComparableStruct, ComparableStruct)> { + (arbitrary_set_comparable_struct(), arbitrary_set_comparable_struct()).prop_perturb(|(s0, s1), mut rng| { + let sets = [&s0, &s1]; + let mut pick_value = || { + let set_idx = usize::from(rng.gen_bool(0.5)); + let set_range = if set_idx == 0 { 0..s0.len() } else { 0..s1.len() }; + if set_range.is_empty() { + ComparableStruct::default() + } else { + let entry_idx = rng.gen_range(set_range); + (sets[set_idx])[entry_idx].clone() + } + }; + let (v0, v1) = (pick_value(), pick_value()); + (s0, v0, v1) + }) + } + } From 54e0d0ab932931ff22c64fa934259b6ec4653747 Mon Sep 17 00:00:00 2001 From: Oliver Anderson Date: Tue, 8 Nov 2022 14:22:34 +0100 Subject: [PATCH 18/18] progress on tests --- identity_core/src/common/ordered_set.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/identity_core/src/common/ordered_set.rs b/identity_core/src/common/ordered_set.rs index 8dee280f6a..96178cf120 100644 --- a/identity_core/src/common/ordered_set.rs +++ b/identity_core/src/common/ordered_set.rs @@ -463,6 +463,10 @@ mod tests { assert_eq!(set.head().unwrap().value, cs2.value); } + // =========================================================================================================================== + // Test key uniqueness invariant with randomly generated input + // =========================================================================================================================== + fn arbitrary_set_comparable_struct() -> impl Strategy> { proptest::arbitrary::any::>().prop_map(|values| { values @@ -476,14 +480,21 @@ mod tests { proptest::arbitrary::any::>().prop_map(|vector| vector.into_iter().collect()) } - fn set_with_values() -> impl Strategy, ComparableStruct, ComparableStruct)> { - (arbitrary_set_comparable_struct(), arbitrary_set_comparable_struct()).prop_perturb(|(s0, s1), mut rng| { + // construct a set together with a pair of values. Given one of these values there is a 50% chance that it is + // contained in the set. + fn set_with_values(f: F) -> impl Strategy, T, T)> + where + T: KeyComparable + Default + Debug + Clone, + U: Strategy, OrderedSet)>, + F: Fn() -> U, + { + f().prop_perturb(|(s0, s1), mut rng| { let sets = [&s0, &s1]; let mut pick_value = || { let set_idx = usize::from(rng.gen_bool(0.5)); let set_range = if set_idx == 0 { 0..s0.len() } else { 0..s1.len() }; if set_range.is_empty() { - ComparableStruct::default() + T::default() } else { let entry_idx = rng.gen_range(set_range); (sets[set_idx])[entry_idx].clone() @@ -493,5 +504,4 @@ mod tests { (s0, v0, v1) }) } - }