Commit 4d489d98 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Sync] Some cleanup in ProtocolEvent

This CL changes the following:
1- Make ProtocolEvent.ToValue() a member method instead of a static
method.
2- Changes most of the methods in ProtocolEvent to be private since
there aren't used outside the class.
3- Replaces some deprecated methods.
3- Some general cleanup here and there in the touched files.

I suggest starting the review from protocol_event.h and
protocol_event.cc

This is mostly a mechanical change.

Bug: 929100
Change-Id: I32d86265fa1b1ac63af993ff34ef46df18b0c2ac
Reviewed-on: https://chromium-review.googlesource.com/c/1456004
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629565}
parent 6cd71d99
......@@ -362,8 +362,7 @@ void SyncInternalsMessageHandler::OnStateChanged(SyncService* sync) {
void SyncInternalsMessageHandler::OnProtocolEvent(
const syncer::ProtocolEvent& event) {
std::unique_ptr<DictionaryValue> value(
syncer::ProtocolEvent::ToValue(event, include_specifics_));
std::unique_ptr<DictionaryValue> value(event.ToValue(include_specifics_));
DispatchEvent(syncer::sync_ui_util::kOnProtocolEvent, *value);
}
......
......@@ -12,14 +12,17 @@ ProtocolEvent::ProtocolEvent() {}
ProtocolEvent::~ProtocolEvent() {}
std::unique_ptr<base::DictionaryValue> ProtocolEvent::ToValue(
const ProtocolEvent& event,
bool include_specifics) {
bool include_specifics) const {
auto dict = std::make_unique<base::DictionaryValue>();
dict->SetDouble("time", event.GetTimestamp().ToJsTime());
dict->SetString("type", event.GetType());
dict->SetString("details", event.GetDetails());
dict->Set("proto", event.GetProtoMessage(include_specifics));
dict->SetDouble("time", GetTimestamp().ToJsTime());
dict->SetString("type", GetType());
dict->SetString("details", GetDetails());
dict->Set("proto", GetProtoMessage(include_specifics));
return dict;
}
base::Time ProtocolEvent::GetTimestampForTesting() const {
return GetTimestamp();
}
} // namespace syncer
......@@ -32,6 +32,17 @@ class ProtocolEvent {
ProtocolEvent();
virtual ~ProtocolEvent();
// Need a virtual copy constructor to copy this object across threads.
virtual std::unique_ptr<ProtocolEvent> Clone() const = 0;
// Assembles the data exposed through the ProtocolEvent's interface into a
// single DictionaryValue.
std::unique_ptr<base::DictionaryValue> ToValue(bool include_specifics) const;
// Returns the time when the request was sent or received.
base::Time GetTimestampForTesting() const;
private:
// Returns the time when the request was sent or received.
virtual base::Time GetTimestamp() const = 0;
......@@ -46,15 +57,6 @@ class ProtocolEvent {
// this event.
virtual std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const = 0;
// Need a virtual copy contructor to copy this object across threads.
virtual std::unique_ptr<ProtocolEvent> Clone() const = 0;
// A static function that assembles the data exposed through the
// ProtocolEvent's interface into a single DictionaryValue.
static std::unique_ptr<base::DictionaryValue> ToValue(
const ProtocolEvent& event,
bool include_specifics);
};
} // namespace syncer
......
......@@ -16,6 +16,10 @@ ClearServerDataRequestEvent::ClearServerDataRequestEvent(
ClearServerDataRequestEvent::~ClearServerDataRequestEvent() {}
std::unique_ptr<ProtocolEvent> ClearServerDataRequestEvent::Clone() const {
return std::make_unique<ClearServerDataRequestEvent>(timestamp_, request_);
}
base::Time ClearServerDataRequestEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -30,13 +34,7 @@ std::string ClearServerDataRequestEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue>
ClearServerDataRequestEvent::GetProtoMessage(bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerMessageToValue(request_, include_specifics));
}
std::unique_ptr<ProtocolEvent> ClearServerDataRequestEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(
new ClearServerDataRequestEvent(timestamp_, request_));
return ClientToServerMessageToValue(request_, include_specifics);
}
} // namespace syncer
......@@ -22,18 +22,15 @@ class ClearServerDataRequestEvent : public ProtocolEvent {
ClearServerDataRequestEvent(base::Time timestamp,
const sync_pb::ClientToServerMessage& request);
~ClearServerDataRequestEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
static std::unique_ptr<base::DictionaryValue> ToValue(
const ProtocolEvent& event);
private:
const base::Time timestamp_;
const sync_pb::ClientToServerMessage request_;
......
......@@ -17,6 +17,11 @@ ClearServerDataResponseEvent::ClearServerDataResponseEvent(
ClearServerDataResponseEvent::~ClearServerDataResponseEvent() {}
std::unique_ptr<ProtocolEvent> ClearServerDataResponseEvent::Clone() const {
return std::make_unique<ClearServerDataResponseEvent>(timestamp_, result_,
response_);
}
base::Time ClearServerDataResponseEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -31,13 +36,7 @@ std::string ClearServerDataResponseEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue>
ClearServerDataResponseEvent::GetProtoMessage(bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerResponseToValue(response_, include_specifics));
}
std::unique_ptr<ProtocolEvent> ClearServerDataResponseEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(
new ClearServerDataResponseEvent(timestamp_, result_, response_));
return ClientToServerResponseToValue(response_, include_specifics);
}
} // namespace syncer
......@@ -25,17 +25,15 @@ class ClearServerDataResponseEvent : public ProtocolEvent {
const sync_pb::ClientToServerResponse& response);
~ClearServerDataResponseEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
static std::unique_ptr<base::DictionaryValue> ToValue(
const ProtocolEvent& event);
private:
const base::Time timestamp_;
const SyncerError result_;
const sync_pb::ClientToServerResponse response_;
......
......@@ -24,6 +24,11 @@ CommitRequestEvent::CommitRequestEvent(
CommitRequestEvent::~CommitRequestEvent() {}
std::unique_ptr<ProtocolEvent> CommitRequestEvent::Clone() const {
return std::make_unique<CommitRequestEvent>(timestamp_, num_items_,
contributing_types_, request_);
}
base::Time CommitRequestEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -42,13 +47,7 @@ std::string CommitRequestEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue> CommitRequestEvent::GetProtoMessage(
bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerMessageToValue(request_, include_specifics));
}
std::unique_ptr<ProtocolEvent> CommitRequestEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(new CommitRequestEvent(
timestamp_, num_items_, contributing_types_, request_));
return ClientToServerMessageToValue(request_, include_specifics);
}
} // namespace syncer
......@@ -27,17 +27,15 @@ class CommitRequestEvent : public ProtocolEvent {
const sync_pb::ClientToServerMessage& request);
~CommitRequestEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
static std::unique_ptr<base::DictionaryValue> ToValue(
const ProtocolEvent& event);
private:
const base::Time timestamp_;
const size_t num_items_;
const ModelTypeSet contributing_types_;
......
......@@ -17,6 +17,10 @@ CommitResponseEvent::CommitResponseEvent(
CommitResponseEvent::~CommitResponseEvent() {}
std::unique_ptr<ProtocolEvent> CommitResponseEvent::Clone() const {
return std::make_unique<CommitResponseEvent>(timestamp_, result_, response_);
}
base::Time CommitResponseEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -31,13 +35,8 @@ std::string CommitResponseEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue> CommitResponseEvent::GetProtoMessage(
bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerResponseToValue(response_, include_specifics));
return ClientToServerResponseToValue(response_, include_specifics);
}
std::unique_ptr<ProtocolEvent> CommitResponseEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(
new CommitResponseEvent(timestamp_, result_, response_));
}
} // namespace syncer
......@@ -25,18 +25,18 @@ class CommitResponseEvent : public ProtocolEvent {
SyncerError result,
const sync_pb::ClientToServerResponse& response);
~CommitResponseEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
static std::unique_ptr<base::DictionaryValue> ToValue(
const ProtocolEvent& event);
private:
const base::Time timestamp_;
const SyncerError result_;
const sync_pb::ClientToServerResponse response_;
......
......@@ -18,6 +18,11 @@ ConfigureGetUpdatesRequestEvent::ConfigureGetUpdatesRequestEvent(
ConfigureGetUpdatesRequestEvent::~ConfigureGetUpdatesRequestEvent() {}
std::unique_ptr<ProtocolEvent> ConfigureGetUpdatesRequestEvent::Clone() const {
return std::make_unique<ConfigureGetUpdatesRequestEvent>(timestamp_, origin_,
request_);
}
base::Time ConfigureGetUpdatesRequestEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -32,13 +37,7 @@ std::string ConfigureGetUpdatesRequestEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue>
ConfigureGetUpdatesRequestEvent::GetProtoMessage(bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerMessageToValue(request_, include_specifics));
}
std::unique_ptr<ProtocolEvent> ConfigureGetUpdatesRequestEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(
new ConfigureGetUpdatesRequestEvent(timestamp_, origin_, request_));
return ClientToServerMessageToValue(request_, include_specifics);
}
} // namespace syncer
......@@ -24,15 +24,15 @@ class ConfigureGetUpdatesRequestEvent : public ProtocolEvent {
sync_pb::SyncEnums::GetUpdatesOrigin origin,
const sync_pb::ClientToServerMessage& request);
~ConfigureGetUpdatesRequestEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
const base::Time timestamp_;
const sync_pb::SyncEnums::GetUpdatesOrigin origin_;
const sync_pb::ClientToServerMessage request_;
......
......@@ -17,6 +17,11 @@ GetUpdatesResponseEvent::GetUpdatesResponseEvent(
GetUpdatesResponseEvent::~GetUpdatesResponseEvent() {}
std::unique_ptr<ProtocolEvent> GetUpdatesResponseEvent::Clone() const {
return std::make_unique<GetUpdatesResponseEvent>(timestamp_, response_,
error_);
}
base::Time GetUpdatesResponseEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -40,13 +45,7 @@ std::string GetUpdatesResponseEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue> GetUpdatesResponseEvent::GetProtoMessage(
bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerResponseToValue(response_, include_specifics));
}
std::unique_ptr<ProtocolEvent> GetUpdatesResponseEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(
new GetUpdatesResponseEvent(timestamp_, response_, error_));
return ClientToServerResponseToValue(response_, include_specifics);
}
} // namespace syncer
......@@ -28,15 +28,15 @@ class GetUpdatesResponseEvent : public ProtocolEvent {
SyncerError error);
~GetUpdatesResponseEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
const base::Time timestamp_;
const sync_pb::ClientToServerResponse response_;
const SyncerError error_;
......
......@@ -14,13 +14,33 @@ NormalGetUpdatesRequestEvent::NormalGetUpdatesRequestEvent(
base::Time timestamp,
const NudgeTracker& nudge_tracker,
const sync_pb::ClientToServerMessage& request)
: NormalGetUpdatesRequestEvent(timestamp,
nudge_tracker.GetNudgedTypes(),
nudge_tracker.GetNotifiedTypes(),
nudge_tracker.GetRefreshRequestedTypes(),
nudge_tracker.IsRetryRequired(),
request) {}
NormalGetUpdatesRequestEvent::NormalGetUpdatesRequestEvent(
base::Time timestamp,
ModelTypeSet nudged_types,
ModelTypeSet notified_types,
ModelTypeSet refresh_requested_types,
bool is_retry,
sync_pb::ClientToServerMessage request)
: timestamp_(timestamp),
nudged_types_(nudge_tracker.GetNudgedTypes()),
notified_types_(nudge_tracker.GetNotifiedTypes()),
refresh_requested_types_(nudge_tracker.GetRefreshRequestedTypes()),
is_retry_(nudge_tracker.IsRetryRequired()),
nudged_types_(nudged_types),
notified_types_(notified_types),
refresh_requested_types_(refresh_requested_types),
is_retry_(is_retry),
request_(request) {}
std::unique_ptr<ProtocolEvent> NormalGetUpdatesRequestEvent::Clone() const {
return std::make_unique<NormalGetUpdatesRequestEvent>(
timestamp_, nudged_types_, notified_types_, refresh_requested_types_,
is_retry_, request_);
}
NormalGetUpdatesRequestEvent::~NormalGetUpdatesRequestEvent() {}
base::Time NormalGetUpdatesRequestEvent::GetTimestamp() const {
......@@ -67,28 +87,7 @@ std::string NormalGetUpdatesRequestEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue>
NormalGetUpdatesRequestEvent::GetProtoMessage(bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerMessageToValue(request_, include_specifics));
}
std::unique_ptr<ProtocolEvent> NormalGetUpdatesRequestEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(new NormalGetUpdatesRequestEvent(
timestamp_, nudged_types_, notified_types_, refresh_requested_types_,
is_retry_, request_));
return ClientToServerMessageToValue(request_, include_specifics);
}
NormalGetUpdatesRequestEvent::NormalGetUpdatesRequestEvent(
base::Time timestamp,
ModelTypeSet nudged_types,
ModelTypeSet notified_types,
ModelTypeSet refresh_requested_types,
bool is_retry,
sync_pb::ClientToServerMessage request)
: timestamp_(timestamp),
nudged_types_(nudged_types),
notified_types_(notified_types),
refresh_requested_types_(refresh_requested_types),
is_retry_(is_retry),
request_(request) {}
} // namespace syncer
......@@ -25,23 +25,21 @@ class NormalGetUpdatesRequestEvent : public ProtocolEvent {
NormalGetUpdatesRequestEvent(base::Time timestamp,
const NudgeTracker& nudge_tracker,
const sync_pb::ClientToServerMessage& request);
NormalGetUpdatesRequestEvent(base::Time timestamp,
ModelTypeSet nudged_types,
ModelTypeSet notified_types,
ModelTypeSet refresh_requested_types,
bool is_retry,
sync_pb::ClientToServerMessage request);
~NormalGetUpdatesRequestEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
NormalGetUpdatesRequestEvent(base::Time timestamp,
ModelTypeSet nudged_types,
ModelTypeSet notified_types,
ModelTypeSet refresh_requested_types,
bool is_retry,
sync_pb::ClientToServerMessage request);
const base::Time timestamp_;
......
......@@ -15,6 +15,10 @@ PollGetUpdatesRequestEvent::PollGetUpdatesRequestEvent(
PollGetUpdatesRequestEvent::~PollGetUpdatesRequestEvent() {}
std::unique_ptr<ProtocolEvent> PollGetUpdatesRequestEvent::Clone() const {
return std::make_unique<PollGetUpdatesRequestEvent>(timestamp_, request_);
}
base::Time PollGetUpdatesRequestEvent::GetTimestamp() const {
return timestamp_;
}
......@@ -29,13 +33,8 @@ std::string PollGetUpdatesRequestEvent::GetDetails() const {
std::unique_ptr<base::DictionaryValue>
PollGetUpdatesRequestEvent::GetProtoMessage(bool include_specifics) const {
return std::unique_ptr<base::DictionaryValue>(
ClientToServerMessageToValue(request_, include_specifics));
return ClientToServerMessageToValue(request_, include_specifics);
}
std::unique_ptr<ProtocolEvent> PollGetUpdatesRequestEvent::Clone() const {
return std::unique_ptr<ProtocolEvent>(
new PollGetUpdatesRequestEvent(timestamp_, request_));
}
} // namespace syncer
......@@ -23,15 +23,15 @@ class PollGetUpdatesRequestEvent : public ProtocolEvent {
PollGetUpdatesRequestEvent(base::Time timestamp,
const sync_pb::ClientToServerMessage& request);
~PollGetUpdatesRequestEvent() override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
base::Time GetTimestamp() const override;
std::string GetType() const override;
std::string GetDetails() const override;
std::unique_ptr<base::DictionaryValue> GetProtoMessage(
bool include_specifics) const override;
std::unique_ptr<ProtocolEvent> Clone() const override;
private:
const base::Time timestamp_;
const sync_pb::ClientToServerMessage request_;
......
......@@ -33,12 +33,16 @@ ProtocolEventBufferTest::~ProtocolEventBufferTest() {}
std::unique_ptr<ProtocolEvent> ProtocolEventBufferTest::MakeTestEvent(
int64_t id) {
sync_pb::ClientToServerMessage message;
return std::unique_ptr<ProtocolEvent>(new PollGetUpdatesRequestEvent(
base::Time::FromInternalValue(id), message));
return std::make_unique<PollGetUpdatesRequestEvent>(
base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromMicroseconds(id)),
message);
}
bool ProtocolEventBufferTest::HasId(const ProtocolEvent& event, int64_t id) {
return event.GetTimestamp() == base::Time::FromInternalValue(id);
return event.GetTimestampForTesting() ==
base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromMicroseconds(id));
}
TEST_F(ProtocolEventBufferTest, AddThenReturnEvents) {
......
......@@ -280,7 +280,7 @@ void SyncInternalsMessageHandler::OnStateChanged(syncer::SyncService* sync) {
void SyncInternalsMessageHandler::OnProtocolEvent(
const syncer::ProtocolEvent& event) {
std::unique_ptr<base::DictionaryValue> value(
syncer::ProtocolEvent::ToValue(event, include_specifics_));
event.ToValue(include_specifics_));
DispatchEvent(syncer::sync_ui_util::kOnProtocolEvent, *value);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment