Helium: Initial proto and Syncable interface definition

blimp/net/helium/syncable_unittest.cc

Index: blimp/net/helium/syncable_unittest.cc
diff --git a/blimp/net/helium/syncable_unittest.cc b/blimp/net/helium/syncable_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6bef75bd562d8f78a4bb3c3fb0b190795cd979ee
--- /dev/null
+++ b/blimp/net/helium/syncable_unittest.cc
@@ -0,0 +1,170 @@
+// 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/net/helium/syncable.h"
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blimp {
+namespace {
+
+// This is a sample implementation that demostrates the implementation
+// of the SyncableMember and SyncableObject
+
+// For simplicity of this example, the ChangeSet will be an integer.
+class IntValueSyncable : public SyncableMember<int> {

[Kevin M, 2016/10/03 22:30:35]

FakeSyncable<int> ?

[scf, 2016/10/03 22:52:33]

renamed to |FakeIntSyncable|. Making a template doesn't seem very useful if its
been used for a single type parameter

+ public:
+  explicit IntValueSyncable(SyncableObject* parent)
+      : SyncableMember(parent), value_(0) {
+    last_modified_ = parent_clock();
+  }
+
+  bool ModifiedSince(const VectorClock& from) override {
+    return from.local_revision() < last_modified_.local_revision();
+  }
+
+  std::unique_ptr<int> CreateChangesetToCurrent(
+      const VectorClock& from) override {
+    return base::MakeUnique<int>(value_);
+  }
+
+  void ApplyChangeset(const VectorClock& from,
+                      const VectorClock& to,
+                      std::unique_ptr<int> changeset) override {
+    // Restore the value
+    value_ = *changeset;
+
+    // Update our clock to the latest clock
+    last_modified_ = to;
+  }
+
+  void ReleaseCheckpointsBefore(const VectorClock& checkpoint) override {
+    last_modified_ = checkpoint;
+  }
+
+  void SetValue(int value) {
+    value_ = value;
+
+    // Increment the parent clock and update our last_modified_ value
+    last_modified_ = IncrementParentClock();
+  }
+
+  int value() { return value_; }
+
+ private:
+  // The last time this object was changed
+  VectorClock last_modified_;
+  int32_t value_;
+
+  DISALLOW_COPY_AND_ASSIGN(IntValueSyncable);
+};
+
+class ParentObjectSyncable : public SyncableObject {
+ public:
+  explicit ParentObjectSyncable(VectorClock clock)
+      : SyncableObject(clock), child1_(this), child2_(this) {}
+
+  std::unique_ptr<helium::ChangesetMessage> CreateChangesetToCurrent(
+      const VectorClock& from) override {
+    std::unique_ptr<helium::ChangesetMessage> changeset =
+        base::MakeUnique<helium::ChangesetMessage>();
+
+    helium::TestChangesetMessage* bm = changeset->mutable_test();
+
+    if (child1_.ModifiedSince(from)) {
+      std::unique_ptr<int> value1 = child1_.CreateChangesetToCurrent(from);
+      bm->set_value1(*value1);
+    }
+
+    if (child2_.ModifiedSince(from)) {
+      std::unique_ptr<int> value2 = child2_.CreateChangesetToCurrent(from);
+      bm->set_value2(*value2);
+    }
+
+    return changeset;
+  }
+
+  void ApplyChangeset(
+      const VectorClock& from,
+      const VectorClock& to,
+      std::unique_ptr<helium::ChangesetMessage> changeset) override {
+    helium::TestChangesetMessage bm = changeset->test();
+
+    int child1_value = bm.value1();
+    if (child1_value != 0) {

[Kevin M, 2016/10/03 22:30:35]

Shouldn't we be calling ModifiedSince() instead of checking against magic
numbers?

[scf, 2016/10/03 22:52:33]

No. We cannot call ModifiedSince(). The other peer might not have been modified.
The idea I had was using |has_fieldname| from protobuf. But since its a scalar
value its not generated (And i didn't want to inflate the proto definition too
much)

+      child1_.ApplyChangeset(from, to, base::MakeUnique<int>(child1_value));
+    }
+
+    int child2_value = bm.value2();
+    if (child2_value != 0) {
+      child2_.ApplyChangeset(from, to, base::MakeUnique<int>(child2_value));
+    }
+  }
+
+  void ReleaseCheckpointsBefore(const VectorClock& checkpoint) override {
+    child1_.ReleaseCheckpointsBefore(checkpoint);
+    child2_.ReleaseCheckpointsBefore(checkpoint);
+  }
+
+  IntValueSyncable& get_child1() { return child1_; }
+  IntValueSyncable& get_child2() { return child2_; }
+
+ private:
+  IntValueSyncable child1_;
+  IntValueSyncable child2_;
+
+  DISALLOW_COPY_AND_ASSIGN(ParentObjectSyncable);
+};
+
+class SyncableTest : public testing::Test {
+ public:
+  SyncableTest()
+      : last_sync_local_(),
+        last_sync_remote_(),
+        parent_local_(last_sync_local_),

[Kevin M, 2016/10/03 22:30:35]

How does this work? last_sync_X_ is passed by-value and therefore will not be
updated as parent_X_ changes. Please beef up the test case so that this omission
gets caught.

[scf, 2016/10/03 22:52:33]

Agree its not needed. The idea was to pass the initial time

+        parent_remote_(last_sync_remote_) {}
+
+  ~SyncableTest() override {}
+
+ protected:
+  VectorClock last_sync_local_;
+  VectorClock last_sync_remote_;
+  ParentObjectSyncable parent_local_;
+  ParentObjectSyncable parent_remote_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SyncableTest);
+};
+
+TEST_F(SyncableTest, DemoAPITest) {

[Kevin M, 2016/10/03 22:30:35]

More descriptive test name?

[scf, 2016/10/03 22:52:33]

Done.

+  // Lets modify a children object
+  parent_local_.get_child1().SetValue(123);
+
+  // At this point |child1| and |parentObject| should have its clock incremented
+  // whereas |child2| should still be the same.
+  EXPECT_TRUE(parent_local_.get_child1().ModifiedSince(last_sync_local_));
+  EXPECT_FALSE(parent_local_.get_child2().ModifiedSince(last_sync_local_));
+
+  std::unique_ptr<helium::ChangesetMessage> changeset =
+      parent_local_.CreateChangesetToCurrent(last_sync_local_);
+
+  VectorClock local_clock = parent_local_.clock();
+  VectorClock remote_clock = local_clock.Invert();
+
+  parent_remote_.ApplyChangeset(last_sync_remote_, remote_clock,
+                                std::move(changeset));
+  last_sync_local_ = local_clock;
+  parent_local_.ReleaseCheckpointsBefore(local_clock);
+  EXPECT_FALSE(parent_local_.get_child1().ModifiedSince(last_sync_local_));
+  EXPECT_FALSE(parent_local_.get_child2().ModifiedSince(last_sync_local_));
+
+  EXPECT_EQ(123, parent_remote_.get_child1().value());
+  EXPECT_EQ(0, parent_remote_.get_child2().value());
+}
+
+}  // namespace
+}  // namespace blimp