Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mobile: Use suffix _ for struct member variables #33278

Merged
merged 2 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mobile/library/cc/engine_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ EngineBuilder& EngineBuilder::setEngineCallbacks(std::unique_ptr<EngineCallbacks
}

EngineBuilder& EngineBuilder::setOnEngineRunning(std::function<void()> closure) {
callbacks_->on_engine_running = std::move(closure);
callbacks_->on_engine_running_ = std::move(closure);
return *this;
}

Expand Down
12 changes: 6 additions & 6 deletions mobile/library/common/engine_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,24 @@ namespace Envoy {

/** The callbacks for the Envoy Engine. */
struct EngineCallbacks {
std::function<void()> on_engine_running = [] {};
std::function<void()> on_exit = [] {};
std::function<void()> on_engine_running_ = [] {};
std::function<void()> on_exit_ = [] {};
};

/** The callbacks for Envoy Logger. */
struct EnvoyLogger {
std::function<void(Logger::Logger::Levels, const std::string&)> on_log =
std::function<void(Logger::Logger::Levels, const std::string&)> on_log_ =
[](Logger::Logger::Levels, const std::string&) {};
std::function<void()> on_exit = [] {};
std::function<void()> on_exit_ = [] {};
};

inline constexpr absl::string_view ENVOY_EVENT_TRACKER_API_NAME = "event_tracker_api";

/** The callbacks for Envoy Event Tracker. */
struct EnvoyEventTracker {
std::function<void(const absl::flat_hash_map<std::string, std::string>&)> on_track =
std::function<void(const absl::flat_hash_map<std::string, std::string>&)> on_track_ =
[](const absl::flat_hash_map<std::string, std::string>&) {};
std::function<void()> on_exit = [] {};
std::function<void()> on_exit_ = [] {};
};

} // namespace Envoy
10 changes: 5 additions & 5 deletions mobile/library/common/internal_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
Assert::addDebugAssertionFailureRecordAction([this](const char* location) {
absl::flat_hash_map<std::string, std::string> event{
{{"name", "assertion"}, {"location", std::string(location)}}};
event_tracker_->on_track(event);
event_tracker_->on_track_(event);
});
bug_handler_registration_ =
Assert::addEnvoyBugFailureRecordAction([this](const char* location) {
absl::flat_hash_map<std::string, std::string> event{
{{"name", "bug"}, {"location", std::string(location)}}};
event_tracker_->on_track(event);
event_tracker_->on_track_(event);
});
}

Expand Down Expand Up @@ -178,7 +178,7 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
server_->serverFactoryContext().scope(),
server_->api().randomGenerator());
dispatcher_->drain(server_->dispatcher());
callbacks_->on_engine_running();
callbacks_->on_engine_running_();
});
} // mutex_

Expand All @@ -196,9 +196,9 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
assert_handler_registration_.reset(nullptr);

if (event_tracker_ != nullptr) {
event_tracker_->on_exit();
event_tracker_->on_exit_();
}
callbacks_->on_exit();
callbacks_->on_exit_();

return run_success ? ENVOY_SUCCESS : ENVOY_FAILURE;
}
Expand Down
10 changes: 5 additions & 5 deletions mobile/library/common/logger/logger_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ void EventTrackingDelegate::logWithStableName(absl::string_view stable_name, abs
}

(*event_tracker_)
->on_track({{"name", "event_log"},
{"log_name", std::string(stable_name)},
{"message", std::string(msg)}});
->on_track_({{"name", "event_log"},
{"log_name", std::string(stable_name)},
{"message", std::string(msg)}});
}

