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.
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
Add packTo and unpackFrom in google.protobuf.Any. #602
Conversation
XCTAssertEqualObjects( | ||
[GPBTypeGoogleApisComPrefix stringByAppendingString:from2.descriptor.name], | ||
any.typeURL); | ||
// XCTAssertEqualObjects(@"", any.value); // don't know why this failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failed: ((@"") equal to (any.value)) failed: ("") is not equal to ("<>")
Why it's "<>"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you asking me?
@@ -49,4 +51,22 @@ NS_ASSUME_NONNULL_BEGIN | |||
- (instancetype)initWithTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970; | |||
@end | |||
|
|||
// Extension to GPBAny to support packFrom and unpackTo for arbitrary messages. | |||
@interface GPBAny (GPBWellKnownTypes) | |||
// Initialize any with the given message. e.g., for google.protobuf.foo, type url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to capitalize "any" so that it reads a little better, or quote it? I find it very difficult to read the documentation because I'm not sure if "any" is part of the sentence, or the type.
#if !defined(NS_BLOCK_ASSERTIONS) | ||
XCTAssertThrows([any wrapsMessageOfClass:[NSString class]]); | ||
#endif | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add #else and test the NO case
53a8c59
to
8a28e06
Compare
NSAssert([self.typeURL hasPrefix:GPBTypeGoogleApisComPrefix], | ||
@"Invalid any type url (%@).", self.typeURL); | ||
if (![self.typeURL hasPrefix:GPBTypeGoogleApisComPrefix]) { | ||
return @""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we return @"", we can treat it as normal return value without additional checking for nil.
In the places calling typeName, we can guarantee that returned value from typeName is always compared with a non-empty string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you rarely if ever need to check for nil in Objective C. A message to nil just returns nil/0. Returning nil is very standard.
XCTAssertTrue([any wrapsMessageOfClass:[from2 class]]); | ||
XCTAssertFalse([any wrapsMessageOfClass:[from class]]); | ||
#if !defined(NS_BLOCK_ASSERTIONS) | ||
XCTAssertThrows([any wrapsMessageOfClass:[NSString class]]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May want to add a comment why the test is conditionalized here.
return [self.typeURL substringFromIndex:[GPBTypeGoogleApisComPrefix length]]; | ||
} | ||
|
||
- (instancetype)initWithMessage:(GPBMessage*)message { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final complaint. Move this to top of implementation block. Then LGTM.
The previous two methods make it easy to transform between any and normal message. unPackeTo will throw error if the type url in any doesn't match the type of the message to be transformed to. is checks any's type url matches the give GPBMessage type.
Add packTo and unpackFrom in google.protobuf.Any.
These two methods make it easy to transform between any and normal message.
unPackeTo will throw error if the type url in any doesn't match the type of the message to be transformed to.