Explorar el Código

Make 'GRPCStatus.Code' an enum (#942)

Motivation:

'GRPCStatus.Code' came straight from the old gRPC Swift implementation,
which in turn was copied almost verbatim from the grpc core library. It
includes the case 'doNotUse' which is documented as existing to force
users to include a default branch. Presumably, this is to ensure that
cases can be added in the future without breaking API. Unfortunately
that just doesn't cut it in Swift and we can enforce that a default
branch is provide by using a struct. This also allows us to add
additional status codes should any be added to the spec in the future.

Modifications:

- Turn `GRPCStatus.Code` into a `struct`, removing the `doNotUse` case
- Add a deprecated `doNotUse` case to the shims
- Add tests

Result:

- Programattically force a default branch when switching over
  'GRPCStatus.Code'
George Barnett hace 5 años
padre
commit
f21f8a5c57

+ 78 - 25
Sources/GRPC/GRPCStatus.swift

@@ -64,9 +64,9 @@ extension GRPCStatus: Equatable {
 extension GRPCStatus: CustomStringConvertible {
   public var description: String {
     if let message = message {
-      return "\(self.code) (\(self.code.rawValue)): \(message)"
+      return "\(self.code): \(message)"
     } else {
-      return "\(self.code) (\(self.code.rawValue))"
+      return "\(self.code)"
     }
   }
 }
@@ -74,53 +74,64 @@ extension GRPCStatus: CustomStringConvertible {
 extension GRPCStatus {
   /// Status codes for gRPC operations (replicated from `status_code_enum.h` in the
   /// [gRPC core library](https://github.com/grpc/grpc)).
-  public enum Code: Int {
+  public struct Code: Hashable, CustomStringConvertible {
+    public let rawValue: Int
+
+    public init?(rawValue: Int) {
+      switch rawValue {
+      case 0 ... 16:
+        self.rawValue = rawValue
+      default:
+        return nil
+      }
+    }
+
+    private init(_ code: Int) {
+      self.rawValue = code
+    }
+
     /// Not an error; returned on success.
-    case ok = 0
+    public static let ok = Code(0)
 
     /// The operation was cancelled (typically by the caller).
-    case cancelled = 1
+    public static let cancelled = Code(1)
 
     /// Unknown error. An example of where this error may be returned is if a
     /// Status value received from another address space belongs to an error-space
     /// that is not known in this address space. Also errors raised by APIs that
     /// do not return enough error information may be converted to this error.
-    case unknown = 2
+    public static let unknown = Code(2)
 
     /// Client specified an invalid argument. Note that this differs from
     /// FAILED_PRECONDITION. INVALID_ARGUMENT indicates arguments that are
     /// problematic regardless of the state of the system (e.g., a malformed file
     /// name).
-    case invalidArgument = 3
+    public static let invalidArgument = Code(3)
 
     /// Deadline expired before operation could complete. For operations that
     /// change the state of the system, this error may be returned even if the
     /// operation has completed successfully. For example, a successful response
     /// from a server could have been delayed long enough for the deadline to
     /// expire.
-    case deadlineExceeded = 4
+    public static let deadlineExceeded = Code(4)
 
     /// Some requested entity (e.g., file or directory) was not found.
-    case notFound = 5
+    public static let notFound = Code(5)
 
     /// Some entity that we attempted to create (e.g., file or directory) already
     /// exists.
-    case alreadyExists = 6
+    public static let alreadyExists = Code(6)
 
     /// The caller does not have permission to execute the specified operation.
     /// PERMISSION_DENIED must not be used for rejections caused by exhausting
     /// some resource (use RESOURCE_EXHAUSTED instead for those errors).
     /// PERMISSION_DENIED must not be used if the caller can not be identified
     /// (use UNAUTHENTICATED instead for those errors).
-    case permissionDenied = 7
-
-    /// The request does not have valid authentication credentials for the
-    /// operation.
-    case unauthenticated = 16
+    public static let permissionDenied = Code(7)
 
     /// Some resource has been exhausted, perhaps a per-user quota, or perhaps the
     /// entire file system is out of space.
-    case resourceExhausted = 8
+    public static let resourceExhausted = Code(8)
 
     /// Operation was rejected because the system is not in a state required for
     /// the operation's execution. For example, directory to be deleted may be
@@ -140,14 +151,14 @@ extension GRPCStatus {
     ///      REST Get/Update/Delete on a resource and the resource on the
     ///      server does not match the condition. E.g., conflicting
     ///      read-modify-write on the same resource.
-    case failedPrecondition = 9
+    public static let failedPrecondition = Code(9)
 
     /// The operation was aborted, typically due to a concurrency issue like
     /// sequencer check failures, transaction aborts, etc.
     ///
     /// See litmus test above for deciding between FAILED_PRECONDITION, ABORTED,
     /// and UNAVAILABLE.
-    case aborted = 10
+    public static let aborted = Code(10)
 
     /// Operation was attempted past the valid range. E.g., seeking or reading
     /// past end of file.
@@ -162,27 +173,69 @@ extension GRPCStatus {
     /// OUT_OF_RANGE. We recommend using OUT_OF_RANGE (the more specific error)
     /// when it applies so that callers who are iterating through a space can
     /// easily look for an OUT_OF_RANGE error to detect when they are done.
-    case outOfRange = 11
+    public static let outOfRange = Code(11)
 
     /// Operation is not implemented or not supported/enabled in this service.
-    case unimplemented = 12
+    public static let unimplemented = Code(12)
 
     /// Internal errors. Means some invariants expected by underlying System has
     /// been broken. If you see one of these errors, Something is very broken.
-    case internalError = 13
+    public static let internalError = Code(13)
 
     /// The service is currently unavailable. This is a most likely a transient
     /// condition and may be corrected by retrying with a backoff.
     ///
     /// See litmus test above for deciding between FAILED_PRECONDITION, ABORTED,
     /// and UNAVAILABLE.
-    case unavailable = 14
+    public static let unavailable = Code(14)
 
     /// Unrecoverable data loss or corruption.
-    case dataLoss = 15
+    public static let dataLoss = Code(15)
+
+    /// The request does not have valid authentication credentials for the
+    /// operation.
+    public static let unauthenticated = Code(16)
 
-    /// Force users to include a default branch:
-    case doNotUse = -1
+    public var description: String {
+      switch self {
+      case .ok:
+        return "ok (\(self.rawValue))"
+      case .cancelled:
+        return "cancelled (\(self.rawValue))"
+      case .unknown:
+        return "unknown (\(self.rawValue))"
+      case .invalidArgument:
+        return "invalid argument (\(self.rawValue))"
+      case .deadlineExceeded:
+        return "deadline exceeded (\(self.rawValue))"
+      case .notFound:
+        return "not found (\(self.rawValue))"
+      case .alreadyExists:
+        return "already exists (\(self.rawValue))"
+      case .permissionDenied:
+        return "permission denied (\(self.rawValue))"
+      case .resourceExhausted:
+        return "resource exhausted (\(self.rawValue))"
+      case .failedPrecondition:
+        return "failed precondition (\(self.rawValue))"
+      case .aborted:
+        return "aborted (\(self.rawValue))"
+      case .outOfRange:
+        return "out of range (\(self.rawValue))"
+      case .unimplemented:
+        return "unimplemented (\(self.rawValue))"
+      case .internalError:
+        return "internal error (\(self.rawValue))"
+      case .unavailable:
+        return "unavailable (\(self.rawValue))"
+      case .dataLoss:
+        return "data loss (\(self.rawValue))"
+      case .unauthenticated:
+        return "unauthenticated (\(self.rawValue))"
+      default:
+        return String(describing: self.rawValue)
+      }
+    }
   }
 }
 

+ 5 - 0
Sources/GRPC/Shims.swift

@@ -401,3 +401,8 @@ extension BidirectionalStreamingCallHandler {
 
 @available(*, deprecated, message: "This protocol is longer required. Please regenerate your code.")
 public protocol GRPCProtobufPayload {}
+
+extension GRPCStatus.Code {
+  @available(*, deprecated, message: "Use a 'default' branch")
+  public static let doNotUse = GRPCStatus.Code.unknown
+}

+ 24 - 1
Tests/GRPCTests/GRPCStatusCodeTests.swift

@@ -102,7 +102,7 @@ class GRPCStatusCodeTests: GRPCTestCase {
   }
 
   func testStatusCodeAndMessageAreRespectedForNon200Responses() throws {
-    let status = GRPCStatus(code: .doNotUse, message: "Not the HTTP error phrase")
+    let status = GRPCStatus(code: .unknown, message: "Not the HTTP error phrase")
 
     let headers: HPACKHeaders = [
       ":status": "\(HTTPResponseStatus.imATeapot.code)",
@@ -122,4 +122,27 @@ class GRPCStatusCodeTests: GRPCTestCase {
       XCTAssertEqual(invalidHTTPStatus.makeGRPCStatus(), status)
     }
   }
+
+  func testCodeFromRawValue() {
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 0), .ok)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 1), .cancelled)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 2), .unknown)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 3), .invalidArgument)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 4), .deadlineExceeded)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 5), .notFound)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 6), .alreadyExists)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 7), .permissionDenied)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 8), .resourceExhausted)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 9), .failedPrecondition)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 10), .aborted)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 11), .outOfRange)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 12), .unimplemented)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 13), .internalError)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 14), .unavailable)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 15), .dataLoss)
+    XCTAssertEqual(GRPCStatus.Code(rawValue: 16), .unauthenticated)
+
+    XCTAssertNil(GRPCStatus.Code(rawValue: -1))
+    XCTAssertNil(GRPCStatus.Code(rawValue: 17))
+  }
 }

