Use milliseconds in bigtable::SetCell (#335)

The timestamps must be in milliseconds, so make it hard for the
user to make a mistake with them.  This fixes #274.

Author: coryan

bigtable/benchmarks/embedded_server_test.cc

@@ -19,6 +19,7 @@
 #include "bigtable/client/table_admin.h"
 
 using namespace bigtable::benchmarks;
+using std::chrono::milliseconds;
 
 TEST(EmbeddedServer, WaitAndShutdown) {
   auto server = CreateEmbeddedServer();
@@ -69,8 +70,8 @@ TEST(EmbeddedServer, TableApply) {
 
   bigtable::SingleRowMutation mutation(
       "row1",
-      {bigtable::SetCell("fam", "col", 0, "val"),
-       bigtable::SetCell("fam", "col", 0, "val")});
+      {bigtable::SetCell("fam", "col", milliseconds(0), "val"),
+       bigtable::SetCell("fam", "col", milliseconds(0), "val")});
 
   EXPECT_EQ(0, server->mutate_row_count());
   table.Apply(std::move(mutation));
@@ -93,9 +94,9 @@ TEST(EmbeddedServer, TableBulkApply) {
 
   bigtable::BulkMutation bulk;
   bulk.emplace_back(bigtable::SingleRowMutation(
-      "row1", {bigtable::SetCell("fam", "col", 0, "val")}));
+      "row1", {bigtable::SetCell("fam", "col", milliseconds(0), "val")}));
   bulk.emplace_back(bigtable::SingleRowMutation(
-      "row2", {bigtable::SetCell("fam", "col", 0, "val")}));
+      "row2", {bigtable::SetCell("fam", "col", milliseconds(0), "val")}));
 
   EXPECT_EQ(0, server->mutate_rows_count());
   table.BulkApply(std::move(bulk));

bigtable/benchmarks/random.cc

@@ -28,8 +28,8 @@ std::string Sample(DefaultPRNG& gen, int n, std::string const& population) {
 
 bigtable::Mutation MakeRandomMutation(DefaultPRNG& gen, int f) {
   std::string field = "field" + std::to_string(f);
-  return bigtable::SetCell(kColumnFamily, std::move(field), 0,
-                           MakeRandomValue(gen));
+  return bigtable::SetCell(kColumnFamily, std::move(field),
+                           std::chrono::milliseconds(0), MakeRandomValue(gen));
 }
 
 std::string MakeRandomValue(DefaultPRNG& generator) {

bigtable/client/idempotent_mutation_policy_test.cc

@@ -13,38 +13,35 @@
 // limitations under the License.
 
 #include "bigtable/client/idempotent_mutation_policy.h"
-
 #include <gmock/gmock.h>
+#include "bigtable/client/testing/chrono_literals.h"
 
 /// @test Verify that the default policy works as expected.
 TEST(IdempotentMutationPolicyTest, Simple) {
+  using namespace bigtable::chrono_literals;
   auto policy = bigtable::DefaultIdempotentMutationPolicy();
   EXPECT_TRUE(policy->is_idempotent(
       bigtable::DeleteFromColumn("fam", "col", 0, 10).op));
   EXPECT_TRUE(policy->is_idempotent(bigtable::DeleteFromFamily("fam").op));
 
   EXPECT_TRUE(
-      policy->is_idempotent(bigtable::SetCell("fam", "col", 0, "v1").op));
+      policy->is_idempotent(bigtable::SetCell("fam", "col", 0_ms, "v1").op));
   EXPECT_FALSE(policy->is_idempotent(bigtable::SetCell("fam", "c2", "v2").op));
-  EXPECT_FALSE(policy->is_idempotent(
-      bigtable::SetCell("f", "c", bigtable::ServerSetTimestamp(), "v").op));
 }
 
 /// @test Verify that bigtable::AlwaysRetryMutationPolicy works as expected.
 TEST(IdempotentMutationPolicyTest, AlwaysRetry) {
+  using namespace bigtable::chrono_literals;
   bigtable::AlwaysRetryMutationPolicy policy;
   EXPECT_TRUE(
       policy.is_idempotent(bigtable::DeleteFromColumn("fam", "col", 0, 10).op));
   EXPECT_TRUE(policy.is_idempotent(bigtable::DeleteFromFamily("fam").op));
 
   EXPECT_TRUE(
-      policy.is_idempotent(bigtable::SetCell("fam", "col", 0, "v1").op));
+      policy.is_idempotent(bigtable::SetCell("fam", "col", 0_ms, "v1").op));
   EXPECT_TRUE(policy.is_idempotent(bigtable::SetCell("fam", "c2", "v2").op));
-  EXPECT_TRUE(policy.is_idempotent(
-      bigtable::SetCell("f", "c", bigtable::ServerSetTimestamp(), "v").op));
 
   auto clone = policy.clone();
-  EXPECT_TRUE(clone->is_idempotent(
-      bigtable::SetCell("f", "c", bigtable::ServerSetTimestamp(), "v").op));
-  EXPECT_TRUE(clone->is_idempotent(bigtable::SetCell("f", "c", 10, "v").op));
+  EXPECT_TRUE(clone->is_idempotent(bigtable::SetCell("f", "c", "v").op));
+  EXPECT_TRUE(clone->is_idempotent(bigtable::SetCell("f", "c", 10_ms, "v").op));
 }

