Conversation
| self.children = Self.children(from: cborValue, parent: self) | ||
| } | ||
|
|
||
| public static func from(data: Data) throws -> Reader { |
There was a problem hiding this comment.
this function calls CBORDecoder which is defined in CRT for actually decoding individual types
| return Reader(nodeInfo: "", cborValue: rootValue, parent: nil) | ||
| } | ||
|
|
||
| private static func children(from cborValue: CBORType?, parent: Reader) -> [Reader] { |
There was a problem hiding this comment.
This function helps form a graph for nested and complex types
| } | ||
| } | ||
|
|
||
| public func writeMap<T>( |
There was a problem hiding this comment.
map and array encoding is particularly complex due to the possibility of nested types. A map can be within an array and vice versa
| try valueWritingClosure(val, writer) | ||
|
|
||
| // If the writer itself doesn't have a cborValue, build it from its children | ||
| if writer.cborValue == nil, !writer.children.isEmpty { |
There was a problem hiding this comment.
its possible cborValue wasnt yet assigned, in that case let's move on and parse the children first then come back
| import Foundation | ||
| @_spi(SmithyReadWrite) import SmithyCBOR | ||
|
|
||
| public struct CBORComparator { |
There was a problem hiding this comment.
this is solely used for testing. we need to compare the semantic values rather than the exact binary values. CRT should have sufficiently tested their de-serializer against SEP prescribed tests to ensure that using the Decoder in protocol test comparison is ok
There was a problem hiding this comment.
Is the testing in CRT side done already? If not, what's the known timeline?
...main/kotlin/software/amazon/smithy/swift/codegen/integration/HTTPBindingProtocolGenerator.kt
Show resolved
Hide resolved
| val bodyContent = test.body.get().replace("\\\"", "\\\\\"") | ||
| val data: String = if (isCbor) { | ||
| // Attempt to decode Base64 data once for CBOR | ||
| try { |
There was a problem hiding this comment.
CBOR data is binary data but for protocol tests it need to be passed as a base64 encoded string
| import Foundation | ||
| @_spi(SmithyReadWrite) import SmithyCBOR | ||
|
|
||
| public struct CBORComparator { |
There was a problem hiding this comment.
Is the testing in CRT side done already? If not, what's the known timeline?
jbelkins
left a comment
jbelkins
left a comment
There was a problem hiding this comment.
Some minor requested changes in the comments. Overall this is excellent, CBOR falls pretty nicely into the frameworks we already have for JSON, XML, etc.
| let readerA = try Reader.from(data: dataA) | ||
| let readerB = try Reader.from(data: dataB) | ||
| return compareReaders(readerA, readerB) | ||
| } |
There was a problem hiding this comment.
This type looks to me like it does the right things, but a few unit tests around it to verify basic cases around lists, maps, etc. would be useful.
smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/SwiftSettings.kt
Show resolved
Hide resolved
...main/kotlin/software/amazon/smithy/swift/codegen/integration/HTTPBindingProtocolGenerator.kt
Show resolved
Hide resolved
...software/amazon/smithy/swift/codegen/integration/protocols/core/StaticHttpBindingResolver.kt
Outdated
Show resolved
Hide resolved
...main/kotlin/software/amazon/smithy/swift/codegen/integration/serde/readwrite/ServiceUtils.kt
Show resolved
Hide resolved
...en/src/main/kotlin/software/amazon/smithy/swift/codegen/swiftmodules/SmithyReadWriteTypes.kt
Outdated
Show resolved
Hide resolved
3 participants
Issue #
Description of changes
Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.