summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
Diffstat (limited to 'sync')
-rw-r--r--sync/api/attachments/attachment_uploader.h4
-rw-r--r--sync/internal_api/attachments/attachment_uploader_impl.cc2
-rw-r--r--sync/internal_api/attachments/attachment_uploader_impl_unittest.cc34
-rw-r--r--sync/internal_api/attachments/fake_attachment_uploader.cc8
-rw-r--r--sync/internal_api/public/write_transaction.h6
-rw-r--r--sync/internal_api/write_transaction.cc4
-rw-r--r--sync/syncable/directory_unittest.cc3
-rw-r--r--sync/syncable/mutable_entry.cc9
-rw-r--r--sync/syncable/mutable_entry.h9
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