Commit 90da2827 authored by Zach Trudo's avatar Zach Trudo Committed by Commit Bot

Remove Priority from ReportQueueConfiguration

Users want to utilize a single ReportQueue in order to
send messages of varying priority. In order to support
this Enqueue has been updated to receive a Priority, and
Priority has been removed from ReportQueueConfiguration

Bug: chromium:1146242
Change-Id: I64ebabe4fb6bb953196ec287e6ea0a5bdd05e2e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522194Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarSwapnil Gupta <swapnilgupta@google.com>
Reviewed-by: default avatarLeonid Baraz <lbaraz@chromium.org>
Commit-Queue: Zach Trudo <zatrudo@google.com>
Cr-Commit-Position: refs/heads/master@{#826554}
parent dd4710d2
...@@ -6589,6 +6589,7 @@ static_library("test_support") { ...@@ -6589,6 +6589,7 @@ static_library("test_support") {
"//components/invalidation/impl:test_support", "//components/invalidation/impl:test_support",
"//components/password_manager/core/browser:test_support", "//components/password_manager/core/browser:test_support",
"//components/policy/core/browser:test_support", "//components/policy/core/browser:test_support",
"//components/policy/proto:reporting_record_proto",
"//components/prefs:test_support", "//components/prefs:test_support",
"//components/reputation/core", "//components/reputation/core",
"//components/reputation/core:proto", "//components/reputation/core:proto",
......
...@@ -244,9 +244,9 @@ class ExtensionInstallEventLogManagerTest : public testing::Test { ...@@ -244,9 +244,9 @@ class ExtensionInstallEventLogManagerTest : public testing::Test {
BuildReport(); BuildReport();
EXPECT_CALL(*mock_report_queue_, EXPECT_CALL(*mock_report_queue_,
ValueEnqueue_(MatchEvents(&events_value_), _)) ValueEnqueue_(MatchEvents(&events_value_), _, _))
.WillOnce( .WillOnce(
Invoke([callback](const base::Value&, Invoke([callback](const base::Value&, reporting::Priority priority,
reporting::MockReportQueue::EnqueueCallback cb) { reporting::MockReportQueue::EnqueueCallback cb) {
*callback = std::move(cb); *callback = std::move(cb);
return reporting::Status::StatusOK(); return reporting::Status::StatusOK();
...@@ -264,9 +264,9 @@ class ExtensionInstallEventLogManagerTest : public testing::Test { ...@@ -264,9 +264,9 @@ class ExtensionInstallEventLogManagerTest : public testing::Test {
BuildReport(); BuildReport();
EXPECT_CALL(*mock_report_queue_, EXPECT_CALL(*mock_report_queue_,
ValueEnqueue_(MatchEvents(&events_value_), _)) ValueEnqueue_(MatchEvents(&events_value_), _, _))
.WillOnce( .WillOnce(
Invoke([](const base::Value&, Invoke([](const base::Value&, reporting::Priority priority,
reporting::MockReportQueue::EnqueueCallback callback) { reporting::MockReportQueue::EnqueueCallback callback) {
std::move(callback).Run(reporting::Status::StatusOK()); std::move(callback).Run(reporting::Status::StatusOK());
return reporting::Status::StatusOK(); return reporting::Status::StatusOK();
......
...@@ -39,7 +39,6 @@ CreateReportQueueConfigGetter(Profile* profile) { ...@@ -39,7 +39,6 @@ CreateReportQueueConfigGetter(Profile* profile) {
std::move(complete_cb) std::move(complete_cb)
.Run(::reporting::ReportQueueConfiguration::Create( .Run(::reporting::ReportQueueConfiguration::Create(
dm_token, ::reporting::Destination::UPLOAD_EVENTS, dm_token, ::reporting::Destination::UPLOAD_EVENTS,
::reporting::Priority::SLOW_BATCH,
base::BindRepeating( base::BindRepeating(
[]() { return ::reporting::Status::StatusOK(); }))); []() { return ::reporting::Status::StatusOK(); })));
}, },
...@@ -119,9 +118,8 @@ void ExtensionInstallEventLogUploader::SetBuildReportQueueConfigurationForTests( ...@@ -119,9 +118,8 @@ void ExtensionInstallEventLogUploader::SetBuildReportQueueConfigurationForTests(
.Run(::reporting::ReportQueueConfiguration::Create( .Run(::reporting::ReportQueueConfiguration::Create(
DMToken::CreateValidTokenForTesting(dm_token), DMToken::CreateValidTokenForTesting(dm_token),
::reporting::Destination::UPLOAD_EVENTS, ::reporting::Destination::UPLOAD_EVENTS,
::reporting::Priority::SLOW_BATCH, base::BindRepeating([]() { base::BindRepeating(
return ::reporting::Status::StatusOK(); []() { return ::reporting::Status::StatusOK(); })));
})));
}, },
dm_token); dm_token);
} }
...@@ -317,6 +315,7 @@ void ExtensionInstallEventLogUploader::EnqueueReport( ...@@ -317,6 +315,7 @@ void ExtensionInstallEventLogUploader::EnqueueReport(
weak_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get()); weak_factory_.GetWeakPtr(), base::ThreadTaskRunnerHandle::Get());
report_queue_->Enqueue(std::move(value_report), report_queue_->Enqueue(std::move(value_report),
::reporting::Priority::SLOW_BATCH,
std::move(on_enqueue_done_cb)); std::move(on_enqueue_done_cb));
} }
......
...@@ -186,9 +186,9 @@ class ExtensionInstallEventLogUploaderTest : public testing::Test { ...@@ -186,9 +186,9 @@ class ExtensionInstallEventLogUploaderTest : public testing::Test {
waiter_.IncreaseCounterLimit(); waiter_.IncreaseCounterLimit();
EXPECT_CALL(*mock_report_queue_, EXPECT_CALL(*mock_report_queue_,
ValueEnqueue_(MatchEvents(&value_report_), _)) ValueEnqueue_(MatchEvents(&value_report_), _, _))
.WillOnce( .WillOnce(
Invoke([=](const base::Value&, Invoke([=](const base::Value&, reporting::Priority priority,
reporting::MockReportQueue::EnqueueCallback callback) { reporting::MockReportQueue::EnqueueCallback callback) {
reporting::Status status = reporting::Status status =
success ? reporting::Status::StatusOK() success ? reporting::Status::StatusOK()
...@@ -212,9 +212,9 @@ class ExtensionInstallEventLogUploaderTest : public testing::Test { ...@@ -212,9 +212,9 @@ class ExtensionInstallEventLogUploaderTest : public testing::Test {
ConvertExtensionProtoToValue(&log_, context), std::move(context)); ConvertExtensionProtoToValue(&log_, context), std::move(context));
EXPECT_CALL(*mock_report_queue_, EXPECT_CALL(*mock_report_queue_,
ValueEnqueue_(MatchEvents(&value_report_), _)) ValueEnqueue_(MatchEvents(&value_report_), _, _))
.WillOnce( .WillOnce(
Invoke([callback](const base::Value&, Invoke([callback](const base::Value&, reporting::Priority priority,
reporting::MockReportQueue::EnqueueCallback cb) { reporting::MockReportQueue::EnqueueCallback cb) {
*callback = std::move(cb); *callback = std::move(cb);
return reporting::Status::StatusOK(); return reporting::Status::StatusOK();
...@@ -335,7 +335,7 @@ TEST_F(ExtensionInstallEventLogUploaderTest, RequestCancelAndSerialize) { ...@@ -335,7 +335,7 @@ TEST_F(ExtensionInstallEventLogUploaderTest, RequestCancelAndSerialize) {
uploader_->CancelUpload(); uploader_->CancelUpload();
Mock::VerifyAndClearExpectations(mock_report_queue_); Mock::VerifyAndClearExpectations(mock_report_queue_);
EXPECT_CALL(*mock_report_queue_, ValueEnqueue_(_, _)).Times(0); EXPECT_CALL(*mock_report_queue_, ValueEnqueue_(_, _, _)).Times(0);
EXPECT_CALL(delegate_, OnExtensionLogUploadSuccess()).Times(0); EXPECT_CALL(delegate_, OnExtensionLogUploadSuccess()).Times(0);
std::move(serialization_callback).Run(&log_); std::move(serialization_callback).Run(&log_);
} }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/policy/messaging_layer/public/report_queue.h" #include "chrome/browser/policy/messaging_layer/public/report_queue.h"
#include "chrome/browser/policy/messaging_layer/util/status.h" #include "chrome/browser/policy/messaging_layer/util/status.h"
#include "components/policy/proto/record.pb.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "third_party/protobuf/src/google/protobuf/message_lite.h" #include "third_party/protobuf/src/google/protobuf/message_lite.h"
...@@ -24,33 +25,41 @@ class MockReportQueue : public ReportQueue { ...@@ -24,33 +25,41 @@ class MockReportQueue : public ReportQueue {
~MockReportQueue() override; ~MockReportQueue() override;
void Enqueue(base::StringPiece record, void Enqueue(base::StringPiece record,
Priority priority,
EnqueueCallback callback) const override { EnqueueCallback callback) const override {
StringPieceEnqueue_(record, std::move(callback)); StringPieceEnqueue_(record, priority, std::move(callback));
} }
void Enqueue(const base::Value& record, void Enqueue(const base::Value& record,
Priority priority,
EnqueueCallback callback) const override { EnqueueCallback callback) const override {
ValueEnqueue_(record, std::move(callback)); ValueEnqueue_(record, priority, std::move(callback));
} }
void Enqueue(google::protobuf::MessageLite* record, void Enqueue(google::protobuf::MessageLite* record,
Priority priority,
EnqueueCallback callback) const override { EnqueueCallback callback) const override {
MessageLiteEnqueue_(record, std::move(callback)); MessageLiteEnqueue_(record, priority, std::move(callback));
} }
MOCK_METHOD(void, MOCK_METHOD(void,
StringPieceEnqueue_, StringPieceEnqueue_,
(base::StringPiece record, EnqueueCallback callback), (base::StringPiece record,
Priority priority,
EnqueueCallback callback),
(const)); (const));
MOCK_METHOD(void, MOCK_METHOD(void,
ValueEnqueue_, ValueEnqueue_,
(const base::Value& record, EnqueueCallback callback), (const base::Value& record,
Priority priority,
EnqueueCallback callback),
(const)); (const));
MOCK_METHOD(void, MOCK_METHOD(void,
MessageLiteEnqueue_, MessageLiteEnqueue_,
(google::protobuf::MessageLite * record, (google::protobuf::MessageLite * record,
Priority priority,
EnqueueCallback callback), EnqueueCallback callback),
(const)); (const));
}; };
......
...@@ -32,7 +32,6 @@ namespace { ...@@ -32,7 +32,6 @@ namespace {
using policy::DMToken; using policy::DMToken;
using reporting::Destination; using reporting::Destination;
using reporting::Priority;
// Usage (in tests only): // Usage (in tests only):
// //
...@@ -113,7 +112,6 @@ class ReportClientTest : public testing::Test { ...@@ -113,7 +112,6 @@ class ReportClientTest : public testing::Test {
#endif // OS_CHROMEOS #endif // OS_CHROMEOS
const DMToken dm_token_ = DMToken::CreateValidTokenForTesting("TOKEN"); const DMToken dm_token_ = DMToken::CreateValidTokenForTesting("TOKEN");
const Destination destination_ = Destination::UPLOAD_EVENTS; const Destination destination_ = Destination::UPLOAD_EVENTS;
const Priority priority_ = Priority::IMMEDIATE;
ReportQueueConfiguration::PolicyCheckCallback policy_checker_callback_ = ReportQueueConfiguration::PolicyCheckCallback policy_checker_callback_ =
base::BindRepeating([]() { return Status::StatusOK(); }); base::BindRepeating([]() { return Status::StatusOK(); });
}; };
...@@ -121,7 +119,7 @@ class ReportClientTest : public testing::Test { ...@@ -121,7 +119,7 @@ class ReportClientTest : public testing::Test {
// Tests that a ReportQueue can be created using the ReportingClient. // Tests that a ReportQueue can be created using the ReportingClient.
TEST_F(ReportClientTest, CreatesReportQueue) { TEST_F(ReportClientTest, CreatesReportQueue) {
auto config_result = ReportQueueConfiguration::Create( auto config_result = ReportQueueConfiguration::Create(
dm_token_, destination_, priority_, policy_checker_callback_); dm_token_, destination_, policy_checker_callback_);
ASSERT_OK(config_result); ASSERT_OK(config_result);
TestEvent<StatusOr<std::unique_ptr<ReportQueue>>> a; TestEvent<StatusOr<std::unique_ptr<ReportQueue>>> a;
...@@ -133,7 +131,7 @@ TEST_F(ReportClientTest, CreatesReportQueue) { ...@@ -133,7 +131,7 @@ TEST_F(ReportClientTest, CreatesReportQueue) {
// Ensures that created ReportQueues are actually different. // Ensures that created ReportQueues are actually different.
TEST_F(ReportClientTest, CreatesTwoDifferentReportQueues) { TEST_F(ReportClientTest, CreatesTwoDifferentReportQueues) {
auto config_result = ReportQueueConfiguration::Create( auto config_result = ReportQueueConfiguration::Create(
dm_token_, destination_, priority_, policy_checker_callback_); dm_token_, destination_, policy_checker_callback_);
EXPECT_TRUE(config_result.ok()); EXPECT_TRUE(config_result.ok());
TestEvent<StatusOr<std::unique_ptr<ReportQueue>>> a1; TestEvent<StatusOr<std::unique_ptr<ReportQueue>>> a1;
...@@ -144,8 +142,8 @@ TEST_F(ReportClientTest, CreatesTwoDifferentReportQueues) { ...@@ -144,8 +142,8 @@ TEST_F(ReportClientTest, CreatesTwoDifferentReportQueues) {
auto report_queue_1 = std::move(result.ValueOrDie()); auto report_queue_1 = std::move(result.ValueOrDie());
TestEvent<StatusOr<std::unique_ptr<ReportQueue>>> a2; TestEvent<StatusOr<std::unique_ptr<ReportQueue>>> a2;
config_result = ReportQueueConfiguration::Create( config_result = ReportQueueConfiguration::Create(dm_token_, destination_,
dm_token_, destination_, priority_, policy_checker_callback_); policy_checker_callback_);
ReportingClient::CreateReportQueue(std::move(config_result.ValueOrDie()), ReportingClient::CreateReportQueue(std::move(config_result.ValueOrDie()),
a2.cb()); a2.cb());
result = a2.result(); result = a2.result();
......
...@@ -51,54 +51,76 @@ ReportQueue::ReportQueue(std::unique_ptr<ReportQueueConfiguration> config, ...@@ -51,54 +51,76 @@ ReportQueue::ReportQueue(std::unique_ptr<ReportQueueConfiguration> config,
} }
void ReportQueue::Enqueue(base::StringPiece record, void ReportQueue::Enqueue(base::StringPiece record,
Priority priority,
EnqueueCallback callback) const { EnqueueCallback callback) const {
AddRecord(record, std::move(callback)); AddRecord(record, priority, std::move(callback));
} }
void ReportQueue::Enqueue(const base::Value& record, void ReportQueue::Enqueue(const base::Value& record,
Priority priority,
EnqueueCallback callback) const { EnqueueCallback callback) const {
std::string json_record; ASSIGN_OR_ONCE_CALLBACK_AND_RETURN(std::string json_record, callback,
if (!base::JSONWriter::Write(record, &json_record)) { ValueToJson(record));
std::move(callback).Run( AddRecord(json_record, priority, std::move(callback));
Status(error::INVALID_ARGUMENT,
"Provided record was not convertable to a std::string"));
return;
}
AddRecord(json_record, std::move(callback));
} }
void ReportQueue::Enqueue(google::protobuf::MessageLite* record, void ReportQueue::Enqueue(google::protobuf::MessageLite* record,
Priority priority,
EnqueueCallback callback) const { EnqueueCallback callback) const {
std::string protobuf_record; ASSIGN_OR_ONCE_CALLBACK_AND_RETURN(std::string protobuf_record, callback,
if (!record->SerializeToString(&protobuf_record)) { ProtoToString(record));
std::move(callback).Run( AddRecord(protobuf_record, priority, std::move(callback));
Status(error::INVALID_ARGUMENT,
"Unabled to serialize record to string. Most likely due to "
"unset required fields."));
return;
}
return AddRecord(protobuf_record, std::move(callback));
} }
void ReportQueue::AddRecord(base::StringPiece record, void ReportQueue::AddRecord(base::StringPiece record,
Priority priority,
EnqueueCallback callback) const { EnqueueCallback callback) const {
const Status status = config_->CheckPolicy(); const Status status = config_->CheckPolicy();
if (!status.ok()) { if (!status.ok()) {
std::move(callback).Run(status); std::move(callback).Run(status);
return; return;
} }
if (priority == Priority::UNDEFINED_PRIORITY) {
std::move(callback).Run(
Status(error::INVALID_ARGUMENT, "Priority must be defined"));
return;
}
sequenced_task_runner_->PostTask( sequenced_task_runner_->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&ReportQueue::SendRecordToStorage, base::Unretained(this), base::BindOnce(&ReportQueue::SendRecordToStorage, base::Unretained(this),
std::string(record), std::move(callback))); std::string(record), priority, std::move(callback)));
} }
void ReportQueue::SendRecordToStorage(base::StringPiece record_data, void ReportQueue::SendRecordToStorage(base::StringPiece record_data,
Priority priority,
EnqueueCallback callback) const { EnqueueCallback callback) const {
storage_->AddRecord(config_->priority(), AugmentRecord(record_data), storage_->AddRecord(priority, AugmentRecord(record_data),
std::move(callback)); std::move(callback));
} }
StatusOr<std::string> ReportQueue::ValueToJson(
const base::Value& record) const {
std::string json_record;
if (!base::JSONWriter::Write(record, &json_record)) {
return Status(error::INVALID_ARGUMENT,
"Provided record was not convertable to a std::string");
}
return json_record;
}
StatusOr<std::string> ReportQueue::ProtoToString(
google::protobuf::MessageLite* record) const {
std::string protobuf_record;
if (!record->SerializeToString(&protobuf_record)) {
return Status(error::INVALID_ARGUMENT,
"Unabled to serialize record to string. Most likely due to "
"unset required fields.");
}
return protobuf_record;
}
Record ReportQueue::AugmentRecord(base::StringPiece record_data) const { Record ReportQueue::AugmentRecord(base::StringPiece record_data) const {
Record record; Record record;
record.set_data(std::string(record_data)); record.set_data(std::string(record_data));
......
...@@ -50,27 +50,31 @@ class ReportQueue { ...@@ -50,27 +50,31 @@ class ReportQueue {
ReportQueue(const ReportQueue& other) = delete; ReportQueue(const ReportQueue& other) = delete;
ReportQueue& operator=(const ReportQueue& other) = delete; ReportQueue& operator=(const ReportQueue& other) = delete;
// Enqueue asynchronously stores and delivers a record. Enqueue will return an // Enqueue asynchronously stores and delivers a record. The |callback| will
// OK status if the task is successfully scheduled. The |callback| will be // be called on any errors. If storage is successful |callback| will be called
// called on any errors. If storage is successful |callback| will be called
// with an OK status. // with an OK status.
// //
// |priority| will Enqueue the record to the specified Priority queue.
//
// The current destinations have the following data requirements: // The current destinations have the following data requirements:
// (destination : requirement) // (destination : requirement)
// UPLOAD_EVENTS : UploadEventsRequest // UPLOAD_EVENTS : UploadEventsRequest
// //
// |record| will be sent as a string with no conversion. // |record| will be sent as a string with no conversion.
virtual void Enqueue(base::StringPiece record, virtual void Enqueue(base::StringPiece record,
Priority priority,
EnqueueCallback callback) const; EnqueueCallback callback) const;
// |record| will be converted to a JSON string with base::JsonWriter::Write. // |record| will be converted to a JSON string with base::JsonWriter::Write.
virtual void Enqueue(const base::Value& record, virtual void Enqueue(const base::Value& record,
Priority priority,
EnqueueCallback callback) const; EnqueueCallback callback) const;
// |record| will be converted to a string with SerializeToString(). The // |record| will be converted to a string with SerializeToString(). The
// handler is responsible for converting the record back to a proto with a // handler is responsible for converting the record back to a proto with a
// ParseFromString() call. // ParseFromString() call.
virtual void Enqueue(google::protobuf::MessageLite* record, virtual void Enqueue(google::protobuf::MessageLite* record,
Priority priority,
EnqueueCallback callback) const; EnqueueCallback callback) const;
protected: protected:
...@@ -78,10 +82,18 @@ class ReportQueue { ...@@ -78,10 +82,18 @@ class ReportQueue {
scoped_refptr<StorageModule> storage); scoped_refptr<StorageModule> storage);
private: private:
void AddRecord(base::StringPiece record, EnqueueCallback callback) const; void AddRecord(base::StringPiece record,
Priority priority,
EnqueueCallback callback) const;
void SendRecordToStorage(base::StringPiece record, void SendRecordToStorage(base::StringPiece record,
Priority priority,
EnqueueCallback callback) const; EnqueueCallback callback) const;
StatusOr<std::string> ValueToJson(const base::Value& record) const;
StatusOr<std::string> ProtoToString(
google::protobuf::MessageLite* record) const;
reporting::Record AugmentRecord(base::StringPiece record_data) const; reporting::Record AugmentRecord(base::StringPiece record_data) const;
std::unique_ptr<ReportQueueConfiguration> config_; std::unique_ptr<ReportQueueConfiguration> config_;
......
...@@ -24,14 +24,12 @@ ReportQueueConfiguration::~ReportQueueConfiguration() = default; ...@@ -24,14 +24,12 @@ ReportQueueConfiguration::~ReportQueueConfiguration() = default;
StatusOr<std::unique_ptr<ReportQueueConfiguration>> StatusOr<std::unique_ptr<ReportQueueConfiguration>>
ReportQueueConfiguration::Create(const policy::DMToken& dm_token, ReportQueueConfiguration::Create(const policy::DMToken& dm_token,
Destination destination, Destination destination,
Priority priority,
PolicyCheckCallback policy_check_callback) { PolicyCheckCallback policy_check_callback) {
auto config = base::WrapUnique<ReportQueueConfiguration>( auto config = base::WrapUnique<ReportQueueConfiguration>(
new ReportQueueConfiguration()); new ReportQueueConfiguration());
RETURN_IF_ERROR(config->SetDMToken(dm_token)); RETURN_IF_ERROR(config->SetDMToken(dm_token));
RETURN_IF_ERROR(config->SetDestination(destination)); RETURN_IF_ERROR(config->SetDestination(destination));
RETURN_IF_ERROR(config->SetPriority(priority));
RETURN_IF_ERROR(config->SetPolicyCheckCallback(policy_check_callback)); RETURN_IF_ERROR(config->SetPolicyCheckCallback(policy_check_callback));
return config; return config;
...@@ -67,12 +65,4 @@ Status ReportQueueConfiguration::SetDestination(Destination destination) { ...@@ -67,12 +65,4 @@ Status ReportQueueConfiguration::SetDestination(Destination destination) {
return Status::StatusOK(); return Status::StatusOK();
} }
Status ReportQueueConfiguration::SetPriority(Priority priority) {
if (priority == Priority::UNDEFINED_PRIORITY) {
return Status(error::INVALID_ARGUMENT, "Priority must be defined");
}
priority_ = priority;
return Status::StatusOK();
}
} // namespace reporting } // namespace reporting
...@@ -20,7 +20,6 @@ namespace reporting { ...@@ -20,7 +20,6 @@ namespace reporting {
// |dm_token| will be attached to all records generated with this queue. // |dm_token| will be attached to all records generated with this queue.
// |destination| indicates what server side handler will be handling // |destination| indicates what server side handler will be handling
// the records that are generated by the ReportQueue. // the records that are generated by the ReportQueue.
// |priority| indicates the priority of the ReportQueue.
// |policy_check_callback_| is a RepeatingCallback that verifies the specific // |policy_check_callback_| is a RepeatingCallback that verifies the specific
// report queue is allowed. // report queue is allowed.
class ReportQueueConfiguration { class ReportQueueConfiguration {
...@@ -40,18 +39,13 @@ class ReportQueueConfiguration { ...@@ -40,18 +39,13 @@ class ReportQueueConfiguration {
// |dm_token| is valid when dm_token.is_valid() is true. // |dm_token| is valid when dm_token.is_valid() is true.
// |destination| is valid when it is any value other than // |destination| is valid when it is any value other than
// Destination::UNDEFINED_DESTINATION. // Destination::UNDEFINED_DESTINATION.
// |priority| is valid when it is any value other than
// Priority::UNDEFINED_PRIORITY.
static StatusOr<std::unique_ptr<ReportQueueConfiguration>> Create( static StatusOr<std::unique_ptr<ReportQueueConfiguration>> Create(
const policy::DMToken& dm_token, const policy::DMToken& dm_token,
Destination destination, Destination destination,
Priority priority,
PolicyCheckCallback policy_check_callback); PolicyCheckCallback policy_check_callback);
reporting::Destination destination() const { return destination_; } reporting::Destination destination() const { return destination_; }
reporting::Priority priority() const { return priority_; }
policy::DMToken dm_token() const { return dm_token_; } policy::DMToken dm_token() const { return dm_token_; }
Status CheckPolicy() const; Status CheckPolicy() const;
...@@ -61,12 +55,10 @@ class ReportQueueConfiguration { ...@@ -61,12 +55,10 @@ class ReportQueueConfiguration {
Status SetDMToken(const policy::DMToken& dm_token); Status SetDMToken(const policy::DMToken& dm_token);
Status SetDestination(reporting::Destination destination); Status SetDestination(reporting::Destination destination);
Status SetPriority(reporting::Priority priority);
Status SetPolicyCheckCallback(PolicyCheckCallback policy_check_callback); Status SetPolicyCheckCallback(PolicyCheckCallback policy_check_callback);
policy::DMToken dm_token_; policy::DMToken dm_token_;
reporting::Destination destination_; reporting::Destination destination_;
reporting::Priority priority_;
PolicyCheckCallback policy_check_callback_; PolicyCheckCallback policy_check_callback_;
}; };
......
...@@ -32,43 +32,31 @@ TEST(ReportQueueConfigurationTest, ParametersMustBeValid) { ...@@ -32,43 +32,31 @@ TEST(ReportQueueConfigurationTest, ParametersMustBeValid) {
// Invalid Parameters // Invalid Parameters
const DMToken invalid_dm_token = DMToken::CreateInvalidTokenForTesting(); const DMToken invalid_dm_token = DMToken::CreateInvalidTokenForTesting();
const Destination invalid_destination = Destination::UNDEFINED_DESTINATION; const Destination invalid_destination = Destination::UNDEFINED_DESTINATION;
const Priority invalid_priority = Priority::UNDEFINED_PRIORITY;
const PolicyCheckCallback invalid_callback; const PolicyCheckCallback invalid_callback;
// Valid Parameters // Valid Parameters
const DMToken valid_dm_token = DMToken::CreateValidTokenForTesting(kDmToken); const DMToken valid_dm_token = DMToken::CreateValidTokenForTesting(kDmToken);
const Destination valid_destination = Destination::UPLOAD_EVENTS; const Destination valid_destination = Destination::UPLOAD_EVENTS;
const Priority valid_priority = Priority::IMMEDIATE;
const PolicyCheckCallback valid_callback = GetSuccessfulCallback(); const PolicyCheckCallback valid_callback = GetSuccessfulCallback();
// Test Invalid DMToken. // Test Invalid DMToken.
EXPECT_FALSE(ReportQueueConfiguration::Create(invalid_dm_token, EXPECT_FALSE(ReportQueueConfiguration::Create(
valid_destination, invalid_dm_token, valid_destination, valid_callback)
valid_priority, valid_callback)
.ok()); .ok());
// Test Invalid Destination. // Test Invalid Destination.
EXPECT_FALSE(ReportQueueConfiguration::Create(valid_dm_token, EXPECT_FALSE(ReportQueueConfiguration::Create(
invalid_destination, valid_dm_token, invalid_destination, valid_callback)
valid_priority, valid_callback)
.ok()); .ok());
// Test Invalid Priority.
EXPECT_FALSE(
ReportQueueConfiguration::Create(valid_dm_token, valid_destination,
invalid_priority, valid_callback)
.ok());
// Test Invalid Callback. // Test Invalid Callback.
EXPECT_FALSE( EXPECT_FALSE(ReportQueueConfiguration::Create(
ReportQueueConfiguration::Create(valid_dm_token, valid_destination, valid_dm_token, valid_destination, invalid_callback)
valid_priority, invalid_callback) .ok());
.ok());
EXPECT_TRUE(ReportQueueConfiguration::Create(
EXPECT_TRUE( valid_dm_token, valid_destination, GetSuccessfulCallback())
ReportQueueConfiguration::Create(valid_dm_token, valid_destination, .ok());
valid_priority, GetSuccessfulCallback())
.ok());
} }
class TestCallbackHandler { class TestCallbackHandler {
...@@ -80,11 +68,10 @@ class TestCallbackHandler { ...@@ -80,11 +68,10 @@ class TestCallbackHandler {
TEST(ReportQueueConfigurationTest, UsesProvidedPolicyCheckCallback) { TEST(ReportQueueConfigurationTest, UsesProvidedPolicyCheckCallback) {
const DMToken dm_token = DMToken::CreateValidTokenForTesting(kDmToken); const DMToken dm_token = DMToken::CreateValidTokenForTesting(kDmToken);
const Destination destination = Destination::UPLOAD_EVENTS; const Destination destination = Destination::UPLOAD_EVENTS;
const Priority priority = Priority::IMMEDIATE;
TestCallbackHandler handler; TestCallbackHandler handler;
auto config_result = ReportQueueConfiguration::Create( auto config_result = ReportQueueConfiguration::Create(
dm_token, destination, priority, dm_token, destination,
base::BindRepeating(&TestCallbackHandler::Callback, base::BindRepeating(&TestCallbackHandler::Callback,
base::Unretained(&handler))); base::Unretained(&handler)));
EXPECT_TRUE(config_result.ok()); EXPECT_TRUE(config_result.ok());
......
...@@ -93,7 +93,7 @@ class ReportQueueTest : public testing::Test { ...@@ -93,7 +93,7 @@ class ReportQueueTest : public testing::Test {
ON_CALL(*this, MockedPolicyCheck).WillByDefault(Return(Status::StatusOK())); ON_CALL(*this, MockedPolicyCheck).WillByDefault(Return(Status::StatusOK()));
StatusOr<std::unique_ptr<ReportQueueConfiguration>> config_result = StatusOr<std::unique_ptr<ReportQueueConfiguration>> config_result =
ReportQueueConfiguration::Create(dm_token_, destination_, priority_, ReportQueueConfiguration::Create(dm_token_, destination_,
policy_check_callback_); policy_check_callback_);
ASSERT_TRUE(config_result.ok()); ASSERT_TRUE(config_result.ok());
...@@ -135,7 +135,7 @@ class ReportQueueTest : public testing::Test { ...@@ -135,7 +135,7 @@ class ReportQueueTest : public testing::Test {
TEST_F(ReportQueueTest, SuccessfulStringRecord) { TEST_F(ReportQueueTest, SuccessfulStringRecord) {
constexpr char kTestString[] = "El-Chupacabra"; constexpr char kTestString[] = "El-Chupacabra";
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(kTestString, a.cb()); report_queue_->Enqueue(kTestString, priority_, a.cb());
EXPECT_OK(a.result()); EXPECT_OK(a.result());
EXPECT_EQ(test_storage_module()->priority(), priority_); EXPECT_EQ(test_storage_module()->priority(), priority_);
EXPECT_EQ(test_storage_module()->record().data(), kTestString); EXPECT_EQ(test_storage_module()->record().data(), kTestString);
...@@ -149,7 +149,7 @@ TEST_F(ReportQueueTest, SuccessfulBaseValueRecord) { ...@@ -149,7 +149,7 @@ TEST_F(ReportQueueTest, SuccessfulBaseValueRecord) {
base::Value test_dict(base::Value::Type::DICTIONARY); base::Value test_dict(base::Value::Type::DICTIONARY);
test_dict.SetStringKey(kTestKey, kTestValue); test_dict.SetStringKey(kTestKey, kTestValue);
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(test_dict, a.cb()); report_queue_->Enqueue(test_dict, priority_, a.cb());
EXPECT_OK(a.result()); EXPECT_OK(a.result());
EXPECT_EQ(test_storage_module()->priority(), priority_); EXPECT_EQ(test_storage_module()->priority(), priority_);
...@@ -166,7 +166,7 @@ TEST_F(ReportQueueTest, SuccessfulProtoRecord) { ...@@ -166,7 +166,7 @@ TEST_F(ReportQueueTest, SuccessfulProtoRecord) {
reporting::test::TestMessage test_message; reporting::test::TestMessage test_message;
test_message.set_test("TEST_MESSAGE"); test_message.set_test("TEST_MESSAGE");
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(&test_message, a.cb()); report_queue_->Enqueue(&test_message, priority_, a.cb());
EXPECT_OK(a.result()); EXPECT_OK(a.result());
EXPECT_EQ(test_storage_module()->priority(), priority_); EXPECT_EQ(test_storage_module()->priority(), priority_);
...@@ -190,7 +190,7 @@ TEST_F(ReportQueueTest, CallSuccessCallbackFailure) { ...@@ -190,7 +190,7 @@ TEST_F(ReportQueueTest, CallSuccessCallbackFailure) {
reporting::test::TestMessage test_message; reporting::test::TestMessage test_message;
test_message.set_test("TEST_MESSAGE"); test_message.set_test("TEST_MESSAGE");
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(&test_message, a.cb()); report_queue_->Enqueue(&test_message, priority_, a.cb());
const auto result = a.result(); const auto result = a.result();
EXPECT_FALSE(result.ok()); EXPECT_FALSE(result.ok());
EXPECT_EQ(result.error_code(), error::UNKNOWN); EXPECT_EQ(result.error_code(), error::UNKNOWN);
...@@ -201,7 +201,7 @@ TEST_F(ReportQueueTest, EnqueueStringFailsOnPolicy) { ...@@ -201,7 +201,7 @@ TEST_F(ReportQueueTest, EnqueueStringFailsOnPolicy) {
.WillOnce(Return(Status(error::UNAUTHENTICATED, "Failing for tests"))); .WillOnce(Return(Status(error::UNAUTHENTICATED, "Failing for tests")));
constexpr char kTestString[] = "El-Chupacabra"; constexpr char kTestString[] = "El-Chupacabra";
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(kTestString, a.cb()); report_queue_->Enqueue(kTestString, priority_, a.cb());
const auto result = a.result(); const auto result = a.result();
EXPECT_FALSE(result.ok()); EXPECT_FALSE(result.ok());
EXPECT_EQ(result.error_code(), error::UNAUTHENTICATED); EXPECT_EQ(result.error_code(), error::UNAUTHENTICATED);
...@@ -213,7 +213,7 @@ TEST_F(ReportQueueTest, EnqueueProtoFailsOnPolicy) { ...@@ -213,7 +213,7 @@ TEST_F(ReportQueueTest, EnqueueProtoFailsOnPolicy) {
reporting::test::TestMessage test_message; reporting::test::TestMessage test_message;
test_message.set_test("TEST_MESSAGE"); test_message.set_test("TEST_MESSAGE");
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(&test_message, a.cb()); report_queue_->Enqueue(&test_message, priority_, a.cb());
const auto result = a.result(); const auto result = a.result();
EXPECT_FALSE(result.ok()); EXPECT_FALSE(result.ok());
EXPECT_EQ(result.error_code(), error::UNAUTHENTICATED); EXPECT_EQ(result.error_code(), error::UNAUTHENTICATED);
...@@ -227,7 +227,7 @@ TEST_F(ReportQueueTest, EnqueueValueFailsOnPolicy) { ...@@ -227,7 +227,7 @@ TEST_F(ReportQueueTest, EnqueueValueFailsOnPolicy) {
base::Value test_dict(base::Value::Type::DICTIONARY); base::Value test_dict(base::Value::Type::DICTIONARY);
test_dict.SetStringKey(kTestKey, kTestValue); test_dict.SetStringKey(kTestKey, kTestValue);
TestEvent<Status> a; TestEvent<Status> a;
report_queue_->Enqueue(test_dict, a.cb()); report_queue_->Enqueue(test_dict, priority_, a.cb());
const auto result = a.result(); const auto result = a.result();
EXPECT_FALSE(result.ok()); EXPECT_FALSE(result.ok());
EXPECT_EQ(result.error_code(), error::UNAUTHENTICATED); EXPECT_EQ(result.error_code(), error::UNAUTHENTICATED);
......
...@@ -30,6 +30,7 @@ proto_library("policy_record_constants") { ...@@ -30,6 +30,7 @@ proto_library("policy_record_constants") {
proto_library("reporting_record_proto") { proto_library("reporting_record_proto") {
visibility = [ visibility = [
"//chrome/browser:browser", "//chrome/browser:browser",
"//chrome/browser:test_support",
"//components/policy/*", "//components/policy/*",
] ]
sources = [ "record.proto" ] sources = [ "record.proto" ]
......
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