Skip to content

Commit

Permalink
mobile: Rename Envoy::Engine to Envoy::InternalEngine (#32147)
Browse files Browse the repository at this point in the history
Having two classes with the same `Engine`  name can be confusing. Since
the `Envoy::Engine` is the actual implementation of `Engine` and it's
supposed to be internal, calling it `EngineImpl` makes more sense.

Signed-off-by: Fredy Wijaya <fredyw@google.com>
  • Loading branch information
fredyw authored Feb 1, 2024
1 parent 9f92f4c commit 6db5d36
Show file tree
Hide file tree
Showing 26 changed files with 118 additions and 112 deletions.
2 changes: 1 addition & 1 deletion mobile/library/cc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ envoy_cc_library(
repository = "@envoy",
visibility = ["//visibility:public"],
deps = [
"//library/common:engine_lib_no_stamp",
"//library/common:internal_engine_lib_no_stamp",
"//library/common/api:c_types",
"//library/common/data:utility_lib",
"//library/common/extensions/key_value/platform:config",
Expand Down
4 changes: 2 additions & 2 deletions mobile/library/cc/engine.cc
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#include "engine.h"

#include "library/common/engine.h"
#include "library/common/internal_engine.h"
#include "library/common/types/c_types.h"

namespace Envoy {
namespace Platform {

Engine::Engine(::Envoy::Engine* engine) : engine_(engine) {}
Engine::Engine(::Envoy::InternalEngine* engine) : engine_(engine) {}

Engine::~Engine() {
if (!engine_->isTerminated()) {
Expand Down
10 changes: 5 additions & 5 deletions mobile/library/cc/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "library/common/types/c_types.h"

namespace Envoy {
class Engine;
class InternalEngine;
class BaseClientIntegrationTest;

namespace Platform {
Expand All @@ -23,10 +23,10 @@ class Engine : public std::enable_shared_from_this<Engine> {
StreamClientSharedPtr streamClient();

envoy_status_t terminate();
Envoy::Engine* engine() { return engine_; }
Envoy::InternalEngine* engine() { return engine_; }

private:
Engine(::Envoy::Engine* engine);
Engine(::Envoy::InternalEngine* engine);

// required to access private constructor
friend class EngineBuilder;
Expand All @@ -35,11 +35,11 @@ class Engine : public std::enable_shared_from_this<Engine> {
// for testing only
friend class ::Envoy::BaseClientIntegrationTest;

Envoy::Engine* engine_;
Envoy::InternalEngine* engine_;
StreamClientSharedPtr stream_client_;
};

using EngineSharedPtr = std::shared_ptr<Engine>;
using InternalEngineSharedPtr = std::shared_ptr<Engine>;

} // namespace Platform
} // namespace Envoy
6 changes: 3 additions & 3 deletions mobile/library/cc/engine_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include "absl/strings/str_join.h"
#include "absl/strings/str_replace.h"
#include "fmt/core.h"
#include "library/common/engine.h"
#include "library/common/internal_engine.h"
#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h"
#include "library/common/extensions/filters/http/local_error/filter.pb.h"
#include "library/common/extensions/filters/http/network_configuration/filter.pb.h"
Expand Down Expand Up @@ -829,8 +829,8 @@ EngineSharedPtr EngineBuilder::build() {

envoy_event_tracker null_tracker{};

Envoy::Engine* envoy_engine =
new Envoy::Engine(callbacks_->asEnvoyEngineCallbacks(), null_logger, null_tracker);
Envoy::InternalEngine* envoy_engine =
new Envoy::InternalEngine(callbacks_->asEnvoyEngineCallbacks(), null_logger, null_tracker);

for (const auto& [name, store] : key_value_stores_) {
// TODO(goaway): This leaks, but it's tied to the life of the engine.
Expand Down
5 changes: 3 additions & 2 deletions mobile/library/cc/stream.cc
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
#include "stream.h"

#include "library/cc/bridge_utility.h"
#include "library/common/engine.h"
#include "library/common/internal_engine.h"
#include "library/common/types/c_types.h"

namespace Envoy {
namespace Platform {

Stream::Stream(Envoy::Engine* engine, envoy_stream_t handle) : engine_(engine), handle_(handle) {}
Stream::Stream(Envoy::InternalEngine* engine, envoy_stream_t handle)
: engine_(engine), handle_(handle) {}

Stream& Stream::sendHeaders(RequestHeadersSharedPtr headers, bool end_stream) {
envoy_headers raw_headers = rawHeaderMapAsEnvoyHeaders(headers->allHeaders());
Expand Down
6 changes: 3 additions & 3 deletions mobile/library/cc/stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
#include "library/common/types/c_types.h"

namespace Envoy {
class Engine;
class InternalEngine;
namespace Platform {

class Stream {
public:
Stream(Envoy::Engine* engine, envoy_stream_t handle);
Stream(Envoy::InternalEngine* engine, envoy_stream_t handle);

Stream& sendHeaders(RequestHeadersSharedPtr headers, bool end_stream);
Stream& sendData(envoy_data data);
Expand All @@ -23,7 +23,7 @@ class Stream {
void cancel();

private:
Envoy::Engine* engine_;
Envoy::InternalEngine* engine_;
envoy_stream_t handle_;
};

Expand Down
2 changes: 1 addition & 1 deletion mobile/library/cc/stream_prototype.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "stream_prototype.h"

#include "library/common/engine.h"
#include "library/common/internal_engine.h"

namespace Envoy {
namespace Platform {
Expand Down
10 changes: 5 additions & 5 deletions mobile/library/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@ licenses(["notice"]) # Apache 2
envoy_mobile_package()

envoy_cc_library(
name = "engine_lib",
name = "internal_engine_lib",
repository = "@envoy",
deps = [
":engine_common_lib_stamped",
":engine_lib_no_stamp",
":internal_engine_lib_no_stamp",
],
)

envoy_cc_library(
name = "engine_lib_no_stamp",
name = "internal_engine_lib_no_stamp",
srcs = [
"engine.cc",
"internal_engine.cc",
],
hdrs = [
"engine.h",
"internal_engine.h",
],
repository = "@envoy",
deps = [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "library/common/engine.h"
#include "library/common/internal_engine.h"

#include "source/common/api/os_sys_calls_impl.h"
#include "source/common/common/lock_guard.h"
Expand All @@ -13,10 +13,10 @@ namespace Envoy {

static std::atomic<envoy_stream_t> current_stream_handle_{0};

envoy_stream_t Engine::initStream() { return current_stream_handle_++; }
envoy_stream_t InternalEngine::initStream() { return current_stream_handle_++; }

Engine::Engine(envoy_engine_callbacks callbacks, envoy_logger logger,
envoy_event_tracker event_tracker)
InternalEngine::InternalEngine(envoy_engine_callbacks callbacks, envoy_logger logger,
envoy_event_tracker event_tracker)
: callbacks_(callbacks), logger_(logger), event_tracker_(event_tracker),
dispatcher_(std::make_unique<Event::ProvisionalDispatcher>()) {
ExtensionRegistry::registerFactories();
Expand All @@ -32,7 +32,7 @@ Engine::Engine(envoy_engine_callbacks callbacks, envoy_logger logger,
Runtime::maybeSetRuntimeGuard("envoy.reloadable_features.dfp_mixed_scheme", true);
}

envoy_status_t Engine::run(const std::string& config, const std::string& log_level) {
envoy_status_t InternalEngine::run(const std::string& config, const std::string& log_level) {
// Start the Envoy on the dedicated thread. Note: due to how the assignment operator works with
// std::thread, main_thread_ is the same object after this call, but its state is replaced with
// that of the temporary. The temporary object's state becomes the default state, which does
Expand All @@ -46,12 +46,12 @@ envoy_status_t Engine::run(const std::string& config, const std::string& log_lev
return run(std::move(options));
}

envoy_status_t Engine::run(std::unique_ptr<Envoy::OptionsImplBase>&& options) {
main_thread_ = std::thread(&Engine::main, this, std::move(options));
envoy_status_t InternalEngine::run(std::unique_ptr<Envoy::OptionsImplBase>&& options) {
main_thread_ = std::thread(&InternalEngine::main, this, std::move(options));
return ENVOY_SUCCESS;
}

envoy_status_t Engine::main(std::unique_ptr<Envoy::OptionsImplBase>&& options) {
envoy_status_t InternalEngine::main(std::unique_ptr<Envoy::OptionsImplBase>&& options) {
// Using unique_ptr ensures main_common's lifespan is strictly scoped to this function.
std::unique_ptr<EngineCommon> main_common;
{
Expand Down Expand Up @@ -145,7 +145,7 @@ envoy_status_t Engine::main(std::unique_ptr<Envoy::OptionsImplBase>&& options) {
return run_success ? ENVOY_SUCCESS : ENVOY_FAILURE;
}

envoy_status_t Engine::terminate() {
envoy_status_t InternalEngine::terminate() {
if (terminated_) {
IS_ENVOY_BUG("attempted to double terminate engine");
return ENVOY_FAILURE;
Expand Down Expand Up @@ -185,34 +185,34 @@ envoy_status_t Engine::terminate() {
return ENVOY_SUCCESS;
}

bool Engine::isTerminated() const { return terminated_; }
bool InternalEngine::isTerminated() const { return terminated_; }

Engine::~Engine() {
InternalEngine::InternalEngine() {
if (!terminated_) {
terminate();
}
}

envoy_status_t Engine::setProxySettings(const char* hostname, const uint16_t port) {
envoy_status_t InternalEngine::setProxySettings(const char* hostname, const uint16_t port) {
return dispatcher_->post([&, host = std::string(hostname), port]() -> void {
connectivity_manager_->setProxySettings(Network::ProxySettings::parseHostAndPort(host, port));
});
}

envoy_status_t Engine::resetConnectivityState() {
envoy_status_t InternalEngine::resetConnectivityState() {
return dispatcher_->post([&]() -> void { connectivity_manager_->resetConnectivityState(); });
}

envoy_status_t Engine::setPreferredNetwork(envoy_network_t network) {
envoy_status_t InternalEngine::setPreferredNetwork(envoy_network_t network) {
return dispatcher_->post([&, network]() -> void {
envoy_netconf_t configuration_key =
Envoy::Network::ConnectivityManagerImpl::setPreferredNetwork(network);
connectivity_manager_->refreshDns(configuration_key, true);
});
}

envoy_status_t Engine::recordCounterInc(absl::string_view elements, envoy_stats_tags tags,
uint64_t count) {
envoy_status_t InternalEngine::recordCounterInc(absl::string_view elements, envoy_stats_tags tags,
uint64_t count) {
return dispatcher_->post(
[&, name = Stats::Utility::sanitizeStatsName(elements), tags, count]() -> void {
ENVOY_LOG(trace, "[pulse.{}] recordCounterInc", name);
Expand All @@ -223,7 +223,7 @@ envoy_status_t Engine::recordCounterInc(absl::string_view elements, envoy_stats_
});
}

Event::ProvisionalDispatcher& Engine::dispatcher() { return *dispatcher_; }
Event::ProvisionalDispatcher& InternalEngine::dispatcher() { return *dispatcher_; }

void statsAsText(const std::map<std::string, uint64_t>& all_stats,
const std::vector<Stats::ParentHistogramSharedPtr>& histograms,
Expand Down Expand Up @@ -264,7 +264,7 @@ void handlerStats(Stats::Store& stats, Buffer::Instance& response) {
statsAsText(all_stats, histograms, response);
}

std::string Engine::dumpStats() {
std::string InternalEngine::dumpStats() {
if (!main_thread_.joinable()) {
return "";
}
Expand All @@ -283,19 +283,19 @@ std::string Engine::dumpStats() {
return stats;
}

Upstream::ClusterManager& Engine::getClusterManager() {
Upstream::ClusterManager& InternalEngine::getClusterManager() {
ASSERT(dispatcher_->isThreadSafe(),
"getClusterManager must be called from the dispatcher's context");
return server_->clusterManager();
}

Stats::Store& Engine::getStatsStore() {
Stats::Store& InternalEngine::getStatsStore() {
ASSERT(dispatcher_->isThreadSafe(), "getStatsStore must be called from the dispatcher's context");
return server_->stats();
}

void Engine::logInterfaces(absl::string_view event,
std::vector<Network::InterfacePair>& interfaces) {
void InternalEngine::logInterfaces(absl::string_view event,
std::vector<Network::InterfacePair>& interfaces) {
std::vector<std::string> names;
names.resize(interfaces.size());
std::transform(interfaces.begin(), interfaces.end(), names.begin(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@

namespace Envoy {

class Engine : public Logger::Loggable<Logger::Id::main> {
class InternalEngine : public Logger::Loggable<Logger::Id::main> {
public:
/**
* Constructor for a new engine instance.
* @param callbacks, the callbacks to use for engine lifecycle monitoring.
* @param logger, the callbacks to use for engine logging.
* @param event_tracker, the event tracker to use for the emission of events.
*/
Engine(envoy_engine_callbacks callbacks, envoy_logger logger, envoy_event_tracker event_tracker);
InternalEngine(envoy_engine_callbacks callbacks, envoy_logger logger,
envoy_event_tracker event_tracker);

/**
* Engine destructor.
* InternalEngine destructor.
*/
~Engine();
InternalEngine();

/**
* Run the engine with the provided configuration.
Expand Down Expand Up @@ -145,7 +146,7 @@ class Engine : public Logger::Loggable<Logger::Id::main> {
bool terminated_{false};
};

using EngineSharedPtr = std::shared_ptr<Engine>;
using EngineWeakPtr = std::weak_ptr<Engine>;
using InternalEngineSharedPtr = std::shared_ptr<InternalEngine>;
using InternalEngineWeakPtr = std::weak_ptr<InternalEngine>;

} // namespace Envoy
4 changes: 2 additions & 2 deletions mobile/library/common/jni/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ envoy_cc_library(
":android_network_utility_lib",
":jni_utility_lib",
"//library/cc:engine_builder_lib",
"//library/common:engine_lib",
"//library/common:internal_engine_lib",
"//library/common/api:c_types",
"//library/common/extensions/cert_validator/platform_bridge:c_types_lib",
"//library/common/extensions/key_value/platform:config",
Expand Down Expand Up @@ -123,7 +123,7 @@ envoy_cc_library(
":jni_impl_lib",
":jni_support_lib",
":jni_utility_lib",
"//library/common:engine_lib",
"//library/common:internal_engine_lib",
"//library/common/jni/import:jni_import_lib",
],
# We need this to ensure that we link this into the .so even though there are no code references.
Expand Down
Loading

0 comments on commit 6db5d36

Please sign in to comment.