From 073ba1170cc3ba7361d48a1f410b3ec507df3bcb Mon Sep 17 00:00:00 2001 From: helly25 Date: Mon, 24 Mar 2025 16:23:53 +0000 Subject: [PATCH] Bump to version 1.2.0 * Added std::optional variants for file reading. * Added `HasBinaryProtoExtension` and `HasTextProtoExtension`. --- CHANGELOG.md | 5 +- MODULE.bazel | 2 +- README.md | 6 ++ mbo/proto/BUILD.bazel | 24 ++++++- mbo/proto/file.cc | 10 +++ mbo/proto/file.h | 127 ++++++++++++++++++++++++++++++++---- mbo/proto/file_impl.h | 41 ++++++++++++ mbo/proto/file_test.cc | 125 +++++++++++++++++++++++++---------- mbo/proto/status_matchers.h | 80 +++++++++++++++++++++++ 9 files changed, 370 insertions(+), 50 deletions(-) create mode 100644 mbo/proto/file_impl.h create mode 100644 mbo/proto/status_matchers.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 233ae87..80ad87c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ -# 1.1.1 +# 1.2.0 + +* Added std::optional variants for file reading. +* Added `HasBinaryProtoExtension` and `HasTextProtoExtension`. # 1.1.0 diff --git a/MODULE.bazel b/MODULE.bazel index 1c3357e..10257dc 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -18,7 +18,7 @@ # Due to flag and test references to the repo name we use the old name for now. module( name = "helly25_proto", - version = "1.1.1", + version = "1.2.0", ) # For local development we include LLVM, Hedron-Compile-Commands, etc. diff --git a/README.md b/README.md index a566ae5..738074a 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,12 @@ TEST(Foo, Wrapper) { * `message` the protocol buffer to write. * Returns `absl::OkStatus()` or an error status. +* function `HasBinaryProtoExtension`(`filesname`) + * Returns whether the filename ends with a well-known extension for binary proto files. + +* function `HasTextProtoExtension`(`filesname`) + * Returns whether the filename ends with a well-known extension for text proto files. + ## Usage ```c++ diff --git a/mbo/proto/BUILD.bazel b/mbo/proto/BUILD.bazel index 05d72f4..476a00e 100644 --- a/mbo/proto/BUILD.bazel +++ b/mbo/proto/BUILD.bazel @@ -22,7 +22,10 @@ licenses(["notice"]) cc_library( name = "file_cc", srcs = ["file.cc"], - hdrs = ["file.h"], + hdrs = [ + "file.h", + "file_impl.h", + ], implementation_deps = [ ":silent_error_collector_cc", "@com_google_absl//absl/log:absl_log", @@ -44,6 +47,7 @@ cc_test( deps = [ ":file_cc", ":matchers_cc", + ":status_matchers_cc", "//mbo/proto/tests:simple_message_cc_proto", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", @@ -145,3 +149,21 @@ cc_test( "@com_google_protobuf//src/google/protobuf/io:tokenizer", ], ) + +cc_library( + name = "status_matchers_cc", + testonly = 1, + hdrs = ["status_matchers.h"], + visibility = [ + # Do not use this elsewhere. Instead: + # - just upgrade your Abseil dependency. + # #include "absl/status/status_matchers" + # - use @helly25_mbo//mbo/testing:status_cc + # #include "mbo/testing/status.h" + "//visibility:private", + ], + deps = [ + "@com_google_absl//absl/status", + "@com_google_googletest//:gtest", + ], +) diff --git a/mbo/proto/file.cc b/mbo/proto/file.cc index 133ec7d..933e41a 100644 --- a/mbo/proto/file.cc +++ b/mbo/proto/file.cc @@ -82,6 +82,16 @@ absl::Status ReadTextProtoFile( } // namespace proto_internal +bool HasBinaryProtoExtension(std::string_view filename) { + return filename.ends_with(".binpb") || // NL + filename.ends_with(".pb"); +} + +bool HasTextProtoExtension(std::string_view filename) { + return filename.ends_with(".txtpb") || // NL + filename.ends_with(".textproto"); +} + absl::Status WriteBinaryProtoFile( const std::filesystem::path& filename, const ::google::protobuf::Message& proto, diff --git a/mbo/proto/file.h b/mbo/proto/file.h index f088735..99deed5 100644 --- a/mbo/proto/file.h +++ b/mbo/proto/file.h @@ -16,34 +16,71 @@ #ifndef MBO_PROTO_FILE_H_ #define MBO_PROTO_FILE_H_ +#include #include +#include #include #include "absl/log/absl_log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "google/protobuf/message.h" +#include "mbo/proto/file_impl.h" // IWYU pragma: export + +// Functionality for reading and writing protos: +// - functions Has(Binary|Text)ProtoExtension +// - concept IsProtoType +// - struct/function Read(Binary|Text)ProtoFile(::(As|OrDie|OrNullopt))? +// - struct/function Write(Binary|Text)ProtoFile namespace mbo::proto { -template -concept IsProtoType = - std::derived_from && !std::same_as; +// Identifies binary proto filenames. +// See https://protobuf.dev/programming-guides/techniques/ +// See https://en.wikipedia.org/wiki/Filename_extension (extension with dot) +// Added [".pb"] which is also fairly common. +bool HasBinaryProtoExtension(std::string_view filename); -namespace proto_internal { +inline bool HasBinaryProtoExtension(const std::same_as auto& filename) { + return HasBinaryProtoExtension(filename.native()); +} -absl::Status ReadBinaryProtoFile( - const std::filesystem::path& filename, - ::google::protobuf::Message& result, - const std::source_location& src_loc); +// Identifies text proto filenames. +// See https://protobuf.dev/programming-guides/techniques/ +// See https://en.wikipedia.org/wiki/Filename_extension (extension with dot) +bool HasTextProtoExtension(std::string_view filename); -absl::Status ReadTextProtoFile( - const std::filesystem::path& filename, - ::google::protobuf::Message& result, - const std::source_location& src_loc); +inline bool HasTextProtoExtension(const std::same_as auto& filename) { + return HasTextProtoExtension(filename.native()); +} -} // namespace proto_internal +// Concept that identifies Proto types as opposed to the base proto `Message`. +template +concept IsProtoType = + std::derived_from && !std::same_as; +// Type erasure proto reader for binary proto files. +// +// Example: +// +// ```c++ +// const MyProto proto = ReadBinaryProtoFile(proto_filename); +// const absl::StatusOr proto_or_error = ReadBinaryProtoFile(proto_filename); +// const std::optional proto_or_nullopt = ReadBinaryProtoFile(proto_filename); +// ``` +// +// +// In the above the first call requires the file to be readable as a `MyProto`. +// The program will abort with a check violation if the file cannot be read. +// +// The second call returns either the read protocol buffer or an error status. +// In this version the return value forces the caller to handle any errors. +// +// The third call returns either the read protocol buffer ot std::nullopt. +// In this verion the caller is responsible for error handling. +// +// The class also supports static direct typed access by functions whose +// addresses can be taken. class ReadBinaryProtoFile { public: // Static read function - so it's address can be taken. @@ -59,6 +96,17 @@ class ReadBinaryProtoFile { return result; } + // Static read function - so it's address can be taken. + template + static std::optional OrNullopt( + const std::filesystem::path& filename, + const std::source_location& src_loc = std::source_location::current()) { + if (auto result = As(filename, src_loc); result.ok()) { + return *std::move(result); + } + return std::nullopt; + } + // Static read function - so it's address can be taken. template static ProtoType OrDie( @@ -92,6 +140,15 @@ class ReadBinaryProtoFile { return result; } + template + operator std::optional() const { // NOLINT(*-explicit-*) + absl::StatusOr proto = *this; + if (!proto.ok()) { + return std::nullopt; + } + return *proto; + } + template operator ProtoType() const { // NOLINT(*-explicit-*) absl::StatusOr proto = *this; @@ -106,11 +163,34 @@ class ReadBinaryProtoFile { mutable bool converted_ = false; }; +// Writes a binary proto file. absl::Status WriteBinaryProtoFile( const std::filesystem::path& filename, const ::google::protobuf::Message& proto, const std::source_location& src_loc = std::source_location::current()); +// Type erasure proto reader for text proto files. +// +// Example: +// +// ```c++ +// const MyProto proto = ReadTextProtoFile(filename); +// const absl::StatusOr proto_or_error = ReadTextProtoFile(filename); +// const std::optional proto_or_nullopt = ReadTextProtoFile(filename); +// ``` +// +// In the above the first call requires the file to be readable as a `MyProto`. +// The program will abort with a check violation if the file cannot be read. +// +// The second call returns either the read protocol buffer or an error status. +// In this version the return value forces the caller to handle any errors. +// +// The third call returns either the read protocol buffer ot std::nullopt. +// In this verion the caller is responsible for error handling. +// +// The class also supports static direct typed access by functions whose +// addresses can be taken. + class ReadTextProtoFile { public: // Static read function - so it's address can be taken. @@ -126,6 +206,17 @@ class ReadTextProtoFile { return result; } + // Static read function - so it's address can be taken. + template + static std::optional OrNullopt( + const std::filesystem::path& filename, + const std::source_location& src_loc = std::source_location::current()) { + if (auto result = As(filename, src_loc); result.ok()) { + return *std::move(result); + } + return std::nullopt; + } + // Static read function - so it's address can be taken. template static ProtoType OrDie( @@ -159,6 +250,15 @@ class ReadTextProtoFile { return result; } + template + operator std::optional() const { // NOLINT(*-explicit-*) + absl::StatusOr proto = *this; + if (!proto.ok()) { + return std::nullopt; + } + return *proto; + } + template operator ProtoType() const { // NOLINT(*-explicit-*) absl::StatusOr proto = *this; @@ -173,6 +273,7 @@ class ReadTextProtoFile { mutable bool converted_ = false; }; +// Writes a text proto file. absl::Status WriteTextProtoFile( const std::filesystem::path& filename, const ::google::protobuf::Message& proto, diff --git a/mbo/proto/file_impl.h b/mbo/proto/file_impl.h new file mode 100644 index 0000000..1926101 --- /dev/null +++ b/mbo/proto/file_impl.h @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: Copyright (c) The helly25/mbo authors (helly25.com), The CPP Proto Builder Authors +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef MBO_PROTO_FILE_IMPL_H_ +#define MBO_PROTO_FILE_IMPL_H_ + +// IWYU pragma: private, include "mbo/proto/file.h" + +#include +#include + +#include "absl/status/status.h" +#include "google/protobuf/message.h" + +namespace mbo::proto::proto_internal { + +absl::Status ReadBinaryProtoFile( + const std::filesystem::path& filename, + ::google::protobuf::Message& result, + const std::source_location& src_loc); + +absl::Status ReadTextProtoFile( + const std::filesystem::path& filename, + ::google::protobuf::Message& result, + const std::source_location& src_loc); + +} // namespace mbo::proto::proto_internal + +#endif // MBO_PROTO_FILE_IMPL_H_ diff --git a/mbo/proto/file_test.cc b/mbo/proto/file_test.cc index d79c0a8..c47a907 100644 --- a/mbo/proto/file_test.cc +++ b/mbo/proto/file_test.cc @@ -22,6 +22,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "mbo/proto/matchers.h" +#include "mbo/proto/status_matchers.h" #include "mbo/proto/tests/simple_message.pb.h" namespace mbo::proto { @@ -30,7 +31,10 @@ namespace { using ::mbo::proto::EqualsProto; using ::mbo::proto::tests::SimpleMessage; +using ::testing::ExplainMatchResult; using ::testing::HasSubstr; +using ::testing::Not; +using ::testing::Optional; struct FileProtoTest : ::testing::Test { [[nodiscard]] static bool WriteFile(const std::filesystem::path& filename, std::string_view data) { @@ -48,29 +52,25 @@ TEST_F(FileProtoTest, BinaryProto) { message.set_one(25); message.add_two(33); message.add_two(42); - ASSERT_TRUE(WriteBinaryProtoFile("test.pb", message).ok()); - const absl::StatusOr result = ReadBinaryProtoFile("test.pb"); - ASSERT_TRUE(result.ok()); - EXPECT_THAT(*result, EqualsProto(message)); + ASSERT_THAT(WriteBinaryProtoFile("test.pb", message), IsOk()); + const absl::StatusOr result_or_status = ReadBinaryProtoFile("test.pb"); + EXPECT_THAT(result_or_status, IsOkAndHolds(EqualsProto(message))); + const std::optional result_or_nullopt = ReadBinaryProtoFile("test.pb"); + EXPECT_THAT(result_or_nullopt, Optional(EqualsProto(message))); EXPECT_THAT(SimpleMessage(ReadBinaryProtoFile("test.pb")), EqualsProto(message)); - EXPECT_THAT(*ReadBinaryProtoFile::As("test.pb"), EqualsProto(message)); + EXPECT_THAT(ReadBinaryProtoFile::As("test.pb"), IsOkAndHolds(EqualsProto(message))); + EXPECT_THAT(ReadBinaryProtoFile::OrNullopt("test.pb"), Optional(EqualsProto(message))); EXPECT_THAT(ReadBinaryProtoFile::OrDie("test.pb"), EqualsProto(message)); } TEST_F(FileProtoTest, BinaryProtoError) { - { - const absl::StatusOr result = ReadBinaryProtoFile("DoesNotExist.pb"); - ASSERT_FALSE(result.ok()); - EXPECT_THAT(result.status().code(), absl::StatusCode::kNotFound); - EXPECT_THAT(result.status().message(), HasSubstr("Cannot open 'DoesNotExist.pb'")); - } - { - ASSERT_TRUE(WriteFile("SomeFile.pb", "\xFF")); - const absl::StatusOr result = ReadBinaryProtoFile("SomeFile.pb"); - ASSERT_FALSE(result.ok()); - EXPECT_THAT(result.status().code(), absl::StatusCode::kAborted); - EXPECT_THAT(result.status().message(), HasSubstr("Cannot parse binary proto file 'SomeFile.pb'")); - } + EXPECT_THAT( + absl::StatusOr(ReadBinaryProtoFile("DoesNotExist.pb")), + StatusIs(absl::StatusCode::kNotFound, HasSubstr("Cannot open 'DoesNotExist.pb'"))); + ASSERT_TRUE(WriteFile("SomeFile.pb", "\xFF")); + EXPECT_THAT( + absl::StatusOr(ReadBinaryProtoFile("SomeFile.pb")), + StatusIs(absl::StatusCode::kAborted, HasSubstr("Cannot parse binary proto file 'SomeFile.pb'"))); } TEST_F(FileProtoTest, TextProto) { @@ -78,28 +78,85 @@ TEST_F(FileProtoTest, TextProto) { message.set_one(25); message.add_two(33); message.add_two(42); - ASSERT_TRUE(WriteTextProtoFile("test.textproto", message).ok()); - const absl::StatusOr result = ReadTextProtoFile("test.textproto"); - ASSERT_TRUE(result.ok()); - EXPECT_THAT(*result, EqualsProto(message)); + ASSERT_THAT(WriteTextProtoFile("test.textproto", message), IsOk()); + const absl::StatusOr result_or_status = ReadTextProtoFile("test.textproto"); + EXPECT_THAT(result_or_status, IsOkAndHolds(EqualsProto(message))); + const std::optional result_or_nullopt = ReadTextProtoFile("test.textproto"); + EXPECT_THAT(result_or_nullopt, Optional(EqualsProto(message))); EXPECT_THAT(SimpleMessage(ReadTextProtoFile("test.textproto")), EqualsProto(message)); - EXPECT_THAT(*ReadTextProtoFile::As("test.textproto"), EqualsProto(message)); + EXPECT_THAT(ReadTextProtoFile::As("test.textproto"), IsOkAndHolds(EqualsProto(message))); + EXPECT_THAT(ReadTextProtoFile::OrNullopt("test.textproto"), Optional(EqualsProto(message))); EXPECT_THAT(ReadTextProtoFile::OrDie("test.textproto"), EqualsProto(message)); } TEST_F(FileProtoTest, TextProtoError) { - { - const absl::StatusOr result = ReadTextProtoFile("DoesNotExist.textproto"); - ASSERT_FALSE(result.ok()); - EXPECT_THAT(result.status().code(), absl::StatusCode::kNotFound); - EXPECT_THAT(result.status().message(), HasSubstr("Cannot open 'DoesNotExist.textproto'")); + EXPECT_THAT( + absl::StatusOr(ReadTextProtoFile("DoesNotExist.textproto")), + StatusIs(absl::StatusCode::kNotFound, HasSubstr("Cannot open 'DoesNotExist.textproto'"))); + ASSERT_TRUE(WriteFile("SomeFile.textproto", "\xFF")); + EXPECT_THAT( + absl::StatusOr(ReadTextProtoFile("SomeFile.textproto")), + StatusIs(absl::StatusCode::kAborted, HasSubstr("Cannot parse text proto file 'SomeFile.textproto'"))); +} + +MATCHER(IsBinaryProtoExtension, "") { + return HasBinaryProtoExtension(arg); +} + +MATCHER(IsTextProtoExtension, "") { + return HasTextProtoExtension(arg); +} + +TEST_F(FileProtoTest, HasBinaryProtoExtension) { + // True: binary proto + for (std::string_view filename : {".binpb", ".pb", "file.binpb", "dir.foo/file.pb", "..pb"}) { + EXPECT_THAT(filename, IsBinaryProtoExtension()); + } + // Type handling + EXPECT_TRUE(HasBinaryProtoExtension(".binpb")); + EXPECT_TRUE(HasBinaryProtoExtension(std::string{".binpb"})); + EXPECT_TRUE(HasBinaryProtoExtension(std::string_view{".binpb"})); + EXPECT_TRUE(HasBinaryProtoExtension(std::filesystem::path(".binpb"))); + // False: text prto + for (std::string_view filename : {".txtpb", ".textproto", "file.txtpb", "dir.foo/file.txtpb", "..txtpb"}) { + EXPECT_THAT(filename, Not(IsBinaryProtoExtension())); + } + // Type handling + EXPECT_FALSE(HasBinaryProtoExtension(".txtpb")); + EXPECT_FALSE(HasBinaryProtoExtension(std::string{".txtpb"})); + EXPECT_FALSE(HasBinaryProtoExtension(std::string_view{".txtpb"})); + EXPECT_FALSE(HasBinaryProtoExtension(std::filesystem::path(".txtpb"))); + // False: others + for (std::string_view filename : {".txtproto", ".proto", "binpb", "pb", "txtpb", "textproto"}) { + EXPECT_THAT(filename, Not(IsBinaryProtoExtension())); + } +} + +TEST_F(FileProtoTest, HasTextProtoExtension) { + // True: text proto + for (std::string_view filename : {".txtpb", ".textproto", "file.txtpb", "dir.foo/file.txtpb", "..txtpb"}) { + EXPECT_THAT(filename, IsTextProtoExtension()); + } + // Type handling + EXPECT_TRUE(HasTextProtoExtension(".txtpb")); + EXPECT_TRUE(HasTextProtoExtension(std::string{".txtpb"})); + EXPECT_TRUE(HasTextProtoExtension(std::string_view{".txtpb"})); + EXPECT_TRUE(HasTextProtoExtension(std::filesystem::path(".txtpb"))); + // False: binary proto + for (std::string_view filename : {".binpb", ".pb", "file.binpb", "dir.foo/file.pb", "..pb"}) { + EXPECT_THAT(filename, Not(IsTextProtoExtension())); + } + for (std::string_view filename : {".txtproto", ".proto", "binpb", "pb", "txtpb", "textproto"}) { + EXPECT_THAT(filename, Not(IsBinaryProtoExtension())); } - { - ASSERT_TRUE(WriteFile("SomeFile.textproto", "\xFF")); - const absl::StatusOr result = ReadTextProtoFile("SomeFile.textproto"); - ASSERT_FALSE(result.ok()); - EXPECT_THAT(result.status().code(), absl::StatusCode::kAborted); - EXPECT_THAT(result.status().message(), HasSubstr("Cannot parse text proto file 'SomeFile.textproto'")); + // Type handling + EXPECT_FALSE(HasTextProtoExtension(".binpb")); + EXPECT_FALSE(HasTextProtoExtension(std::string{".binpb"})); + EXPECT_FALSE(HasTextProtoExtension(std::string_view{".binpb"})); + EXPECT_FALSE(HasTextProtoExtension(std::filesystem::path(".binpb"))); + // False: others + for (std::string_view filename : {".txtproto", ".proto", "binpb", "pb", "txtpb", "textproto"}) { + EXPECT_THAT(filename, Not(IsBinaryProtoExtension())); } } diff --git a/mbo/proto/status_matchers.h b/mbo/proto/status_matchers.h new file mode 100644 index 0000000..230c9a3 --- /dev/null +++ b/mbo/proto/status_matchers.h @@ -0,0 +1,80 @@ +// SPDX-FileCopyrightText: Copyright (c) The helly25/mbo authors (helly25.com), The CPP Proto Builder Authors +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// ----------------------------------------------------------------------------- +// Do not use this elsewhere. Instead: +// - just upgrade your Abseil dependency. +// #include "absl/status/status_matchers" +// - use @helly25_mbo//mbo/testing:status_cc +// #include "mbo/testing/status.h" +// +// These are layman's implementations to simplify testwriting without incuring +// additional requirements for external libraries. The code will deleted in the +// future and replaced with Abseil functionality. +// ----------------------------------------------------------------------------- + +#ifndef MBO_PROTO_STATUS_MATCHERS_H_ +#define MBO_PROTO_STATUS_MATCHERS_H_ + +#include +#include + +#include "absl/status/status.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace mbo::proto { + +using ::testing::ExplainMatchResult; +using ::testing::HasSubstr; +using ::testing::Not; +using ::testing::Optional; + +MATCHER(IsOk, "") { + const absl::Status status = [&] { + if constexpr (std::same_as>) { + return arg; + } else { + return arg.status(); + } + }(); + if (status.ok()) { + *result_listener << "argument is OK"; + return true; + } + *result_listener << "argument is not OK and has status " << status.ToString(); + return false; +} + +MATCHER_P(IsOkAndHolds, matcher, "") { + return ExplainMatchResult(IsOk(), arg, result_listener) && ExplainMatchResult(matcher, *arg, result_listener); +} + +MATCHER_P2(StatusIs, code_matcher, message_matcher, "") { + const absl::Status status = [&] { + if constexpr (std::same_as>) { + return arg; + } else { + return arg.status(); + } + }(); + return ExplainMatchResult(Not(IsOk()), status, result_listener) + && ExplainMatchResult(code_matcher, status.code(), result_listener) + && ExplainMatchResult(message_matcher, status.message(), result_listener); +} + +} // namespace mbo::proto + +#endif // MBO_PROTO_STATUS_MATCHERS_H_