Skip to content

Commit

Permalink
mobile: Switch to absl::AnyInvocable for engine_types.h (#33523)
Browse files Browse the repository at this point in the history
This PR switches from std::function to absl::AnyInvocable since absl::AnyInvocable is more flexible, such that it does not have a copyability requirement. The change should mostly be backward compatible.

This change also undeprecates setOnEngineRunning and introduces setOnEngineExit since it makes adding engine callbacks much nicer.

This PR also removes the comment about Swift/C++ interop since that support has been removed in #33484.

Signed-off-by: Fredy Wijaya <fredyw@google.com>
  • Loading branch information
fredyw authored Apr 15, 2024
1 parent a02a826 commit 5219730
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 21 deletions.
7 changes: 6 additions & 1 deletion mobile/library/cc/engine_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,16 @@ EngineBuilder& EngineBuilder::setEngineCallbacks(std::unique_ptr<EngineCallbacks
return *this;
}

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

EngineBuilder& EngineBuilder::setOnEngineExit(absl::AnyInvocable<void()> closure) {
callbacks_->on_exit_ = std::move(closure);
return *this;
}

EngineBuilder& EngineBuilder::setEventTracker(std::unique_ptr<EnvoyEventTracker> event_tracker) {
event_tracker_ = std::move(event_tracker);
return *this;
Expand Down
4 changes: 2 additions & 2 deletions mobile/library/cc/engine_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ class EngineBuilder {
EngineBuilder& setLogLevel(Logger::Logger::Levels log_level);
EngineBuilder& setLogger(std::unique_ptr<EnvoyLogger> logger);
EngineBuilder& setEngineCallbacks(std::unique_ptr<EngineCallbacks> callbacks);
[[deprecated("Use EngineBuilder::setEngineCallbacks instead")]] EngineBuilder&
setOnEngineRunning(std::function<void()> closure);
EngineBuilder& setOnEngineRunning(absl::AnyInvocable<void()> closure);
EngineBuilder& setOnEngineExit(absl::AnyInvocable<void()> closure);
EngineBuilder& setEventTracker(std::unique_ptr<EnvoyEventTracker> event_tracker);
EngineBuilder& addConnectTimeoutSeconds(int connect_timeout_seconds);
EngineBuilder& addDnsRefreshSeconds(int dns_refresh_seconds);
Expand Down
1 change: 1 addition & 0 deletions mobile/library/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ envoy_cc_library(
repository = "@envoy",
deps = [
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/functional:any_invocable",
"@com_google_absl//absl/strings",
"@envoy//source/common/common:base_logger_lib",
],
Expand Down
24 changes: 7 additions & 17 deletions mobile/library/common/engine_types.h
Original file line number Diff line number Diff line change
@@ -1,45 +1,35 @@
#pragma once

//==================================================================================================
// READ THIS BEFORE UPDATING THIS FILE
//==================================================================================================
// Keep the code here (including the includes) as simple as possible given that this file will be
// directly included by Swift and the Swift/C++ interop is far from complete. Including headers or
// having code that is not supported by Swift may lead into weird compilation errors that can be
// difficult to debug.
// For more information, see
// https://github.com/apple/swift/blob/swift-5.7.3-RELEASE/docs/CppInteroperability/CppInteroperabilityStatus.md

#include <functional>
#include <string>

#include "source/common/common/base_logger.h"

#include "absl/container/flat_hash_map.h"
#include "absl/functional/any_invocable.h"
#include "absl/strings/string_view.h"

namespace Envoy {

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

/** The callbacks for Envoy Logger. */
struct EnvoyLogger {
std::function<void(Logger::Logger::Levels, const std::string&)> on_log_ =
absl::AnyInvocable<void(Logger::Logger::Levels, const std::string&)> on_log_ =
[](Logger::Logger::Levels, const std::string&) {};
std::function<void()> on_exit_ = [] {};
absl::AnyInvocable<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_ =
absl::AnyInvocable<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_ = [] {};
absl::AnyInvocable<void()> on_exit_ = [] {};
};

} // namespace Envoy
7 changes: 6 additions & 1 deletion mobile/test/kotlin/integration/SendDataTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ class SendDataTest {
.newStreamPrototype()
.setOnResponseHeaders { headers, _, _ -> responseStatus = headers.httpStatus }
.setOnResponseData { _, endStream, _ ->
// Sometimes Envoy Mobile may send an empty data (0 byte) with `endStream` set to true
// to indicate an end of stream. So, we should only do `expectation.countDown()`
// when the `endStream` is true.
responseEndStream = endStream
expectation.countDown()
if (endStream) {
expectation.countDown()
}
}
.setOnError { _, _ -> fail("Unexpected error") }
.start()
Expand Down

0 comments on commit 5219730

Please sign in to comment.