From ef7de69ee3cc86117af06a692753165c82ca0a4c Mon Sep 17 00:00:00 2001
From: Miroslav Kovar <miroslav.kovar@absa.africa>
Date: Wed, 16 Aug 2023 16:43:37 +0200
Subject: [PATCH] Address code review

Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
---
 Cargo.lock                                | 14 --------------
 did_doc_sov/src/extra_fields/aip1.rs      |  1 +
 did_doc_sov/src/extra_fields/didcommv1.rs |  1 +
 did_doc_sov/src/extra_fields/didcommv2.rs |  1 +
 did_doc_sov/src/lib.rs                    |  7 ++++++-
 did_doc_sov/tests/serde.rs                | 10 +++++++++-
 6 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 7d1e2a5015..f09ea55f3f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -427,13 +427,6 @@ dependencies = [
  "bs58 0.4.0",
  "chrono",
  "derive_builder 0.10.2",
- "did_doc",
- "did_doc_sov",
- "did_key",
- "did_parser",
- "did_peer",
- "did_resolver",
- "did_resolver_registry",
  "diddoc_legacy",
  "env_logger 0.9.3",
  "futures",
@@ -441,7 +434,6 @@ dependencies = [
  "log",
  "messages",
  "openssl",
- "public_key",
  "rand 0.7.3",
  "regex",
  "serde",
@@ -466,12 +458,7 @@ dependencies = [
  "aries_vcx_core",
  "async-trait",
  "derive_builder 0.11.2",
- "did_key",
- "did_peer",
- "did_resolver_registry",
- "did_resolver_sov",
  "log",
- "public_key",
  "serde",
  "serde_json",
  "thiserror",
@@ -3166,7 +3153,6 @@ version = "0.57.1"
 dependencies = [
  "chrono",
  "derive_more",
- "did_doc_sov",
  "diddoc_legacy",
  "isolang",
  "lazy_static",
diff --git a/did_doc_sov/src/extra_fields/aip1.rs b/did_doc_sov/src/extra_fields/aip1.rs
index e8f9155724..0ec8e4f44c 100644
--- a/did_doc_sov/src/extra_fields/aip1.rs
+++ b/did_doc_sov/src/extra_fields/aip1.rs
@@ -1,4 +1,5 @@
 use serde::{Deserialize, Serialize};
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Default)]
+#[serde(deny_unknown_fields)]
 pub struct ExtraFieldsAIP1 {}
diff --git a/did_doc_sov/src/extra_fields/didcommv1.rs b/did_doc_sov/src/extra_fields/didcommv1.rs
index a9c95559b3..393cbb1821 100644
--- a/did_doc_sov/src/extra_fields/didcommv1.rs
+++ b/did_doc_sov/src/extra_fields/didcommv1.rs
@@ -4,6 +4,7 @@ use super::{AcceptType, KeyKind};
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Default)]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct ExtraFieldsDidCommV1 {
     priority: u32,
     recipient_keys: Vec<KeyKind>,
diff --git a/did_doc_sov/src/extra_fields/didcommv2.rs b/did_doc_sov/src/extra_fields/didcommv2.rs
index a48cf855fb..e66390dfb2 100644
--- a/did_doc_sov/src/extra_fields/didcommv2.rs
+++ b/did_doc_sov/src/extra_fields/didcommv2.rs
@@ -4,6 +4,7 @@ use super::{AcceptType, KeyKind};
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Default)]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct ExtraFieldsDidCommV2 {
     routing_keys: Vec<KeyKind>,
     #[serde(default)]
diff --git a/did_doc_sov/src/lib.rs b/did_doc_sov/src/lib.rs
index b5c1960c06..445d59e5cf 100644
--- a/did_doc_sov/src/lib.rs
+++ b/did_doc_sov/src/lib.rs
@@ -164,7 +164,12 @@ impl Serialize for DidDocumentSov {
 
         for service_sov in &self.services {
             let service: Service<ExtraFieldsSov> = service_sov.clone().try_into().map_err(serde::ser::Error::custom)?;
-            builder = builder.add_service(service);
+            // Not very efficient, but
+            // * we don't expect many services
+            // * does not require allowing to remove services from existing DDO or builder
+            if !self.did_doc.service().iter().any(|s| s.id() == service.id()) {
+                builder = builder.add_service(service);
+            }
         }
 
         builder.build().serialize(serializer)
diff --git a/did_doc_sov/tests/serde.rs b/did_doc_sov/tests/serde.rs
index 17fe1cb454..a2d43d251f 100644
--- a/did_doc_sov/tests/serde.rs
+++ b/did_doc_sov/tests/serde.rs
@@ -68,7 +68,7 @@ const DID_DOC_DATA: &'static str = r#"
 "#;
 
 #[test]
-fn test_serde() {
+fn test_deserialization() {
     let did_doc = serde_json::from_str::<DidDocumentSov>(DID_DOC_DATA).unwrap();
     assert_eq!(did_doc.id().to_string(), "did:sov:HR6vs6GEZ8rHaVgjg2WodM");
     assert_eq!(did_doc.verification_method().len(), 2);
@@ -136,3 +136,11 @@ fn test_serde() {
         panic!("Expected reference key kind");
     }
 }
+
+#[test]
+fn test_deserialization_and_serialization() {
+    let did_doc_1 = serde_json::from_str::<DidDocumentSov>(DID_DOC_DATA).unwrap();
+    let serialized = serde_json::to_string_pretty(&did_doc_1).unwrap();
+    let did_doc_2 = serde_json::from_str::<DidDocumentSov>(&serialized).unwrap();
+    assert_eq!(did_doc_1, did_doc_2);
+}