Skip to content

Commit 11ff813

Browse files
tbkkatbkka
committed
Correctly handle repeated NullValue fields
Just as NullValue parses `null` and serializes as `null` when in a singular field, it should do the same when NullValue is in a repeated field. This requires a new test proto (since there are no `repeated NullValue` fields in the existing test protos) and tests to verify it, as well as the serialization/deserialization support.
1 parent 148a105 commit 11ff813

File tree

8 files changed

+210
-2
lines changed

8 files changed

+210
-2
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ TEST_PROTOS= \
123123
Protos/unittest_swift_extension4.proto \
124124
Protos/unittest_swift_fieldorder.proto \
125125
Protos/unittest_swift_groups.proto \
126+
Protos/unittest_swift_json.proto \
126127
Protos/unittest_swift_naming.proto \
127128
Protos/unittest_swift_naming_no_prefix.proto \
128129
Protos/unittest_swift_naming_number_prefix.proto \

‎Protos/unittest_swift_json.proto‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Protos/unittest_swift_json.proto
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2020 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See http://swift.org/LICENSE.txt for license information
8+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
9+
10+
syntax = "proto3";
11+
12+
package protobuf_unittest;
13+
14+
import "google/protobuf/struct.proto";
15+
16+
message SwiftJSONTest {
17+
// This case was omitted from test_messages_proto3.proto
18+
repeated google.protobuf.NullValue repeated_null_value = 318;
19+
}

‎Reference/unittest_swift_json.pb.swift‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// DO NOT EDIT.
2+
// swift-format-ignore-file
3+
//
4+
// Generated by the Swift generator plugin for the protocol buffer compiler.
5+
// Source: unittest_swift_json.proto
6+
//
7+
// For information on using the generated types, please see the documentation:
8+
// https://github.com/apple/swift-protobuf/
9+
10+
// Protos/unittest_swift_json.proto
11+
// This source file is part of the Swift.org open source project
12+
//
13+
// Copyright (c) 2020 Apple Inc. and the Swift project authors
14+
// Licensed under Apache License v2.0 with Runtime Library Exception
15+
//
16+
// See http://swift.org/LICENSE.txt for license information
17+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
18+
19+
import Foundation
20+
import SwiftProtobuf
21+
22+
// If the compiler emits an error on this type, it is because this file
23+
// was generated by a version of the `protoc` Swift plug-in that is
24+
// incompatible with the version of SwiftProtobuf to which you are linking.
25+
// Please ensure that you are building against the same version of the API
26+
// that was used to generate this file.
27+
fileprivate struct _GeneratedWithProtocGenSwiftVersion: SwiftProtobuf.ProtobufAPIVersionCheck {
28+
struct _2: SwiftProtobuf.ProtobufAPIVersion_2 {}
29+
typealias Version = _2
30+
}
31+
32+
struct ProtobufUnittest_SwiftJSONTest {
33+
// SwiftProtobuf.Message conformance is added in an extension below. See the
34+
// `Message` and `Message+*Additions` files in the SwiftProtobuf library for
35+
// methods supported on all messages.
36+
37+
/// This case was omitted from test_messages_proto3.proto
38+
var repeatedNullValue: [SwiftProtobuf.Google_Protobuf_NullValue] = []
39+
40+
var unknownFields = SwiftProtobuf.UnknownStorage()
41+
42+
init() {}
43+
}
44+
45+
// MARK: - Code below here is support for the SwiftProtobuf runtime.
46+
47+
fileprivate let _protobuf_package = "protobuf_unittest"
48+
49+
extension ProtobufUnittest_SwiftJSONTest: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, SwiftProtobuf._ProtoNameProviding {
50+
static let protoMessageName: String = _protobuf_package + ".SwiftJSONTest"
51+
static let _protobuf_nameMap: SwiftProtobuf._NameMap = [
52+
318: .standard(proto: "repeated_null_value"),
53+
]
54+
55+
mutating func decodeMessage<D: SwiftProtobuf.Decoder>(decoder: inout D) throws {
56+
while let fieldNumber = try decoder.nextFieldNumber() {
57+
// The use of inline closures is to circumvent an issue where the compiler
58+
// allocates stack space for every case branch when no optimizations are
59+
// enabled. https://github.com/apple/swift-protobuf/issues/1034
60+
switch fieldNumber {
61+
case 318: try { try decoder.decodeRepeatedEnumField(value: &self.repeatedNullValue) }()
62+
default: break
63+
}
64+
}
65+
}
66+
67+
func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
68+
if !self.repeatedNullValue.isEmpty {
69+
try visitor.visitPackedEnumField(value: self.repeatedNullValue, fieldNumber: 318)
70+
}
71+
try unknownFields.traverse(visitor: &visitor)
72+
}
73+
74+
static func ==(lhs: ProtobufUnittest_SwiftJSONTest, rhs: ProtobufUnittest_SwiftJSONTest) -> Bool {
75+
if lhs.repeatedNullValue != rhs.repeatedNullValue {return false}
76+
if lhs.unknownFields != rhs.unknownFields {return false}
77+
return true
78+
}
79+
}

