Skip to content

Commit

Permalink
Remove the AnyMetadata class and use free functions instead.
Browse files Browse the repository at this point in the history
The way AnyMetadata works is by holding two pointers to the fields of
google.protobuf.Any, and its methods `PackFrom` and `UnpackTo` mutates these
pointers directly.

This works but is a bit overcomplicated. A whole new object is created to
mutate members of Any directly, breaking isolation. Each Any object will also
need to unnecessarily create room to hold an AnyMetadata object, which is at
minimum the size of two pointers.

By removing AnyMetadata and using free functions, the call site passes
`const Value& foo()` and `Value* mutable_foo()` directly into `PackFrom` and
`UnpackTo`. This is more idiomatic and will play well with hasbits.

The only catch here is that in some templated implementations of `PackFrom` and
`UnpackTo`, caller will need access to T::FullMessageName(), which is in
private scope. The workaround here is to have a single templated helper
function that is made into a friend of google.protobuf.Any, so that it has the
right visibility.

PiperOrigin-RevId: 670722407
  • Loading branch information
tonyliaoss authored and copybara-github committed Sep 3, 2024
1 parent 1f237ef commit 920d5c3
Show file tree
Hide file tree
Showing 10 changed files with 333 additions and 239 deletions.
26 changes: 15 additions & 11 deletions src/google/protobuf/any.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "google/protobuf/any.h"

