Skip to content

Commit

Permalink
mobile: Use suffix _ for struct member variables (envoyproxy#33278)
Browse files Browse the repository at this point in the history
This PR updates the code to use suffix _ for struct member variables as per Envoy Code Style.

Risk Level: low (rename only)
Testing: CI
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Fredy Wijaya <fredyw@google.com>
  • Loading branch information
fredyw committed Apr 3, 2024
1 parent 3e9aca4 commit 06ea913
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 49 deletions.
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

0 comments on commit 06ea913

Please sign in to comment.