[feature] #3130: Add type-state-pattern to divide client into sync and async parts #4004

pesterev · 2023-10-18T05:48:20Z

Description

This PR adds traits to implement type-state-pattern on Client to divide it into sync and async parts.
I'm going to open also two pull requests:

Add async client features
iroha_awc and iroha_hyper crates implementations

Linked issue

Closes #3130

Benefits

Checklist

I've read CONTRIBUTING.md
I've used the standard signed-off commit format (or will squash just before merging)
All applicable CI checks pass (or I promised to make them pass later)
(optional) I've written unit tests for the code changes
Rewrite doc tests and examples
I replied to all comments after code review, marking all implemented changes with thumbs

Open a pull request that adds async client features
Open a pull request that add implementations of the iroha_awc and iroha_hyper crates

…ient into sync and async parts Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>

coveralls · 2023-10-18T05:55:46Z

Pull Request Test Coverage Report for Build 6556759437

16 of 109 (14.68%) changed or added relevant lines in 5 files are covered.
6828 unchanged lines in 128 files lost coverage.
Overall coverage decreased (-3.4%) to 56.032%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
client/src/http.rs	0	2	0.0%
client_cli/src/main.rs	0	6	0.0%
core/test_network/src/lib.rs	0	8	0.0%
client/src/http_default.rs	8	32	25.0%
client/src/client.rs	8	61	13.11%

Files with Coverage Reduction	New Missed Lines	%
client/src/lib.rs	1	0.0%
cli/src/torii/pagination.rs	1	98.9%
config/src/block_sync.rs	1	95.0%
config/src/network.rs	1	93.75%
config/src/torii.rs	1	96.43%
config/src/wasm.rs	1	87.5%
core/src/smartcontracts/isi/block.rs	1	87.5%
config/base/derive/src/proxy.rs	2	98.11%
config/src/kura.rs	2	79.41%
config/src/lib.rs	2	0.0%

Totals
Change from base Build 5423219773:	-3.4%
Covered Lines:	21936
Relevant Lines:	39149

💛 - Coveralls

Arjentix · 2023-10-19T15:37:43Z

.gitignore

@@ -38,6 +38,7 @@ include/generated/*
 peers.list
 cmake-build*
 result
+.obsidian/


Wow, what's that?

That is a note-taking app. I use it to render and edit our documentation. Pretty useful.

Arjentix · 2023-10-19T15:49:53Z

client/src/client.rs

-    ) -> Result<Self> {
+    ) -> Result<Client<SyncRequestManager<T>>> {


I think we prefer Self style

Arjentix · 2023-10-19T15:51:41Z

client/src/client.rs

-            request,
-            sorting,
+            request: request.clone(),
+            sorting: sorting.clone(),


I can't see why whis change is needed

Arjentix · 2023-10-19T16:10:32Z

client/src/http.rs

+pub trait AsyncSendRequest {
+    type RequestBuilder;
+
+    fn send<'async_trait>(
+        &'async_trait self,
+        rb: Self::RequestBuilder,
+    ) -> ::std::pin::Pin<
+        Box<
+            dyn ::std::future::Future<Output = Result<http::Response<Vec<u8>>>>
+                + Send
+                + 'async_trait,
+        >,
+    >;
+}


Is this an expanstion of async_trait crate?

I considered this crate, but it will require users to use this crate as I understand. If we agree with it, then let's use it.
Btw the native async traits have already stabilized and seem can appear in one of the coming releases of Rust.

Arjentix · 2023-10-19T16:16:36Z

client/tests/integration/asset.rs

+        let assets = test_client
+            .seek(result)
+            .collect::<QueryResult<Vec<_>>>()
+            .expect("Valid");


Can you explain how does seek() work here?

DCNick3 · 2023-10-20T10:22:01Z

client/src/client.rs

@@ -248,16 +250,41 @@ pub struct ResultSet<T> {
    client_cursor: usize,
 }

-impl<T: Clone> Iterator for ResultSet<T>
+pub struct SeekResultSet<'a, T, R>


Is there a particular reason this struct is called Seek? Also, why does it always uses SyncRequestManager? Is it more like SyncResultSet?

pesterev self-assigned this Oct 18, 2023

[feature] hyperledger-iroha#3130: Add type-state-pattern to divide cl…

82760e4

…ient into sync and async parts Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>

github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Oct 18, 2023

pesterev force-pushed the separate-sync-async-client branch from bc53519 to 82760e4 Compare October 18, 2023 05:49

DCNick3 self-assigned this Oct 18, 2023

Arjentix reviewed Oct 19, 2023

View reviewed changes

DCNick3 reviewed Oct 20, 2023

View reviewed changes

mversic self-assigned this Oct 23, 2023

mversic closed this Jan 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] #3130: Add type-state-pattern to divide client into sync and async parts #4004

[feature] #3130: Add type-state-pattern to divide client into sync and async parts #4004

pesterev commented Oct 18, 2023 •

edited

Loading

coveralls commented Oct 18, 2023

Arjentix Oct 19, 2023

pesterev Oct 20, 2023

Arjentix Oct 19, 2023

Arjentix Oct 19, 2023

Arjentix Oct 19, 2023

pesterev Oct 20, 2023

Arjentix Oct 19, 2023

DCNick3 Oct 20, 2023

		) -> Result<Self> {
		) -> Result<Client<SyncRequestManager<T>>> {

[feature] #3130: Add type-state-pattern to divide client into sync and async parts #4004

[feature] #3130: Add type-state-pattern to divide client into sync and async parts #4004

Conversation

pesterev commented Oct 18, 2023 • edited Loading

Description

Linked issue

Benefits

Checklist

coveralls commented Oct 18, 2023

Pull Request Test Coverage Report for Build 6556759437

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pesterev commented Oct 18, 2023 •

edited

Loading