bigtable/client/internal/bulk_mutator_test.cc

@@ -13,10 +13,9 @@
 // limitations under the License.
 
 #include "bigtable/client/internal/bulk_mutator.h"
-
 #include <google/bigtable/v2/bigtable_mock.grpc.pb.h>
-
 #include "bigtable/client/internal/make_unique.h"
+#include "bigtable/client/testing/chrono_literals.h"
 
 /// Define types and functions used in the tests.
 namespace {
@@ -35,12 +34,13 @@ TEST(MultipleRowsMutatorTest, Simple) {
   namespace btproto = ::google::bigtable::v2;
   namespace bt = ::bigtable;
   using namespace ::testing;
+  using namespace bigtable::chrono_literals;
 
   // In this test we create a Mutation for two rows, which succeeds in the
   // first RPC request.  First create the mutation.
   bt::BulkMutation mut(
-      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0, "baz")}),
-      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0, "qux")}));
+      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0_ms, "baz")}),
+      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0_ms, "qux")}));
 
   // Prepare the mocks.  The mutator should issue a RPC which must return a
   // stream of responses, we prepare the stream first because it is easier than
@@ -88,12 +88,13 @@ TEST(MultipleRowsMutatorTest, RetryPartialFailure) {
   namespace btproto = ::google::bigtable::v2;
   namespace bt = ::bigtable;
   using namespace ::testing;
+  using namespace bigtable::chrono_literals;
 
   // In this test we create a Mutation for two rows, one of which will fail.
   // First create the mutation.
   bt::BulkMutation mut(
-      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0, "baz")}),
-      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0, "qux")}));
+      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0_ms, "baz")}),
+      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0_ms, "qux")}));
 
   // Prepare the mocks for the request.  First create a stream response which
   // indicates a partial failure.
@@ -162,12 +163,13 @@ TEST(MultipleRowsMutatorTest, PermanentFailure) {
   namespace btproto = ::google::bigtable::v2;
   namespace bt = ::bigtable;
   using namespace ::testing;
+  using namespace bigtable::chrono_literals;
 
   // In this test we handle a recoverable and one unrecoverable failures.
   // Create a bulk mutation with two SetCell() mutations.
   bt::BulkMutation mut(
-      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0, "baz")}),
-      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0, "qux")}));
+      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0_ms, "baz")}),
+      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0_ms, "qux")}));
 
   // Make the first RPC return one recoverable and one unrecoverable failures.
   auto r1 = bigtable::internal::make_unique<MockReader>();
@@ -237,12 +239,13 @@ TEST(MultipleRowsMutatorTest, PartialStream) {
   namespace btproto = ::google::bigtable::v2;
   namespace bt = ::bigtable;
   using namespace ::testing;
+  using namespace bigtable::chrono_literals;
 
   // We are going to test the case where the stream does not contain a response
   // for all requests.  Create a BulkMutation with two entries.
   bt::BulkMutation mut(
-      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0, "baz")}),
-      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0, "qux")}));
+      bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", 0_ms, "baz")}),
+      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0_ms, "qux")}));
 
   // This will be the stream returned by the first request.  It is missing
   // information about one of the mutations.
@@ -305,11 +308,12 @@ TEST(MultipleRowsMutatorTest, RetryOnlyIdempotent) {
   namespace btproto = ::google::bigtable::v2;
   namespace bt = ::bigtable;
   using namespace ::testing;
+  using namespace bigtable::chrono_literals;
 
   // Create a BulkMutation with a non-idempotent mutation.
   bt::BulkMutation mut(
       bt::SingleRowMutation("foo", {bt::SetCell("fam", "col", "baz")}),
-      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0, "qux")}),
+      bt::SingleRowMutation("bar", {bt::SetCell("fam", "col", 0_ms, "qux")}),
       bt::SingleRowMutation("baz", {bt::SetCell("fam", "col", "v")}));
 
   // We will setup the mock to return recoverable failures for idempotent

