From 5ceae5f3b0f4792fe8f6f0856308fffcc2cbc24f Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Tue, 23 Jul 2024 09:36:08 -0700 Subject: [PATCH] [ObjC] Update tests to use both unknown fields apis. Also fixes edge case where merging into an autocreated message for a field wasn't marking the field as set in the parent. PiperOrigin-RevId: 655196339 --- objectivec/GPBMessage.m | 1 + objectivec/Tests/GPBCodedInputStreamTests.m | 41 +++++++++++++- .../Tests/GPBMessageTests+Serialization.m | 17 ++++++ objectivec/Tests/GPBMessageTests.m | 55 +++++++++++++++++++ objectivec/Tests/GPBUnknownFieldSetTest.m | 4 ++ objectivec/Tests/GPBUnknownFieldsTest.m | 1 - objectivec/Tests/GPBUtilitiesTests.m | 36 ++++++++++++ 7 files changed, 152 insertions(+), 3 deletions(-) diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m index 9ef625b01f..1761eab485 100644 --- a/objectivec/GPBMessage.m +++ b/objectivec/GPBMessage.m @@ -2085,6 +2085,7 @@ static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) { - (BOOL)mergeFromData:(NSData *)data extensionRegistry:(nullable id)extensionRegistry error:(NSError **)errorPtr { + GPBBecomeVisibleToAutocreator(self); GPBCodedInputStream *input = [[GPBCodedInputStream alloc] initWithData:data]; @try { [self mergeFromCodedInputStream:input extensionRegistry:extensionRegistry endingTag:0]; diff --git a/objectivec/Tests/GPBCodedInputStreamTests.m b/objectivec/Tests/GPBCodedInputStreamTests.m index 6bf775dded..29802eb15a 100644 --- a/objectivec/Tests/GPBCodedInputStreamTests.m +++ b/objectivec/Tests/GPBCodedInputStreamTests.m @@ -11,7 +11,9 @@ #import "GPBCodedInputStream_PackagePrivate.h" #import "GPBCodedOutputStream.h" #import "GPBTestUtilities.h" +#import "GPBUnknownField.h" #import "GPBUnknownFieldSet_PackagePrivate.h" +#import "GPBUnknownFields.h" #import "GPBUtilities_PackagePrivate.h" #import "GPBWireFormat.h" #import "objectivec/Tests/Unittest.pbobjc.h" @@ -268,20 +270,55 @@ TestAllTypes* message = [self allSetRepeatedCount:kGPBDefaultRepeatCount]; NSData* rawBytes = message.data; - // Create two parallel inputs. Parse one as unknown fields while using - // skipField() to skip each field on the other. Expect the same tags. + TestEmptyMessage* empty = [TestEmptyMessage parseFromData:rawBytes error:NULL]; + XCTAssertNotNil(empty); + GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] initFromMessage:empty] autorelease]; + NSMutableArray* fieldNumbers = [NSMutableArray arrayWithCapacity:ufs.count]; + for (GPBUnknownField* field in ufs) { + GPBWireFormat wireFormat; + switch (field.type) { + case GPBUnknownFieldTypeFixed32: + wireFormat = GPBWireFormatFixed32; + break; + case GPBUnknownFieldTypeFixed64: + wireFormat = GPBWireFormatFixed64; + break; + case GPBUnknownFieldTypeVarint: + wireFormat = GPBWireFormatVarint; + break; + case GPBUnknownFieldTypeLengthDelimited: + wireFormat = GPBWireFormatLengthDelimited; + break; + case GPBUnknownFieldTypeGroup: + wireFormat = GPBWireFormatStartGroup; + break; + case GPBUnknownFieldTypeLegacy: + XCTFail(@"Legacy field type not expected"); + wireFormat = GPBWireFormatVarint; + break; + } + uint32_t tag = GPBWireFormatMakeTag(field.number, wireFormat); + [fieldNumbers addObject:@(tag)]; + } + + // Check the tags compared to what's in the UnknownFields to confirm the stream is + // skipping as expected (this covers the tags within a group also). GPBCodedInputStream* input1 = [GPBCodedInputStream streamWithData:rawBytes]; GPBCodedInputStream* input2 = [GPBCodedInputStream streamWithData:rawBytes]; GPBUnknownFieldSet* unknownFields = [[[GPBUnknownFieldSet alloc] init] autorelease]; + NSUInteger idx = 0; while (YES) { int32_t tag = [input1 readTag]; XCTAssertEqual(tag, [input2 readTag]); if (tag == 0) { + XCTAssertEqual(idx, fieldNumbers.count); break; } + XCTAssertEqual(tag, [fieldNumbers[idx] intValue]); [unknownFields mergeFieldFrom:tag input:input1]; [input2 skipField:tag]; + ++idx; } } diff --git a/objectivec/Tests/GPBMessageTests+Serialization.m b/objectivec/Tests/GPBMessageTests+Serialization.m index f3e8abce15..b42bfdd564 100644 --- a/objectivec/Tests/GPBMessageTests+Serialization.m +++ b/objectivec/Tests/GPBMessageTests+Serialization.m @@ -419,6 +419,17 @@ // All the values should be in unknown fields. + GPBUnknownFields *ufs = [[GPBUnknownFields alloc] initFromMessage:msg]; + XCTAssertEqual(ufs.count, 3U); + uint64_t varint; + XCTAssertTrue([ufs getFirst:Message2_FieldNumber_OptionalEnum varint:&varint]); + XCTAssertEqual(varint, (uint64_t)Message3_Enum_Extra3); + XCTAssertTrue([ufs getFirst:Message2_FieldNumber_RepeatedEnumArray varint:&varint]); + XCTAssertEqual(varint, (uint64_t)Message3_Enum_Extra3); + XCTAssertTrue([ufs getFirst:Message2_FieldNumber_OneofEnum varint:&varint]); + XCTAssertEqual(varint, (uint64_t)Message3_Enum_Extra3); + [ufs release]; + GPBUnknownFieldSet *unknownFields = msg.unknownFields; XCTAssertEqual([unknownFields countOfFields], 3U); @@ -1398,6 +1409,9 @@ int32_t val = -1; XCTAssertTrue([msg1.knownMapField getEnum:&val forKey:0]); XCTAssertEqual(val, Proto2MapEnum_Proto2MapEnumFoo); + GPBUnknownFields *ufs = [[GPBUnknownFields alloc] initFromMessage:msg1]; + XCTAssertEqual(ufs.count, 1U); + [ufs release]; XCTAssertEqual(msg1.unknownFields.countOfFields, 1U); data = [msg1 data]; @@ -1410,6 +1424,9 @@ XCTAssertEqual(msg2.unknownMapField.count, 1U); XCTAssertTrue([msg2.unknownMapField getEnum:&val forKey:0]); XCTAssertEqual(val, Proto2MapEnumPlusExtra_EProto2MapEnumExtra); + ufs = [[GPBUnknownFields alloc] initFromMessage:msg2]; + XCTAssertTrue(ufs.empty); + [ufs release]; XCTAssertEqual(msg2.unknownFields.countOfFields, 0U); XCTAssertEqualObjects(orig, msg2); diff --git a/objectivec/Tests/GPBMessageTests.m b/objectivec/Tests/GPBMessageTests.m index e70f898e3e..f3272dbaf2 100644 --- a/objectivec/Tests/GPBMessageTests.m +++ b/objectivec/Tests/GPBMessageTests.m @@ -13,8 +13,10 @@ #import "GPBDictionary_PackagePrivate.h" #import "GPBMessage_PackagePrivate.h" #import "GPBTestUtilities.h" +#import "GPBUnknownField.h" #import "GPBUnknownFieldSet_PackagePrivate.h" #import "GPBUnknownField_PackagePrivate.h" +#import "GPBUnknownFields.h" #import "objectivec/Tests/Unittest.pbobjc.h" #import "objectivec/Tests/UnittestImport.pbobjc.h" #import "objectivec/Tests/UnittestObjc.pbobjc.h" @@ -501,6 +503,11 @@ [message setUnknownFields:unknownFields]; + GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] init] autorelease]; + [ufs addFieldNumber:1234 fixed32:1234]; + [ufs addFieldNumber:2345 varint:54321]; + [message mergeUnknownFields:ufs extensionRegistry:nil]; + NSString *description = [message description]; XCTAssertGreaterThan([description length], 0U); @@ -985,6 +992,19 @@ XCTAssertFalse([message hasOptionalNestedMessage]); [message.optionalNestedMessage setUnknownFields:unknownFields]; XCTAssertTrue([message hasOptionalNestedMessage]); + + message.optionalNestedMessage = nil; + XCTAssertFalse([message hasOptionalNestedMessage]); + GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] init] autorelease]; + [ufs addFieldNumber:1 varint:1]; + [message.optionalNestedMessage mergeUnknownFields:ufs extensionRegistry:nil]; + XCTAssertTrue([message hasOptionalNestedMessage]); + + message.optionalNestedMessage = nil; + XCTAssertFalse([message hasOptionalNestedMessage]); + [ufs clear]; // Also make sure merging zero length forces it to become visible. + [message.optionalNestedMessage mergeUnknownFields:ufs extensionRegistry:nil]; + XCTAssertTrue([message hasOptionalNestedMessage]); } - (void)testSetAutocreatedSubmessageToSelf { @@ -1481,6 +1501,19 @@ XCTAssertFalse([msg hasExtension:[UnittestRoot repeatedNestedEnumExtension]]); XCTAssertFalse([msg hasExtension:[UnittestRoot repeatedForeignEnumExtension]]); + GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] initFromMessage:msg] autorelease]; + XCTAssertEqual(ufs.count, 3); + uint64_t varint; + XCTAssertTrue([ufs getFirst:[UnittestRoot optionalNestedEnumExtension].fieldNumber + varint:&varint]); + XCTAssertEqual(varint, 10); + XCTAssertTrue([ufs getFirst:[UnittestRoot repeatedNestedEnumExtension].fieldNumber + varint:&varint]); + XCTAssertEqual(varint, 11); + XCTAssertTrue([ufs getFirst:[UnittestRoot repeatedForeignEnumExtension].fieldNumber + varint:&varint]); + XCTAssertEqual(varint, 12); + GPBUnknownFieldSet *unknownFields = msg.unknownFields; GPBUnknownField *field = [unknownFields getField:[UnittestRoot optionalNestedEnumExtension].fieldNumber]; @@ -1523,6 +1556,18 @@ expected = @[ @4, @6 ]; XCTAssertEqualObjects([msg getExtension:[UnittestRoot repeatedForeignEnumExtension]], expected); + ufs = [[[GPBUnknownFields alloc] initFromMessage:msg] autorelease]; + XCTAssertEqual(ufs.count, 3); + XCTAssertTrue([ufs getFirst:[UnittestRoot optionalNestedEnumExtension].fieldNumber + varint:&varint]); + XCTAssertEqual(varint, 10); + XCTAssertTrue([ufs getFirst:[UnittestRoot repeatedNestedEnumExtension].fieldNumber + varint:&varint]); + XCTAssertEqual(varint, 11); + XCTAssertTrue([ufs getFirst:[UnittestRoot repeatedForeignEnumExtension].fieldNumber + varint:&varint]); + XCTAssertEqual(varint, 12); + unknownFields = msg.unknownFields; field = [unknownFields getField:[UnittestRoot optionalNestedEnumExtension].fieldNumber]; XCTAssertNotNil(field); @@ -1840,6 +1885,9 @@ [unknowns mergeVarintField:123 value:456]; GPBMessage *message = [GPBMessage message]; [message setUnknownFields:unknowns]; + GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] init] autorelease]; + [ufs addFieldNumber:1234 varint:5678]; + [message mergeUnknownFields:ufs extensionRegistry:nil]; NSData *data = [message data]; GPBMessage *message2 = [GPBMessage parseFromData:data extensionRegistry:nil error:NULL]; XCTAssertEqualObjects(message, message2); @@ -1850,12 +1898,19 @@ [unknowns1 mergeVarintField:123 value:456]; GPBMessage *message1 = [GPBMessage message]; [message1 setUnknownFields:unknowns1]; + GPBUnknownFields *ufs1 = [[[GPBUnknownFields alloc] init] autorelease]; + [ufs1 addFieldNumber:1234 varint:5678]; + [message1 mergeUnknownFields:ufs1 extensionRegistry:nil]; GPBUnknownFieldSet *unknowns2 = [[[GPBUnknownFieldSet alloc] init] autorelease]; [unknowns2 mergeVarintField:789 value:987]; [unknowns2 mergeVarintField:654 value:321]; GPBMessage *message2 = [GPBMessage message]; [message2 setUnknownFields:unknowns2]; + GPBUnknownFields *ufs2 = [[[GPBUnknownFields alloc] init] autorelease]; + [ufs2 addFieldNumber:2345 fixed32:6789]; + [ufs2 addFieldNumber:3456 fixed32:7890]; + [message2 mergeUnknownFields:ufs2 extensionRegistry:nil]; NSMutableData *delimitedData = [NSMutableData data]; [delimitedData appendData:[message1 delimitedData]]; diff --git a/objectivec/Tests/GPBUnknownFieldSetTest.m b/objectivec/Tests/GPBUnknownFieldSetTest.m index 1e055b0dd0..890ce8f4ad 100644 --- a/objectivec/Tests/GPBUnknownFieldSetTest.m +++ b/objectivec/Tests/GPBUnknownFieldSetTest.m @@ -282,6 +282,10 @@ TestEmptyMessage* destination2 = [TestEmptyMessage message]; [destination2 mergeFrom:source3]; + XCTAssertEqualObjects(destination1.unknownFields, destination2.unknownFields); + XCTAssertEqualObjects(destination1.unknownFields, source3.unknownFields); + XCTAssertEqualObjects(destination2.unknownFields, source3.unknownFields); + XCTAssertEqualObjects(destination1.data, destination2.data); XCTAssertEqualObjects(destination1.data, source3.data); XCTAssertEqualObjects(destination2.data, source3.data); diff --git a/objectivec/Tests/GPBUnknownFieldsTest.m b/objectivec/Tests/GPBUnknownFieldsTest.m index 5565f674b5..8c2adcf32c 100644 --- a/objectivec/Tests/GPBUnknownFieldsTest.m +++ b/objectivec/Tests/GPBUnknownFieldsTest.m @@ -918,7 +918,6 @@ static NSData* DataForGroupsOfDepth(NSUInteger depth) { uint64_t fixed64 = 0; XCTAssertTrue([group getFirst:3 fixed64:&fixed64]); XCTAssertEqual(fixed64, 0x123456789abcdef0LL); - XCTAssertEqual(m.unknownFields.countOfFields, (NSUInteger)1); m = [TestEmptyMessage parseFromData:DataFromBytes(35, 50, 0, 36) error:NULL]; // length delimited, length 0 diff --git a/objectivec/Tests/GPBUtilitiesTests.m b/objectivec/Tests/GPBUtilitiesTests.m index d05a345523..fb0175ecca 100644 --- a/objectivec/Tests/GPBUtilitiesTests.m +++ b/objectivec/Tests/GPBUtilitiesTests.m @@ -251,11 +251,27 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { return result; } +// Helper to add an unknown field data to messages. +static void AddUnknownFields(GPBMessage *message, int num) { + GPBUnknownFields *ufs = [[GPBUnknownFields alloc] init]; + [ufs addFieldNumber:num varint:num]; + [message mergeUnknownFields:ufs extensionRegistry:nil]; + [ufs release]; +} + +static BOOL HasUnknownFields(GPBMessage *message) { + GPBUnknownFields *ufs = [[GPBUnknownFields alloc] initFromMessage:message]; + BOOL result = !ufs.empty; + [ufs release]; + return result; +} + - (void)testDropMessageUnknownFieldsRecursively { TestAllExtensions *message = [TestAllExtensions message]; // Give it unknownFields. message.unknownFields = UnknownFieldsSetHelper(777); + AddUnknownFields(message, 1777); // Given it extensions that include a message with unknown fields of its own. { @@ -266,18 +282,21 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { OptionalGroup_extension *optionalGroup = [OptionalGroup_extension message]; optionalGroup.a = 123; optionalGroup.unknownFields = UnknownFieldsSetHelper(779); + AddUnknownFields(optionalGroup, 1779); [message setExtension:[UnittestRoot optionalGroupExtension] value:optionalGroup]; // Message TestAllTypes_NestedMessage *nestedMessage = [TestAllTypes_NestedMessage message]; nestedMessage.bb = 456; nestedMessage.unknownFields = UnknownFieldsSetHelper(778); + AddUnknownFields(nestedMessage, 1778); [message setExtension:[UnittestRoot optionalNestedMessageExtension] value:nestedMessage]; // Repeated Group RepeatedGroup_extension *repeatedGroup = [[RepeatedGroup_extension alloc] init]; repeatedGroup.a = 567; repeatedGroup.unknownFields = UnknownFieldsSetHelper(780); + AddUnknownFields(repeatedGroup, 1780); [message addExtension:[UnittestRoot repeatedGroupExtension] value:repeatedGroup]; [repeatedGroup release]; @@ -285,6 +304,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { nestedMessage = [[TestAllTypes_NestedMessage alloc] init]; nestedMessage.bb = 678; nestedMessage.unknownFields = UnknownFieldsSetHelper(781); + AddUnknownFields(nestedMessage, 1781); [message addExtension:[UnittestRoot repeatedNestedMessageExtension] value:nestedMessage]; [nestedMessage release]; } @@ -292,6 +312,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { // Confirm everything is there. XCTAssertNotNil(message); + XCTAssertTrue(HasUnknownFields(message)); XCTAssertNotNil(message.unknownFields); XCTAssertTrue([message hasExtension:[UnittestRoot optionalInt32Extension]]); @@ -301,6 +322,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { [message getExtension:[UnittestRoot optionalGroupExtension]]; XCTAssertNotNil(optionalGroup); XCTAssertEqual(optionalGroup.a, 123); + XCTAssertTrue(HasUnknownFields(optionalGroup)); XCTAssertNotNil(optionalGroup.unknownFields); } @@ -310,6 +332,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { [message getExtension:[UnittestRoot optionalNestedMessageExtension]]; XCTAssertNotNil(nestedMessage); XCTAssertEqual(nestedMessage.bb, 456); + XCTAssertTrue(HasUnknownFields(nestedMessage)); XCTAssertNotNil(nestedMessage.unknownFields); } @@ -320,6 +343,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { RepeatedGroup_extension *repeatedGroup = repeatedGroups.firstObject; XCTAssertNotNil(repeatedGroup); XCTAssertEqual(repeatedGroup.a, 567); + XCTAssertTrue(HasUnknownFields(repeatedGroup)); XCTAssertNotNil(repeatedGroup.unknownFields); } @@ -331,6 +355,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { TestAllTypes_NestedMessage *repeatedNestedMessage = repeatedNestedMessages.firstObject; XCTAssertNotNil(repeatedNestedMessage); XCTAssertEqual(repeatedNestedMessage.bb, 678); + XCTAssertTrue(HasUnknownFields(repeatedNestedMessage)); XCTAssertNotNil(repeatedNestedMessage.unknownFields); } @@ -340,6 +365,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { // Confirm unknowns are gone from within the messages. XCTAssertNotNil(message); + XCTAssertFalse(HasUnknownFields(message)); XCTAssertNil(message.unknownFields); XCTAssertTrue([message hasExtension:[UnittestRoot optionalInt32Extension]]); @@ -349,6 +375,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { [message getExtension:[UnittestRoot optionalGroupExtension]]; XCTAssertNotNil(optionalGroup); XCTAssertEqual(optionalGroup.a, 123); + XCTAssertFalse(HasUnknownFields(optionalGroup)); XCTAssertNil(optionalGroup.unknownFields); } @@ -358,6 +385,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { [message getExtension:[UnittestRoot optionalNestedMessageExtension]]; XCTAssertNotNil(nestedMessage); XCTAssertEqual(nestedMessage.bb, 456); + XCTAssertFalse(HasUnknownFields(nestedMessage)); XCTAssertNil(nestedMessage.unknownFields); } @@ -368,6 +396,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { RepeatedGroup_extension *repeatedGroup = repeatedGroups.firstObject; XCTAssertNotNil(repeatedGroup); XCTAssertEqual(repeatedGroup.a, 567); + XCTAssertFalse(HasUnknownFields(repeatedGroup)); XCTAssertNil(repeatedGroup.unknownFields); } @@ -379,6 +408,7 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { TestAllTypes_NestedMessage *repeatedNestedMessage = repeatedNestedMessages.firstObject; XCTAssertNotNil(repeatedNestedMessage); XCTAssertEqual(repeatedNestedMessage.bb, 678); + XCTAssertFalse(HasUnknownFields(repeatedNestedMessage)); XCTAssertNil(repeatedNestedMessage.unknownFields); } } @@ -389,10 +419,12 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { { ForeignMessage *foreignMessage = [ForeignMessage message]; foreignMessage.unknownFields = UnknownFieldsSetHelper(100); + AddUnknownFields(foreignMessage, 1000); [message.mapInt32ForeignMessage setObject:foreignMessage forKey:100]; foreignMessage = [ForeignMessage message]; foreignMessage.unknownFields = UnknownFieldsSetHelper(101); + AddUnknownFields(foreignMessage, 1001); [message.mapStringForeignMessage setObject:foreignMessage forKey:@"101"]; } @@ -403,12 +435,14 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { { ForeignMessage *foreignMessage = [message.mapInt32ForeignMessage objectForKey:100]; XCTAssertNotNil(foreignMessage); + XCTAssertTrue(HasUnknownFields(foreignMessage)); XCTAssertNotNil(foreignMessage.unknownFields); } { ForeignMessage *foreignMessage = [message.mapStringForeignMessage objectForKey:@"101"]; XCTAssertNotNil(foreignMessage); + XCTAssertTrue(HasUnknownFields(foreignMessage)); XCTAssertNotNil(foreignMessage.unknownFields); } @@ -421,12 +455,14 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) { { ForeignMessage *foreignMessage = [message.mapInt32ForeignMessage objectForKey:100]; XCTAssertNotNil(foreignMessage); + XCTAssertFalse(HasUnknownFields(foreignMessage)); XCTAssertNil(foreignMessage.unknownFields); } { ForeignMessage *foreignMessage = [message.mapStringForeignMessage objectForKey:@"101"]; XCTAssertNotNil(foreignMessage); + XCTAssertFalse(HasUnknownFields(foreignMessage)); XCTAssertNil(foreignMessage.unknownFields); } }