feat: add efficient move support to mutation builder temporaries (#989)

Fixes: #379

Most of the testing for this is done via static_asserts that ensure that a chain of method calls results in a call to .Build() that returns an rvalue. That ensures that the right functions are being called. Then I actually call the code and test that it works, but this part of the test doesn't really verify that moving happened, just that the code path works.

google/cloud/spanner/mutations.h

@@ -78,6 +78,8 @@ class Mutation {
   friend void PrintTo(Mutation const& m, std::ostream* os);
 
  private:
+  google::spanner::v1::Mutation& proto() & { return m_; }
+
   template <typename Op>
   friend class internal::WriteMutationBuilder;
   friend class internal::DeleteMutationBuilder;
@@ -101,32 +103,41 @@ class WriteMutationBuilder {
  public:
   WriteMutationBuilder(std::string table_name,
                        std::vector<std::string> column_names) {
-    auto& field = Op::mutable_field(m_);
+    auto& field = Op::mutable_field(m_.proto());
     field.set_table(std::move(table_name));
     field.mutable_columns()->Reserve(static_cast<int>(column_names.size()));
     for (auto& name : column_names) {
       *field.add_columns() = std::move(name);
     }
   }
 
-  Mutation Build() const& { return Mutation(m_); }
-  Mutation Build() && { return Mutation(std::move(m_)); }
+  Mutation Build() const& { return m_; }
+  Mutation&& Build() && { return std::move(m_); }
 
-  WriteMutationBuilder& AddRow(std::vector<Value> values) {
-    auto& lv = *Op::mutable_field(m_).add_values();
+  WriteMutationBuilder& AddRow(std::vector<Value> values) & {
+    auto& lv = *Op::mutable_field(m_.proto()).add_values();
     for (auto& v : values) {
       std::tie(std::ignore, *lv.add_values()) = internal::ToProto(std::move(v));
     }
     return *this;
   }
 
+  WriteMutationBuilder&& AddRow(std::vector<Value> values) && {
+    return std::move(AddRow(std::move(values)));
+  }
+
   template <typename... Ts>
-  WriteMutationBuilder& EmplaceRow(Ts&&... values) {
+  WriteMutationBuilder& EmplaceRow(Ts&&... values) & {
     return AddRow({Value(std::forward<Ts>(values))...});
   }
 
+  template <typename... Ts>
+  WriteMutationBuilder&& EmplaceRow(Ts&&... values) && {
+    return std::move(EmplaceRow(std::forward<Ts>(values)...));
+  }
+
  private:
-  google::spanner::v1::Mutation m_;
+  Mutation m_;
 };
 
 struct InsertOp {
@@ -160,16 +171,16 @@ struct ReplaceOp {
 class DeleteMutationBuilder {
  public:
   DeleteMutationBuilder(std::string table_name, KeySet keys) {
-    auto& field = *m_.mutable_delete_();
+    auto& field = *m_.proto().mutable_delete_();
     field.set_table(std::move(table_name));
     *field.mutable_key_set() = internal::ToProto(std::move(keys));
   }
 
-  Mutation Build() const& { return Mutation(m_); }
-  Mutation Build() && { return Mutation(std::move(m_)); }
+  Mutation Build() const& { return m_; }
+  Mutation&& Build() && { return std::move(m_); }
 
  private:
-  google::spanner::v1::Mutation m_;
+  Mutation m_;
 };
 
 }  // namespace internal

google/cloud/spanner/mutations_test.cc

@@ -22,6 +22,8 @@
 #include <sstream>
 #include <string>
 #include <tuple>
+#include <type_traits>
+#include <utility>
 #include <vector>
 
 namespace google {
@@ -326,6 +328,108 @@ TEST(MutationsTest, DeleteSimple) {
   EXPECT_THAT(actual, IsProtoEqual(expected));
 }
 
+TEST(MutationsTest, FluentInsertBuilder) {
+  static_assert(
+      std::is_rvalue_reference<decltype(std::declval<InsertMutationBuilder>()
+                                            .EmplaceRow()
+                                            .AddRow({})
+                                            .Build())>::value,
+      "Build() should return an rvalue if called fluently on a temporary");
+
+  std::string const data(128, 'x');
+  std::string blob = data;
+  Mutation m = InsertMutationBuilder("table-name", {"col_a"})
+                   .EmplaceRow(std::move(blob))
+                   .AddRow({Value(data)})
+                   .Build();
+  auto actual = std::move(m).as_proto();
+  EXPECT_EQ(2, actual.insert().values().size());
+  EXPECT_EQ(data, actual.insert().values(0).values(0).string_value());
+  EXPECT_EQ(data, actual.insert().values(1).values(0).string_value());
+}
+
+TEST(MutationsTest, FluentUpdateBuilder) {
+  static_assert(
+      std::is_rvalue_reference<decltype(std::declval<UpdateMutationBuilder>()
+                                            .EmplaceRow()
+                                            .AddRow({})
+                                            .Build())>::value,
+      "Build() should return an rvalue if called fluently on a temporary");
+
+  std::string const data(128, 'x');
+  std::string blob = data;
+  Mutation m = UpdateMutationBuilder("table-name", {"col_a"})
+                   .EmplaceRow(std::move(blob))
+                   .AddRow({Value(data)})
+                   .Build();
+  auto actual = std::move(m).as_proto();
+  EXPECT_EQ(2, actual.update().values().size());
+  EXPECT_EQ(data, actual.update().values(0).values(0).string_value());
+  EXPECT_EQ(data, actual.update().values(1).values(0).string_value());
+}
+
+TEST(MutationsTest, FluentInsertOrUpdateBuilder) {
+  static_assert(
+      std::is_rvalue_reference<decltype(
+          std::declval<InsertOrUpdateMutationBuilder>()
+              .EmplaceRow()
+              .AddRow({})
+              .Build())>::value,
+      "Build() should return an rvalue if called fluently on a temporary");
+
+  std::string const data(128, 'x');
+  std::string blob = data;
+  Mutation m = InsertOrUpdateMutationBuilder("table-name", {"col_a"})
+                   .EmplaceRow(std::move(blob))
+                   .AddRow({Value(data)})
+                   .Build();
+  auto actual = std::move(m).as_proto();
+  EXPECT_EQ(2, actual.insert_or_update().values().size());
+  EXPECT_EQ(data, actual.insert_or_update().values(0).values(0).string_value());
+  EXPECT_EQ(data, actual.insert_or_update().values(1).values(0).string_value());
+}
+
+TEST(MutationsTest, FluentReplaceBuilder) {
+  static_assert(
+      std::is_rvalue_reference<decltype(std::declval<ReplaceMutationBuilder>()
+                                            .EmplaceRow()
+                                            .AddRow({})
+                                            .Build())>::value,
+      "Build() should return an rvalue if called fluently on a temporary");
+
+  std::string const data(128, 'x');
+  std::string blob = data;
+  Mutation m = ReplaceMutationBuilder("table-name", {"col_a"})
+                   .EmplaceRow(std::move(blob))
+                   .AddRow({Value(data)})
+                   .Build();
+  auto actual = std::move(m).as_proto();
+  EXPECT_EQ(2, actual.replace().values().size());
+  EXPECT_EQ(data, actual.replace().values(0).values(0).string_value());
+  EXPECT_EQ(data, actual.replace().values(1).values(0).string_value());
+}
+
+TEST(MutationsTest, FluentDeleteBuilder) {
+  static_assert(
+      std::is_rvalue_reference<decltype(
+          std::declval<DeleteMutationBuilder>().Build())>::value,
+      "Build() should return an rvalue if called fluently on a temporary");
+
+  auto ks = KeySet().AddKey(MakeKey("key-to-delete"));
+  Mutation m = DeleteMutationBuilder("table-name", ks).Build();
+  auto actual = std::move(m).as_proto();
+  spanner_proto::Mutation expected;
+  ASSERT_TRUE(TextFormat::ParseFromString(
+      R"pb(
+        delete: {
+          table: "table-name"
+          key_set: { keys: { values { string_value: "key-to-delete" } } }
+        }
+      )pb",
+      &expected));
+  EXPECT_THAT(actual, IsProtoEqual(expected));
+}
+
 }  // namespace
 }  // namespace SPANNER_CLIENT_NS
 }  // namespace spanner