Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

feat: refactor peer class #63

Merged
merged 6 commits into from
Jul 16, 2020
Merged

Conversation

wemeetagain
Copy link
Member

@wemeetagain wemeetagain commented Jul 15, 2020

Resolves #54 and #56

  • Peer has been renamed/replaced with PeerStreams class that manages incoming and outgoing streams.
  • _addPeer and _removePeer have been slightly modified to suit - _removePeer closes a peer's streams
  • getSubscribers now uses this.topics rather than peer.topics to determine the subscribers for a topic

PeerStreams manages:

  • id - the PeerId of the peer
  • protocol - the protocol negotiated with the peer (previously was an array of protocols, which is described in Outgoing peer connections - peer protocols #54 )
  • _rawOutboundStream (previously named this.conn) - this is the stream returned from conn.newStream
  • outboundStream (previously named this.stream) - this is the pushable stream to write outgoing messages to. Data pushed here is sent to _rawOutboundStream after first being transformed with a length-prefix encoding transform.
    It now manages an inbound stream as well
  • _rawInboundStream - this is the stream returned in the callback attached to registrar.handle. Called when a peer dials us.
  • inboundStream - this is an abortable stream with first a length-prefix decoding tranform applied. This is the stream that should be read from to receive incoming rpc messages.
  • async attachOutboundStream(stream) - method to attach a new outgoing stream for management. If the stream is the first outgoing stream to be attached, this method triggers a stream:outbound event. If there is already an existing outgoing stream, the existing stream is gently shutdown (without triggering a close event) first, then the new stream is attached.
  • attachInboundStream(stream) - method to attach a new incoming stream for management. If the stream is the first incoming stream to be attached, this method triggers a stream:inbound event. If there is already an existing incoming stream, the existing stream is gently shutdown (without triggering a close event) first, then the new stream is attached.
  • write(data) - convenience method to push data to the outboundStream, if it exists. If it doesn't, this method throws an error.
  • close() - method to shutdown both incoming and outgoing streams, triggers a close event. In conjunction with the base Pubsub impl, the close event will _removePeer.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!!

src/index.js Outdated
@@ -166,15 +168,16 @@ class PubsubBaseProtocol extends EventEmitter {
* @private
* @param {Object} props
* @param {string} props.protocol
* @param {DuplexStream} props.strean
* @param {Stream} props.stream
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param {Stream} props.stream
* @param {IterableStream} props.stream

Per https://gist.github.com/alanshaw/591dc7dd54e4f99338a347ef568d6ee9, we should call this an IterableStream. This is not a node stream and might create confusion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I called it DuplexIterableStream so its known to be a duplex object ({ sink, source })

src/index.js Outdated Show resolved Hide resolved
src/peerStreams.js Outdated Show resolved Hide resolved
src/peerStreams.js Outdated Show resolved Hide resolved
src/peerStreams.js Outdated Show resolved Hide resolved
src/peerStreams.js Outdated Show resolved Hide resolved
src/peerStreams.js Outdated Show resolved Hide resolved
@wemeetagain wemeetagain changed the base branch from master to v0.6.x July 16, 2020 18:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outgoing peer connections - peer protocols
2 participants