diff options
Diffstat (limited to 'sync')
-rw-r--r-- | sync/api/attachments/attachment_uploader.h | 4 | ||||
-rw-r--r-- | sync/internal_api/attachments/attachment_uploader_impl.cc | 2 | ||||
-rw-r--r-- | sync/internal_api/attachments/attachment_uploader_impl_unittest.cc | 34 | ||||
-rw-r--r-- | sync/internal_api/attachments/fake_attachment_uploader.cc | 8 | ||||
-rw-r--r-- | sync/internal_api/public/write_transaction.h | 6 | ||||
-rw-r--r-- | sync/internal_api/write_transaction.cc | 4 | ||||
-rw-r--r-- | sync/syncable/directory_unittest.cc | 3 | ||||
-rw-r--r-- | sync/syncable/mutable_entry.cc | 9 | ||||
-rw-r--r-- | sync/syncable/mutable_entry.h | 9 |
9 files changed, 34 insertions, 45 deletions
diff --git a/sync/api/attachments/attachment_uploader.h b/sync/api/attachments/attachment_uploader.h index 12fe1a3..4774451 100644 --- a/sync/api/attachments/attachment_uploader.h +++ b/sync/api/attachments/attachment_uploader.h @@ -34,8 +34,8 @@ class SYNC_EXPORT AttachmentUploader { // |callback| will be invoked when the operation has completed (successfully // or otherwise). // - // |callback| will receive an UploadResult code and an updated AttachmentId - // containing the server address of the newly uploaded attachment. + // |callback| will receive an UploadResult code and the AttachmentId of the + // newly uploaded attachment. virtual void UploadAttachment(const Attachment& attachment, const UploadCallback& callback) = 0; }; diff --git a/sync/internal_api/attachments/attachment_uploader_impl.cc b/sync/internal_api/attachments/attachment_uploader_impl.cc index 7e12350..c2bda36f 100644 --- a/sync/internal_api/attachments/attachment_uploader_impl.cc +++ b/sync/internal_api/attachments/attachment_uploader_impl.cc @@ -137,8 +137,6 @@ void AttachmentUploaderImpl::UploadState::OnURLFetchComplete( AttachmentId attachment_id = attachment_.GetId(); if (source->GetResponseCode() == net::HTTP_OK) { result = UPLOAD_SUCCESS; - // TODO(maniscalco): Update the attachment id with server address - // information before passing it to the callback (bug 371522). } else if (source->GetResponseCode() == net::HTTP_UNAUTHORIZED) { // TODO(maniscalco): One possibility is that we received a 401 because our // access token has expired. We should probably fetch a new access token diff --git a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc index 90cb07d..14fcfc7 100644 --- a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc +++ b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc @@ -187,7 +187,7 @@ class AttachmentUploaderImplTest : public testing::Test, const AttachmentUploader::UploadCallback& upload_callback() const; std::vector<HttpRequest>& http_requests_received(); std::vector<AttachmentUploader::UploadResult>& upload_results(); - std::vector<AttachmentId>& updated_attachment_ids(); + std::vector<AttachmentId>& attachment_ids(); MockOAuth2TokenService& token_service(); base::MessageLoopForIO& message_loop(); RequestHandler& request_handler(); @@ -195,7 +195,7 @@ class AttachmentUploaderImplTest : public testing::Test, private: // An UploadCallback invoked by AttachmentUploaderImpl. void UploadDone(const AttachmentUploader::UploadResult& result, - const AttachmentId& updated_attachment_id); + const AttachmentId& attachment_id); base::MessageLoopForIO message_loop_; scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_; @@ -207,7 +207,7 @@ class AttachmentUploaderImplTest : public testing::Test, base::Closure signal_upload_done_; std::vector<HttpRequest> http_requests_received_; std::vector<AttachmentUploader::UploadResult> upload_results_; - std::vector<AttachmentId> updated_attachment_ids_; + std::vector<AttachmentId> attachment_ids_; scoped_ptr<MockOAuth2TokenService> token_service_; // Must be last data member. @@ -315,8 +315,8 @@ AttachmentUploaderImplTest::upload_results() { } std::vector<AttachmentId>& -AttachmentUploaderImplTest::updated_attachment_ids() { - return updated_attachment_ids_; +AttachmentUploaderImplTest::attachment_ids() { + return attachment_ids_; } MockOAuth2TokenService& AttachmentUploaderImplTest::token_service() { @@ -333,10 +333,10 @@ RequestHandler& AttachmentUploaderImplTest::request_handler() { void AttachmentUploaderImplTest::UploadDone( const AttachmentUploader::UploadResult& result, - const AttachmentId& updated_attachment_id) { + const AttachmentId& attachment_id) { DCHECK(CalledOnValidThread()); upload_results_.push_back(result); - updated_attachment_ids_.push_back(updated_attachment_id); + attachment_ids_.push_back(attachment_id); DCHECK(!signal_upload_done_.is_null()); signal_upload_done_.Run(); } @@ -432,8 +432,8 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_HappyCase) { // See that the done callback was invoked with the right arguments. ASSERT_EQ(1U, upload_results().size()); EXPECT_EQ(AttachmentUploader::UPLOAD_SUCCESS, upload_results()[0]); - ASSERT_EQ(1U, updated_attachment_ids().size()); - EXPECT_EQ(attachment.GetId(), updated_attachment_ids()[0]); + ASSERT_EQ(1U, attachment_ids().size()); + EXPECT_EQ(attachment.GetId(), attachment_ids()[0]); // See that the HTTP server received one request. ASSERT_EQ(1U, http_requests_received().size()); @@ -448,10 +448,6 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_HappyCase) { const std::string header_value(std::string("Bearer ") + kAccessToken); EXPECT_THAT(http_request.headers, testing::Contains(testing::Pair(header_name, header_value))); - - // TODO(maniscalco): Once AttachmentUploaderImpl is capable of updating the - // AttachmentId with server address information about the attachment, add some - // checks here to verify it works properly (bug 371522). } // Verify two overlapping calls to upload the same attachment result in only one @@ -513,8 +509,8 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_FailToGetToken) { // See that the done callback was invoked. ASSERT_EQ(1U, upload_results().size()); EXPECT_EQ(AttachmentUploader::UPLOAD_UNSPECIFIED_ERROR, upload_results()[0]); - ASSERT_EQ(1U, updated_attachment_ids().size()); - EXPECT_EQ(attachment.GetId(), updated_attachment_ids()[0]); + ASSERT_EQ(1U, attachment_ids().size()); + EXPECT_EQ(attachment.GetId(), attachment_ids()[0]); // See that no HTTP request was received. ASSERT_EQ(0U, http_requests_received().size()); @@ -535,8 +531,8 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_ServiceUnavilable) { // See that the done callback was invoked. ASSERT_EQ(1U, upload_results().size()); EXPECT_EQ(AttachmentUploader::UPLOAD_UNSPECIFIED_ERROR, upload_results()[0]); - ASSERT_EQ(1U, updated_attachment_ids().size()); - EXPECT_EQ(attachment.GetId(), updated_attachment_ids()[0]); + ASSERT_EQ(1U, attachment_ids().size()); + EXPECT_EQ(attachment.GetId(), attachment_ids()[0]); // See that the HTTP server received one request. ASSERT_EQ(1U, http_requests_received().size()); @@ -573,8 +569,8 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_BadToken) { // See that the done callback was invoked. ASSERT_EQ(1U, upload_results().size()); EXPECT_EQ(AttachmentUploader::UPLOAD_UNSPECIFIED_ERROR, upload_results()[0]); - ASSERT_EQ(1U, updated_attachment_ids().size()); - EXPECT_EQ(attachment.GetId(), updated_attachment_ids()[0]); + ASSERT_EQ(1U, attachment_ids().size()); + EXPECT_EQ(attachment.GetId(), attachment_ids()[0]); // See that the HTTP server received one request. ASSERT_EQ(1U, http_requests_received().size()); diff --git a/sync/internal_api/attachments/fake_attachment_uploader.cc b/sync/internal_api/attachments/fake_attachment_uploader.cc index 743b534..1ffbe5b 100644 --- a/sync/internal_api/attachments/fake_attachment_uploader.cc +++ b/sync/internal_api/attachments/fake_attachment_uploader.cc @@ -25,11 +25,9 @@ void FakeAttachmentUploader::UploadAttachment(const Attachment& attachment, DCHECK(!attachment.GetId().GetProto().unique_id().empty()); UploadResult result = UPLOAD_SUCCESS; - AttachmentId updated_id = attachment.GetId(); - // TODO(maniscalco): Update the attachment id with server address information - // before passing it to the callback. - base::MessageLoop::current()->PostTask( - FROM_HERE, base::Bind(callback, result, updated_id)); + AttachmentId id = attachment.GetId(); + base::MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(callback, result, id)); } } // namespace syncer diff --git a/sync/internal_api/public/write_transaction.h b/sync/internal_api/public/write_transaction.h index 06d1354..2dbcdaa 100644 --- a/sync/internal_api/public/write_transaction.h +++ b/sync/internal_api/public/write_transaction.h @@ -52,9 +52,9 @@ class SYNC_EXPORT WriteTransaction : public BaseTransaction { syncer::SyncChangeProcessor::ContextRefreshStatus refresh_status, const std::string& context); - // Attachment id was just updated. Propagate this change to all entries that - // refer this attachment id and set is_on_server for corresponding records. - void UpdateEntriesWithAttachmentId(const AttachmentId& attachment_id); + // Update all entries that refer to |attachment_id| indicating that + // |attachment_id| has been uploaded to the sync server. + void UpdateEntriesMarkAttachmentAsOnServer(const AttachmentId& attachment_id); protected: WriteTransaction() {} diff --git a/sync/internal_api/write_transaction.cc b/sync/internal_api/write_transaction.cc index 70e9029..5415564 100644 --- a/sync/internal_api/write_transaction.cc +++ b/sync/internal_api/write_transaction.cc @@ -81,7 +81,7 @@ void WriteTransaction::SetDataTypeContext( // See crbug.com/360280 } -void WriteTransaction::UpdateEntriesWithAttachmentId( +void WriteTransaction::UpdateEntriesMarkAttachmentAsOnServer( const AttachmentId& attachment_id) { syncable::Directory::Metahandles handles; GetDirectory()->GetMetahandlesByAttachmentId( @@ -90,7 +90,7 @@ void WriteTransaction::UpdateEntriesWithAttachmentId( iter != handles.end(); ++iter) { syncable::MutableEntry entry(transaction_, syncable::GET_BY_HANDLE, *iter); - entry.UpdateAttachmentIdWithServerInfo(attachment_id.GetProto()); + entry.MarkAttachmentAsOnServer(attachment_id.GetProto()); } } diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc index adc4004..efc97e6 100644 --- a/sync/syncable/directory_unittest.cc +++ b/sync/syncable/directory_unittest.cc @@ -1649,8 +1649,7 @@ TEST_F(SyncableDirectoryTest, MutableEntry_UpdateAttachmentId) { ASSERT_FALSE(entry_metadata.record(1).is_on_server()); ASSERT_FALSE(entry.GetIsUnsynced()); - // TODO(pavely): When we add server info to proto, add test for it here. - entry.UpdateAttachmentIdWithServerInfo(attachment_id_proto); + entry.MarkAttachmentAsOnServer(attachment_id_proto); ASSERT_TRUE(entry_metadata.record(0).is_on_server()); ASSERT_FALSE(entry_metadata.record(1).is_on_server()); diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc index a506e4f..b1c1ff4 100644 --- a/sync/syncable/mutable_entry.cc +++ b/sync/syncable/mutable_entry.cc @@ -246,19 +246,18 @@ void MutableEntry::PutAttachmentMetadata( } } -void MutableEntry::UpdateAttachmentIdWithServerInfo( - const sync_pb::AttachmentIdProto& updated_attachment_id) { +void MutableEntry::MarkAttachmentAsOnServer( + const sync_pb::AttachmentIdProto& attachment_id) { DCHECK(kernel_); - DCHECK(!updated_attachment_id.unique_id().empty()); + DCHECK(!attachment_id.unique_id().empty()); write_transaction()->TrackChangesTo(kernel_); sync_pb::AttachmentMetadata& attachment_metadata = kernel_->mutable_ref(ATTACHMENT_METADATA); for (int i = 0; i < attachment_metadata.record_size(); ++i) { sync_pb::AttachmentMetadataRecord* record = attachment_metadata.mutable_record(i); - if (record->id().unique_id() != updated_attachment_id.unique_id()) + if (record->id().unique_id() != attachment_id.unique_id()) continue; - *record->mutable_id() = updated_attachment_id; record->set_is_on_server(true); } kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles); diff --git a/sync/syncable/mutable_entry.h b/sync/syncable/mutable_entry.h index 1709475..615fe80 100644 --- a/sync/syncable/mutable_entry.h +++ b/sync/syncable/mutable_entry.h @@ -62,11 +62,10 @@ class SYNC_EXPORT_PRIVATE MutableEntry : public ModelNeutralMutableEntry { void PutAttachmentMetadata( const sync_pb::AttachmentMetadata& attachment_metadata); - // Update attachment metadata, replace all records matching attachment id's - // unique id with updated attachment id that contains server info. - // Set is_in_server for corresponding records. - void UpdateAttachmentIdWithServerInfo( - const sync_pb::AttachmentIdProto& updated_attachment_id); + // Update attachment metadata for |attachment_id| to indicate that this + // attachment has been uploaded to the sync server. + void MarkAttachmentAsOnServer( + const sync_pb::AttachmentIdProto& attachment_id); private: // Kind of redundant. We should reduce the number of pointers |