#include "absl/strings/cord.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/generated_message_util.h"
Expand All @@ -19,21 +20,24 @@ namespace google {
namespace protobuf {
namespace internal {

bool AnyMetadata::PackFrom(Arena* arena, const Message& message) {
return PackFrom(arena, message, kTypeGoogleApisComPrefix);
using UrlType = std::string;
using ValueType = std::string;

bool InternalPackFrom(const Message& message, UrlType* dst_url,
ValueType* dst_value) {
return InternalPackFromLite(message, kTypeGoogleApisComPrefix,
message.GetTypeName(), dst_url, dst_value);
}

bool AnyMetadata::PackFrom(Arena* arena, const Message& message,
absl::string_view type_url_prefix) {
type_url_->Set(GetTypeUrl(message.GetTypeName(), type_url_prefix), arena);
return message.SerializeToString(value_->Mutable(arena));
bool InternalPackFrom(const Message& message, absl::string_view type_url_prefix,
UrlType* dst_url, ValueType* dst_value) {
return InternalPackFromLite(message, type_url_prefix, message.GetTypeName(),
dst_url, dst_value);
}

bool AnyMetadata::UnpackTo(Message* message) const {
if (!InternalIs(message->GetTypeName())) {
return false;
}
return message->ParseFromString(value_->Get());
bool InternalUnpackTo(absl::string_view type_url, const ValueType& value,
Message* message) {
return InternalUnpackToLite(message->GetTypeName(), type_url, value, message);
}

bool GetAnyFieldDescriptors(const Message& message,
Expand Down
143 changes: 74 additions & 69 deletions src/google/protobuf/any.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

#include "absl/strings/string_view.h"
#include "google/protobuf/port.h"
#include "google/protobuf/arenastring.h"
#include "absl/strings/cord.h"
#include "google/protobuf/message_lite.h"

// Must be included last.
Expand All @@ -36,75 +36,80 @@ PROTOBUF_EXPORT extern const char kTypeGoogleProdComPrefix[];
std::string GetTypeUrl(absl::string_view message_name,
absl::string_view type_url_prefix);

template <typename T>
absl::string_view GetAnyMessageName() {
return T::FullMessageName();
}

// Helper class used to implement google::protobuf::Any.
class PROTOBUF_EXPORT AnyMetadata {
typedef ArenaStringPtr UrlType;
typedef ArenaStringPtr ValueType;
public:
// AnyMetadata does not take ownership of "type_url" and "value".
constexpr AnyMetadata(UrlType* type_url, ValueType* value)
: type_url_(type_url), value_(value) {}
AnyMetadata(const AnyMetadata&) = delete;
AnyMetadata& operator=(const AnyMetadata&) = delete;

// Packs a message using the default type URL prefix: "type.googleapis.com".
// The resulted type URL will be "type.googleapis.com/<message_full_name>".
// Returns false if serializing the message failed.
template <typename T>
bool PackFrom(Arena* arena, const T& message) {
return InternalPackFrom(arena, message, kTypeGoogleApisComPrefix,
T::FullMessageName());
}

bool PackFrom(Arena* arena, const Message& message);

// Packs a message using the given type URL prefix. The type URL will be
// constructed by concatenating the message type's full name to the prefix
// with an optional "/" separator if the prefix doesn't already end with "/".
// For example, both PackFrom(message, "type.googleapis.com") and
// PackFrom(message, "type.googleapis.com/") yield the same result type
// URL: "type.googleapis.com/<message_full_name>".
// Returns false if serializing the message failed.
template <typename T>
bool PackFrom(Arena* arena, const T& message,
absl::string_view type_url_prefix) {
return InternalPackFrom(arena, message, type_url_prefix,
T::FullMessageName());
}

bool PackFrom(Arena* arena, const Message& message,
absl::string_view type_url_prefix);

// Unpacks the payload into the given message. Returns false if the message's
// type doesn't match the type specified in the type URL (i.e., the full
// name after the last "/" of the type URL doesn't match the message's actual
// full name) or parsing the payload has failed.
template <typename T>
bool UnpackTo(T* message) const {
return InternalUnpackTo(T::FullMessageName(), message);
}

bool UnpackTo(Message* message) const;

// Checks whether the type specified in the type URL matches the given type.
// A type is considered matching if its full name matches the full name after
// the last "/" in the type URL.
template <typename T>
bool Is() const {
return InternalIs(T::FullMessageName());
}

private:
bool InternalPackFrom(Arena* arena, const MessageLite& message,
absl::string_view type_url_prefix,
absl::string_view type_name);
bool InternalUnpackTo(absl::string_view type_name,
MessageLite* message) const;
bool InternalIs(absl::string_view type_name) const;

UrlType* type_url_;
ValueType* value_;
};
#define URL_TYPE std::string
#define VALUE_TYPE std::string

// Helper functions that only require 'lite' messages to work.
PROTOBUF_EXPORT bool InternalPackFromLite(const MessageLite& message,
absl::string_view type_url_prefix,
absl::string_view type_name,
URL_TYPE* dst_url,
VALUE_TYPE* dst_value);
PROTOBUF_EXPORT bool InternalUnpackToLite(absl::string_view type_name,
absl::string_view type_url,
const VALUE_TYPE& value,
MessageLite* dst_message);
PROTOBUF_EXPORT bool InternalIsLite(absl::string_view type_name,
absl::string_view type_url);

// Packs a message using the default type URL prefix: "type.googleapis.com".
// The resulted type URL will be "type.googleapis.com/<message_full_name>".
// Returns false if serializing the message failed.
template <typename T>
bool InternalPackFrom(const T& message, URL_TYPE* dst_url,
VALUE_TYPE* dst_value) {
return InternalPackFromLite(message, kTypeGoogleApisComPrefix,
GetAnyMessageName<T>(), dst_url, dst_value);
}
PROTOBUF_EXPORT bool InternalPackFrom(const Message& message, URL_TYPE* dst_url,
VALUE_TYPE* dst_value);

// Packs a message using the given type URL prefix. The type URL will be
// constructed by concatenating the message type's full name to the prefix
// with an optional "/" separator if the prefix doesn't already end with "/".
// For example, both InternalPackFrom(message, "type.googleapis.com") and
// InternalPackFrom(message, "type.googleapis.com/") yield the same result type
// URL: "type.googleapis.com/<message_full_name>".
// Returns false if serializing the message failed.
template <typename T>
bool InternalPackFrom(const T& message, absl::string_view type_url_prefix,
URL_TYPE* dst_url, VALUE_TYPE* dst_value) {
return InternalPackFromLite(message, type_url_prefix, GetAnyMessageName<T>(),
dst_url, dst_value);
}
PROTOBUF_EXPORT bool InternalPackFrom(const Message& message,
absl::string_view type_url_prefix,
URL_TYPE* dst_url, VALUE_TYPE* dst_value);

// Unpacks the payload into the given message. Returns false if the message's
// type doesn't match the type specified in the type URL (i.e., the full
// name after the last "/" of the type URL doesn't match the message's actual
// full name) or parsing the payload has failed.
template <typename T>
bool InternalUnpackTo(absl::string_view type_url, const VALUE_TYPE& value,
T* message) {
return InternalUnpackToLite(GetAnyMessageName<T>(), type_url, value, message);
}
PROTOBUF_EXPORT bool InternalUnpackTo(absl::string_view type_url,
const VALUE_TYPE& value,
Message* message);

// Checks whether the type specified in the type URL matches the given type.
// A type is considered matching if its full name matches the full name after
// the last "/" in the type URL.
template <typename T>
bool InternalIs(absl::string_view type_url) {
return InternalIsLite(GetAnyMessageName<T>(), type_url);
}

#undef URL_TYPE
#undef VALUE_TYPE

// Get the proto type name from Any::type_url value. For example, passing
// "type.googleapis.com/rpc.QueryOrigin" will return "rpc.QueryOrigin" in
Expand Down
36 changes: 21 additions & 15 deletions src/google/protobuf/any_lite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <cstdlib>
#include <string>

#include "absl/strings/cord.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
Expand All @@ -20,6 +21,13 @@ namespace google {
namespace protobuf {
namespace internal {

using UrlType = std::string;
using ValueType = std::string;

const char kAnyFullTypeName[] = "google.protobuf.Any";
const char kTypeGoogleApisComPrefix[] = "type.googleapis.com/";
const char kTypeGoogleProdComPrefix[] = "type.googleprod.com/";

std::string GetTypeUrl(absl::string_view message_name,
absl::string_view type_url_prefix) {
if (!type_url_prefix.empty() &&
Expand All @@ -36,27 +44,25 @@ bool EndsWithTypeName(absl::string_view type_url, absl::string_view type_name) {
absl::EndsWith(type_url, type_name);
}

const char kAnyFullTypeName[] = "google.protobuf.Any";
const char kTypeGoogleApisComPrefix[] = "type.googleapis.com/";
const char kTypeGoogleProdComPrefix[] = "type.googleprod.com/";

bool AnyMetadata::InternalPackFrom(Arena* arena, const MessageLite& message,
absl::string_view type_url_prefix,
absl::string_view type_name) {
type_url_->Set(GetTypeUrl(type_name, type_url_prefix), arena);
return message.SerializeToString(value_->Mutable(arena));
bool InternalPackFromLite(const MessageLite& message,
absl::string_view type_url_prefix,
absl::string_view type_name, UrlType* dst_url,
ValueType* dst_value) {
*dst_url = GetTypeUrl(type_name, type_url_prefix);
return message.SerializeToString(dst_value);
}

bool AnyMetadata::InternalUnpackTo(absl::string_view type_name,
MessageLite* message) const {
if (!InternalIs(type_name)) {
bool InternalUnpackToLite(absl::string_view type_name,
absl::string_view type_url, const ValueType& value,
MessageLite* dst_message) {
if (!InternalIsLite(type_name, type_url)) {
return false;
}
return message->ParseFromString(value_->Get());
return dst_message->ParseFromString(value);
}

bool AnyMetadata::InternalIs(absl::string_view type_name) const {
return EndsWithTypeName(type_url_->Get(), type_name);
bool InternalIsLite(absl::string_view type_name, absl::string_view type_url) {
return EndsWithTypeName(type_url, type_name);
}

bool ParseAnyTypeUrl(absl::string_view type_url, std::string* url_prefix,
Expand Down
5 changes: 0 additions & 5 deletions src/google/protobuf/any_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ namespace google {
namespace protobuf {
namespace {

TEST(AnyMetadataTest, ConstInit) {
PROTOBUF_CONSTINIT static internal::AnyMetadata metadata(nullptr, nullptr);
(void)metadata;
}

TEST(AnyTest, TestPackAndUnpack) {
protobuf_unittest::TestAny submessage;
submessage.set_int32_value(12345);
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/compiler/cpp/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ void FileGenerator::GenerateSharedHeaderCode(io::Printer* p) {
NamespaceOpener ns(ProtobufNamespace(options_), p);
p->Emit(R"cc(
namespace internal {
class AnyMetadata;
template <typename T>
::absl::string_view GetAnyMessageName();
} // namespace internal
)cc");
}},
Expand Down
Loading

0 comments on commit 920d5c3

Please sign in to comment.