Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor code cleanup for JSON additions #935

Merged
merged 4 commits into from
Jan 6, 2020
Merged

Minor code cleanup for JSON additions #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a63a7c
Minor code cleanup for JSON additions
tbkka Jan 3, 2020
ad1f7ba
Change `if` to `guard`, test decoding empty string
tbkka Jan 3, 2020
3b33076
Expand JSON test helpers to also verify the UTF-8 data methods
tbkka Jan 3, 2020
e4811ab
Fix whitespace, update comment
tbkka Jan 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions Sources/SwiftProtobuf/Message+JSONAdditions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ extension Message {
public func jsonString(
options: JSONEncodingOptions = JSONEncodingOptions()
) throws -> String {
if let m = self as? _CustomJSONCodable {
return try m.encodedJSONString(options: options)
}
let data = try jsonUTF8Data(options: options)
return String(data: data, encoding: String.Encoding.utf8)!
}
Expand Down Expand Up @@ -90,23 +93,23 @@ extension Message {
) throws {
self.init()
try jsonUTF8Data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
if body.count > 0 {
var decoder = JSONDecoder(source: body, options: options)
if !decoder.scanner.skipOptionalNull() {
try decoder.decodeFullObject(message: &self)
} else if Self.self is _CustomJSONCodable.Type {
if let message = try (Self.self as! _CustomJSONCodable.Type)
.decodedFromJSONNull() {
self = message as! Self
} else {
throw JSONDecodingError.illegalNull
}
// Empty input is valid for binary, but not for JSON.
guard body.count > 0 else {
throw JSONDecodingError.truncated
}
var decoder = JSONDecoder(source: body, options: options)
if decoder.scanner.skipOptionalNull() {
if let customCodable = Self.self as? _CustomJSONCodable.Type,
let message = try customCodable.decodedFromJSONNull() {
self = message as! Self
} else {
throw JSONDecodingError.illegalNull
}
if !decoder.scanner.complete {
throw JSONDecodingError.trailingGarbage
}
} else {
try decoder.decodeFullObject(message: &self)
}
if !decoder.scanner.complete {
throw JSONDecodingError.trailingGarbage
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion Tests/LinuxMain.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ extension Test_JSON {
("testRepeatedInt32", testRepeatedInt32),
("testRepeatedString", testRepeatedString),
("testRepeatedNestedMessage", testRepeatedNestedMessage),
("testOneof", testOneof)
("testOneof", testOneof),
("testEmptyMessage", testEmptyMessage)
]
}

Expand Down
51 changes: 49 additions & 2 deletions Tests/SwiftProtobufTests/TestHelpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,22 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
} catch let e {
XCTFail("Failed to serialize JSON: \(e)\n \(configured)", file: file, line: line)
}

do {
let encodedData = try configured.jsonUTF8Data()
let encodedOptString = String(data: encodedData, encoding: String.Encoding.utf8)
XCTAssertNotNil(encodedOptString)
let encodedString = encodedOptString!
XCTAssert(expected == encodedString, "Did not encode correctly: got \(encodedString)", file: file, line: line)
do {
let decoded = try MessageTestType(jsonUTF8Data: encodedData)
XCTAssert(decoded == configured, "Encode/decode cycle should generate equal object: \(decoded) != \(configured)", file: file, line: line)
} catch {
XCTFail("Encode/decode cycle should not throw error decoding: \(encodedString), but it threw \(error)", file: file, line: line)
}
} catch let e {
XCTFail("Failed to serialize JSON: \(e)\n \(configured)", file: file, line: line)
}
}

/// Verify the preferred encoding/decoding of a particular object.
Expand Down Expand Up @@ -200,8 +216,8 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
do {
let encoded = try decoded.jsonString()
do {
let redecoded = try MessageTestType(jsonString: json)
XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded)", file: file, line: line)
let redecoded = try MessageTestType(jsonString: encoded)
XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded) from \(encoded)", file: file, line: line)
XCTAssertEqual(decoded, redecoded, file: file, line: line)
} catch {
XCTFail("Swift should have recoded/redecoded without error: \(encoded)", file: file, line: line)
Expand All @@ -213,6 +229,29 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
XCTFail("Swift should have decoded without error but got \(e): \(json)", file: file, line: line)
return
}

do {
let jsonData = json.data(using: String.Encoding.utf8)!
let decoded: MessageTestType = try MessageTestType(jsonUTF8Data: jsonData)
XCTAssert(check(decoded), "Condition failed for \(decoded) from binary \(json)", file: file, line: line)

do {
let encoded = try decoded.jsonUTF8Data()
let encodedString = String(data: encoded, encoding: String.Encoding.utf8)!
do {
let redecoded = try MessageTestType(jsonUTF8Data: encoded)
XCTAssert(check(redecoded), "Condition failed for redecoded \(redecoded) from binary \(encodedString)", file: file, line: line)
XCTAssertEqual(decoded, redecoded, file: file, line: line)
} catch {
XCTFail("Swift should have recoded/redecoded without error: \(encodedString)", file: file, line: line)
}
} catch let e {
XCTFail("Swift should have recoded without error but got \(e)\n \(decoded)", file: file, line: line)
}
} catch let e {
XCTFail("Swift should have decoded without error but got \(e): \(json)", file: file, line: line)
return
}
}

