Helium: Initial proto and Syncable interface definition

blimp/net/helium/syncable_unittest.cc

+// 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/bind.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 {
+
+// TODO(scf): Will delete this before checkin in. This is just to
+// ilustrate a sample implementation. Will replace with mocks.

[Brian Goldman, 2016/09/29 19:44:23]

Actually I favor keeping IntValueSyncable around for documentation purposes.

+
+// For simplicity of this example, the ChangeSet will be an integer.
+class IntValueSyncable : public Syncable<int> {
+ public:
+  explicit IntValueSyncable(VectorClock* parent_clock)
+      : Syncable(parent_clock), value_(0) {
+    // We initialize the last time we modified with the parent clock
+    last_modified_ = Syncable::parent_clock();
+  }
+
+  bool ModifiedSince(const VectorClock& from) override {
+    // If something was modified from the last time we synchronized with
+    // my current clock.
+    return from.local_revision() < last_modified_.local_revision();
+  }
+
+  void CreateChangesetToCurrent(const VectorClock& from,
+                                int* changeset) override {
+    *changeset = value_;
+  }
+
+  void ApplyChangeset(const VectorClock& from,
+                      const VectorClock& to,
+                      const 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 get_value() { return value_; }
+
+ private:
+  // The last time this object was changed
+  VectorClock last_modified_;
+  int32_t value_;
+};
+
+class ParentObjectSyncable : public SyncableObject {
+ public:
+  explicit ParentObjectSyncable(VectorClock clock)
+      : SyncableObject(clock), child1_(&clock_), child2_(&clock_) {}
+
+  void CreateChangesetToCurrent(
+      const VectorClock& from,
+      const CreateChangesetCallback& response_callback) override {
+    std::unique_ptr<helium::Changeset> changeset =
+        base::MakeUnique<helium::Changeset>();
+
+    int child1_value = 0;
+    if (child1_.ModifiedSince(from)) {
+      child1_.CreateChangesetToCurrent(from, &child1_value);
+    }
+
+    int child2_value = 0;
+    if (child2_.ModifiedSince(from)) {
+      child2_.CreateChangesetToCurrent(from, &child2_value);
+    }
+
+    // HACK: For simplicity of this example I will encode the first
+    // value in |session_id| and the second in |message_id|
+    BlimpMessage* bm = changeset->mutable_legacy()->add_messages();
+    bm->set_session_id(child1_value);
+    bm->set_message_id(child2_value);
+
+    response_callback.Run(std::move(changeset));
+  }
+
+  void ApplyChangeset(const VectorClock& from,
+                      const VectorClock& to,
+                      const helium::Changeset& content,
+                      const base::Closure& done) override {
+    BlimpMessage bm = content.legacy().messages(0);
+
+    // HACK: For simplicity of this example I will encode the first
+    // value in |session_id| and the second in |message_id|
+
+    int child1_value = bm.session_id();
+    if (child1_value != 0) {
+      child1_.ApplyChangeset(from, to, child1_value);
+    }
+
+    int child2_value = bm.message_id();
+    if (child2_value != 0) {
+      child2_.ApplyChangeset(from, to, child2_value);
+    }
+
+    done.Run();
+  }
+
+  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_;
+};
+
+class SyncableTest : public testing::Test {
+ public:
+  SyncableTest()
+      : last_sync_local_(),
+        last_sync_remote_(),
+        parent_local_(last_sync_local_),
+        parent_remote_(last_sync_remote_) {}
+  ~SyncableTest() override {}
+
+  void OnLocalChangesetCreated(std::unique_ptr<helium::Changeset> changeset) {
+    VectorClock local_clock = parent_local_.get_clock();
+    VectorClock remote_clock = local_clock.Invert();
+    base::Closure done = base::Bind(&SyncableTest::OnRemoteChangesetApplied,
+                                    base::Unretained(this));
+    parent_remote_.ApplyChangeset(last_sync_remote_, remote_clock,
+                                  *(changeset.get()), done);
+    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_));
+  }
+
+  void OnRemoteChangesetApplied() {
+    EXPECT_EQ(123, parent_remote_.get_child1().get_value());
+    EXPECT_EQ(0, parent_remote_.get_child2().get_value());
+  }
+
+ protected:
+  VectorClock last_sync_local_;
+  VectorClock last_sync_remote_;

[scf, 2016/09/29 18:26:53]

One thought also is have this bit of information in the SyncableObject itself.
We might as well add the object id in there for making state management simpler.
@kmarshall: thoughts?

+  ParentObjectSyncable parent_local_;
+  ParentObjectSyncable parent_remote_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SyncableTest);
+};
+
+TEST_F(SyncableTest, Test1) {

[CJ, 2016/09/29 20:36:55]

Usually we want to give our Test names a little more description. What are you
testing here?

Also, we seem to only be testing one aspect of your class. Perhaps more tests?

[scf, 2016/09/29 23:13:39]

Good catch. I'm not really testing much here other than the test code and my
assumptions. So I will rename this to DemoAPITest

+  // 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_));
+
+  ParentObjectSyncable::CreateChangesetCallback callback = base::Bind(
+      &SyncableTest::OnLocalChangesetCreated, base::Unretained(this));
+  parent_local_.CreateChangesetToCurrent(last_sync_local_, callback);
+}
+
+}  // namespace
+}  // namespace blimp