refactor(service): change service interface return type (#392)

* feat(protocol): add an impl interface macro refactor rename cargo fmt * rebase master * remove useless code
nervosnetwork · Aug 12, 2020 · b355d7a · b355d7a
1 parent e678e92
commit b355d7a
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 102 deletions.
diff --git a/benchmark/governance/mod.rs b/benchmark/governance/mod.rs
@@ -9,6 +9,7 @@ use derive_more::{Display, From};
 
 use binding_macro::{genesis, hook_after, service, tx_hook_after, tx_hook_before};
 use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK, StoreMap};
+use protocol::try_service_response;
 use protocol::types::{Address, Hash, ServiceContext, ServiceContextParams};
 
 use asset::types::TransferPayload;
@@ -86,10 +87,9 @@ impl<A: Assets, SDK: ServiceSDK> GovernanceService<A, SDK> {
         };
 
         // Pledge the tx failure fee before executed the transaction.
-        match self.asset.transfer_(&ctx, payload) {
-            Ok(_) => ServiceResponse::from_succeed("".to_owned()),
-            Err(e) => ServiceResponse::from_error(e.code, e.error_message),
-        }
+        let res = self.asset.transfer_(&ctx, payload);
+        try_service_response!(res);
+        ServiceResponse::from_succeed(String::new())
     }
 
     #[tx_hook_after]
@@ -108,10 +108,9 @@ impl<A: Assets, SDK: ServiceSDK> GovernanceService<A, SDK> {
             value:    1,
         };
 
-        match self.asset.transfer_(&ctx, payload) {
-            Ok(_) => ServiceResponse::from_succeed("".to_owned()),
-            Err(e) => ServiceResponse::from_error(e.code, e.error_message),
-        }
+        let res = self.asset.transfer_(&ctx, payload);
+        try_service_response!(res);
+        ServiceResponse::from_succeed(String::new())
     }
 
     #[hook_after]

diff --git a/built-in-services/asset/src/lib.rs b/built-in-services/asset/src/lib.rs
@@ -8,64 +8,40 @@ use std::collections::BTreeMap;
 
 use binding_macro::{cycles, genesis, service};
 use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK, StoreMap};
+use protocol::try_service_response;
 use protocol::types::{Address, Bytes, Hash, ServiceContext};
 
 use crate::types::{
     ApproveEvent, ApprovePayload, Asset, AssetBalance, CreateAssetPayload, GetAllowancePayload,
     GetAllowanceResponse, GetAssetPayload, GetBalancePayload, GetBalanceResponse,
     InitGenesisPayload, TransferEvent, TransferFromEvent, TransferFromPayload, TransferPayload,
 };
-macro_rules! impl_assets {
-    ($self: expr, $method: ident, $ctx: expr) => {{
-        let res = $self.$method($ctx.clone());
-        if res.is_error() {
-            Err(ServiceResponse::from_error(res.code, res.error_message))
-        } else {
-            Ok(res.succeed_data)
-        }
-    }};
-    ($self: expr, $method: ident, $ctx: expr, $payload: expr) => {{
-        let res = $self.$method($ctx.clone(), $payload);
-        if res.is_error() {
-            Err(ServiceResponse::from_error(res.code, res.error_message))
-        } else {
-            Ok(res.succeed_data)
-        }
-    }};
-}
 
 pub const ASSET_SERVICE_NAME: &str = "asset";
 
 pub trait Assets {
-    fn create_(
-        &mut self,
-        ctx: &ServiceContext,
-        payload: CreateAssetPayload,
-    ) -> Result<Asset, ServiceResponse<()>>;
+    fn create_(&mut self, ctx: &ServiceContext, payload: CreateAssetPayload)
+        -> ServiceResponse<()>;
 
     fn balance_(
         &self,
         ctx: &ServiceContext,
         payload: GetBalancePayload,
-    ) -> Result<GetBalanceResponse, ServiceResponse<()>>;
+    ) -> ServiceResponse<GetBalanceResponse>;
 
