[RFC][WIP] Create StreamServer class

[martin-rueegg]

Hi there

I have not seen any contribution guidelines so I hope this is an acceptable way for a change request. Please kindly let me know if an other way (e.g. creating an issue first) is preferred.

I needed to implement a stream server. Unfortunately I have not found a way to achieve this with your existing classes. Eventually, I wanted to extend your ReadableResourceStream, but given that the properties are private and it is marked final, I decided to create a new base class (AbstractReadableResourceStreamBase) instead and then extend the current ReadableResourceStream and the new StreamServerStream from it. This has the advantage, that the method pipe() is not included in the latter, as it would not be supported anyway (unless all connections would be piped to a new endpoint. But that would require quite a different implementation). As a consequence, I had also done the same move for the appropriate interfaces.

So, basically I did the following:

1. Interfaces
   1. rename ReadableStreamInterface -> ReadableStreamBaseInterface
   2. create new ReadableStreamInterface extending ReadableStreamBaseInterface
   3. move method pipe() from ReadableStreamInterface -> ReadableStreamBaseInterface
   4. create new ReadableStreamInterface extending ReadableStreamBaseInterface
2. Classes
   1. rename ReadableResourceStream -> AbstractReadableResourceStreamBase
   2. with AbstractReadableResourceStreamBase
      • mark abstract, rather than final
      • implementing ReadableStreamBaseInterface, rather than ReadableStreamInterface
   3. create new ReadableResourceStream
      • extending AbstractReadableResourceStreamBase
      • implementing ReadableStreamInterface
      • move method pipe() from AbstractReadableResourceStreamBase -> ReadableResourceStream
   4. create new StreamServerStream
      • extending AbstractReadableResourceStreamBase
      • implementing StreamServerInterface
      • extend __construct() to create new stream_socket_server() from string argument
      • overload handleData() te get the new client stream and emit event new

If you agree, that this would be a useful extension, the following needs to be done (at least):

• [ ] Fix coding style to match project guidelines.
• [ ] Update class, interface and method comments of the server related files
• [ ] in StreamServerStream::__construct()
  • [ ] extended check if a resource is given to make sure it's a server?
  • [ ] allow tcp or other protocols too.
• [ ] Write tests
• [ ] maybe add an option to give a class name to be used to wrap the stream with, and sending that instance with the emit, e.g DuplexResourceStream::class which would then be the type of the first argument to the listeners.
• [ ] ...?

Looking forward to your comments and suggestions.

Thank you for your time, Martin.

[WyriHaximus]

Before diving into your PR, how similar is it to https://github.com/reactphp/socket ?

[martin-rueegg]

Thanks, @WyriHaximus, for your comment.

Actually, it seems to be what UnixServer is doing!

Gosh, I was looking into reactphp/socket last night but could not see it! Now it's there ... Hahaha.

Ok, forget this PR then.

[WyriHaximus]

@martin-rueegg no problem, and thank you for the time putting this together. Quickly skimmed over it and I'm impressed by what I see 👍 .

[martin-rueegg]

hey @WyriHaximus,

you're welcome and thanks for the compliments.

I have one question, which I take the opportunity of this PR to ask:

Why are the classes made final? What is the rationale behind it to not allow developers to extend your classes?

Thanks, Martin.

[WyriHaximus]

@martin-rueegg TL;DR to encourage composition over inheritance: https://github.com/reactphp/stream/commit/49ecaa434e6be851c3bdef225358f53095956d97

[martin-rueegg]

I see. Thanks for your quick responses. Awesome!