From d96d585c1d1d430a26f01692a3b348072c85ecab Mon Sep 17 00:00:00 2001
From: Patrik Stas <patrik.stas@absa.africa>
Date: Sat, 21 Oct 2023 13:59:44 +0200
Subject: [PATCH] Refactor ServiceBuilder

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
---
 did_doc/src/schema/service.rs            | 61 ++++++------------------
 did_resolver_sov/src/resolution/utils.rs | 10 ++--
 2 files changed, 20 insertions(+), 51 deletions(-)

diff --git a/did_doc/src/schema/service.rs b/did_doc/src/schema/service.rs
index 5b0db23b7f..73024227bf 100644
--- a/did_doc/src/schema/service.rs
+++ b/did_doc/src/schema/service.rs
@@ -8,14 +8,12 @@ use super::{
 };
 use crate::error::DidDocumentBuilderError;
 
-pub type ServiceTypeAlias = OneOrList<String>;
-
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
 #[serde(rename_all = "camelCase")]
 pub struct Service<E> {
     id: Uri,
     #[serde(rename = "type")]
-    service_type: ServiceTypeAlias,
+    service_type: OneOrList<String>,
     service_endpoint: Url,
     #[serde(flatten)]
     extra: E,
@@ -30,7 +28,7 @@ impl<E> Service<E> {
         &self.id
     }
 
-    pub fn service_type(&self) -> &ServiceTypeAlias {
+    pub fn service_type(&self) -> &OneOrList<String> {
         &self.service_type
     }
 
@@ -45,13 +43,6 @@ impl<E> Service<E> {
 
 #[derive(Debug)]
 pub struct ServiceBuilder<E> {
-    id: Uri,
-    service_endpoint: Url,
-    extra: E,
-}
-
-#[derive(Debug)]
-pub struct ServiceBuilderWithServiceType<E> {
     id: Uri,
     service_type: HashSet<String>,
     service_endpoint: Url,
@@ -62,6 +53,7 @@ impl<E> ServiceBuilder<E> {
     pub fn new(id: Uri, service_endpoint: Url, extra: E) -> Self {
         Self {
             id,
+            service_type: Default::default(),
             service_endpoint,
             extra,
         }
@@ -70,54 +62,29 @@ impl<E> ServiceBuilder<E> {
     pub fn add_service_type(
         self,
         service_type: String,
-    ) -> Result<ServiceBuilderWithServiceType<E>, DidDocumentBuilderError> {
+    ) -> Result<ServiceBuilder<E>, DidDocumentBuilderError> {
         if service_type.is_empty() {
-            return Err(DidDocumentBuilderError::MissingField("type"));
+            return Err(DidDocumentBuilderError::InvalidInput(
+                "Invalid service type: empty string".into(),
+            ));
         }
-        let mut service_types = HashSet::new();
-        service_types.insert(service_type);
-        Ok(ServiceBuilderWithServiceType {
-            id: self.id,
-            service_type: service_types,
-            service_endpoint: self.service_endpoint,
-            extra: self.extra,
-        })
-    }
-
-    pub fn add_service_types(
-        self,
-        service_types: Vec<String>,
-    ) -> Result<ServiceBuilderWithServiceType<E>, DidDocumentBuilderError> {
-        if service_types.is_empty() {
-            return Err(DidDocumentBuilderError::MissingField("type"));
+        if self.service_type.contains(&service_type) {
+            return Err(DidDocumentBuilderError::InvalidInput(
+                "Service type was already included".into(),
+            ));
         }
-        let service_types = service_types.into_iter().collect::<HashSet<_>>();
-        Ok(ServiceBuilderWithServiceType {
+        let mut service_types = self.service_type.clone();
+        service_types.insert(service_type);
+        Ok(ServiceBuilder {
             id: self.id,
             service_type: service_types,
             service_endpoint: self.service_endpoint,
             extra: self.extra,
         })
     }
-}
-
-impl<E> ServiceBuilderWithServiceType<E> {
-    pub fn add_service_type(
-        mut self,
-        service_type: String,
-    ) -> Result<Self, DidDocumentBuilderError> {
-        if service_type.is_empty() {
-            return Err(DidDocumentBuilderError::MissingField("type"));
-        }
-        self.service_type.insert(service_type);
-        Ok(self)
-    }
 
     pub fn build(self) -> Service<E> {
         let service_type = match self.service_type.len() {
-            // SAFETY: The only way to get to this state is to add at least one service type
-            0 => unreachable!(),
-            // SAFETY: We know that the length is non-zero
             1 => OneOrList::One(self.service_type.into_iter().next().unwrap()),
             _ => OneOrList::List(self.service_type.into_iter().collect()),
         };
diff --git a/did_resolver_sov/src/resolution/utils.rs b/did_resolver_sov/src/resolution/utils.rs
index df3b797c7d..a895a159ce 100644
--- a/did_resolver_sov/src/resolution/utils.rs
+++ b/did_resolver_sov/src/resolution/utils.rs
@@ -81,13 +81,15 @@ pub(super) async fn ledger_response_to_ddo<E: Default>(
             .filter(|t| *t != DidSovServiceType::Unknown)
             .map(|t| t.to_string())
             .collect();
-        Service::builder(
+        let mut builder = Service::builder(
             service_id,
             endpoint.endpoint.as_str().try_into()?,
             Default::default(),
-        )
-        .add_service_types(service_types)?
-        .build()
+        );
+        for service_type in service_types {
+            builder = builder.add_service_type(service_type)?;
+        }
+        builder.build()
     };
 
     // TODO: Use multibase instead of base58