Reorder some of the server logic.

[glbrntt]

Motivation:

Recently we introduced the GRPCPayload protocol to abstract over the
request and response payload types so that we can support message types
other than Protobuf (such as Flatbuffers).

However, in doing so we removed our previous assumption that the payload
would (de)serialize from and to a Data. To avoid the needless copying
back and forth the payload writing (i.e. length prefixing) was moved to
the server codec (where serializion happens). This however requires the
type of the message to be known and therefore must happen after the
pipeline has been configured for the RPC in question.

In order to support compression on the server the message readers and
writers must be aware of the encoding being used and require access to
the headers. This happens (/happened) in the HTTP1ToRawGRPCServerCodec
before the RPC is configured. It would be possible to pass this through
with a user-event or extra data in the message type but would result in
fairly difficult to follow code with state spread across multiple
handlers.

Modifications:

Reorder the server channel operations.

• Previously the server channel did:
  1. verify content-type, handle message framing (HTTP1ToRawGRPCServerCodec)
  2. requests are routed (GRPCChannelHandler)
  3. requests are converted to typed gRPC messages (GRPCServerCodec)
  4. requests are passed to user handlers
• Now the server does:
  1. requests are routed and validated as gRPC
     (GRPCServerRequestRoutingHandler)
  2. requests are converted from HTTP/1, message framing is handler, messages
     are converted typed gRPC messages (HTTP1ToGRPCServerCodec)
  3. requests are passed to user handlers

As such:

• GRPCChannelHandler was renamed GRPCServerRequestRoutingHandler
• GRPCServerRequestRoutingHandler now reads HTTPServerRequestPart and
  writes HTTPServerResponsePart
• GRPCServerRequestRoutingHandler verifies the content-type and then
  constructs the pipeline
• HTTP1ToRawGRPCServerCodec is renamed HTTP1ToGRPCServerCodec and is
  now generic over the request and response type
• HTTP1ToGRPCServerCodec now comes after GRPCServerRequestRoutingHandler
• HTTP1ToGRPCServerCodec deals with message framing as well as
  (de/)serializion
• The HTTPRequestHead is now passed from the HTTP1ToGRPCServerCodec
  to the BaseCallHandler, which now passes it to the appropriate call
  handler (instead of passing it through the generated code as part of
  the call context).
• Tests updated, dead code removed.

This also paves the way for providing a server state machine, similar to
the client (i.e. it would essentially replace the contents of
HTTP1ToGRPCServerCodec).

Result:

Functionally, there should be no change. However this change makes it
possible for compression to be added to the server.

[MrMage]

Good stuff! Really couldn't find much that could be improved.

[MrMage]

Would this comment still be relevant?

[glbrntt]

I don't think so: it's now no longer possible to make the observer until processHead because we don't pass it in the CallHandlerContext. The problem we had before was that the status promise could fail before the handler reached the pipeline; now it must be in the pipeline when the observer is created.

[MrMage]

But the delaying part kind of still applies, doesn't it? That's what I was thinking about. I.e. just explain why we need a factory at all.

[glbrntt]

Oh I see, makes sense to explain that then.

[MrMage]

nit: I prefer to put the expectation first (also elsewhere I guess) — that's what I learned at Google. I don't want to force my will onto the entire project, though.

[glbrntt]

I do it this way for practical reasons: the type inference comes from the first arg in XCTAssertEqual.

There are two useful aspects to this:

• if the left hand side is non-optional, then the right hand side must be also be non-options. If the left hand side is optional, the right hand side doesn't have to be optional.
• the type inference saves some typing; i.e. we can have .internalError instead of GRPCStatus.Code.internalError

[MrMage]

I could have sworn that I had both of these in my tests with the other order as well, are you sure?

E.g. I thought had written tests like this in the past:

XCTAssertEqual(0, optionalInt)
XCTAssertEqual(.someCase, someEnum)

[glbrntt]

Not so sure now, no. Seems to work. I definitely hit this problem in the past, maybe there was a bug before 🤷‍♂️

[MrMage]

Good job! Leaving the remaining comments to your discretion.