[NoCheck] Make Uploader's arguments NotNullUniquePtrs
Change-Id: Ibb1165296089b6ebf63d6518b669c6fadd3b5046
Reviewed-on: https://fuchsia-review.googlesource.com/c/cobalt/+/663827
Fuchsia-Auto-Submit: Zach Bush <zmbush@google.com>
Reviewed-by: Cameron Dale <camrdale@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
diff --git a/src/lib/clearcut/BUILD.gn b/src/lib/clearcut/BUILD.gn
index c8b6a4c..8959a1a 100644
--- a/src/lib/clearcut/BUILD.gn
+++ b/src/lib/clearcut/BUILD.gn
@@ -23,6 +23,7 @@
":clearcut_proto",
"$cobalt_root/src:logging",
"$cobalt_root/src/lib/util:clock",
+ "$cobalt_root/src/lib/util:not_null",
"$cobalt_root/src/lib/util:sleeper",
"$cobalt_root/src/logger:internal_metrics",
"$cobalt_root/src/public/lib:http_client",
diff --git a/src/lib/clearcut/uploader.cc b/src/lib/clearcut/uploader.cc
index af02c9d..0fd7cb1 100644
--- a/src/lib/clearcut/uploader.cc
+++ b/src/lib/clearcut/uploader.cc
@@ -20,11 +20,11 @@
using cobalt::util::SteadyClockInterface;
ClearcutUploader::ClearcutUploader(
- std::string url, std::unique_ptr<HTTPClient> client,
+ std::string url, util::NotNullUniquePtr<HTTPClient> client,
// TODO(fxbug.dev/85571): NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
int64_t upload_timeout_millis, int64_t initial_backoff_millis,
- std::unique_ptr<SteadyClockInterface> steady_clock,
- std::unique_ptr<cobalt::util::SleeperInterface> sleeper)
+ util::NotNullUniquePtr<SteadyClockInterface> steady_clock,
+ util::NotNullUniquePtr<cobalt::util::SleeperInterface> sleeper)
: url_(std::move(url)),
client_(std::move(client)),
upload_timeout_(std::chrono::milliseconds(upload_timeout_millis)),
@@ -33,10 +33,7 @@
sleeper_(std::move(sleeper)),
pause_uploads_until_(steady_clock_->now()) // Set this to now() so that we
// can immediately upload.
-{
- CHECK(steady_clock_);
- CHECK(sleeper_);
-}
+{}
Status ClearcutUploader::UploadEvents(LogRequest* log_request, int32_t max_retries) {
int32_t i = 0;
diff --git a/src/lib/clearcut/uploader.h b/src/lib/clearcut/uploader.h
index f8fb8f9..f15f9d5 100644
--- a/src/lib/clearcut/uploader.h
+++ b/src/lib/clearcut/uploader.h
@@ -11,6 +11,7 @@
#include "src/lib/clearcut/clearcut.pb.h"
#include "src/lib/util/clock.h"
+#include "src/lib/util/not_null.h"
#include "src/lib/util/sleeper.h"
#include "src/logger/internal_metrics.h"
#include "src/public/lib/http_client.h"
@@ -68,13 +69,13 @@
// steady_clock: Optionally replace the system steady clock. Mostly useful for tests.
//
// sleeper: Optionally replace the system sleep. Mostly useful for tests.
- ClearcutUploader(std::string url, std::unique_ptr<HTTPClient> client,
+ ClearcutUploader(std::string url, util::NotNullUniquePtr<HTTPClient> client,
int64_t upload_timeout_millis = 0,
int64_t initial_backoff_millis = kInitialBackoffMillis,
- std::unique_ptr<cobalt::util::SteadyClockInterface> steady_clock =
- std::make_unique<cobalt::util::SteadyClock>(),
- std::unique_ptr<cobalt::util::SleeperInterface> sleeper =
- std::make_unique<cobalt::util::Sleeper>());
+ util::NotNullUniquePtr<cobalt::util::SteadyClockInterface> steady_clock =
+ util::MakeNotNullUniquePtr<cobalt::util::SteadyClock>(),
+ util::NotNullUniquePtr<cobalt::util::SleeperInterface> sleeper =
+ util::MakeNotNullUniquePtr<cobalt::util::Sleeper>());
Status UploadEvents(LogRequest *log_request, int32_t max_retries) override;
@@ -83,12 +84,12 @@
Status TryUploadEvents(LogRequest *log_request, std::chrono::steady_clock::time_point deadline);
const std::string url_;
- const std::unique_ptr<HTTPClient> client_;
+ const util::PinnedUniquePtr<HTTPClient> client_;
const std::chrono::milliseconds upload_timeout_;
const std::chrono::milliseconds initial_backoff_;
- const std::unique_ptr<cobalt::util::SteadyClockInterface> steady_clock_;
- const std::unique_ptr<cobalt::util::SleeperInterface> sleeper_;
+ const util::PinnedUniquePtr<cobalt::util::SteadyClockInterface> steady_clock_;
+ const util::PinnedUniquePtr<cobalt::util::SleeperInterface> sleeper_;
friend class UploaderTest;
diff --git a/src/lib/clearcut/uploader_test.cc b/src/lib/clearcut/uploader_test.cc
index f384d50..f6042c1 100644
--- a/src/lib/clearcut/uploader_test.cc
+++ b/src/lib/clearcut/uploader_test.cc
@@ -12,6 +12,7 @@
#include "src/lib/clearcut/clearcut.pb.h"
#include "src/lib/util/clock.h"
+#include "src/lib/util/not_null.h"
#include "src/lib/util/sleeper.h"
#include "src/logging.h"
@@ -83,16 +84,14 @@
class UploaderTest : public ::testing::Test {
protected:
void DoSetUp(int64_t upload_timeout_millis) {
- auto unique_client = std::make_unique<FakeHttpClient>();
- client = unique_client.get();
- auto unique_clock = std::make_unique<IncrementingSteadyClock>(kFakeClockIncrement);
- fake_clock = unique_clock.get();
- auto unique_sleeper = std::make_unique<FakeSleeper>();
- fake_sleeper = unique_sleeper.get();
+ client = new FakeHttpClient();
+ fake_clock = new IncrementingSteadyClock(kFakeClockIncrement);
+ fake_sleeper = new FakeSleeper();
fake_sleeper->set_incrementing_steady_clock(fake_clock);
uploader = std::make_unique<ClearcutUploader>(
- kFakeClearcutURL, std::move(unique_client), upload_timeout_millis,
- kInitialBackoffMillisForTest, std::move(unique_clock), std::move(unique_sleeper));
+ kFakeClearcutURL, util::TESTONLY_TakeRawPointer(client), upload_timeout_millis,
+ kInitialBackoffMillisForTest, util::TESTONLY_TakeRawPointer(fake_clock),
+ util::TESTONLY_TakeRawPointer(fake_sleeper));
}
void SetUp() override { DoSetUp(kUploadTimeoutMillis); }
diff --git a/src/lib/util/not_null.h b/src/lib/util/not_null.h
index cc675ba..ef6bc74 100644
--- a/src/lib/util/not_null.h
+++ b/src/lib/util/not_null.h
@@ -95,9 +95,9 @@
return NotNullUniquePtr<U>(std::make_unique<T>(std::forward<Args>(args)...));
}
-// TESTONLY_TakeRawPointer is a hack to allow a test to keep a reference to a pointer while passing
-// a NotNullUniquePtr to another class. It is currently only used in testing Cobalt 1.0 local
-// aggregation and is not the preferred solution.
+// TESTONLY_TakeRawPointer takes a (possible null) pointer to an object of type T. If the pointer is
+// non-null, it will return a NotNullUniquePtr<T> that takes ownership of the provided pointer.
+// Otherwise it will CHECK-fail.
template <typename T>
constexpr NotNullUniquePtr<T> TESTONLY_TakeRawPointer(T* other) {
CHECK(other) << "Expected non-null other in TESTONLY_TakeRawPointer";
@@ -166,8 +166,8 @@
// Access the contained ptr. Guaranteed never to return null.
[[nodiscard]] T* get() const noexcept { return ptr_.get(); }
- T* operator->() { return ptr_.operator->(); }
- T& operator*() { return ptr_.operator*(); }
+ T* operator->() const noexcept { return ptr_.operator->(); }
+ T& operator*() const noexcept { return ptr_.operator*(); }
private:
std::unique_ptr<T> ptr_;
diff --git a/src/public/cobalt_service.cc b/src/public/cobalt_service.cc
index b265169..856e301 100644
--- a/src/public/cobalt_service.cc
+++ b/src/public/cobalt_service.cc
@@ -69,7 +69,9 @@
cfg->target_pipeline->TakeClearcutUploader();
if (!clearcut_uploader) {
clearcut_uploader = std::make_unique<lib::clearcut::ClearcutUploader>(
- cfg->target_pipeline->clearcut_endpoint(), cfg->target_pipeline->TakeHttpClient());
+ cfg->target_pipeline->clearcut_endpoint(),
+ util::WrapNotNullUniquePtrOrDefault<lib::EmptyHTTPClient>(
+ cfg->target_pipeline->TakeHttpClient()));
}
auto shipping_manager = std::make_unique<uploader::ClearcutV1ShippingManager>(
diff --git a/src/public/lib/http_client.h b/src/public/lib/http_client.h
index e7efb3e..99d0152 100644
--- a/src/public/lib/http_client.h
+++ b/src/public/lib/http_client.h
@@ -81,6 +81,14 @@
virtual ~HTTPClient() = default;
};
+class EmptyHTTPClient : public HTTPClient {
+ public:
+ StatusOr<HTTPResponse> PostSync(HTTPRequest request,
+ std::chrono::steady_clock::time_point deadline) override {
+ return Status::OkStatus();
+ }
+};
+
} // namespace cobalt::lib
#endif // COBALT_SRC_PUBLIC_LIB_HTTP_CLIENT_H_
diff --git a/src/uploader/shipping_manager_test.cc b/src/uploader/shipping_manager_test.cc
index bcb23c9..35970e5 100644
--- a/src/uploader/shipping_manager_test.cc
+++ b/src/uploader/shipping_manager_test.cc
@@ -14,6 +14,7 @@
#include <gtest/gtest.h>
#include "src/lib/clearcut/clearcut.pb.h"
+#include "src/lib/util/not_null.h"
#include "src/lib/util/posix_file_system.h"
#include "src/logger/fake_logger.h"
#include "src/logger/internal_metrics.h"
@@ -138,13 +139,12 @@
std::unique_ptr<FakeActivityListener> listener = nullptr,
DiagnosticsInterface* diagnostics = nullptr) {
UploadScheduler upload_scheduler(schedule_interval, min_interval);
- auto http_client = std::make_unique<FakeHTTPClient>();
- http_client_ = http_client.get();
+ http_client_ = new FakeHTTPClient();
shipping_manager_ = std::make_unique<ClearcutV1ShippingManager>(
upload_scheduler, observation_store_, encrypt_to_shuffler_.get(),
encrypt_to_analyzer_.get(),
- std::make_unique<lib::clearcut::ClearcutUploader>("https://test.com",
- std::move(http_client)),
+ std::make_unique<lib::clearcut::ClearcutUploader>(
+ "https://test.com", util::TESTONLY_TakeRawPointer(http_client_)),
std::move(listener), diagnostics,
/*log_source_id=*/11, /*max_attempts_per_upload=*/1);
shipping_manager_->Start();
