Simplify compression representation. (#688)

Motivation:

Since we don't support compression, there's no need for any scaffolding
that we do have to be public. Moreover there's no need to over
complicate something which isn't yet user facing.

Modifications:

Remove `CompressionMechanism` and replace it with
`CompressionAlgorithm`; an enum containing all compression algorithms we
support. At the moment this is limited to `identity`.

Result:

Simpler code, smaller public API.

Sources/GRPC/CompressionAlgorithm.swift

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2020, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/// Supported message compression algorithms.
+///
+/// These algorithms are indicated in the "grpc-encoding" header. As such, a lack of "grpc-encoding"
+/// header indicates that there is no message compression.
+enum CompressionAlgorithm: String {
+  /// Identity compression; "no" compression but indicated via the "grpc-encoding" header.
+  case identity
+}

Sources/GRPC/CompressionMechanism.swift

@@ -1,76 +0,0 @@
-/*
- * Copyright 2019, gRPC Authors All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import Foundation
-
-public enum CompressionError: Error {
-  case unsupported(CompressionMechanism)
-}
-
-/// The mechanism to use for message compression.
-public enum CompressionMechanism: String, CaseIterable {
-  // No compression was indicated.
-  case none
-
-  // Compression indicated via a header.
-  case gzip
-  case deflate
-  case snappy
-  // This is the same as `.none` but was indicated via a "grpc-encoding" and may be listed
-  // in the "grpc-accept-encoding" header. If this is the compression mechanism being used
-  // then the compression flag should be indicated in length-prefxied messages (see
-  // `LengthPrefixedMessageReader`).
-  case identity
-
-  // Compression indicated via a header, but not one listed in the specification.
-  case unknown
-
-  init(value: String?) {
-    self = value.map { CompressionMechanism(rawValue: $0) ?? .unknown } ?? .none
-  }
-
-  /// Whether the compression flag in gRPC length-prefixed messages should be set or not.
-  ///
-  /// See `LengthPrefixedMessageReader` for the message format.
-  public var requiresFlag: Bool {
-    switch self {
-    case .none:
-      return false
-
-    case .identity, .gzip, .deflate, .snappy, .unknown:
-      return true
-    }
-  }
-
-  /// Whether the given compression is supported.
-  public var supported: Bool {
-    switch self {
-    case .identity, .none:
-      return true
-
-    case .gzip, .deflate, .snappy, .unknown:
-      return false
-    }
-  }
-
-  /// A string containing the supported compression mechanisms to list in the "grpc-accept-encoding" header.
-  static let acceptEncodingHeader: String = {
-    return CompressionMechanism
-      .allCases
-      .filter { $0.supported && $0.requiresFlag }
-      .map { $0.rawValue }
-      .joined(separator: ", ")
-  }()
-}

Sources/GRPC/GRPCClientStateMachine.swift

@@ -561,17 +561,20 @@ extension GRPCClientStateMachine.State {
     }
 
     // What compression mechanism is the server using, if any?
-    let compression = CompressionMechanism(value: headers.first(name: GRPCHeaderName.encoding))
+    let compression: CompressionAlgorithm?
 
-    // From: https://github.com/grpc/grpc/blob/master/doc/compression.md
-    //
-    // "If a server sent data which is compressed by an algorithm that is not supported by the
-    // client, an INTERNAL error status will occur on the client side."
-    guard compression.supported else {
-      return .failure(.unsupportedMessageEncoding(compression.rawValue))
+    if let encodingHeader = headers.first(name: GRPCHeaderName.encoding) {
+      compression = CompressionAlgorithm(rawValue: encodingHeader)
+      // The algorithm isn't supported.
+      if compression == nil {
+        return .failure(.unsupportedMessageEncoding(encodingHeader))
+      }
+    } else {
+      // No compression was specified, this is fine.
+      compression = nil
     }
 
-    let reader = LengthPrefixedMessageReader(compressionMechanism: compression)
+    let reader = LengthPrefixedMessageReader(compression: compression)
     return .success(.reading(arity, reader))
   }
 

Sources/GRPC/HTTP1ToRawGRPCServerCodec.swift

@@ -35,7 +35,7 @@ public enum _RawGRPCServerResponsePart {
 
 /// A simple channel handler that translates HTTP1 data types into gRPC packets, and vice versa.
 ///
-/// This codec allows us to use the "raw" gRPC protocol on a low level, with further handlers operationg the protocol
+/// This codec allows us to use the "raw" gRPC protocol on a low level, with further handlers operating the protocol
 /// on a "higher" level.
 ///
 /// We use HTTP1 (instead of HTTP2) primitives, as these are easier to work with than raw HTTP2
@@ -50,8 +50,7 @@ public final class HTTP1ToRawGRPCServerCodec {
     var accessLog = Logger(subsystem: .serverAccess)
     accessLog[metadataKey: MetadataKey.requestID] = logger[metadataKey: MetadataKey.requestID]
     self.accessLog = accessLog
-
-    self.messageReader = LengthPrefixedMessageReader(compressionMechanism: .none)
+    self.messageReader = LengthPrefixedMessageReader()
   }
 
   // 1-byte for compression flag, 4-bytes for message length.

Sources/GRPC/LengthPrefixedMessageReader.swift

@@ -32,10 +32,10 @@ import Logging
 /// [gRPC Protocol](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md)
 internal struct LengthPrefixedMessageReader {
   /// The mechanism that messages will be compressed with.
-  var compressionMechanism: CompressionMechanism
+  var compression: CompressionAlgorithm?
 
-  init(compressionMechanism: CompressionMechanism) {
-    self.compressionMechanism = compressionMechanism
+  init(compression: CompressionAlgorithm? = nil) {
+    self.compression = compression
   }
 
   /// The result of trying to parse a message with the bytes we currently have.
@@ -150,7 +150,7 @@ internal struct LengthPrefixedMessageReader {
   }
 
   private func handleCompressionFlag(enabled flagEnabled: Bool) throws {
-    if flagEnabled && !(self.compressionMechanism.requiresFlag && self.compressionMechanism.supported) {
+    if flagEnabled && self.compression == nil {
       throw GRPCError.CompressionUnsupported().captureContext()
     }
   }

Sources/GRPC/LengthPrefixedMessageWriter.swift

@@ -19,10 +19,15 @@ import NIO
 internal struct LengthPrefixedMessageWriter {
   static let metadataLength = 5
 
-  private let compression: CompressionMechanism
+  /// The compression algorithm to use, if one should be used.
+  private let compression: CompressionAlgorithm?
 
-  init(compression: CompressionMechanism) {
-    precondition(compression.supported, "compression mechanism \(compression) is not supported")
+  /// Whether the compression message flag should be set.
+  private var shouldSetCompressionFlag: Bool {
+    return self.compression != nil
+  }
+
+  init(compression: CompressionAlgorithm? = nil) {
     self.compression = compression
   }
 
@@ -38,7 +43,7 @@ internal struct LengthPrefixedMessageWriter {
     buffer.reserveCapacity(LengthPrefixedMessageWriter.metadataLength + message.count)
 
     //! TODO: Add compression support, use the length and compressed content.
-    buffer.writeInteger(Int8(compression.requiresFlag ? 1 : 0))
+    buffer.writeInteger(Int8(self.shouldSetCompressionFlag ? 1 : 0))
     buffer.writeInteger(UInt32(message.count))
     buffer.writeBytes(message)
   }

Sources/GRPC/ReadWriteStates.swift

@@ -28,7 +28,7 @@ struct PendingWriteState {
   var arity: MessageArity
 
   /// The compression used when writing messages.
-  var compression: CompressionMechanism
+  var compression: CompressionAlgorithm?
 
   /// The 'content-type' being written.
   var contentType: ContentType

Tests/GRPCTests/GRPCClientStateMachineTests.swift

@@ -714,7 +714,7 @@ extension GRPCClientStateMachineTests {
 
     switch stateMachine.state {
     case let .clientActiveServerActive(_, .reading(_, reader)):
-      XCTAssertEqual(reader.compressionMechanism, .identity)
+      XCTAssertEqual(reader.compression, .identity)
     default:
       XCTFail("unexpected state \(stateMachine.state)")
     }
@@ -738,7 +738,7 @@ extension GRPCClientStateMachineTests {
     headers.add(name: "grpc-encoding", value: "not-a-known-compression-(probably)")
 
     stateMachine.receiveResponseHeaders(headers).assertFailure {
-      XCTAssertEqual($0, .unsupportedMessageEncoding("unknown"))
+      XCTAssertEqual($0, .unsupportedMessageEncoding("not-a-known-compression-(probably)"))
     }
   }
 
@@ -930,12 +930,12 @@ extension Result {
 
 extension ReadState {
   static func one() -> ReadState {
-    let reader = LengthPrefixedMessageReader(compressionMechanism: .none)
+    let reader = LengthPrefixedMessageReader()
     return .reading(.one, reader)
   }
 
   static func many() -> ReadState {
-    let reader = LengthPrefixedMessageReader(compressionMechanism: .none)
+    let reader = LengthPrefixedMessageReader()
     return .reading(.many, reader)
   }
 

Tests/GRPCTests/LengthPrefixedMessageReaderTests.swift

@@ -23,7 +23,7 @@ class LengthPrefixedMessageReaderTests: GRPCTestCase {
 
   override func setUp() {
     super.setUp()
-    self.reader = LengthPrefixedMessageReader(compressionMechanism: .none)
+    self.reader = LengthPrefixedMessageReader()
   }
 
   var allocator = ByteBufferAllocator()
@@ -206,24 +206,10 @@ class LengthPrefixedMessageReaderTests: GRPCTestCase {
     self.assertMessagesEqual(expected: [0x00, 0x01], actual: try reader.nextMessage())
   }
 
-  func testNextMessageThrowsWhenCompressionMechanismIsNotSupported() throws {
-    // Unknown should never be supported.
-    reader.compressionMechanism = .unknown
-    XCTAssertFalse(reader.compressionMechanism.supported)
-
-    var buffer = byteBuffer(withBytes: lengthPrefixedTwoByteMessage(withCompression: true))
-    reader.append(buffer: &buffer)
-
-    XCTAssertThrowsError(try reader.nextMessage()) { error in
-      let errorWithContext = error as? GRPCError.WithContext
-      XCTAssertTrue(errorWithContext?.error is GRPCError.CompressionUnsupported)
-    }
-  }
-
   func testNextMessageThrowsWhenCompressionFlagIsSetButNotExpected() throws {
-    // Default compression mechanism is `.none` which requires that no
+    // Default compression mechanism is `nil` which requires that no
     // compression flag is set as it indicates a lack of message encoding header.
-    XCTAssertFalse(reader.compressionMechanism.requiresFlag)
+    XCTAssertNil(self.reader.compression)
 
     var buffer = byteBuffer(withBytes: lengthPrefixedTwoByteMessage(withCompression: true))
     reader.append(buffer: &buffer)
@@ -236,9 +222,7 @@ class LengthPrefixedMessageReaderTests: GRPCTestCase {
 
   func testNextMessageDoesNotThrowWhenCompressionFlagIsExpectedButNotSet() throws {
     // `.identity` should always be supported and requires a flag.
-    reader.compressionMechanism = .identity
-    XCTAssertTrue(reader.compressionMechanism.supported)
-    XCTAssertTrue(reader.compressionMechanism.requiresFlag)
+    reader.compression = .identity
 
     var buffer = byteBuffer(withBytes: lengthPrefixedTwoByteMessage())
     reader.append(buffer: &buffer)

Tests/GRPCTests/XCTestManifests.swift

@@ -477,7 +477,6 @@ extension LengthPrefixedMessageReaderTests {
         ("testNextMessageReturnsNilWhenNotAllMessageBytesAreAvailable", testNextMessageReturnsNilWhenNotAllMessageBytesAreAvailable),
         ("testNextMessageReturnsNilWhenNotAllMessageLengthIsAvailable", testNextMessageReturnsNilWhenNotAllMessageLengthIsAvailable),
         ("testNextMessageThrowsWhenCompressionFlagIsSetButNotExpected", testNextMessageThrowsWhenCompressionFlagIsSetButNotExpected),
-        ("testNextMessageThrowsWhenCompressionMechanismIsNotSupported", testNextMessageThrowsWhenCompressionMechanismIsNotSupported),
         ("testNextMessageWhenMultipleMessagesAreBuffered", testNextMessageWhenMultipleMessagesAreBuffered),
     ]
 }