[CodeGen Protobuf support] Message serializer and deserializer #1764
Conversation
Motivation: In the generated code we are specifying types for a serializer and deserializer. As we want to support SwiftProtobuf, we need to implement the generic serializer and deserializer for Protobuf messages, which will be the default if the user doesn't specify other IDL serializer and deserializer. Modifications: Created the serializer and deserializer structs conforming to `GRPCCore.MessageSerializer` and `GRPCCOre.MessageDeserializer` respectively and implemented their serialize and deserialize functions. Added tests for them. Result: The generated code can reference protobuf serializer and deserializer types, as they will be supported in GRPC.
Sources/GRPCProtobuf/Coding.swift
Outdated
| let data = Data(serializedMessageBytes) | ||
| let message = try Message(serializedData: data) | ||
| return message |
There was a problem hiding this comment.
I think we can use the contiguous bytes init for Message and skip Data entirely: let message = try Message(contiguousBytes: serializedMessageBytes)
Package.swift
Outdated
| dependencies: [ | ||
| .grpcCore, | ||
| .grpcProtobuf, | ||
| .echoModel |
There was a problem hiding this comment.
We don't need this dependency
There was a problem hiding this comment.
But we do need a .protobuf dependency
| import SwiftProtobuf | ||
| import XCTest | ||
|
|
||
| final class CodingTests: XCTestCase { |
There was a problem hiding this comment.
nit: these names are used in test output, use ProtobufCodingTests instead
| struct TestMessage: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase { | ||
|
|
||
| var text: String = "" | ||
| var unknownFields = SwiftProtobuf.UnknownStorage() | ||
| static var protoMessageName: String = "Test" + ".ServiceRequest" | ||
| init() {} | ||
|
|
||
| mutating func decodeMessage<D>(decoder: inout D) throws where D: SwiftProtobuf.Decoder { | ||
| while let fieldNumber = try decoder.nextFieldNumber() { | ||
| switch fieldNumber { | ||
| case 1: try { try decoder.decodeSingularStringField(value: &self.text) }() | ||
| default: break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func traverse<V>(visitor: inout V) throws where V: SwiftProtobuf.Visitor { | ||
| if !self.text.isEmpty { | ||
| try visitor.visitSingularStringField(value: self.text, fieldNumber: 1) | ||
| } | ||
| try unknownFields.traverse(visitor: &visitor) | ||
| } | ||
|
|
||
| public static func == (lhs: TestMessage, rhs: TestMessage) -> Bool { | ||
| if lhs.text != rhs.text { return false } | ||
| if lhs.unknownFields != rhs.unknownFields { return false } | ||
| return true | ||
| } | ||
|
|
||
| public var isInitialized: Bool { | ||
| if self.text.isEmpty { return false } | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
You don't need to create a message, SwiftProtobuf includes a number of "well known" messages, e.g. https://github.com/apple/swift-protobuf/blob/2540c49bd8b8ab33a4603ce1ae1c33db6e54365b/Sources/SwiftProtobuf/timestamp.pb.swift#L141
There was a problem hiding this comment.
I've created it because I needed to override the "isInitialized" var so there is a time when it is false, in order to have a message that can't be serialised. And since I had to implement the struct for one case, I thought I can use it for all of them.
There was a problem hiding this comment.
Oh right I see, in that case can we take a slightly different approach by using an existing message type for the round-trip test and then having this test message being the bare minimum implementation of SwiftProtobuf.Message which where decode and traverse only throw an error?
| let invalidData = "%%%%%££££".data(using: .utf8) | ||
| let bytes = [UInt8](invalidData!) |
There was a problem hiding this comment.
You can just do let bytes = Array("%%%%%££££".utf8)
| * limitations under the License. | ||
| */ | ||
|
|
||
| import EchoModel |
There was a problem hiding this comment.
We don't need this import
Sources/GRPCProtobuf/Coding.swift
Outdated
| } | ||
| } | ||
|
|
||
| public init() {} |
There was a problem hiding this comment.
Normal style is for the init() to come before other methods (usually it's: properties, init, methods)
Sources/GRPCProtobuf/Coding.swift
Outdated
| } catch let error { | ||
| throw RPCError( | ||
| code: .invalidArgument, | ||
| message: "The message could not be serialized.", |
There was a problem hiding this comment.
nit, we can bit a bit more specific and add the type of the message:
| message: "The message could not be serialized.", | |
| message: "Can't serialize message of type \(type(of: message).", |
Sources/GRPCProtobuf/Coding.swift
Outdated
| } catch let error { | ||
| throw RPCError( | ||
| code: .invalidArgument, | ||
| message: "The data could not be deserialized into a Message.", |
There was a problem hiding this comment.
Same here, we can be a bit more specific: "Can't deserialize to message of type \(Message.self)"
| } | ||
| } | ||
|
|
||
| struct TestMessage: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase { |
There was a problem hiding this comment.
_MessageImplementationBase is prefixed with an underscore which means it's not part of the public API so you shouldn't be using it
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Two tiny nits but looks good otherwise
…1764) Motivation: In the generated code we are specifying types for a serializer and deserializer. As we want to support SwiftProtobuf, we need to implement the generic serializer and deserializer for Protobuf messages, which will be the default if the user doesn't specify other IDL serializer and deserializer. Modifications: Created the serializer and deserializer structs conforming to `GRPCCore.MessageSerializer` and `GRPCCOre.MessageDeserializer` respectively and implemented their serialize and deserialize functions. Added tests for them. Result: The generated code can reference protobuf serializer and deserializer types, as they will be supported in GRPC.
2 participants
Motivation:
In the generated code we are specifying types for a serializer and deserializer. As we want to support SwiftProtobuf, we need to implement the generic serializer and deserializer for Protobuf messages, which will be the default if the user doesn't specify other IDL serializer and deserializer.
Modifications:
Created the serializer and deserializer structs conforming to
GRPCCore.MessageSerializerandGRPCCOre.MessageDeserializerrespectively and implemented theirserialize()anddeserialize()functions. Added tests for them.Result:
The generated code can reference protobuf serializer and deserializer types, as they will be supported in GRPC.