core: Remove ProtocolName trait in favor of always using strings
#2966
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for opening this! I can't push to your branch unfortunately but here is a patch that should fix a few of the hardest errors. Hope this helps :) Index: core/src/either.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/either.rs b/core/src/either.rs
--- a/core/src/either.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/either.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -22,7 +22,7 @@
use crate::{
muxing::StreamMuxer,
transport::{ListenerId, Transport, TransportError, TransportEvent},
- Multiaddr, ProtocolName,
+ Multiaddr,
};
use futures::{
io::{IoSlice, IoSliceMut},
@@ -310,20 +310,6 @@
}
}
-#[derive(Debug, Clone)]
-pub enum EitherName {
- A(ProtocolName),
- B(ProtocolName),
-}
-
-impl EitherName {
- fn protocol_name(&self) -> &str {
- match self {
- EitherName::A(a) => a.protocol_name(),
- EitherName::B(b) => b.protocol_name(),
- }
- }
-}
#[pin_project(project = EitherTransportProj)]
#[derive(Debug)]
#[must_use = "transports do nothing unless polled"]
Index: core/src/upgrade.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade.rs b/core/src/upgrade.rs
--- a/core/src/upgrade.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -135,21 +135,26 @@
// }
// }
+#[derive(Clone, Debug, Eq, PartialEq)]
pub struct ProtocolName(Cow<'static, str>);
impl ProtocolName {
fn protocol_name(&self) -> &str {
- self.0.as_ref()
+ self.0.as_ref()
}
}
impl AsRef<str> for ProtocolName {
fn as_ref(&self) -> &str {
- self.0.as_ref()
+ self.0.as_ref()
}
}
-
+impl AsRef<[u8]> for ProtocolName {
+ fn as_ref(&self) -> &[u8] {
+ self.0.as_ref().as_bytes()
+ }
+}
/// Common trait for upgrades that can be applied on inbound substreams, outbound substreams,
/// or both.
Index: core/src/upgrade/apply.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/apply.rs b/core/src/upgrade/apply.rs
--- a/core/src/upgrade/apply.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/apply.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -23,7 +23,7 @@
use futures::{future::Either, prelude::*};
use log::debug;
use multistream_select::{self, DialerSelectFuture, ListenerSelectFuture};
-use std::{iter, mem, pin::Pin, task::Context, task::Poll};
+use std::{mem, pin::Pin, task::Context, task::Poll};
pub use multistream_select::Version;
@@ -53,10 +53,7 @@
C: AsyncRead + AsyncWrite + Unpin,
U: InboundUpgrade<Negotiated<C>>,
{
- let iter = up
- .protocol_info()
- .into_iter()
- .map(NameWrap as fn(_) -> NameWrap<_>);
+ let iter = up.protocol_info().into_iter();
let future = multistream_select::listener_select_proto(conn, iter);
InboundUpgradeApply {
inner: InboundUpgradeApplyState::Init {
@@ -72,10 +69,7 @@
C: AsyncRead + AsyncWrite + Unpin,
U: OutboundUpgrade<Negotiated<C>>,
{
- let iter = up
- .protocol_info()
- .into_iter()
- .map(NameWrap as fn(_) -> NameWrap<_>);
+ let iter = up.protocol_info().into_iter();
let future = multistream_select::dialer_select_proto(conn, iter, v);
OutboundUpgradeApply {
inner: OutboundUpgradeApplyState::Init {
@@ -100,7 +94,7 @@
U: InboundUpgrade<Negotiated<C>>,
{
Init {
- future: ListenerSelectFuture<C, NameWrap<ProtocolName>>,
+ future: ListenerSelectFuture<C, ProtocolName>,
upgrade: U,
},
Upgrade {
@@ -138,7 +132,7 @@
}
};
self.inner = InboundUpgradeApplyState::Upgrade {
- future: Box::pin(upgrade.upgrade_inbound(io, info.0)),
+ future: Box::pin(upgrade.upgrade_inbound(io, info)),
};
}
InboundUpgradeApplyState::Upgrade { mut future } => {
@@ -180,7 +174,7 @@
U: OutboundUpgrade<Negotiated<C>>,
{
Init {
- future: DialerSelectFuture<C, NameWrapIter<<U::InfoIter as IntoIterator>::IntoIter>>,
+ future: DialerSelectFuture<C, <U::InfoIter as IntoIterator>::IntoIter>,
upgrade: U,
},
Upgrade {
@@ -218,7 +212,7 @@
}
};
self.inner = OutboundUpgradeApplyState::Upgrade {
- future: Box::pin(upgrade.upgrade_outbound(connection, info.0)),
+ future: Box::pin(upgrade.upgrade_outbound(connection, info)),
};
}
OutboundUpgradeApplyState::Upgrade { mut future } => {
@@ -244,15 +238,3 @@
}
}
}
-
-type NameWrapIter<I> = iter::Map<I, fn(<I as Iterator>::Item) -> NameWrap<<I as Iterator>::Item>>;
-
-/// Wrapper type to expose an `AsRef<[u8]>` impl for all types implementing `ProtocolName`.
-#[derive(Clone)]
-struct NameWrap<N>(N);
-
-impl<N> AsRef<[u8]> for NameWrap<N> {
- fn as_ref(&self) -> &[u8] {
- self.0.protocol_name().as_bytes()
- }
-}
Index: core/src/upgrade/denied.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/denied.rs b/core/src/upgrade/denied.rs
--- a/core/src/upgrade/denied.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/denied.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -18,7 +18,7 @@
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
-use crate::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo, ProtocolName};
+use crate::upgrade::{InboundUpgrade, OutboundUpgrade, ProtocolName, UpgradeInfo};
use futures::future;
use std::iter;
use void::Void;
Index: core/src/upgrade/either.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/either.rs b/core/src/upgrade/either.rs
--- a/core/src/upgrade/either.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/either.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -19,9 +19,10 @@
// DEALINGS IN THE SOFTWARE.
use crate::{
- either::{EitherError, EitherFuture2, EitherName, EitherOutput},
- upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo, ProtocolName},
+ either::{EitherError, EitherFuture2, EitherOutput},
+ upgrade::{InboundUpgrade, OutboundUpgrade, ProtocolName, UpgradeInfo},
};
+use either::Either;
/// A type to represent two possible upgrade types (inbound or outbound).
#[derive(Debug, Clone)]
@@ -35,15 +36,13 @@
A: UpgradeInfo,
B: UpgradeInfo,
{
- type InfoIter = EitherIter<
- <A::InfoIter as IntoIterator>::IntoIter,
- <B::InfoIter as IntoIterator>::IntoIter,
- >;
+ type InfoIter =
+ Either<<A::InfoIter as IntoIterator>::IntoIter, <B::InfoIter as IntoIterator>::IntoIter>;
fn protocol_info(&self) -> Self::InfoIter {
match self {
- EitherUpgrade::A(a) => EitherIter::A(a.protocol_info().into_iter()),
- EitherUpgrade::B(b) => EitherIter::B(b.protocol_info().into_iter()),
+ EitherUpgrade::A(a) => Either::Left(a.protocol_info().into_iter()),
+ EitherUpgrade::B(b) => Either::Right(b.protocol_info().into_iter()),
}
}
}
@@ -58,14 +57,9 @@
type Future = EitherFuture2<A::Future, B::Future>;
fn upgrade_inbound(self, sock: C, info: ProtocolName) -> Self::Future {
- match (self, info) {
- (EitherUpgrade::A(a), EitherName::A(info)) => {
- EitherFuture2::A(a.upgrade_inbound(sock, info))
- }
- (EitherUpgrade::B(b), EitherName::B(info)) => {
- EitherFuture2::B(b.upgrade_inbound(sock, info))
- }
- _ => panic!("Invalid invocation of EitherUpgrade::upgrade_inbound"),
+ match self {
+ EitherUpgrade::A(a) => EitherFuture2::A(a.upgrade_inbound(sock, info)),
+ EitherUpgrade::B(b) => EitherFuture2::B(b.upgrade_inbound(sock, info)),
}
}
}
@@ -79,44 +73,10 @@
type Error = EitherError<EA, EB>;
type Future = EitherFuture2<A::Future, B::Future>;
- fn upgrade_outbound(self, sock: C, info: EitherName) -> Self::Future {
- match (self, info) {
- (EitherUpgrade::A(a), EitherName::A(info)) => {
- EitherFuture2::A(a.upgrade_outbound(sock, info))
- }
- (EitherUpgrade::B(b), EitherName::B(info)) => {
- EitherFuture2::B(b.upgrade_outbound(sock, info))
- }
- _ => panic!("Invalid invocation of EitherUpgrade::upgrade_outbound"),
- }
- }
-}
-
-/// A type to represent two possible `Iterator` types.
-#[derive(Debug, Clone)]
-pub enum EitherIter<A, B> {
- A(A),
- B(B),
-}
-
-impl<A, B> Iterator for EitherIter<A, B>
-where
- A: Iterator,
- B: Iterator,
-{
- type Item = EitherName;
-
- fn next(&mut self) -> Option<Self::Item> {
+ fn upgrade_outbound(self, sock: C, info: ProtocolName) -> Self::Future {
match self {
- EitherIter::A(a) => a.next().map(EitherName::A),
- EitherIter::B(b) => b.next().map(EitherName::B),
- }
- }
-
- fn size_hint(&self) -> (usize, Option<usize>) {
- match self {
- EitherIter::A(a) => a.size_hint(),
- EitherIter::B(b) => b.size_hint(),
+ EitherUpgrade::A(a) => EitherFuture2::A(a.upgrade_outbound(sock, info)),
+ EitherUpgrade::B(b) => EitherFuture2::B(b.upgrade_outbound(sock, info)),
}
}
}
Index: core/src/upgrade/from_fn.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/from_fn.rs b/core/src/upgrade/from_fn.rs
--- a/core/src/upgrade/from_fn.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/from_fn.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -70,8 +70,7 @@
fun: F,
}
-impl<F> UpgradeInfo for FromFnUpgrade<F>
-{
+impl<F> UpgradeInfo for FromFnUpgrade<F> {
type InfoIter = iter::Once<ProtocolName>;
fn protocol_info(&self) -> Self::InfoIter {
Index: core/src/upgrade/map.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/map.rs b/core/src/upgrade/map.rs
--- a/core/src/upgrade/map.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/map.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -18,7 +18,7 @@
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
-use crate::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo, ProtocolName};
+use crate::upgrade::{InboundUpgrade, OutboundUpgrade, ProtocolName, UpgradeInfo};
use futures::prelude::*;
use std::{pin::Pin, task::Context, task::Poll};
Index: core/src/upgrade/optional.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/optional.rs b/core/src/upgrade/optional.rs
--- a/core/src/upgrade/optional.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/optional.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -18,7 +18,7 @@
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
-use crate::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo, ProtocolName};
+use crate::upgrade::{InboundUpgrade, OutboundUpgrade, ProtocolName, UpgradeInfo};
/// Upgrade that can be disabled at runtime.
///
Index: core/src/upgrade/pending.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/pending.rs b/core/src/upgrade/pending.rs
--- a/core/src/upgrade/pending.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/pending.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -37,8 +37,7 @@
}
}
-impl UpgradeInfo for PendingUpgrade
-{
+impl UpgradeInfo for PendingUpgrade {
type InfoIter = iter::Once<ProtocolName>;
fn protocol_info(&self) -> Self::InfoIter {
@@ -46,8 +45,7 @@
}
}
-impl<C> InboundUpgrade<C> for PendingUpgrade
-{
+impl<C> InboundUpgrade<C> for PendingUpgrade {
type Output = Void;
type Error = Void;
type Future = future::Pending<Result<Self::Output, Self::Error>>;
@@ -57,8 +55,7 @@
}
}
-impl<C> OutboundUpgrade<C> for PendingUpgrade
-{
+impl<C> OutboundUpgrade<C> for PendingUpgrade {
type Output = Void;
type Error = Void;
type Future = future::Pending<Result<Self::Output, Self::Error>>;
Index: core/src/upgrade/ready.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/ready.rs b/core/src/upgrade/ready.rs
--- a/core/src/upgrade/ready.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/ready.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -36,8 +36,7 @@
}
}
-impl UpgradeInfo for ReadyUpgrade
-{
+impl UpgradeInfo for ReadyUpgrade {
type InfoIter = iter::Once<ProtocolName>;
fn protocol_info(&self) -> Self::InfoIter {
@@ -45,8 +44,7 @@
}
}
-impl<C> InboundUpgrade<C> for ReadyUpgrade
-{
+impl<C> InboundUpgrade<C> for ReadyUpgrade {
type Output = C;
type Error = Void;
type Future = future::Ready<Result<Self::Output, Self::Error>>;
@@ -56,8 +54,7 @@
}
}
-impl<C> OutboundUpgrade<C> for ReadyUpgrade
-{
+impl<C> OutboundUpgrade<C> for ReadyUpgrade {
type Output = C;
type Error = Void;
type Future = future::Ready<Result<Self::Output, Self::Error>>;
Index: core/src/upgrade/select.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade/select.rs b/core/src/upgrade/select.rs
--- a/core/src/upgrade/select.rs (revision 8fd23bc0eff8fea08e10a4750a3321c8f963f034)
+++ b/core/src/upgrade/select.rs (revision 1ef648184017f9c2d7420f84776364b131d57fc4)
@@ -19,8 +19,9 @@
// DEALINGS IN THE SOFTWARE.
use crate::{
- either::{EitherError, EitherFuture2, EitherName, EitherOutput},
+ either::{EitherError, EitherFuture2, EitherOutput},
upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo},
+ ProtocolName,
};
/// Upgrade that combines two upgrades into one. Supports all the protocols supported by either
@@ -44,16 +45,16 @@
A: UpgradeInfo,
B: UpgradeInfo,
{
- type InfoIter = InfoIterChain<
+ type InfoIter = std::iter::Chain<
<A::InfoIter as IntoIterator>::IntoIter,
<B::InfoIter as IntoIterator>::IntoIter,
>;
fn protocol_info(&self) -> Self::InfoIter {
- InfoIterChain(
- self.0.protocol_info().into_iter(),
- self.1.protocol_info().into_iter(),
- )
+ self.0
+ .protocol_info()
+ .into_iter()
+ .chain(self.1.protocol_info().into_iter())
}
}
@@ -66,11 +67,26 @@
type Error = EitherError<EA, EB>;
type Future = EitherFuture2<A::Future, B::Future>;
- fn upgrade_inbound(self, sock: C, info: EitherName) -> Self::Future {
- match info {
- EitherName::A(info) => EitherFuture2::A(self.0.upgrade_inbound(sock, info)),
- EitherName::B(info) => EitherFuture2::B(self.1.upgrade_inbound(sock, info)),
+ fn upgrade_inbound(self, sock: C, info: ProtocolName) -> Self::Future {
+ if self
+ .0
+ .protocol_info()
+ .into_iter()
+ .any(|candidate| candidate == info)
+ {
+ return EitherFuture2::A(self.0.upgrade_inbound(sock, info));
}
+
+ if self
+ .1
+ .protocol_info()
+ .into_iter()
+ .any(|candidate| candidate == info)
+ {
+ return EitherFuture2::B(self.1.upgrade_inbound(sock, info));
+ }
+
+ unreachable!("selected protocol must be suppored by one of the upgrades")
}
}
@@ -83,39 +99,25 @@
type Error = EitherError<EA, EB>;
type Future = EitherFuture2<A::Future, B::Future>;
- fn upgrade_outbound(self, sock: C, info: EitherName) -> Self::Future {
- match info {
- EitherName::A(info) => EitherFuture2::A(self.0.upgrade_outbound(sock, info)),
- EitherName::B(info) => EitherFuture2::B(self.1.upgrade_outbound(sock, info)),
+ fn upgrade_outbound(self, sock: C, info: ProtocolName) -> Self::Future {
+ if self
+ .0
+ .protocol_info()
+ .into_iter()
+ .any(|candidate| candidate == info)
+ {
+ return EitherFuture2::A(self.0.upgrade_outbound(sock, info));
}
- }
-}
-/// Iterator that combines the protocol names of twp upgrades.
-#[derive(Debug, Clone)]
-pub struct InfoIterChain<A, B>(A, B);
-
-impl<A, B> Iterator for InfoIterChain<A, B>
-where
- A: Iterator,
- B: Iterator,
-{
- type Item = EitherName;
-
- fn next(&mut self) -> Option<Self::Item> {
- if let Some(info) = self.0.next() {
- return Some(EitherName::A(info));
+ if self
+ .1
+ .protocol_info()
+ .into_iter()
+ .any(|candidate| candidate == info)
+ {
+ return EitherFuture2::B(self.1.upgrade_outbound(sock, info));
}
- if let Some(info) = self.1.next() {
- return Some(EitherName::B(info));
- }
- None
- }
- fn size_hint(&self) -> (usize, Option<usize>) {
- let (min1, max1) = self.0.size_hint();
- let (min2, max2) = self.1.size_hint();
- let max = max1.and_then(move |m1| max2.and_then(move |m2| m1.checked_add(m2)));
- (min1.saturating_add(min2), max)
+ unreachable!("selected protocol must be suppored by one of the upgrades")
}
}
|
thomaseizinger
changed the title
ProtocolName trait in favor of always using strings
Just out of curiosity. Any particular reason the order is important, or is it just a quirk or custom to import alphabetically? :) |
I just ran the auto-formatter! I believe it sorts the imports alphabetically! It is |
|
@thomaseizinger I am trying to figure things out. So please ignore the commit text. :) |
All good, just ping me whenever you have a question or want me to take another look 😊 |
1 similar comment
All good, just ping me whenever you have a question or want me to take another look 😊 |
2 tasks
|
Setting to draft until ready. |
|
rust-libp2p/swarm/src/upgrade.rs Lines 89 to 96 in 5b3612b But it is impl'd in rust-libp2p/swarm/src/handler/multi.rs Lines 472 to 475 in 5b3612b Same goes for |
|
I am not too familiar with those traits but I assume they were implemented for a reason. Do they represent a blocker for you? |
|
Okay, I had a quick look at this and the reason these traits are implemented instead of the regular This is required for I think this shouldn't be much of a concern for your current endeavor though. You should be able to remove |
|
Cross referencing #3128 which would have been slightly easier to implement with these changes. |
|
Thanks for the notification! I think these two PRs are sufficiently orthogonal to treat them independently. |
|
@efarg How are you progressing on this? Let me know if you are stuck with something where I can help :) I am keen to land this simplification! |
|
@thomaseizinger sorry it's still very incomplete: https://github.com/efarg/rust-libp2p/tree/2966 |
|
@efarg You seem to have disabled push access for me so I can't push a commit to this branch :) Anyway, here is a patch that fixes the multi handler: Index: core/src/upgrade.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/upgrade.rs b/core/src/upgrade.rs
--- a/core/src/upgrade.rs (revision 327bdcb3b9889ed3a5d3ebb4346200785bf684ce)
+++ b/core/src/upgrade.rs (revision 1efa7bb73cd029932512060bb24d4aa344b2af58)
@@ -135,11 +135,15 @@
// }
// }
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct ProtocolName(Cow<'static, str>);
impl ProtocolName {
- fn protocol_name(&self) -> &str {
+ pub fn new(protocol: impl Into<Cow<'static, str>>) -> Self {
+ Self(protocol.into())
+ }
+
+ pub fn protocol_name(&self) -> &str {
self.0.as_ref()
}
}
Index: swarm/src/handler/multi.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/swarm/src/handler/multi.rs b/swarm/src/handler/multi.rs
--- a/swarm/src/handler/multi.rs (revision 327bdcb3b9889ed3a5d3ebb4346200785bf684ce)
+++ b/swarm/src/handler/multi.rs (revision 1efa7bb73cd029932512060bb24d4aa344b2af58)
@@ -37,7 +37,7 @@
error,
fmt::{self, Debug},
hash::Hash,
- iter::{self, FromIterator},
+ iter::FromIterator,
task::{Context, Poll},
time::Duration,
};
@@ -112,7 +112,8 @@
.fold(
(Upgrade::new(), Info::new(), Duration::from_secs(0)),
|(mut upg, mut inf, mut timeout), (k, (u, i, t))| {
- upg.upgrades.push((k.clone(), u));
+ upg.add_upgrade(k.clone(), u);
+
inf.infos.push((k, i));
timeout = cmp::max(timeout, t);
(upg, inf, timeout)
@@ -385,23 +386,13 @@
}
fn inbound_protocol(&self) -> <Self::Handler as ConnectionHandler>::InboundProtocol {
- Upgrade {
- upgrades: self
- .handlers
- .iter()
- .map(|(k, h)| (k.clone(), h.inbound_protocol()))
- .collect(),
- }
- }
-}
+ let mut upgrade = Upgrade::new();
-/// Index and protocol name pair used as `UpgradeInfo::Info`.
-#[derive(Debug, Clone)]
-pub struct IndexedProtoName(usize, ProtocolName);
+ for (key, handler) in &self.handlers {
+ upgrade.add_upgrade(key.clone(), handler.inbound_protocol());
+ }
-impl IndexedProtoName {
- fn protocol_name(&self) -> &[u8] {
- self.1.protocol_name()
+ upgrade
}
}
@@ -427,17 +418,33 @@
/// Inbound and outbound upgrade for all [`ConnectionHandler`]s.
#[derive(Clone)]
pub struct Upgrade<K, H> {
- upgrades: Vec<(K, H)>,
+ handler_by_protocol: HashMap<ProtocolName, K>,
+ upgrades_by_handler: HashMap<K, H>,
}
impl<K, H> Upgrade<K, H> {
fn new() -> Self {
Upgrade {
- upgrades: Vec::new(),
+ handler_by_protocol: Default::default(),
+ upgrades_by_handler: Default::default(),
+ }
+ }
+}
+
+impl<K, H> Upgrade<K, H>
+where
+ K: Clone + Eq + Hash,
+ H: InboundUpgradeSend,
+{
+ fn add_upgrade(&mut self, key: K, upgrade: H) {
+ for proto in upgrade.protocol_info() {
+ self.handler_by_protocol.insert(proto, key.clone());
}
+ self.upgrades_by_handler.insert(key, upgrade);
}
}
+// TODO: Derive this?
impl<K, H> fmt::Debug for Upgrade<K, H>
where
K: fmt::Debug + Eq + Hash,
@@ -445,7 +452,8 @@
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Upgrade")
- .field("upgrades", &self.upgrades)
+ .field("handler_by_protocol", &self.handler_by_protocol)
+ .field("upgrades_by_handler", &self.upgrades_by_handler)
.finish()
}
}
@@ -455,14 +463,12 @@
H: UpgradeInfoSend,
K: Send + 'static,
{
- type InfoIter = std::vec::IntoIter<IndexedProtoName>;
+ type InfoIter = std::vec::IntoIter<ProtocolName>;
fn protocol_info(&self) -> Self::InfoIter {
- self.upgrades
- .iter()
- .enumerate()
- .flat_map(|(i, (_, h))| iter::repeat(i).zip(h.protocol_info()))
- .map(|(i, h)| IndexedProtoName(i, h))
+ self.handler_by_protocol
+ .keys()
+ .cloned()
.collect::<Vec<_>>()
.into_iter()
}
@@ -471,15 +477,26 @@
impl<K, H> InboundUpgradeSend for Upgrade<K, H>
where
H: InboundUpgradeSend,
- K: Send + 'static,
+ K: Eq + Hash + Send + 'static,
{
type Output = (K, <H as InboundUpgradeSend>::Output);
type Error = (K, <H as InboundUpgradeSend>::Error);
type Future = BoxFuture<'static, Result<Self::Output, Self::Error>>;
- fn upgrade_inbound(mut self, resource: NegotiatedSubstream, info: ProtocolName) -> Self::Future {
- let IndexedProtoName(index, info) = info;
- let (key, upgrade) = self.upgrades.remove(index);
+ fn upgrade_inbound(
+ mut self,
+ resource: NegotiatedSubstream,
+ info: ProtocolName,
+ ) -> Self::Future {
+ let key = self
+ .handler_by_protocol
+ .remove(&info)
+ .expect("to only negotiate protocols we support");
+ let upgrade = self
+ .upgrades_by_handler
+ .remove(&key)
+ .expect("to only negotiate protocols we support");
+
upgrade
.upgrade_inbound(resource, info)
.map(move |out| match out {
@@ -493,15 +510,26 @@
impl<K, H> OutboundUpgradeSend for Upgrade<K, H>
where
H: OutboundUpgradeSend,
- K: Send + 'static,
+ K: Eq + Hash + Send + 'static,
{
type Output = (K, <H as OutboundUpgradeSend>::Output);
type Error = (K, <H as OutboundUpgradeSend>::Error);
type Future = BoxFuture<'static, Result<Self::Output, Self::Error>>;
- fn upgrade_outbound(mut self, resource: NegotiatedSubstream, info: ProtocolName) -> Self::Future {
- let IndexedProtoName(index, info) = info;
- let (key, upgrade) = self.upgrades.remove(index);
+ fn upgrade_outbound(
+ mut self,
+ resource: NegotiatedSubstream,
+ info: ProtocolName,
+ ) -> Self::Future {
+ let key = self
+ .handler_by_protocol
+ .remove(&info)
+ .expect("to only negotiate protocols we support");
+ let upgrade = self
+ .upgrades_by_handler
+ .remove(&key)
+ .expect("to only negotiate protocols we support");
+
upgrade
.upgrade_outbound(resource, info)
.map(move |out| match out {
@@ -520,12 +548,11 @@
{
let mut set = HashSet::new();
for infos in iter {
- for i in infos.protocol_info() {
- let v = Vec::from(i.protocol_name());
- if set.contains(&v) {
- return Err(DuplicateProtonameError(v));
+ for proto in infos.protocol_info() {
+ if set.contains(&proto) {
+ return Err(DuplicateProtonameError(proto));
} else {
- set.insert(v);
+ set.insert(proto);
}
}
}
@@ -534,22 +561,20 @@
/// It is an error if two handlers share the same protocol name.
#[derive(Debug, Clone)]
-pub struct DuplicateProtonameError(Vec<u8>);
+pub struct DuplicateProtonameError(ProtocolName);
impl DuplicateProtonameError {
/// The protocol name bytes that occured in more than one handler.
+ // TODO: Change this to a `&str`
pub fn protocol_name(&self) -> &[u8] {
- &self.0
+ self.0.protocol_name().as_bytes()
}
}
impl fmt::Display for DuplicateProtonameError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- if let Ok(s) = std::str::from_utf8(&self.0) {
- write!(f, "duplicate protocol name: {}", s)
- } else {
- write!(f, "duplicate protocol name: {:?}", self.0)
- }
+ // TODO: Directly implement `fmt::Display` on `ProtocolName`
+ write!(f, "duplicate protocol name: {}", self.0.protocol_name())
}
}
Index: swarm/src/handler/pending.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/swarm/src/handler/pending.rs b/swarm/src/handler/pending.rs
--- a/swarm/src/handler/pending.rs (revision 327bdcb3b9889ed3a5d3ebb4346200785bf684ce)
+++ b/swarm/src/handler/pending.rs (revision 1efa7bb73cd029932512060bb24d4aa344b2af58)
@@ -26,7 +26,7 @@
use crate::NegotiatedSubstream;
use libp2p_core::{
upgrade::{InboundUpgrade, OutboundUpgrade, PendingUpgrade},
- Multiaddr,
+ Multiaddr, ProtocolName,
};
use std::task::{Context, Poll};
use void::Void;
@@ -53,7 +53,10 @@
type InboundOpenInfo = ();
fn listen_protocol(&self) -> SubstreamProtocol<Self::InboundProtocol, Self::InboundOpenInfo> {
- SubstreamProtocol::new(PendingUpgrade::new(self.protocol_name.clone()), ())
+ SubstreamProtocol::new(
+ PendingUpgrade::new(ProtocolName::new(self.protocol_name.clone())),
+ (),
+ )
}
fn inject_fully_negotiated_inbound(
Index: swarm/src/lib.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs
--- a/swarm/src/lib.rs (revision 327bdcb3b9889ed3a5d3ebb4346200785bf684ce)
+++ b/swarm/src/lib.rs (revision 1efa7bb73cd029932512060bb24d4aa344b2af58)
@@ -90,7 +90,6 @@
multihash::Multihash,
muxing::StreamMuxerBox,
transport::{self, ListenerId, TransportError, TransportEvent},
- upgrade::ProtocolName,
Endpoint, Executor, Multiaddr, Negotiated, PeerId, Transport,
};
use registry::{AddressIntoIter, Addresses};
@@ -1384,7 +1383,7 @@
.inbound_protocol()
.protocol_info()
.into_iter()
- .map(|info| info.protocol_name().to_vec())
+ .map(|info| info.protocol_name().as_bytes().to_vec())
.collect();
// If no executor has been explicitly configured, try to set up a thread pool.
Index: swarm/src/upgrade.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/swarm/src/upgrade.rs b/swarm/src/upgrade.rs
--- a/swarm/src/upgrade.rs (revision 327bdcb3b9889ed3a5d3ebb4346200785bf684ce)
+++ b/swarm/src/upgrade.rs (revision 1efa7bb73cd029932512060bb24d4aa344b2af58)
@@ -22,7 +22,6 @@
use futures::prelude::*;
use libp2p_core::upgrade;
-use std::iter;
/// Implemented automatically on all types that implement [`UpgradeInfo`](upgrade::UpgradeInfo)
/// and `Send + 'static`.
@@ -30,8 +29,8 @@
/// Do not implement this trait yourself. Instead, please implement
/// [`UpgradeInfo`](upgrade::UpgradeInfo).
pub trait UpgradeInfoSend: Send + 'static {
-/// Equivalent to [`UpgradeInfo::Info`](upgrade::UpgradeInfo::Info).
-//type Info: upgrade::ProtocolName + Clone + Send + 'static;
+ /// Equivalent to [`UpgradeInfo::Info`](upgrade::UpgradeInfo::Info).
+ //type Info: upgrade::ProtocolName + Clone + Send + 'static;
/// Equivalent to [`UpgradeInfo::InfoIter`](upgrade::UpgradeInfo::InfoIter).
type InfoIter: Iterator<Item = upgrade::ProtocolName> + Send + 'static;
@@ -39,8 +38,12 @@
fn protocol_info(&self) -> Self::InfoIter;
}
-impl UpgradeInfoSend for upgrade::ProtocolName {
- type InfoIter = iter::Once<upgrade::ProtocolName>;
+impl<T> UpgradeInfoSend for T
+where
+ T: upgrade::UpgradeInfo + Send + 'static,
+ <T::InfoIter as IntoIterator>::IntoIter: Send + 'static,
+{
+ type InfoIter = <<T as upgrade::UpgradeInfo>::InfoIter as IntoIterator>::IntoIter;
fn protocol_info(&self) -> Self::InfoIter {
upgrade::UpgradeInfo::protocol_info(self).into_iter()
@@ -61,7 +64,11 @@
type Future: Future<Output = Result<Self::Output, Self::Error>> + Send + 'static;
/// Equivalent to [`OutboundUpgrade::upgrade_outbound`](upgrade::OutboundUpgrade::upgrade_outbound).
- fn upgrade_outbound(self, socket: NegotiatedSubstream, info: upgrade::ProtocolName) -> Self::Future;
+ fn upgrade_outbound(
+ self,
+ socket: NegotiatedSubstream,
+ info: upgrade::ProtocolName,
+ ) -> Self::Future;
}
impl<T> OutboundUpgradeSend for T
@@ -75,7 +82,11 @@
type Error = T::Error;
type Future = T::Future;
- fn upgrade_outbound(self, socket: NegotiatedSubstream, info: upgrade::ProtocolName) -> Self::Future {
+ fn upgrade_outbound(
+ self,
+ socket: NegotiatedSubstream,
+ info: upgrade::ProtocolName,
+ ) -> Self::Future {
upgrade::OutboundUpgrade::upgrade_outbound(self, socket, info)
}
}
@@ -94,7 +105,11 @@
type Future: Future<Output = Result<Self::Output, Self::Error>> + Send + 'static;
/// Equivalent to [`InboundUpgrade::upgrade_inbound`](upgrade::InboundUpgrade::upgrade_inbound).
- fn upgrade_inbound(self, socket: NegotiatedSubstream, info: upgrade::ProtocolName) -> Self::Future;
+ fn upgrade_inbound(
+ self,
+ socket: NegotiatedSubstream,
+ info: upgrade::ProtocolName,
+ ) -> Self::Future;
}
impl<T> InboundUpgradeSend for T
@@ -108,7 +123,11 @@
type Error = T::Error;
type Future = T::Future;
- fn upgrade_inbound(self, socket: NegotiatedSubstream, info: upgrade::ProtocolName) -> Self::Future {
+ fn upgrade_inbound(
+ self,
+ socket: NegotiatedSubstream,
+ info: upgrade::ProtocolName,
+ ) -> Self::Future {
upgrade::InboundUpgrade::upgrade_inbound(self, socket, info)
}
}
@@ -134,7 +153,11 @@
type Error = T::Error;
type Future = T::Future;
- fn upgrade_outbound(self, socket: NegotiatedSubstream, info: upgrade::ProtocolName) -> Self::Future {
+ fn upgrade_outbound(
+ self,
+ socket: NegotiatedSubstream,
+ info: upgrade::ProtocolName,
+ ) -> Self::Future {
OutboundUpgradeSend::upgrade_outbound(self.0, socket, info)
}
}
@@ -144,7 +167,11 @@
type Error = T::Error;
type Future = T::Future;
- fn upgrade_inbound(self, socket: NegotiatedSubstream, info: upgrade::ProtocolName) -> Self::Future {
+ fn upgrade_inbound(
+ self,
+ socket: NegotiatedSubstream,
+ info: upgrade::ProtocolName,
+ ) -> Self::Future {
InboundUpgradeSend::upgrade_inbound(self.0, socket, info)
}
} |
I think that happened because my github token expired a couple days back. Not sure. Sorry. Were you pushing to master or 2966? |
To master, i.e. where this PR is opened from. There should be a checkbox on the right side of this PR, next to the list of "participants" where you can select "allow edits from maintainers". Make sure that is checked! |
|
In any case, did the patch help you? :) |
|
@efarg Friendly ping. Did you manage to make any progress on this? |
|
This pull request has merge conflicts. Could you please resolve them @efarg? 🙏 |
|
Replaced by #3746. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@thomaseizinger