+ 3 - 3
Tests/GRPCTests/GRPCStatusTests.swift

@@ -29,7 +29,7 @@ class GRPCStatusTests: GRPCTestCase {
     )
 
     XCTAssertEqual(
-      "internalError (13)",
+      "internal error (13)",
       String(describing: GRPCStatus(code: .internalError, message: nil))
     )
   }
@@ -41,12 +41,12 @@ class GRPCStatusTests: GRPCTestCase {
     )
 
     XCTAssertEqual(
-      "resourceExhausted (8): a resource was exhausted",
+      "resource exhausted (8): a resource was exhausted",
       String(describing: GRPCStatus(code: .resourceExhausted, message: "a resource was exhausted"))
     )
 
     XCTAssertEqual(
-      "failedPrecondition (9): invalid state",
+      "failed precondition (9): invalid state",
       String(describing: GRPCStatus(code: .failedPrecondition, message: "invalid state"))
     )
   }

+ 1 - 0
Tests/GRPCTests/XCTestManifests.swift

@@ -582,6 +582,7 @@ extension GRPCStatusCodeTests {
     static let __allTests__GRPCStatusCodeTests = [
         ("testBadGateway", testBadGateway),
         ("testBadRequest", testBadRequest),
+        ("testCodeFromRawValue", testCodeFromRawValue),
         ("testForbidden", testForbidden),
         ("testGatewayTimeout", testGatewayTimeout),
         ("testNotFound", testNotFound),