LambdaDelegate::LambdaDelegate(std::unique_ptr<EnvoyLogger> logger,
Expand All @@ -27,12 +27,12 @@ LambdaDelegate::LambdaDelegate(std::unique_ptr<EnvoyLogger> logger,

LambdaDelegate::~LambdaDelegate() {
restoreDelegate();
logger_->on_exit();
logger_->on_exit_();
}

void LambdaDelegate::log(absl::string_view msg, const spdlog::details::log_msg& log_msg) {
// Logger::Levels is simply an alias to spdlog::level::level_enum, so we can safely cast it.
logger_->on_log(static_cast<Logger::Levels>(log_msg.level), std::string(msg));
logger_->on_log_(static_cast<Logger::Levels>(log_msg.level), std::string(msg));
}

DefaultDelegate::DefaultDelegate(absl::Mutex& mutex, DelegatingLogSinkSharedPtr log_sink)
Expand Down
16 changes: 8 additions & 8 deletions mobile/library/jni/jni_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
if (on_start_context != nullptr) {
jobject retained_on_start_context =
env->NewGlobalRef(on_start_context); // Required to keep context in memory
callbacks->on_engine_running = [retained_on_start_context] {
callbacks->on_engine_running_ = [retained_on_start_context] {
Envoy::JNI::JniHelper jni_helper(Envoy::JNI::getEnv());
Envoy::JNI::LocalRefUniquePtr<jclass> java_on_engine_running_class =
jni_helper.getObjectClass(retained_on_start_context);
Expand All @@ -58,7 +58,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
// This will need to be updated for https://github.com/envoyproxy/envoy-mobile/issues/332
jni_helper.getEnv()->DeleteGlobalRef(retained_on_start_context);
};
callbacks->on_exit = [] {
callbacks->on_exit_ = [] {
// Note that this is not dispatched because the thread that
// needs to be detached is the engine thread.
// This function is called from the context of the engine's
Expand All @@ -72,8 +72,8 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
std::unique_ptr<Envoy::EnvoyLogger> logger = std::make_unique<Envoy::EnvoyLogger>();
if (envoy_logger_context != nullptr) {
const jobject retained_logger_context = env->NewGlobalRef(envoy_logger_context);
logger->on_log = [retained_logger_context](Envoy::Logger::Logger::Levels level,
const std::string& message) {
logger->on_log_ = [retained_logger_context](Envoy::Logger::Logger::Levels level,
const std::string& message) {
Envoy::JNI::JniHelper jni_helper(Envoy::JNI::getEnv());
Envoy::JNI::LocalRefUniquePtr<jstring> java_message =
jni_helper.newStringUtf(message.c_str());
Expand All @@ -85,7 +85,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
jni_helper.callVoidMethod(retained_logger_context, java_log_method_id, java_level,
java_message.get());
};
logger->on_exit = [retained_logger_context] {
logger->on_exit_ = [retained_logger_context] {
Envoy::JNI::getEnv()->DeleteGlobalRef(retained_logger_context);
};
}
Expand All @@ -96,8 +96,8 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
std::make_unique<Envoy::EnvoyEventTracker>();
if (event_tracker_context != nullptr) {
const jobject retained_event_tracker_context = env->NewGlobalRef(event_tracker_context);
event_tracker->on_track = [retained_event_tracker_context](
const absl::flat_hash_map<std::string, std::string>& events) {
event_tracker->on_track_ = [retained_event_tracker_context](
const absl::flat_hash_map<std::string, std::string>& events) {
Envoy::JNI::JniHelper jni_helper(Envoy::JNI::getEnv());
Envoy::JNI::LocalRefUniquePtr<jobject> java_events =
Envoy::JNI::cppMapToJavaMap(jni_helper, events);
Expand All @@ -108,7 +108,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
jni_helper.callVoidMethod(retained_event_tracker_context, java_track_method_id,
java_events.get());
};
event_tracker->on_exit = [retained_event_tracker_context] {
event_tracker->on_exit_ = [retained_event_tracker_context] {
Envoy::JNI::getEnv()->DeleteGlobalRef(retained_event_tracker_context);
};
}
Expand Down
10 changes: 5 additions & 5 deletions mobile/library/objective-c/EnvoyEngineImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ - (instancetype)initWithRunningCallback:(nullable void (^)())onEngineRunning

std::unique_ptr<Envoy::EngineCallbacks> native_callbacks =
std::make_unique<Envoy::EngineCallbacks>();
native_callbacks->on_engine_running = [onEngineRunning = std::move(onEngineRunning)] {
native_callbacks->on_engine_running_ = [onEngineRunning = std::move(onEngineRunning)] {
// This code block runs inside the Envoy event loop. Therefore, an explicit autoreleasepool
// block is necessary to act as a breaker for any Objective-C allocation that happens.
@autoreleasepool {
Expand All @@ -385,7 +385,7 @@ - (instancetype)initWithRunningCallback:(nullable void (^)())onEngineRunning
}
}
};
native_callbacks->on_exit = [] {
native_callbacks->on_exit_ = [] {
// This code block runs inside the Envoy event loop. Therefore, an explicit autoreleasepool
// block is necessary to act as a breaker for any Objective-C allocation that happens.
@autoreleasepool {
Expand All @@ -395,8 +395,8 @@ - (instancetype)initWithRunningCallback:(nullable void (^)())onEngineRunning

std::unique_ptr<Envoy::EnvoyLogger> native_logger = std::make_unique<Envoy::EnvoyLogger>();
if (logger) {
native_logger->on_log = [logger = std::move(logger)](Envoy::Logger::Logger::Levels level,
const std::string &message) {
native_logger->on_log_ = [logger = std::move(logger)](Envoy::Logger::Logger::Levels level,
const std::string &message) {
// This code block runs inside the Envoy event loop. Therefore, an explicit autoreleasepool
// block is necessary to act as a breaker for any Objective-C allocation that happens.
@autoreleasepool {
Expand All @@ -408,7 +408,7 @@ - (instancetype)initWithRunningCallback:(nullable void (^)())onEngineRunning
std::unique_ptr<Envoy::EnvoyEventTracker> native_event_tracker =
std::make_unique<Envoy::EnvoyEventTracker>();
if (eventTracker) {
native_event_tracker->on_track =
native_event_tracker->on_track_ =
[eventTracker =
std::move(eventTracker)](const absl::flat_hash_map<std::string, std::string> &events) {
NSMutableDictionary *newMap = [NSMutableDictionary new];
Expand Down
12 changes: 6 additions & 6 deletions mobile/test/cc/engine_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ namespace Envoy {
TEST(EngineTest, SetLogger) {
std::atomic logging_was_called{false};
auto logger = std::make_unique<EnvoyLogger>();
logger->on_log = [&](Logger::Logger::Levels, const std::string&) { logging_was_called = true; };
logger->on_log_ = [&](Logger::Logger::Levels, const std::string&) { logging_was_called = true; };

absl::Notification engine_running;
auto engine_callbacks = std::make_unique<EngineCallbacks>();
engine_callbacks->on_engine_running = [&] { engine_running.Notify(); };
engine_callbacks->on_engine_running_ = [&] { engine_running.Notify(); };
Platform::EngineBuilder engine_builder;
Platform::EngineSharedPtr engine =
engine_builder.setLogLevel(Logger::Logger::debug)
Expand Down Expand Up @@ -66,7 +66,7 @@ TEST(EngineTest, SetLogger) {
TEST(EngineTest, SetEngineCallbacks) {
absl::Notification engine_running;
auto engine_callbacks = std::make_unique<EngineCallbacks>();
engine_callbacks->on_engine_running = [&] { engine_running.Notify(); };
engine_callbacks->on_engine_running_ = [&] { engine_running.Notify(); };
Platform::EngineBuilder engine_builder;
Platform::EngineSharedPtr engine =
engine_builder.setLogLevel(Logger::Logger::debug)
Expand Down Expand Up @@ -117,18 +117,18 @@ TEST(EngineTest, SetEngineCallbacks) {
TEST(EngineTest, SetEventTracker) {
absl::Notification engine_running;
auto engine_callbacks = std::make_unique<EngineCallbacks>();
engine_callbacks->on_engine_running = [&] { engine_running.Notify(); };
engine_callbacks->on_engine_running_ = [&] { engine_running.Notify(); };

absl::Notification on_track;
auto event_tracker = std::make_unique<EnvoyEventTracker>();
event_tracker->on_track = [&](const absl::flat_hash_map<std::string, std::string>& events) {
event_tracker->on_track_ = [&](const absl::flat_hash_map<std::string, std::string>& events) {
if (events.count("name") && events.at("name") == "assertion") {
EXPECT_EQ(events.at("location"), "foo_location");
on_track.Notify();
}
};
absl::Notification on_track_exit;
event_tracker->on_exit = [&] { on_track_exit.Notify(); };
event_tracker->on_exit_ = [&] { on_track_exit.Notify(); };

Platform::EngineBuilder engine_builder;
Platform::EngineSharedPtr engine =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TestEventTrackerFilterConfig {
absl::flat_hash_map<std::string, std::string> attributes() { return attributes_; };
void track(const absl::flat_hash_map<std::string, std::string>& events) {
if (event_tracker_ != nullptr) {
(*event_tracker_)->on_track(events);
(*event_tracker_)->on_track_(events);
}
}

Expand Down
16 changes: 8 additions & 8 deletions mobile/test/common/internal_engine_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ struct TestEngine {

std::unique_ptr<EngineCallbacks> createDefaultEngineCallbacks(EngineTestContext& test_context) {
std::unique_ptr<EngineCallbacks> engine_callbacks = std::make_unique<EngineCallbacks>();
engine_callbacks->on_engine_running = [&] { test_context.on_engine_running.Notify(); };
engine_callbacks->on_exit = [&] { test_context.on_exit.Notify(); };
engine_callbacks->on_engine_running_ = [&] { test_context.on_engine_running.Notify(); };
engine_callbacks->on_exit_ = [&] { test_context.on_exit.Notify(); };
return engine_callbacks;
}

Expand Down Expand Up @@ -230,12 +230,12 @@ TEST_F(InternalEngineTest, RecordCounter) {
TEST_F(InternalEngineTest, Logger) {
EngineTestContext test_context{};
auto logger = std::make_unique<EnvoyLogger>();
logger->on_log = [&](Logger::Logger::Levels, const std::string&) {
logger->on_log_ = [&](Logger::Logger::Levels, const std::string&) {
if (!test_context.on_log.HasBeenNotified()) {
test_context.on_log.Notify();
}
};
logger->on_exit = [&] { test_context.on_log_exit.Notify(); };
logger->on_exit_ = [&] { test_context.on_log_exit.Notify(); };
std::unique_ptr<InternalEngine> engine = std::make_unique<InternalEngine>(
createDefaultEngineCallbacks(test_context), std::move(logger), /*event_tracker=*/nullptr);
engine->run(MINIMAL_TEST_CONFIG, LEVEL_DEBUG);
Expand Down Expand Up @@ -276,7 +276,7 @@ TEST_F(InternalEngineTest, EventTrackerRegistersAPI) {
EngineTestContext test_context{};

auto event_tracker = std::make_unique<EnvoyEventTracker>();
event_tracker->on_track = [&](const absl::flat_hash_map<std::string, std::string>& events) {
event_tracker->on_track_ = [&](const absl::flat_hash_map<std::string, std::string>& events) {
if (events.count("foo") && events.at("foo") == "bar") {
test_context.on_event.Notify();
}
Expand All @@ -291,7 +291,7 @@ TEST_F(InternalEngineTest, EventTrackerRegistersAPI) {
Api::External::retrieveApi(ENVOY_EVENT_TRACKER_API_NAME));
EXPECT_TRUE(registered_event_tracker != nullptr && *registered_event_tracker != nullptr);

(*registered_event_tracker)->on_track({{"foo", "bar"}});
(*registered_event_tracker)->on_track_({{"foo", "bar"}});

ASSERT_TRUE(test_context.on_event.WaitForNotificationWithTimeout(absl::Seconds(3)));
engine->terminate();
Expand All @@ -302,7 +302,7 @@ TEST_F(InternalEngineTest, EventTrackerRegistersAssertionFailureRecordAction) {
EngineTestContext test_context{};

auto event_tracker = std::make_unique<EnvoyEventTracker>();
event_tracker->on_track = [&](const absl::flat_hash_map<std::string, std::string>& events) {
event_tracker->on_track_ = [&](const absl::flat_hash_map<std::string, std::string>& events) {
if (events.count("name") && events.at("name") == "assertion") {
EXPECT_EQ(events.at("location"), "foo_location");
test_context.on_event.Notify();
Expand All @@ -329,7 +329,7 @@ TEST_F(InternalEngineTest, EventTrackerRegistersEnvoyBugRecordAction) {
EngineTestContext test_context{};

auto event_tracker = std::make_unique<EnvoyEventTracker>();
event_tracker->on_track = [&](const absl::flat_hash_map<std::string, std::string>& events) {
event_tracker->on_track_ = [&](const absl::flat_hash_map<std::string, std::string>& events) {
if (events.count("name") && events.at("name") == "bug") {
EXPECT_EQ(events.at("location"), "foo_location");
test_context.on_event.Notify();
Expand Down
8 changes: 4 additions & 4 deletions mobile/test/common/logger/logger_delegate_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ TEST_F(LambdaDelegateTest, LogCb) {
std::string actual_msg;

auto logger = std::make_unique<EnvoyLogger>();
logger->on_log = [&](Logger::Levels, const std::string& message) { actual_msg = message; };
logger->on_log_ = [&](Logger::Levels, const std::string& message) { actual_msg = message; };
LambdaDelegate delegate(std::move(logger), Registry::getSink());

ENVOY_LOG_MISC(error, expected_msg);
Expand All @@ -41,7 +41,7 @@ TEST_F(LambdaDelegateTest, LogCbWithLevels) {
std::string actual_msg;

auto logger = std::make_unique<EnvoyLogger>();
logger->on_log = [&](Logger::Levels, const std::string& message) { actual_msg = message; };
logger->on_log_ = [&](Logger::Levels, const std::string& message) { actual_msg = message; };
LambdaDelegate delegate(std::move(logger), Registry::getSink());

// Set the log to critical. The message should not be logged.
Expand All @@ -65,7 +65,7 @@ TEST_F(LambdaDelegateTest, ReleaseCb) {

{
auto logger = std::make_unique<EnvoyLogger>();
logger->on_exit = [&] { released = true; };
logger->on_exit_ = [&] { released = true; };
LambdaDelegate(std::move(logger), Registry::getSink());
}

Expand All @@ -89,7 +89,7 @@ TEST_P(LambdaDelegateWithLevelTest, Log) {
Logger::Levels actual_level;
std::string actual_msg;
auto logger = std::make_unique<EnvoyLogger>();
logger->on_log = [&](Logger::Levels level, const std::string& message) {
logger->on_log_ = [&](Logger::Levels level, const std::string& message) {
actual_level = level;
actual_msg = message;
};
Expand Down