summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--sync/engine/commit_util.cc5
-rw-r--r--sync/engine/directory_commit_contribution_unittest.cc62
-rw-r--r--sync/syncable/entry.cc4
-rw-r--r--sync/syncable/entry.h4
-rw-r--r--sync/syncable/entry_kernel.cc8
-rw-r--r--sync/syncable/entry_kernel.h1
-rw-r--r--sync/test/fake_server/fake_server_entity.cc11
-rw-r--r--sync/test/fake_server/fake_server_entity.h9
-rw-r--r--sync/test/fake_server/permanent_entity.cc2
-rw-r--r--sync/test/fake_server/unique_client_entity.cc9
-rw-r--r--sync/test/fake_server/unique_client_entity.h4
-rw-r--r--sync/tools/testserver/chromiumsync.py35
12 files changed, 138 insertions, 16 deletions
diff --git a/sync/engine/commit_util.cc b/sync/engine/commit_util.cc
index 36b7c06..ad29e4f 100644
--- a/sync/engine/commit_util.cc
+++ b/sync/engine/commit_util.cc
@@ -128,7 +128,10 @@ void BuildCommitItem(
} else {
new_parent_id = meta_entry.GetParentId();
}
- sync_entry->set_parent_id_string(SyncableIdToProto(new_parent_id));
+
+ if (meta_entry.ShouldMaintainHierarchy()) {
+ sync_entry->set_parent_id_string(SyncableIdToProto(new_parent_id));
+ }
// If our parent has changed, send up the old one so the server
// can correctly deal with multiple parents.
diff --git a/sync/engine/directory_commit_contribution_unittest.cc b/sync/engine/directory_commit_contribution_unittest.cc
index b342b54..894db97 100644
--- a/sync/engine/directory_commit_contribution_unittest.cc
+++ b/sync/engine/directory_commit_contribution_unittest.cc
@@ -274,6 +274,68 @@ TEST_F(DirectoryCommitContributionTest, DeletedBookmarksWithSpecifics) {
bm_cc->CleanUp();
}
+// Test that bookmarks support hierarchy.
+TEST_F(DirectoryCommitContributionTest, HierarchySupport_Bookmark) {
+
+ // Create a normal-looking bookmark item.
+ int64 bm1;
+ {
+ syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, dir());
+ bm1 = CreateSyncedItem(&trans, BOOKMARKS, "bm1");
+ syncable::MutableEntry e(&trans, syncable::GET_BY_HANDLE, bm1);
+
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::BookmarkSpecifics* bm_specifics = specifics.mutable_bookmark();
+ bm_specifics->set_url("http://www.chrome.com");
+ bm_specifics->set_title("Chrome");
+ e.PutSpecifics(specifics);
+
+ e.PutIsDel(false);
+ e.PutIsUnsynced(true);
+
+ EXPECT_TRUE(e.ShouldMaintainHierarchy());
+ }
+
+ DirectoryTypeDebugInfoEmitter emitter(BOOKMARKS, &type_observers_);
+ scoped_ptr<DirectoryCommitContribution> bm_cc(
+ DirectoryCommitContribution::Build(dir(), BOOKMARKS, 25, &emitter));
+
+ sync_pb::ClientToServerMessage message;
+ bm_cc->AddToCommitMessage(&message);
+ const sync_pb::CommitMessage& commit_message = message.commit();
+ bm_cc->CleanUp();
+
+ ASSERT_EQ(1, commit_message.entries_size());
+ EXPECT_TRUE(commit_message.entries(0).has_parent_id_string());
+ EXPECT_FALSE(commit_message.entries(0).parent_id_string().empty());
+}
+
+// Test that preferences do not support hierarchy.
+TEST_F(DirectoryCommitContributionTest, HierarchySupport_Preferences) {
+ // Create a normal-looking prefs item.
+ int64 pref1;
+ {
+ syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, dir());
+ pref1 = CreateUnsyncedItem(&trans, PREFERENCES, "pref1");
+ syncable::MutableEntry e(&trans, syncable::GET_BY_HANDLE, pref1);
+
+ EXPECT_FALSE(e.ShouldMaintainHierarchy());
+ }
+
+ DirectoryTypeDebugInfoEmitter emitter(PREFERENCES, &type_observers_);
+ scoped_ptr<DirectoryCommitContribution> pref_cc(
+ DirectoryCommitContribution::Build(dir(), PREFERENCES, 25, &emitter));
+
+ sync_pb::ClientToServerMessage message;
+ pref_cc->AddToCommitMessage(&message);
+ const sync_pb::CommitMessage& commit_message = message.commit();
+ pref_cc->CleanUp();
+
+ ASSERT_EQ(1, commit_message.entries_size());
+ EXPECT_FALSE(commit_message.entries(0).has_parent_id_string());
+ EXPECT_TRUE(commit_message.entries(0).parent_id_string().empty());
+}
+
// Creates some unsynced items, pretends to commit them, and hands back a
// specially crafted response to the syncer in order to test commit response
// processing. The response simulates a succesful commit scenario.
diff --git a/sync/syncable/entry.cc b/sync/syncable/entry.cc
index 0a4a44f..5bee5ee 100644
--- a/sync/syncable/entry.cc
+++ b/sync/syncable/entry.cc
@@ -121,6 +121,10 @@ bool Entry::ShouldMaintainPosition() const {
return kernel_->ShouldMaintainPosition();
}
+bool Entry::ShouldMaintainHierarchy() const {
+ return kernel_->ShouldMaintainHierarchy();
+}
+
std::ostream& operator<<(std::ostream& s, const Blob& blob) {
for (Blob::const_iterator i = blob.begin(); i != blob.end(); ++i)
s << std::hex << std::setw(2)
diff --git a/sync/syncable/entry.h b/sync/syncable/entry.h
index 619c84b..5faa6e8 100644
--- a/sync/syncable/entry.h
+++ b/sync/syncable/entry.h
@@ -245,6 +245,10 @@ class SYNC_EXPORT Entry {
// sort ordering relative to its siblings under the same parent.
bool ShouldMaintainPosition() const;
+ // Returns true if this is an entry that is expected to maintain hierarchy.
+ // ie. Whether or not the PARENT_ID field contains useful information.
+ bool ShouldMaintainHierarchy() const;
+
Directory* dir() const;
const EntryKernel GetKernelCopy() const {
diff --git a/sync/syncable/entry_kernel.cc b/sync/syncable/entry_kernel.cc
index 0821999..d945d50 100644
--- a/sync/syncable/entry_kernel.cc
+++ b/sync/syncable/entry_kernel.cc
@@ -56,6 +56,14 @@ bool EntryKernel::ShouldMaintainPosition() const {
&& !(!ref(UNIQUE_SERVER_TAG).empty() && ref(IS_DIR));
}
+bool EntryKernel::ShouldMaintainHierarchy() const {
+ // We maintain hierarchy for bookmarks, device info, and top-level folders,
+ // but no other types. Note that the Nigori node consists of a single
+ // top-level folder, so it's included in this set.
+ return (GetModelTypeFromSpecifics(ref(SPECIFICS)) == syncer::BOOKMARKS)
+ || (!ref(UNIQUE_SERVER_TAG).empty());
+}
+
namespace {
// Utility function to loop through a set of enum values and add the
diff --git a/sync/syncable/entry_kernel.h b/sync/syncable/entry_kernel.h
index 6380420..d270c4b 100644
--- a/sync/syncable/entry_kernel.h
+++ b/sync/syncable/entry_kernel.h
@@ -341,6 +341,7 @@ struct SYNC_EXPORT_PRIVATE EntryKernel {
ModelType GetModelType() const;
ModelType GetServerModelType() const;
bool ShouldMaintainPosition() const;
+ bool ShouldMaintainHierarchy() const;
// Dumps all kernel info into a DictionaryValue and returns it.
// Transfers ownership of the DictionaryValue to the caller.
diff --git a/sync/test/fake_server/fake_server_entity.cc b/sync/test/fake_server/fake_server_entity.cc
index b2aa6f9..602a266 100644
--- a/sync/test/fake_server/fake_server_entity.cc
+++ b/sync/test/fake_server/fake_server_entity.cc
@@ -64,6 +64,13 @@ string FakeServerEntity::CreateId(const ModelType& model_type,
}
// static
+std::string FakeServerEntity::GetTopLevelId(const ModelType& model_type) {
+ return FakeServerEntity::CreateId(
+ model_type,
+ syncer::ModelTypeToRootTag(model_type));
+}
+
+// static
ModelType FakeServerEntity::GetModelTypeFromId(const string& id) {
vector<string> tokens;
size_t token_count = Tokenize(id, kIdSeparator, &tokens);
@@ -80,8 +87,8 @@ FakeServerEntity::FakeServerEntity(const string& id,
const ModelType& model_type,
int64 version,
const string& name)
- : id_(id),
- model_type_(model_type),
+ : model_type_(model_type),
+ id_(id),
version_(version),
name_(name) {}
diff --git a/sync/test/fake_server/fake_server_entity.h b/sync/test/fake_server/fake_server_entity.h
index 7f30bb0..b7c5990 100644
--- a/sync/test/fake_server/fake_server_entity.h
+++ b/sync/test/fake_server/fake_server_entity.h
@@ -26,6 +26,9 @@ class FakeServerEntity {
static std::string CreateId(const syncer::ModelType& model_type,
const std::string& inner_id);
+ // Returns the ID string of the top level node for the specified type.
+ static std::string GetTopLevelId(const syncer::ModelType& model_type);
+
virtual ~FakeServerEntity();
const std::string& GetId() const;
syncer::ModelType GetModelType() const;
@@ -51,13 +54,13 @@ class FakeServerEntity {
void SerializeBaseProtoFields(sync_pb::SyncEntity* sync_entity);
+ // The ModelType that categorizes this entity.
+ syncer::ModelType model_type_;
+
private:
// The entity's ID.
std::string id_;
- // The ModelType that categorizes this entity.
- syncer::ModelType model_type_;
-
// The version of this entity.
int64 version_;
diff --git a/sync/test/fake_server/permanent_entity.cc b/sync/test/fake_server/permanent_entity.cc
index 3ceb5c8..5b1631c 100644
--- a/sync/test/fake_server/permanent_entity.cc
+++ b/sync/test/fake_server/permanent_entity.cc
@@ -56,7 +56,7 @@ FakeServerEntity* PermanentEntity::CreateTopLevel(const ModelType& model_type) {
<< "invalid.";
string server_tag = syncer::ModelTypeToRootTag(model_type);
string name = syncer::ModelTypeToString(model_type);
- string id = FakeServerEntity::CreateId(model_type, server_tag);
+ string id = FakeServerEntity::GetTopLevelId(model_type);
sync_pb::EntitySpecifics entity_specifics;
AddDefaultFieldValue(model_type, &entity_specifics);
return new PermanentEntity(id,
diff --git a/sync/test/fake_server/unique_client_entity.cc b/sync/test/fake_server/unique_client_entity.cc
index e9ce2e9..39d3a4c 100644
--- a/sync/test/fake_server/unique_client_entity.cc
+++ b/sync/test/fake_server/unique_client_entity.cc
@@ -11,6 +11,7 @@
#include "sync/internal_api/public/base/model_type.h"
#include "sync/protocol/sync.pb.h"
#include "sync/test/fake_server/fake_server_entity.h"
+#include "sync/test/fake_server/permanent_entity.h"
using std::string;
@@ -34,7 +35,6 @@ FakeServerEntity* UniqueClientEntity::CreateNew(
model_type,
client_entity.version(),
client_entity.name(),
- client_entity.parent_id_string(),
client_entity.client_defined_unique_tag(),
client_entity.specifics(),
client_entity.ctime(),
@@ -49,7 +49,6 @@ FakeServerEntity* UniqueClientEntity::CreateUpdatedVersion(
current_server_entity->GetModelType(),
client_entity.version(),
client_entity.name(),
- client_entity.parent_id_string(),
client_entity.client_defined_unique_tag(),
client_entity.specifics(),
client_entity.ctime(),
@@ -61,13 +60,11 @@ UniqueClientEntity::UniqueClientEntity(
const ModelType& model_type,
int64 version,
const string& name,
- const string& parent_id,
const string& client_defined_unique_tag,
const sync_pb::EntitySpecifics& specifics,
int64 creation_time,
int64 last_modified_time)
: FakeServerEntity(id, model_type, version, name),
- parent_id_(parent_id),
client_defined_unique_tag_(client_defined_unique_tag),
specifics_(specifics),
creation_time_(creation_time),
@@ -76,7 +73,7 @@ UniqueClientEntity::UniqueClientEntity(
string UniqueClientEntity::GetParentId() const {
// The parent ID for this type of entity should always be its ModelType's
// root node.
- return parent_id_;
+ return FakeServerEntity::GetTopLevelId(model_type_);
}
sync_pb::SyncEntity* UniqueClientEntity::SerializeAsProto() {
@@ -86,7 +83,7 @@ sync_pb::SyncEntity* UniqueClientEntity::SerializeAsProto() {
sync_pb::EntitySpecifics* specifics = sync_entity->mutable_specifics();
specifics->CopyFrom(specifics_);
- sync_entity->set_parent_id_string(parent_id_);
+ sync_entity->set_parent_id_string(GetParentId());
sync_entity->set_client_defined_unique_tag(client_defined_unique_tag_);
sync_entity->set_ctime(creation_time_);
sync_entity->set_mtime(last_modified_time_);
diff --git a/sync/test/fake_server/unique_client_entity.h b/sync/test/fake_server/unique_client_entity.h
index 6e876e3..fecf044 100644
--- a/sync/test/fake_server/unique_client_entity.h
+++ b/sync/test/fake_server/unique_client_entity.h
@@ -39,14 +39,12 @@ class UniqueClientEntity : public FakeServerEntity {
const syncer::ModelType& model_type,
int64 version,
const std::string& name,
- const std::string& parent_id,
const std::string& client_defined_unique_tag,
const sync_pb::EntitySpecifics& specifics,
int64 creation_time,
int64 last_modified_time);
- // All member values have equivalent fields in SyncEntity.
- std::string parent_id_;
+ // These member values have equivalent fields in SyncEntity.
std::string client_defined_unique_tag_;
sync_pb::EntitySpecifics specifics_;
int64 creation_time_;
diff --git a/sync/tools/testserver/chromiumsync.py b/sync/tools/testserver/chromiumsync.py
index 661366a..2930059 100644
--- a/sync/tools/testserver/chromiumsync.py
+++ b/sync/tools/testserver/chromiumsync.py
@@ -624,6 +624,12 @@ class SyncDataModel(object):
spec = [x for x in self._PERMANENT_ITEM_SPECS if x.tag == tag][0]
return self._MakeCurrentId(spec.sync_type, '<server tag>%s' % tag)
+ def _TypeToTypeRootId(self, model_type):
+ """Returns the server ID for the type root node of the given type."""
+ tag = [x.tag for x in self._PERMANENT_ITEM_SPECS
+ if x.sync_type == model_type][0]
+ return self._ServerTagToId(tag)
+
def _ClientTagToId(self, datatype, tag):
"""Determine the server ID from a client-unique tag.
@@ -840,6 +846,9 @@ class SyncDataModel(object):
if entry.parent_id_string == ROOT_ID:
# This is generally allowed.
return True
+ if (not entry.HasField('parent_id_string') and
+ entry.HasField('client_defined_unique_tag')):
+ return True # Unique client tag items do not need to specify a parent.
if entry.parent_id_string not in self._entries:
print 'Warning: Client sent unknown ID. Should never happen.'
return False
@@ -939,6 +948,11 @@ class SyncDataModel(object):
# ID generation state is stored in 'commit_session'.
self._RewriteIdsAsServerIds(entry, cache_guid, commit_session)
+ # Sets the parent ID field for a client-tagged item. The client is allowed
+ # to not specify parents for these types of items. The server can figure
+ # out on its own what the parent ID for this entry should be.
+ self._RewriteParentIdForUniqueClientEntry(entry)
+
# Perform the optimistic concurrency check on the entry's version number.
# Clients are not allowed to commit unless they indicate that they've seen
# the most recent version of an object.
@@ -1035,6 +1049,27 @@ class SyncDataModel(object):
datatype, old_migration_version, inner_id = parsed_id
return self._MakeCurrentId(datatype, inner_id)
+ def _RewriteParentIdForUniqueClientEntry(self, entry):
+ """Sets the entry's parent ID field to the appropriate value.
+
+ The client must always set enough of the specifics of the entries it sends
+ up such that the server can identify its type. (See crbug.com/373859)
+
+ The client is under no obligation to set the parent ID field. The server
+ can always infer what the appropriate parent for this model type should be.
+ Having the client not send the parent ID is a step towards the removal of
+ type root nodes. (See crbug.com/373869)
+
+ This server implements these features by "faking" the existing of a parent
+ ID early on in the commit processing.
+
+ This function has no effect on non-client-tagged items.
+ """
+ if not entry.HasField('client_defined_unique_tag'):
+ return # Skip this processing for non-client-tagged types.
+ data_type = GetEntryType(entry)
+ entry.parent_id_string = self._TypeToTypeRootId(data_type)
+
def TriggerMigration(self, datatypes):
"""Cause a migration to occur for a set of datatypes on this account.
generated by cgit v1.1 at 2024-09-09 21:13:24 +0000