Skip to content

Commit

Permalink
* Added support for com_google_protobuf >= Protocol Buffers v28. (#4)
Browse files Browse the repository at this point in the history
* Moved `ParseTextProtoHelper` into namespace `mbo::proto::proto_internal`.
* Changed `ParseTextProtoHelper` to only accept derived proto messages on operator access.
* Changed `ParseTextProtoHelper` to die on parsing failure.
* Added `ParseText<T>() -> absl::StatusOr<T>`.
* Improved error message detail for all parsing functions.
  • Loading branch information
helly25 authored Jul 16, 2024
1 parent d4b6dd4 commit e5c8015
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 484 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Enabling bzlmod results in protobuf version issues...
build --noenable_bzlmod
common --noenable_bzlmod

# To debug bazel options, uncomment next line.
# build --announce_rc
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
# 0.5

* Added support for com_google_protobuf >= Protocol Buffers v28.
* Moved `ParseTextProtoHelper` into namespace `mbo::proto::proto_internal`.
* Changed `ParseTextProtoHelper` to only accept derived proto messages on operator access.
* Changed `ParseTextProtoHelper` to die on parsing failure.
* Added `ParseText<T>() -> absl::StatusOr<T>`.
* Improved error message detail for all parsing functions.

# 0.4

* Changed `ParseTextProto`, `ParseTextProtoOrDie` to only work for actual derived proto types.
Expand Down
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,22 @@ The project is formatted with specific `clang-format` settings which require cla
* namespace: `mbo::proto`

* `ParseTextProtoOrDie`(`text_proto` [, `std::source_location`])
* `text_proto` is a text proto best identified as a raw-string with marker 'pb'.
* `text_proto` is a text proto best identified as a raw-string with marker `pb` or `proto`.
* If `text_proto` cannot be parsed into the receiving proto type, then the function will fail.
* Prefer this function over template function `ParseTextOrDie`.

* `ParseTextOrDie`<`Proto`>(`text_proto` [, `std::source_location`])
* `text_proto` is a text proto best identified as a raw-string with marker 'pb'.
* `text_proto` is a text proto best identified as a raw-string with marker `pb` or `proto`.
* `Proto` is the type to produce.
* If `text_proto` cannot be parsed as a `Proto`, then the function will fail.
* Use this only if the `Proto` type cannot be inferred by `ParserTextProtoOrDie`.

* `ParseTest`<`Proto`>(`text_proto` [, `std::source_location`]) -> `absl::StatusOr`<`Proto`>
* `text_proto` is a text proto best identified as a raw-string with marker `pb` or `proto`.
* `Proto` is the type to produce.
* If `text_proto` cannot be parse as a `Proto`, then the function returns a non-`absl::OkStatus`.
* Use this function in cases where errors are expected.

## Usage

BUILD.bazel:
Expand Down Expand Up @@ -179,6 +185,8 @@ cp ../cpp-proto-builder/proto_builder/oss/tests/simple_message.proto proto/mbo/p
patch <proto/mbo/proto/parse_proto_text.diff
```
The diff files are available in the repository history.
## Proto Matchers
The clone was made from Google's [CPP-proto-builder](https://github.com/google/cpp-proto-builder).
Expand All @@ -199,6 +207,8 @@ cp ../cpp-proto-builder/proto_builder/oss/testing/proto_test_util.* proto/mbo/te
patch <proto/mbo/testing/proto_test_util.diff
```
The diff files are available in the repository history.
The include guards were updated and the namespace changed to `testing::proto` which allows to
import the whole namespace easily. Further logging was switched directly to
[Abseil logging](https://abseil.io/docs/cpp/guides/logging) (this was not an option when I wrote
Expand Down
219 changes: 0 additions & 219 deletions mbo/proto/matchers.diff

This file was deleted.

67 changes: 37 additions & 30 deletions mbo/proto/parse_text_proto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
#include "google/protobuf/io/tokenizer.h"
#include "google/protobuf/text_format.h"
#include "absl/base/log_severity.h"
#include "absl/log/absl_log.h"
#include "absl/strings/str_format.h"
#include "absl/strings/string_view.h"


namespace mbo::proto::internal {
namespace mbo::proto::proto_internal {
namespace {

// Collects errors from proto parsing.
Expand All @@ -39,36 +39,38 @@ class SilentErrorCollector : public google::protobuf::io::ErrorCollector {
absl::LogSeverity severity = absl::LogSeverity::kError;
};

// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
#if GOOGLE_PROTOBUF_VERSION >= 5028000
void RecordError(int line, int column, std::string_view message) override {
#else
void AddError(int line, int column, const std::string& message) override {
errors_.push_back({
.line = line,
.column = column,
.message = message,
.severity = absl::LogSeverity::kError,
});
#endif
errors_.push_back({.line = line,
.column = column,
.message = std::string(message),
.severity = absl::LogSeverity::kError});
}

// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
#if GOOGLE_PROTOBUF_VERSION >= 5028000
void RecordWarning(int line, int column, std::string_view message) override {
#else
void AddWarning(int line, int column, const std::string& message) override {
errors_.push_back({
.line = line,
.column = column,
.message = message,
.severity = absl::LogSeverity::kWarning,
});
#endif
errors_.push_back({.line = line,
.column = column,
.message = std::string(message),
.severity = absl::LogSeverity::kWarning});
}

std::string GetErrorStr() const;
const std::vector<ErrorInfo>& GetErrors() const { return errors_; }
std::string GetErrors() const;
const std::vector<ErrorInfo>& errors() const { return errors_; }

private:
std::vector<ErrorInfo> errors_;
};

std::string SilentErrorCollector::GetErrorStr() const {
std::string SilentErrorCollector::GetErrors() const {
std::string result;
for (const auto& error : GetErrors()) {
for (const auto& error : errors()) {
absl::StrAppendFormat(&result, "Line %d, Col %d: %s\n", error.line,
error.column, error.message);
}
Expand All @@ -77,23 +79,28 @@ std::string SilentErrorCollector::GetErrorStr() const {

} // namespace

absl::Status ParseTextInternal(std::string_view text, ::google::protobuf::Message* message,
std::source_location loc) {
absl::Status ParseTextInternal(std::string_view text_proto, ::google::protobuf::Message* message,
std::string_view func, std::source_location loc) {
google::protobuf::TextFormat::Parser parser;
SilentErrorCollector error_collector;
parser.RecordErrorsTo(&error_collector);
if (parser.ParseFromString(std::string(text), message)) {
if (parser.ParseFromString(std::string(text_proto), message)) {
return absl::OkStatus();
}
return absl::InvalidArgumentError(
absl::StrFormat("%s\nFile: '%s', Line: %d", error_collector.GetErrorStr(),
loc.file_name(), loc.line()));
absl::StrFormat("%s<%s>\nFile: '%s', Line: %d: %s\nError: %s",
func, message->GetDescriptor()->name(), loc.file_name(), loc.line(), loc.function_name(), error_collector.GetErrors()));
}

void ParseTextOrDieInternal(std::string_view text, ::google::protobuf::Message* message, std::string_view func, std::source_location loc) {
absl::Status result = internal::ParseTextInternal(text, message, loc);
ABSL_CHECK_OK(result) << func << "<" << message->GetDescriptor()->name() << ">"
<< " @" << loc.file_name() << ":" << loc.line() << ": " << result;
void ParseTextOrDieInternal(
std::string_view text,
::google::protobuf::Message* message,
std::string_view func,
std::source_location loc) {
const absl::Status status = ParseTextInternal(text, message, func, loc);
if (!status.ok()) {
ABSL_LOG(FATAL) << "Check failed: " << status;
}
}

} // namespace mbo::proto::internal
} // namespace mbo::proto::proto_internal
Loading

0 comments on commit e5c8015

Please sign in to comment.