Crystal: Refactor Sockets API

[straight-shoota]

See #4277 (issue-comment) for the initial idea.

There are quite a few errors in the API design related to sockets:

TCPServer inherits from TCPSocket which is conceptually true, but TCPSocket is a Socket which inherits from IO - yet a server cannot be used as an IO. The same is true for UNIXServer.

UDPSocket also inherits from Socket and IO, yet cannot be used as an IO either since UDP communication does not work as a stream.

IPSocket as parent of UDPSocket and TCPSocket defines accessors local_address and remote_address, yet TCPServer does not have single remote address. Neither does a UDPSocket in listening mode.

While it's not such a big issue, I would also suggest to put all these types in the Socket namespace. These socket types are the only non-basic types in the global name space. And large parts of the API are already in the Socket namespace. TCPSocket -> Socket::TCP, TCPServer -> Socket::Server::TCP (or Socket::TCPServer) etc. I will use namespaced types from now on but that should not influence the logical considerations.

TCP and UNIX sockets should be an IO because they are essentially streaming interfaces. The basic Socket however should not be an IO because Socket::UDP cannot be used as an IO. This would call for an intermediary class Socket::IO as parent for Socket::TCP and Socket::UNIX which inherits from IO.

• Socket
  
  
  
  • Socket::UDP
  
  
  • Socket::IO < IO
  
  
  • Socket::TCP
  
  
  • Socket::UNIX
  
  

I'm not entirely sure about Socket as a base type. It needs to be a module because IO is a class, but I don't know if it has any useful functionality common to streaming and non-streaming sockets. It's main purpose is to give access to lower level socket functions.

I'm also not sure we need IPSocket at all, it adds very little.

The server sockets can't inherit from Socket::TCP and Socket::UNIX so they're don't need to be related. It could be considered having a module which can be included by both, but I don't think there is much commonality, at least not for the public API.

• Socket::Server
  
  
  
  • Socket::Server::TCP
  
  
  • Socket::Server::UNIX
  
  

[bew]

May be related: https://github.com/crystal-lang/crystal/issues/4317#issuecomment-295789662

[straight-shoota]

@bew yeah, this assumes UNIX sockets are only used as streams. Anything else doesn't make sense.

[straight-shoota]

Additionally, I would suggest that all sockets and servers can be created using appropriate Socket::IPAddress or Socket::UNIXAddress as constructor parameters.

Maybe these could also be renamed to Socket::Address::IP and Socket::Address::UNIX to clean up the namespace and follow the same naming as for sockets and servers.

Socket and Socket::Server could have as static method .new which receives a Socket::Address and creates the specific type based on the address type. A nice addition would be a parser for Socket::Address which receives a string and creates the appropriate type based on whether it contains a host + port or a unix path.
This would make it very easy to use a simple string as a config setting for users to transparently choose either a UNIX or TCP connection type (see also #2735 (issue-comment)).

[ysbaddaden]

I can hear the conceptual requirement that TCPServer and UNIXServer shouldn't eventually inherit from IO, and it's maybe possible to achieve. I'll welcome a solution that doesn't merely wrap a TCPSocket and delegates some selected methods, but let's stay focused on this single problem.

Honestly, introducing many namespaces only introduces complicity for it's own sake and doesn't solve anything. For example Socket::Server::TCP spells hideously when compared to TCPServer —the OpenSSL namespaces are a perfect example of how hideous and user unfriendly it can lead to (you can't remember proper names without looking at documentation).

Last but not least, the TCPServer or UNIXSocket classes are simple abstractions for very common usages (e.g. start a TCP server, connect to an UNIX socket, send/receive a message over UDP). Uncommon usages must use Socket directly. For example UNIX DGRAM sockets are so uncommon we only have one single reported use case (wpa_supplicant): we musn't bother about it (use Socket).

[ysbaddaden]

Listing problems is fine, but detailing what's accessible to each level (general / TCP / UDP and UNIX sockets and servers) would be even better, especially since the underlying system implementation just mixes everything —so Socket should be capable to intermix everything, even being an IO, because of advanced usages, such as Bluetooth or RAW sockets.

That would really help to map what should be accessible, what shouldn't, and so on, and decide whether such a refactor makes sense, or if it will inexorably lead to a horrible implementation (because system sockets are an horrible blob).

[bararchy]

Right now working on Bluetooth socket , its a hacky ride because of current implementation.
I would love to somehow remove the AF settings from an enum

[straight-shoota]

The only solution to avoid convoluted APIs is to separate the low-level API currently provided by Socket from the more specific implementations. The low-level API needs to be able to provide access to everything you can possibly do. But TCPSocket needs no methods for sending and receiving datagrams for example.

Implementationwise, this could be achieved by wrapping the low-level API in the higher-level classes, though that would not be particularly great. Another approach could be non-public modules that provide the required functionality. But I guess that's gonna cause some issues, just thinking about API docs. Alternatively, we could have a very basic, internal API similar to the APIs in the Crystal::System namespace which is used by both the low-level and high-level types.

I'll try to sketch the basics of each type, leaving out any detailed configuration settings to get a more clear overview:

(we could probably have a module Socket::TCPSettings and include it in Socket::TCP and Socket::Server::TCP to apply tcp settings for both server and client sockets)

abstract class Socket::IO < IO::Syscall
  getter file_descriptor : Int32

  def self.open(*args, **kwargs, &block : self ->) # individual implementations in subclasses
end

class Socket::TCP < Socket::IO
  getter local_address : Socket::Address::IP
  getter remote_address : Socket::Address::IP

  def self.new(address : IPAddress, local_address : IPAddress = nil)
  def self.new(host : String, port : Int32)
end

class Socket::UNIX < Socket::IO
  getter remote_address : Socket::Address::UNIX?

  def self.pair : {self, self}
end

class Socket::UDP
  property? broadcast : Bool
  getter remote_address : Socket::Address:IP?
  getter local_address : Socket::Address:IP?

  def self.new
  def self.new(address : IPAddress)

  def file_descriptor : Int32

  def connect(address : IPAddress)
  def connect(host : String, port : Int32)
  def connected?
  def disconnect

  def bind(address : Socket::Address::IP)
  def bound? : Bool
  def close

  def send(message, address : Socket::Address::IP? = nil)
  def receive(message : Bytes), {Int32, Socket::Address::IP}
end

And the servers:

abstract class Socket::Server
  getter local_address : Socket::Address

  def accept(&block : Socket::IO ->)
  def accept?(&block : Socket::IO ->)
  def accept? : Socket::IO?

  def close
end

class Socket::Server::TCP < Socket::Server
  getter file_descriptor : Int32
  getter local_address : Socket::Address:IP

  def self.new(address : Socket::Address::IP, backlog = nil, reuse_port = false)
  def self.new(host : String, port : Int32, backlog = nil, reuse_port = false)
  def self.new(port : Int32, backlog = nil, reuse_port = false)
end

class Socket::Server::UNIX < Socket::Server
  getter file_descriptor : Int32
  getter local_address : Socket::Address:UNIX

  def self.new(address : Socket::Address::UNIX, backlog = nil)
  def self.new(path : String, backlog= nil)

  def close(delete = true)
end

[RX14]

this could be achieved by wrapping the low-level API in the higher-level classes, though that would not be particularly great

Why not just do this? Have a big low-level Socket class, and then a TCPServer struct which wraps a pointer to that socket and makes it usable.