bigtable/client/mutations.cc

@@ -17,13 +17,14 @@
 
 namespace bigtable {
 inline namespace BIGTABLE_CLIENT_NS {
-Mutation SetCell(std::string family, std::string column, std::int64_t timestamp,
-                 std::string value) {
+Mutation SetCell(std::string family, std::string column,
+                 std::chrono::milliseconds timestamp, std::string value) {
   Mutation m;
   auto& set_cell = *m.op.mutable_set_cell();
   set_cell.set_family_name(std::move(family));
   set_cell.set_column_qualifier(std::move(column));
-  set_cell.set_timestamp_micros(timestamp);
+  set_cell.set_timestamp_micros(
+      std::chrono::duration_cast<std::chrono::microseconds>(timestamp).count());
   set_cell.set_value(std::move(value));
   return m;
 }

bigtable/client/mutations.h

@@ -38,8 +38,8 @@ struct Mutation {
 };
 
 /// Create a mutation to set a cell value.
-Mutation SetCell(std::string family, std::string column, std::int64_t timestamp,
-                 std::string value);
+Mutation SetCell(std::string family, std::string column,
+                 std::chrono::milliseconds timestamp, std::string value);
 
 /**
  * Create a mutation to set a cell value where the server sets the time.

bigtable/client/mutations_test.cc

@@ -15,14 +15,17 @@
 #include "bigtable/client/mutations.h"
 #include <gmock/gmock.h>
 #include <google/rpc/error_details.pb.h>
+#include "bigtable/client/testing/chrono_literals.h"
+
+using namespace bigtable::chrono_literals;
 
 /// @test Verify that SetCell() works as expected.
 TEST(MutationsTest, SetCell) {
-  auto actual = bigtable::SetCell("family", "col", 1234, "value");
+  auto actual = bigtable::SetCell("family", "col", 1234_ms, "value");
   ASSERT_TRUE(actual.op.has_set_cell());
   EXPECT_EQ("family", actual.op.set_cell().family_name());
   EXPECT_EQ("col", actual.op.set_cell().column_qualifier());
-  EXPECT_EQ(1234, actual.op.set_cell().timestamp_micros());
+  EXPECT_EQ(1234000, actual.op.set_cell().timestamp_micros());
   EXPECT_EQ("value", actual.op.set_cell().value());
 
   auto server_set = bigtable::SetCell("fam", "col", "v");
@@ -40,8 +43,8 @@ TEST(MutationsTest, SetCell) {
   // work around that optimization and test the move behavior.
   std::string val(1000000, 'a');
   auto val_data = val.data();
-  auto moved =
-      bigtable::SetCell(std::move(fam), std::move(col), 2345, std::move(val));
+  auto moved = bigtable::SetCell(std::move(fam), std::move(col), 2345_ms,
+                                 std::move(val));
   ASSERT_TRUE(moved.op.has_set_cell());
   EXPECT_EQ("fam2", moved.op.set_cell().family_name());
   EXPECT_EQ("col2", moved.op.set_cell().column_qualifier());
@@ -113,7 +116,7 @@ TEST(MutationsTest, DeleteFromRow) {
 // @test Verify that FailedMutation works as expected.
 TEST(MutationsTest, FailedMutation) {
   bigtable::SingleRowMutation mut("foo",
-                                  {bigtable::SetCell("f", "c", 0, "val")});
+                                  {bigtable::SetCell("f", "c", 0_ms, "val")});
 
   // Create an overly complicated detail status, the idea is to make
   // sure it works in this case.
@@ -153,9 +156,9 @@ TEST(MutationsTest, MutipleRowMutations) {
 
   actual
       .emplace_back(bigtable::SingleRowMutation(
-          "foo1", {bigtable::SetCell("f", "c", 0, "v1")}))
+          "foo1", {bigtable::SetCell("f", "c", 0_ms, "v1")}))
       .push_back(bigtable::SingleRowMutation(
-          "foo2", {bigtable::SetCell("f", "c", 0, "v2")}));
+          "foo2", {bigtable::SetCell("f", "c", 0_ms, "v2")}));
 
   actual.MoveTo(&request);
   ASSERT_EQ(2, request.entries_size());
@@ -164,11 +167,11 @@ TEST(MutationsTest, MutipleRowMutations) {
 
   std::vector<bigtable::SingleRowMutation> vec{
       bigtable::SingleRowMutation("foo1",
-                                  {bigtable::SetCell("f", "c", 0, "v1")}),
+                                  {bigtable::SetCell("f", "c", 0_ms, "v1")}),
       bigtable::SingleRowMutation("foo2",
-                                  {bigtable::SetCell("f", "c", 0, "v2")}),
+                                  {bigtable::SetCell("f", "c", 0_ms, "v2")}),
       bigtable::SingleRowMutation("foo3",
-                                  {bigtable::SetCell("f", "c", 0, "v3")}),
+                                  {bigtable::SetCell("f", "c", 0_ms, "v3")}),
   };
   bigtable::BulkMutation from_vec(vec.begin(), vec.end());
 
@@ -180,9 +183,9 @@ TEST(MutationsTest, MutipleRowMutations) {
 
   bigtable::BulkMutation from_il{
       bigtable::SingleRowMutation("foo2",
-                                  {bigtable::SetCell("f", "c", 0, "v2")}),
+                                  {bigtable::SetCell("f", "c", 0_ms, "v2")}),
       bigtable::SingleRowMutation("foo3",
-                                  {bigtable::SetCell("f", "c", 0, "v3")}),
+                                  {bigtable::SetCell("f", "c", 0_ms, "v3")}),
   };
   from_il.MoveTo(&request);
   ASSERT_EQ(2, request.entries_size());
@@ -195,8 +198,8 @@ TEST(MutationsTest, SingleRowMutationMultipleVariadic) {
   std::string const row_key = "row-key-1";
 
   bigtable::SingleRowMutation actual(
-      row_key, bigtable::SetCell("family", "c1", 1000, "V1000"),
-      bigtable::SetCell("family", "c2", 2000, "V2000"));
+      row_key, bigtable::SetCell("family", "c1", 1_ms, "V1000"),
+      bigtable::SetCell("family", "c2", 2_ms, "V2000"));
 
   google::bigtable::v2::MutateRowsRequest::Entry entry;
   (void)entry.add_mutations();
@@ -212,7 +215,7 @@ TEST(MutationsTest, SingleRowMutationSingleVariadic) {
   std::string const row_key = "row-key-1";
 
   bigtable::SingleRowMutation actual(
-      row_key, bigtable::SetCell("family", "c1", 1000, "V1000"));
+      row_key, bigtable::SetCell("family", "c1", 1_ms, "V1000"));
 
   google::bigtable::v2::MutateRowsRequest::Entry entry;
   (void)entry.add_mutations();

bigtable/client/table_apply_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "bigtable/client/table.h"
+#include "bigtable/client/testing/chrono_literals.h"
 #include "bigtable/client/testing/table_test_fixture.h"
 
 /// Define helper types and functions for this test.
@@ -21,14 +22,17 @@ class TableApplyTest : public bigtable::testing::TableTestFixture {};
 }  // anonymous namespace
 
 /// @test Verify that Table::Apply() works in a simplest case.
+
+using namespace bigtable::chrono_literals;
+
 TEST_F(TableApplyTest, Simple) {
   using namespace ::testing;
 
   EXPECT_CALL(*bigtable_stub_, MutateRow(_, _, _))
       .WillOnce(Return(grpc::Status::OK));
 
   table_.Apply(bigtable::SingleRowMutation(
-      "bar", {bigtable::SetCell("fam", "col", 0, "val")}));
+      "bar", {bigtable::SetCell("fam", "col", 0_ms, "val")}));
 }
 
 /// @test Verify that Table::Apply() raises an exception on permanent failures.
@@ -41,12 +45,12 @@ TEST_F(TableApplyTest, Failure) {
 
 #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
   EXPECT_THROW(table_.Apply(bigtable::SingleRowMutation(
-                   "bar", {bigtable::SetCell("fam", "col", 0, "val")})),
+                   "bar", {bigtable::SetCell("fam", "col", 0_ms, "val")})),
                std::exception);
 #else
   EXPECT_DEATH_IF_SUPPORTED(
       table_.Apply(bigtable::SingleRowMutation(
-          "bar", {bigtable::SetCell("fam", "col", 0, "val")})),
+          "bar", {bigtable::SetCell("fam", "col", 0_ms, "val")})),
       "exceptions are disabled");
 #endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
 }
@@ -65,7 +69,7 @@ TEST_F(TableApplyTest, Retry) {
       .WillOnce(Return(grpc::Status::OK));
 
   table_.Apply(bigtable::SingleRowMutation(
-      "bar", {bigtable::SetCell("fam", "col", 0, "val")}));
+      "bar", {bigtable::SetCell("fam", "col", 0_ms, "val")}));
 }
 
 /// @test Verify that Table::Apply() retries only idempotent mutations.