commit 07a61ccf0226a5a370bf93a3f275e561f07beb44
author Taylor Cramer <cramertj@google.com> Fri Jul 29 22:55:40 2022 +0000
committer CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Jul 29 22:55:40 2022 +0000
tree b7c10f256f5267b971d69df3b4dae3a9eaa479bd
parent c8bb7b15b66e57e9b0c7f58104dc1912b028745e

pw_protobuf: Add StreamEncoder::CloseEncoder

Previously, `StreamEncoder` would only perform writes to its parent
after destruction occurred. This resulted in callers frequently needing
to introduce new scopes, often with a comment explaining the API to
readers.

This change adds the `CloseEncoder` method, which explicitly finalizes
the child encoder and performs the writes to the parent, allowing users
both to close an encoder before the end of its lexical scope and to make
their code more explicit and self-descriptive if desired.

Change-Id: Ieedf99048da3fe2d87602eea1ae85165c378d70a
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/103922
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Taylor Cramer <cramertj@google.com>

pw_protobuf/encoder.cc

diff --git a/pw_protobuf/encoder.cc b/pw_protobuf/encoder.cc
index b7ddba3..c7209f2 100644
--- a/pw_protobuf/encoder.cc
+++ b/pw_protobuf/encoder.cc
@@ -65,7 +65,7 @@
   return StreamEncoder(*this, nested_buffer, write_when_empty);
 }
 
-StreamEncoder::~StreamEncoder() {
+void StreamEncoder::CloseEncoder() {
   // If this was an invalidated StreamEncoder which cannot be used, permit the
   // object to be cleanly destructed by doing nothing.
   if (nested_field_number_ == kFirstReservedNumber) {

pw_protobuf/encoder_test.cc

diff --git a/pw_protobuf/encoder_test.cc b/pw_protobuf/encoder_test.cc
index c4700a6..4833814 100644
--- a/pw_protobuf/encoder_test.cc
+++ b/pw_protobuf/encoder_test.cc
@@ -502,5 +502,18 @@
   ASSERT_EQ(parent.status(), Status::InvalidArgument());
 }
 
+TEST(StreamEncoder, ManualCloseEncoderWrites) {
+  std::byte encode_buffer[32];
+  MemoryEncoder parent(encode_buffer);
+  StreamEncoder child = parent.GetNestedEncoder(kTestProtoNestedField);
+  child.CloseEncoder();
+  ASSERT_EQ(parent.status(), OkStatus());
+  const size_t kExpectedSize =
+      varint::EncodedSize(
+          FieldKey(kTestProtoNestedField, WireType::kDelimited)) +
+      varint::EncodedSize(0);
+  ASSERT_EQ(parent.size(), kExpectedSize);
+}
+
 }  // namespace
 }  // namespace pw::protobuf

pw_protobuf/public/pw_protobuf/encoder.h

diff --git a/pw_protobuf/public/pw_protobuf/encoder.h b/pw_protobuf/public/pw_protobuf/encoder.h
index 7582a81..2f8701f 100644
--- a/pw_protobuf/public/pw_protobuf/encoder.h
+++ b/pw_protobuf/public/pw_protobuf/encoder.h
@@ -128,7 +128,7 @@
   //
   // Postcondition: If this encoder is a nested one, the parent encoder is
   //     unlocked and proto encoding may resume on the parent.
-  ~StreamEncoder();
+  ~StreamEncoder() { CloseEncoder(); }
 
   // Disallow copy/assign to avoid confusion about who owns the buffer.
   StreamEncoder& operator=(const StreamEncoder& other) = delete;
@@ -138,6 +138,19 @@
   // parent_ pointer to become invalid.
   StreamEncoder& operator=(StreamEncoder&& other) = delete;
 
+  // Closes this encoder, finalizing its output.
+  //
+  // This method is called automatically by `StreamEncoder`'s destructor, but
+  // may be invoked manually in order to close an encoder before the end of its
+  // lexical scope.
+  //
+  // Precondition: Encoder has no active child encoder.
+  //
+  // Postcondition: If this encoder is a nested one, the parent encoder is
+  //     unlocked and proto encoding may resume on the parent. No more writes
+  //     to this encoder may be performed.
+  void CloseEncoder();
+
   // Forwards the conservative write limit of the underlying
   // pw::stream::Writer.
   //