‎Sources/SwiftProtobuf/JSONDecoder.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,8 @@ internal struct JSONDecoder: Decoder {
502502
return
503503
}
504504
while true {
505-
let e: E = try scanner.nextEnumValue()
505+
var e = E()
506+
try decodeSingularEnumField(value: &e)
506507
value.append(e)
507508
if scanner.skipOptionalArrayEnd() {
508509
return

‎Sources/SwiftProtobuf/JSONEncodingVisitor.swift‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,10 +280,18 @@ internal struct JSONEncodingVisitor: Visitor {
280280
}
281281

282282
mutating func visitRepeatedEnumField<E: Enum>(value: [E], fieldNumber: Int) throws {
283+
let options = self.options
283284
let alwaysPrintEnumsAsInts = options.alwaysPrintEnumsAsInts
284285
try _visitRepeated(value: value, fieldNumber: fieldNumber) {
285286
(encoder: inout JSONEncoder, v: E) throws in
286-
if !alwaysPrintEnumsAsInts, let n = v.name {
287+
// TODO: This cast is expensive to repeat for every item
288+
// in the array. To eliminate it, make `encodedJSONString`
289+
// static, and cast E.self to _CustomJSONCodable.Type outside
290+
// of this loop so we can use that static method.
291+
if let e = v as? _CustomJSONCodable {
292+
let json = try e.encodedJSONString(options: options)
293+
encoder.append(text: json)
294+
} else if !alwaysPrintEnumsAsInts, let n = v.name {
287295
encoder.appendQuoted(name: n)
288296
} else {
289297
encoder.putEnumInt(value: v.rawValue)

‎Tests/LinuxMain.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ extension Test_JSON_Conformance {
641641
("testNullSupport_oneofNullValue", testNullSupport_oneofNullValue),
642642
("testNullSupport_oneofNullValue_alternate", testNullSupport_oneofNullValue_alternate),
643643
("testNullSupport_oneofNullValue_numeric", testNullSupport_oneofNullValue_numeric),
644+
("testNullSupport_repeatedNullValue", testNullSupport_repeatedNullValue),
644645
("testNullSupport_Repeated", testNullSupport_Repeated),
645646
("testNullSupport_RepeatedValue", testNullSupport_RepeatedValue),
646647
("testNullConformance", testNullConformance),

‎Tests/SwiftProtobufTests/Test_JSON_Conformance.swift‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,26 @@ class Test_JSON_Conformance: XCTestCase {
200200
}
201201
}
202202

203+
func testNullSupport_repeatedNullValue() throws {
204+
let valueNull = "{\"repeatedNullValue\": [0, \"NULL_VALUE\", null]}"
205+
let decoded: ProtobufUnittest_SwiftJSONTest
206+
do {
207+
decoded = try ProtobufUnittest_SwiftJSONTest(jsonString: valueNull)
208+
XCTAssertNotEqual(decoded, ProtobufUnittest_SwiftJSONTest())
209+
XCTAssertEqual(3, decoded.repeatedNullValue.count)
210+
} catch let e {
211+
XCTFail("Decode failed with error \(e): \(valueNull)")
212+
return
213+
}
214+
215+
do {
216+
let recoded = try decoded.jsonString()
217+
XCTAssertEqual(recoded, "{\"repeatedNullValue\":[null,null,null]}")
218+
} catch let e {
219+
XCTFail("JSON encode failed with error: \(e)")
220+
}
221+
}
222+
203223
func testNullSupport_Repeated() throws {
204224
// Nulls within repeated lists are errors
205225
let json1 = "{\"repeatedBoolWrapper\":[true, null, false]}"

‎Tests/SwiftProtobufTests/unittest_swift_json.pb.swift‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// DO NOT EDIT.
2+
// swift-format-ignore-file
3+
//
4+
// Generated by the Swift generator plugin for the protocol buffer compiler.
5+
// Source: unittest_swift_json.proto
6+
//
7+
// For information on using the generated types, please see the documentation:
8+
// https://github.com/apple/swift-protobuf/
9+
10+
// Protos/unittest_swift_json.proto
11+
// This source file is part of the Swift.org open source project
12+
//
13+
// Copyright (c) 2020 Apple Inc. and the Swift project authors
14+
// Licensed under Apache License v2.0 with Runtime Library Exception
15+
//
16+
// See http://swift.org/LICENSE.txt for license information
17+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
18+
19+
import Foundation
20+
import SwiftProtobuf
21+
22+
// If the compiler emits an error on this type, it is because this file
23+
// was generated by a version of the `protoc` Swift plug-in that is
24+
// incompatible with the version of SwiftProtobuf to which you are linking.
25+
// Please ensure that you are building against the same version of the API
26+
// that was used to generate this file.
27+
fileprivate struct _GeneratedWithProtocGenSwiftVersion: SwiftProtobuf.ProtobufAPIVersionCheck {
28+
struct _2: SwiftProtobuf.ProtobufAPIVersion_2 {}
29+
typealias Version = _2
30+
}
31+
32+
struct ProtobufUnittest_SwiftJSONTest {
33+
// SwiftProtobuf.Message conformance is added in an extension below. See the
34+
// `Message` and `Message+*Additions` files in the SwiftProtobuf library for
35+
// methods supported on all messages.
36+
37+
/// This case was omitted from test_messages_proto3.proto
38+
var repeatedNullValue: [SwiftProtobuf.Google_Protobuf_NullValue] = []
39+
40+
var unknownFields = SwiftProtobuf.UnknownStorage()
41+
42+
init() {}
43+
}
44+
45+
// MARK: - Code below here is support for the SwiftProtobuf runtime.
46+
47+
fileprivate let _protobuf_package = "protobuf_unittest"
48+
49+
extension ProtobufUnittest_SwiftJSONTest: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, SwiftProtobuf._ProtoNameProviding {
50+
static let protoMessageName: String = _protobuf_package + ".SwiftJSONTest"
51+
static let _protobuf_nameMap: SwiftProtobuf._NameMap = [
52+
318: .standard(proto: "repeated_null_value"),
53+
]
54+
55+
mutating func decodeMessage<D: SwiftProtobuf.Decoder>(decoder: inout D) throws {
56+
while let fieldNumber = try decoder.nextFieldNumber() {
57+
// The use of inline closures is to circumvent an issue where the compiler
58+
// allocates stack space for every case branch when no optimizations are
59+
// enabled. https://github.com/apple/swift-protobuf/issues/1034
60+
switch fieldNumber {
61+
case 318: try { try decoder.decodeRepeatedEnumField(value: &self.repeatedNullValue) }()
62+
default: break
63+
}
64+
}
65+
}
66+
67+
func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
68+
if !self.repeatedNullValue.isEmpty {
69+
try visitor.visitPackedEnumField(value: self.repeatedNullValue, fieldNumber: 318)
70+
}
71+
try unknownFields.traverse(visitor: &visitor)
72+
}
73+
74+
static func ==(lhs: ProtobufUnittest_SwiftJSONTest, rhs: ProtobufUnittest_SwiftJSONTest) -> Bool {
75+
if lhs.repeatedNullValue != rhs.repeatedNullValue {return false}
76+
if lhs.unknownFields != rhs.unknownFields {return false}
77+
return true
78+
}
79+
}

0 commit comments

Comments
 (0)