-    fn transfer_(
-        &mut self,
-        ctx: &ServiceContext,
-        payload: TransferPayload,
-    ) -> Result<(), ServiceResponse<()>>;
+    fn transfer_(&mut self, ctx: &ServiceContext, payload: TransferPayload) -> ServiceResponse<()>;
 
     fn transfer_from_(
         &mut self,
         ctx: &ServiceContext,
         payload: TransferFromPayload,
-    ) -> Result<(), ServiceResponse<()>>;
+    ) -> ServiceResponse<()>;
 
     fn allowance_(
         &self,
         ctx: &ServiceContext,
         payload: GetAllowancePayload,
-    ) -> Result<GetAllowanceResponse, ServiceResponse<()>>;
+    ) -> ServiceResponse<GetAllowanceResponse>;
 }
 
 pub struct AssetService<SDK> {
@@ -78,40 +54,38 @@ impl<SDK: ServiceSDK> Assets for AssetService<SDK> {
         &mut self,
         ctx: &ServiceContext,
         payload: CreateAssetPayload,
-    ) -> Result<Asset, ServiceResponse<()>> {
-        impl_assets!(self, create_asset, ctx, payload)
+    ) -> ServiceResponse<()> {
+        let res = self.create_asset(ctx.clone(), payload);
+        try_service_response!(res);
+        ServiceResponse::from_succeed(())
     }
 
     fn balance_(
         &self,
         ctx: &ServiceContext,
         payload: GetBalancePayload,
-    ) -> Result<GetBalanceResponse, ServiceResponse<()>> {
-        impl_assets!(self, get_balance, ctx, payload)
+    ) -> ServiceResponse<GetBalanceResponse> {
+        self.get_balance(ctx.clone(), payload)
     }
 
-    fn transfer_(
-        &mut self,
-        ctx: &ServiceContext,
-        payload: TransferPayload,
-    ) -> Result<(), ServiceResponse<()>> {
-        impl_assets!(self, transfer, ctx, payload)
+    fn transfer_(&mut self, ctx: &ServiceContext, payload: TransferPayload) -> ServiceResponse<()> {
+        self.transfer(ctx.clone(), payload)
     }
 
     fn transfer_from_(
         &mut self,
         ctx: &ServiceContext,
         payload: TransferFromPayload,
-    ) -> Result<(), ServiceResponse<()>> {
-        impl_assets!(self, transfer_from, ctx, payload)
+    ) -> ServiceResponse<()> {
+        self.transfer_from(ctx.clone(), payload)
     }
 
     fn allowance_(
         &self,
         ctx: &ServiceContext,
         payload: GetAllowancePayload,
-    ) -> Result<GetAllowanceResponse, ServiceResponse<()>> {
-        impl_assets!(self, get_allowance, ctx, payload)
+    ) -> ServiceResponse<GetAllowanceResponse> {
+        self.get_allowance(ctx.clone(), payload)
     }
 }
 

diff --git a/built-in-services/metadata/src/lib.rs b/built-in-services/metadata/src/lib.rs
@@ -5,38 +5,19 @@ use binding_macro::{cycles, genesis, service};
 use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK};
 use protocol::types::{Metadata, ServiceContext, METADATA_KEY};
 
