Add LwwRegister CRDT

blimp/helium/lww_register_unittest.cc

Index: blimp/helium/lww_register_unittest.cc
diff --git a/blimp/helium/lww_register_unittest.cc b/blimp/helium/lww_register_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fe60b67868b62ade3d2dac37b83337e09d92a46b
--- /dev/null
+++ b/blimp/helium/lww_register_unittest.cc
@@ -0,0 +1,171 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "blimp/helium/lww_register.h"
+
+#include <string>
+
+#include "blimp/helium/version_vector.h"
+#include "blimp/helium/version_vector_generator.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h"
+
+namespace blimp {
+namespace helium {
+namespace {
+
+class LwwRegisterTest : public testing::Test {
+ protected:
+  VersionVectorGenerator client_gen_;

[Kevin M, 2016/10/19 00:27:08]

Fields must come after methods.

[steimel, 2016/10/19 16:40:49]

Done.

+  VersionVectorGenerator engine_gen_;
+
+  Bias bias_;
+
+  std::unique_ptr<LwwRegister<int>> client_lww_register_;
+  std::unique_ptr<LwwRegister<int>> engine_lww_register_;
+
+  void set_bias(Bias bias) { bias_ = bias; }

[Kevin M, 2016/10/19 00:27:08]

Do we need setters? Can't the test code just access bias_ directly?

[steimel, 2016/10/19 16:40:49]

Done.

+
+  LwwRegister<int>* getClient() {

[Kevin M, 2016/10/19 00:27:08]

Camel casing isn't allowed as per the C++ style guide.

[steimel, 2016/10/19 16:40:49]

Done.

+    if (client_lww_register_) {
+      return client_lww_register_.get();
+    }
+    client_lww_register_ = base::MakeUnique<LwwRegister<int>>(
+        &client_gen_, bias_, RunningAs::Client);
+    return client_lww_register_.get();
+  }
+
+  LwwRegister<int>* getEngine() {
+    if (engine_lww_register_) {
+      return engine_lww_register_.get();
+    }
+    engine_lww_register_ = base::MakeUnique<LwwRegister<int>>(

[Kevin M, 2016/10/19 00:27:08]

Why do we need to lazily instantiate this? Why can't we just create them all the
time in SetUp()?

[steimel, 2016/10/19 16:40:49]

Discussed offline and using Initialize method instead

+        &engine_gen_, bias_, RunningAs::Engine);
+    return engine_lww_register_.get();
+  }
+
+  void SyncFromClient() {
+    Sync(getClient(), getEngine(), engine_gen_.current(),
+         client_gen_.current());
+  }
+
+  void SyncFromEngine() {
+    Sync(getEngine(), getClient(), client_gen_.current(),
+         engine_gen_.current());
+  }
+
+  void Sync(LwwRegister<int>* from_lww_register,

[Kevin M, 2016/10/19 00:27:08]

Comment on this method

[steimel, 2016/10/19 16:40:50]

Done.

+            LwwRegister<int>* to_lww_register,
+            VersionVector from,
+            VersionVector to);
+};
+
+void LwwRegisterTest::Sync(LwwRegister<int>* from_lww_register,
+                           LwwRegister<int>* to_lww_register,
+                           VersionVector from,
+                           VersionVector to) {
+  std::string changeset;
+  google::protobuf::io::StringOutputStream raw_output_stream(&changeset);
+  google::protobuf::io::CodedOutputStream output_stream(&raw_output_stream);
+
+  from_lww_register->CreateChangesetToCurrent(from.Invert(), &output_stream);
+
+  google::protobuf::io::ArrayInputStream raw_input_stream(changeset.data(),
+                                                          changeset.size());
+  google::protobuf::io::CodedInputStream input_stream(&raw_input_stream);
+  to_lww_register->ApplyChangeset(from, to.Invert(), &input_stream);
+}
+
+TEST_F(LwwRegisterTest, SetSetsStateAndIncrementsLocalVersion) {
+  set_bias(Bias::Client);
+  auto lww_register = getClient();
+
+  VersionVector earlier_version = client_gen_.current();
+  lww_register->Set(42);
+  VersionVector current_version = client_gen_.current();
+
+  EXPECT_EQ(42, lww_register->Get());
+  EXPECT_LT(earlier_version.local_revision(), current_version.local_revision());
+  EXPECT_TRUE(lww_register->ModifiedSince(earlier_version));
+  EXPECT_FALSE(lww_register->ModifiedSince(current_version));
+}
+
+TEST_F(LwwRegisterTest, ApplyChangesetLaterChangesetChangesState) {
+  set_bias(Bias::Client);
+  auto client = getClient();
+  auto engine = getEngine();
+
+  client->Set(123);
+  SyncFromClient();
+
+  EXPECT_EQ(123, engine->Get());
+}
+
+TEST_F(LwwRegisterTest, ApplyChangesetEarlierChangesetDoesNotChangeState) {
+  set_bias(Bias::Client);
+  auto client = getClient();
+  auto engine = getEngine();
+
+  client->Set(123);
+  SyncFromClient();
+
+  engine->Set(456);
+  SyncFromClient();
+
+  EXPECT_EQ(456, engine->Get());
+}
+
+TEST_F(LwwRegisterTest,
+       ApplyChangesetConflictClientWinsClientDoesNotChangeState) {
+  set_bias(Bias::Client);
+  auto client = getClient();
+  auto engine = getEngine();
+
+  client->Set(123);
+  engine->Set(456);
+  SyncFromEngine();
+
+  EXPECT_EQ(123, client->Get());
+}
+
+TEST_F(LwwRegisterTest, ApplyChangesetConflictClientWinsRemoteChangesState) {
+  set_bias(Bias::Client);
+  auto client = getClient();
+  auto engine = getEngine();
+
+  client->Set(123);
+  engine->Set(456);
+  SyncFromClient();
+
+  EXPECT_EQ(123, engine->Get());
+}
+
+TEST_F(LwwRegisterTest, ApplyChangesetConflictEngineWinsClientChangesState) {

[Kevin M, 2016/10/19 00:27:08]

Such a literal description doesn't lend itself well to brevity. Can you
summarize this more succinctly?

ApplyConflictEngineWins?

[steimel, 2016/10/19 16:40:49]

Done.

+  set_bias(Bias::Engine);
+  auto client = getClient();
+  auto engine = getEngine();
+
+  client->Set(123);
+  engine->Set(456);
+  SyncFromEngine();
+
+  EXPECT_EQ(456, client->Get());
+}
+
+TEST_F(LwwRegisterTest,
+       ApplyChangesetConflictEngineWinsRemoteDoesNotChangeState) {

[Kevin M, 2016/10/19 00:27:08]

You can probably merge the postcondition checks in these "Conflict + Engine
Wins" test cases since the test setup is identical.

[steimel, 2016/10/19 16:40:49]

In one case, I sync to the engine, in the other I sync to the client. I can't
just call one sync and then the other, since then the second sync is different.
So in order to properly emulate it, I'd have to get the changesets from each one
and then apply each one (splitting the Sync method in half essentially). I think
this is more complexity than it's worth, but we should discuss offline

+  set_bias(Bias::Engine);
+  auto client = getClient();
+  auto engine = getEngine();
+
+  client->Set(123);
+  engine->Set(456);
+  SyncFromClient();
+
+  EXPECT_EQ(456, engine->Get());
+}
+
+}  // namespace
+}  // namespace helium
+}  // namespace blimp