func assertTextFormatDecodeSucceeds(_ text: String, file: XCTestFileArgType = #file, line: UInt = #line, check: (MessageTestType) throws -> Bool) {
Expand Down Expand Up @@ -278,6 +317,14 @@ extension PBTestHelpers where MessageTestType: SwiftProtobuf.Message & Equatable
} catch {
// Yay! It failed!
}

let jsonData = json.data(using: String.Encoding.utf8)!
do {
let _ = try MessageTestType(jsonUTF8Data: jsonData, options: options)
XCTFail("Swift decode should have failed for binary: \(json)", file: file, line: line)
} catch {
// Yay! It failed again!
}
}

func assertTextFormatDecodeFails(_ text: String, file: XCTestFileArgType = #file, line: UInt = #line) {
Expand Down
38 changes: 33 additions & 5 deletions Tests/SwiftProtobufTests/Test_JSON.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,19 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeSucceeds("{\"optionalDouble\":\"1.5e+1\"}") {$0.optionalDouble == 15}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"15e-1\"}") {$0.optionalDouble == 1.5}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"1.0e0\"}") {$0.optionalDouble == 1.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"-0\"}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":\"0\"}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":-0}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":0}") {$0.optionalDouble == 0.0}
// We preserve signed zero when decoding
let d1 = try MessageTestType(jsonString: "{\"optionalDouble\":\"-0\"}")
XCTAssertEqual(d1.optionalDouble, 0.0)
XCTAssertEqual(d1.optionalDouble.sign, .minus)
let d2 = try MessageTestType(jsonString: "{\"optionalDouble\":-0}")
XCTAssertEqual(d2.optionalDouble, 0.0)
XCTAssertEqual(d2.optionalDouble.sign, .minus)
// But re-encoding treats the field as defaulted, so the sign gets lost
assertJSONDecodeSucceeds("{\"optionalDouble\":\"-0\"}") {$0.optionalDouble == 0.0}
assertJSONDecodeSucceeds("{\"optionalDouble\":-0}") {$0.optionalDouble == 0.0}

// Malformed numbers should fail
assertJSONDecodeFails("{\"optionalDouble\":Infinity}")
assertJSONDecodeFails("{\"optionalDouble\":-Infinity}") // Must be quoted
Expand Down Expand Up @@ -472,10 +481,13 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertRoundTripJSON {$0.optionalDouble = 2.22507385850720138309e-308}
}

func testOptionalFloat() {
func testOptionalFloat() throws {
assertJSONEncode("{\"optionalFloat\":1.0}") {(o: inout MessageTestType) in
o.optionalFloat = 1.0
}
assertJSONEncode("{\"optionalFloat\":-1.0}") {(o: inout MessageTestType) in
o.optionalFloat = -1.0
}
assertJSONEncode("{\"optionalFloat\":\"Infinity\"}") {(o: inout MessageTestType) in
o.optionalFloat = Float.infinity
}
Expand All @@ -485,6 +497,7 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeSucceeds("{\"optionalFloat\":\"Inf\"}") {$0.optionalFloat == Float.infinity}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-Inf\"}") {$0.optionalFloat == -Float.infinity}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1\"}") {$0.optionalFloat == 1}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-1\"}") {$0.optionalFloat == -1}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1.0\"}") {$0.optionalFloat == 1.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1.5\"}") {$0.optionalFloat == 1.5}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"1.5e1\"}") {$0.optionalFloat == 15}
Expand All @@ -499,8 +512,16 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeSucceeds("{\"optionalFloat\":1.0e0}") {$0.optionalFloat == 1.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"0\"}") {$0.optionalFloat == 0.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":0}") {$0.optionalFloat == 0.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-0\"}") {$0.optionalFloat == -0.0 && $0.optionalFloat.sign == .minus}
assertJSONDecodeSucceeds("{\"optionalFloat\":-0}") {$0.optionalFloat == 0 && $0.optionalFloat.sign == .minus}
// We preserve signed zero when decoding
let d1 = try MessageTestType(jsonString: "{\"optionalFloat\":\"-0\"}")
XCTAssertEqual(d1.optionalFloat, 0.0)
XCTAssertEqual(d1.optionalFloat.sign, .minus)
let d2 = try MessageTestType(jsonString: "{\"optionalFloat\":-0}")
XCTAssertEqual(d2.optionalFloat, 0.0)
XCTAssertEqual(d2.optionalFloat.sign, .minus)
// But re-encoding treats the field as defaulted, so the sign gets lost
assertJSONDecodeSucceeds("{\"optionalFloat\":\"-0\"}") {$0.optionalFloat == 0.0}
assertJSONDecodeSucceeds("{\"optionalFloat\":-0}") {$0.optionalFloat == 0.0}
// Malformed numbers should fail
assertJSONDecodeFails("{\"optionalFloat\":Infinity}")
assertJSONDecodeFails("{\"optionalFloat\":-Infinity}") // Must be quoted
Expand Down Expand Up @@ -874,6 +895,13 @@ class Test_JSON: XCTestCase, PBTestHelpers {
assertJSONDecodeFails("{\"oneofUint32\":1,\"oneofString\":\"abc\"}")
assertJSONDecodeFails("{\"oneofString\":\"abc\",\"oneofUint32\":1}")
}

func testEmptyMessage() {
assertJSONDecodeSucceeds("{}") {MessageTestType -> Bool in true}
assertJSONDecodeFails("")
assertJSONDecodeFails("{")
assertJSONDecodeFails("}")
}
}


Expand Down