-macro_rules! impl_meatdata {
-    ($self: expr, $method: ident, $ctx: expr) => {{
-        let res = $self.$method($ctx.clone());
-        if res.is_error() {
-            Err(ServiceResponse::from_error(res.code, res.error_message))
-        } else {
-            Ok(res.succeed_data)
-        }
-    }};
-    ($self: expr, $method: ident, $ctx: expr, $payload: expr) => {{
-        let res = $self.$method($ctx.clone(), $payload);
-        if res.is_error() {
-            Err(ServiceResponse::from_error(res.code, res.error_message))
-        } else {
-            Ok(res.succeed_data)
-        }
-    }};
-}
-
 pub const METADATA_SERVICE_NAME: &str = "metadata";
 
 pub trait MetaData {
-    fn get_(&self, ctx: &ServiceContext) -> Result<Metadata, ServiceResponse<()>>;
+    fn get_(&self, ctx: &ServiceContext) -> ServiceResponse<Metadata>;
 }
 
 pub struct MetadataService<SDK> {
     sdk: SDK,
 }
 
 impl<SDK: ServiceSDK> MetaData for MetadataService<SDK> {
-    fn get_(&self, ctx: &ServiceContext) -> Result<Metadata, ServiceResponse<()>> {
-        impl_meatdata!(self, get_metadata, ctx)
+    fn get_(&self, ctx: &ServiceContext) -> ServiceResponse<Metadata> {
+        self.get_metadata(ctx.clone())
     }
 }
 

diff --git a/built-in-services/multi-signature/src/lib.rs b/built-in-services/multi-signature/src/lib.rs
@@ -8,7 +8,6 @@ use std::collections::HashMap;
 use std::panic::catch_unwind;
 
 use binding_macro::{cycles, genesis, service};
-
 use common_crypto::{Crypto, Secp256k1};
 use protocol::traits::{ExecutorParams, ServiceResponse, ServiceSDK};
 use protocol::types::{Address, Bytes, Hash, ServiceContext, SignedTransaction};
@@ -21,41 +20,22 @@ use crate::types::{
     UpdateAccountPayload, VerifySignaturePayload, Witness,
 };
 
+pub const MULTI_SIG_SERVICE_NAME: &str = "multi_signature";
 const MAX_MULTI_SIGNATURE_RECURSION_DEPTH: u8 = 8;
 const MAX_PERMISSION_ACCOUNTS: u8 = 16;
 
-macro_rules! impl_multisig {
-    ($self: expr, $method: ident, $ctx: expr) => {{
-        let res = $self.$method($ctx.clone());
-        if res.is_error() {
-            Err(ServiceResponse::from_error(res.code, res.error_message))
-        } else {
-            Ok(res.succeed_data)
-        }
-    }};
-    ($self: expr, $method: ident, $ctx: expr, $payload: expr) => {{
-        let res = $self.$method($ctx.clone(), $payload);
-        if res.is_error() {
-            Err(ServiceResponse::from_error(res.code, res.error_message))
-        } else {
-            Ok(res.succeed_data)
-        }
-    }};
-}
-pub const MULTI_SIG_SERVICE_NAME: &str = "multi_signature";
-
 pub trait MultiSignature {
     fn verify_signature_(
         &self,
         ctx: &ServiceContext,
         payload: SignedTransaction,
-    ) -> Result<(), ServiceResponse<()>>;
+    ) -> ServiceResponse<()>;
 
     fn generate_account_(
         &mut self,
         ctx: &ServiceContext,
         payload: GenerateMultiSigAccountPayload,
-    ) -> Result<GenerateMultiSigAccountResponse, ServiceResponse<()>>;
+    ) -> ServiceResponse<GenerateMultiSigAccountResponse>;
 }
 
 pub struct MultiSignatureService<SDK> {
@@ -67,16 +47,16 @@ impl<SDK: ServiceSDK> MultiSignature for MultiSignatureService<SDK> {
         &self,
         ctx: &ServiceContext,
         payload: SignedTransaction,
-    ) -> Result<(), ServiceResponse<()>> {
-        impl_multisig!(self, verify_signature, ctx, payload)
+    ) -> ServiceResponse<()> {
+        self.verify_signature(ctx.clone(), payload)
     }
 
     fn generate_account_(
         &mut self,
         ctx: &ServiceContext,
         payload: GenerateMultiSigAccountPayload,
-    ) -> Result<GenerateMultiSigAccountResponse, ServiceResponse<()>> {
-        impl_multisig!(self, generate_account, ctx, payload)
+    ) -> ServiceResponse<GenerateMultiSigAccountResponse> {
+        self.generate_account(ctx.clone(), payload)
     }
 }
 

diff --git a/protocol/src/traits/binding.rs b/protocol/src/traits/binding.rs
@@ -5,6 +5,16 @@ use crate::traits::{ExecutorParams, ServiceResponse};
 use crate::types::{Address, Block, Hash, MerkleRoot, Receipt, ServiceContext, SignedTransaction};
 use crate::ProtocolResult;
 
+#[macro_export]
+macro_rules! try_service_response {
+    ($service_resp: expr) => {{
+        if $service_resp.is_error() {
+            return ServiceResponse::from_error($service_resp.code, $service_resp.error_message);
+        }
+        $service_resp.succeed_data
+    }};
+}
+
 pub trait SDKFactory<SDK: ServiceSDK> {
     fn get_sdk(&self, name: &str) -> ProtocolResult<SDK>;
 }