summaryrefslogtreecommitdiffstats
path: root/components/sync_driver
diff options
context:
space:
mode:
Diffstat (limited to 'components/sync_driver')
-rw-r--r--components/sync_driver/generic_change_processor.cc32
-rw-r--r--components/sync_driver/generic_change_processor.h12
-rw-r--r--components/sync_driver/generic_change_processor_unittest.cc36
3 files changed, 57 insertions, 23 deletions
diff --git a/components/sync_driver/generic_change_processor.cc b/components/sync_driver/generic_change_processor.cc
index 08238e5..a6614f8 100644
--- a/components/sync_driver/generic_change_processor.cc
+++ b/components/sync_driver/generic_change_processor.cc
@@ -112,6 +112,7 @@ GenericChangeProcessor::GenericChangeProcessor(
attachment_service_proxy_.reset(new syncer::AttachmentServiceProxy(
base::MessageLoopProxy::current(),
attachment_service_weak_ptr_factory_->GetWeakPtr()));
+ UploadAllAttachmentsNotOnServer();
} else {
attachment_service_proxy_.reset(new syncer::AttachmentServiceProxy(
base::MessageLoopProxy::current(),
@@ -408,7 +409,7 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
// Keep track of brand new attachments so we can persist them on this device
// and upload them to the server.
- syncer::AttachmentIdList new_attachments;
+ syncer::AttachmentIdSet new_attachments;
syncer::WriteTransaction trans(from_here, share_handle());
@@ -470,7 +471,7 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
NOTREACHED();
return error;
}
- UploadAttachments(new_attachments);
+ attachment_service_->UploadAttachments(new_attachments);
}
return syncer::SyncError();
@@ -485,7 +486,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionAdd(
const std::string& type_str,
const syncer::WriteTransaction& trans,
syncer::WriteNode* sync_node,
- syncer::AttachmentIdList* new_attachments) {
+ syncer::AttachmentIdSet* new_attachments) {
// TODO(sync): Handle other types of creation (custom parents, folders,
// etc.).
syncer::ReadNode root_node(&trans);
@@ -553,8 +554,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionAdd(
SetAttachmentMetadata(attachment_ids, sync_node);
// Return any newly added attachments.
- new_attachments->insert(
- new_attachments->end(), attachment_ids.begin(), attachment_ids.end());
+ new_attachments->insert(attachment_ids.begin(), attachment_ids.end());
if (merge_result_.get()) {
merge_result_->set_num_items_added(merge_result_->num_items_added() + 1);
}
@@ -569,7 +569,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
const std::string& type_str,
const syncer::WriteTransaction& trans,
syncer::WriteNode* sync_node,
- syncer::AttachmentIdList* new_attachments) {
+ syncer::AttachmentIdSet* new_attachments) {
// TODO(zea): consider having this logic for all possible changes?
const syncer::SyncDataLocal sync_data_local(change.sync_data());
@@ -655,8 +655,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
SetAttachmentMetadata(attachment_ids, sync_node);
// Return any newly added attachments.
- new_attachments->insert(
- new_attachments->end(), attachment_ids.begin(), attachment_ids.end());
+ new_attachments->insert(attachment_ids.begin(), attachment_ids.end());
if (merge_result_.get()) {
merge_result_->set_num_items_modified(merge_result_->num_items_modified() +
@@ -703,14 +702,17 @@ syncer::UserShare* GenericChangeProcessor::share_handle() const {
return share_handle_;
}
-void GenericChangeProcessor::UploadAttachments(
- const syncer::AttachmentIdList& attachment_ids) {
+void GenericChangeProcessor::UploadAllAttachmentsNotOnServer() {
DCHECK(CalledOnValidThread());
- DCHECK(attachment_service_.get() != NULL);
-
- syncer::AttachmentIdSet attachment_id_set;
- attachment_id_set.insert(attachment_ids.begin(), attachment_ids.end());
- attachment_service_->UploadAttachments(attachment_id_set);
+ DCHECK(attachment_service_.get());
+ syncer::AttachmentIdSet id_set;
+ {
+ syncer::ReadTransaction trans(FROM_HERE, share_handle());
+ trans.GetAttachmentIdsToUpload(type_, &id_set);
+ }
+ if (!id_set.empty()) {
+ attachment_service_->UploadAttachments(id_set);
+ }
}
} // namespace sync_driver
diff --git a/components/sync_driver/generic_change_processor.h b/components/sync_driver/generic_change_processor.h
index 05a7368..c370c3f 100644
--- a/components/sync_driver/generic_change_processor.h
+++ b/components/sync_driver/generic_change_processor.h
@@ -114,7 +114,7 @@ class GenericChangeProcessor : public ChangeProcessor,
const std::string& type_str,
const syncer::WriteTransaction& trans,
syncer::WriteNode* sync_node,
- syncer::AttachmentIdList* new_attachments);
+ syncer::AttachmentIdSet* new_attachments);
// Logically part of ProcessSyncChanges.
//
@@ -125,13 +125,11 @@ class GenericChangeProcessor : public ChangeProcessor,
const std::string& type_str,
const syncer::WriteTransaction& trans,
syncer::WriteNode* sync_node,
- syncer::AttachmentIdList* new_attachments);
+ syncer::AttachmentIdSet* new_attachments);
- // Upload |attachments| to the sync server.
- //
- // This function assumes that attachments were already stored in
- // AttachmentStore.
- void UploadAttachments(const syncer::AttachmentIdList& attachment_ids);
+ // Begin uploading attachments that have not yet been uploaded to the sync
+ // server.
+ void UploadAllAttachmentsNotOnServer();
const syncer::ModelType type_;
diff --git a/components/sync_driver/generic_change_processor_unittest.cc b/components/sync_driver/generic_change_processor_unittest.cc
index 623a509..f3c580e 100644
--- a/components/sync_driver/generic_change_processor_unittest.cc
+++ b/components/sync_driver/generic_change_processor_unittest.cc
@@ -11,6 +11,7 @@
#include "base/strings/stringprintf.h"
#include "components/sync_driver/data_type_error_handler_mock.h"
#include "components/sync_driver/sync_api_component_factory.h"
+#include "sync/api/attachments/attachment_id.h"
#include "sync/api/attachments/fake_attachment_store.h"
#include "sync/api/fake_syncable_service.h"
#include "sync/api/sync_change.h"
@@ -142,9 +143,12 @@ class SyncGenericChangeProcessorTest : public testing::Test {
test_user_share_->user_share());
}
test_user_share_->encryption_handler()->Init();
+ ConstructGenericChangeProcessor(type);
+ }
+
+ void ConstructGenericChangeProcessor(syncer::ModelType type) {
scoped_refptr<syncer::AttachmentStore> attachment_store(
new syncer::FakeAttachmentStore(base::MessageLoopProxy::current()));
-
scoped_ptr<MockAttachmentService> mock_attachment_service(
new MockAttachmentService(attachment_store));
// GenericChangeProcessor takes ownership of the AttachmentService, but we
@@ -446,6 +450,36 @@ TEST_F(SyncGenericChangeProcessorTest, AttachmentUploaded) {
EXPECT_EQ(1U, attachment_ids.size());
}
+// Verify that upon construction, all attachments not yet on the server are
+// scheduled for upload.
+TEST_F(SyncGenericChangeProcessorTest, UploadAllAttachmentsNotOnServer) {
+ // Create two attachment ids. id2 will be marked as "on server".
+ syncer::AttachmentId id1 = syncer::AttachmentId::Create();
+ syncer::AttachmentId id2 = syncer::AttachmentId::Create();
+ {
+ // Write an entry containing these two attachment ids.
+ syncer::WriteTransaction trans(FROM_HERE, user_share());
+ syncer::ReadNode root(&trans);
+ ASSERT_EQ(syncer::BaseNode::INIT_OK, root.InitTypeRoot(kType));
+ syncer::WriteNode node(&trans);
+ node.InitUniqueByCreation(kType, root, "some node");
+ sync_pb::AttachmentMetadata metadata;
+ sync_pb::AttachmentMetadataRecord* record1 = metadata.add_record();
+ *record1->mutable_id() = id1.GetProto();
+ sync_pb::AttachmentMetadataRecord* record2 = metadata.add_record();
+ *record2->mutable_id() = id2.GetProto();
+ record2->set_is_on_server(true);
+ node.SetAttachmentMetadata(metadata);
+ }
+
+ // Construct the GenericChangeProcessor and see that it asks the
+ // AttachmentService to upload id1 only.
+ ConstructGenericChangeProcessor(kType);
+ ASSERT_EQ(1U, mock_attachment_service()->attachment_id_sets()->size());
+ ASSERT_THAT(mock_attachment_service()->attachment_id_sets()->front(),
+ testing::UnorderedElementsAre(id1));
+}
+
} // namespace
} // namespace sync_driver
generated by cgit v1.1 at 2025-05-10 09:42:17 +0000