From 2f6cd587f95a326e44129af16d0467b828555654 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 19:31:02 -0400 Subject: [PATCH 01/14] api: return opaque engine handle in `onEngineRunning` To support interop between the Swift/Kotlin and C++ APIs. Signed-off-by: JP Simard --- docs/root/api/starting_envoy.rst | 5 ++++- docs/root/intro/version_history.rst | 1 + library/cc/engine.cc | 2 ++ library/cc/engine.h | 2 ++ library/objective-c/EnvoyEngine.h | 5 +++-- library/objective-c/EnvoyEngineImpl.m | 8 ++++++-- library/swift/EngineBuilder.swift | 7 ++++--- library/swift/mocks/MockEnvoyEngine.swift | 2 +- 8 files changed, 23 insertions(+), 9 deletions(-) diff --git a/docs/root/api/starting_envoy.rst b/docs/root/api/starting_envoy.rst index 841d4442db..4bcfefae51 100644 --- a/docs/root/api/starting_envoy.rst +++ b/docs/root/api/starting_envoy.rst @@ -313,13 +313,16 @@ completes. This interface provides the ability to observe when Envoy has complet ready to start dispatching requests. Any requests sent through Envoy before this setup completes will be queued automatically, and this function is typically used purely for observability. +The closure passes an opaque handle to the engine so that it can be passed to Envoy's C++ APIs if +desired. + **Example**:: // Kotlin builder.setOnEngineRunning { /*do something*/ } // Swift - builder.setOnEngineRunning { /*do something*/ } + builder.setOnEngineRunning { engineHandle in /*do something*/ } ~~~~~~~~~~~~~ ``setLogger`` diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 1385bfc3fa..afad524137 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -14,6 +14,7 @@ Breaking changes: - iOS: enable usage of ``NWPathMonitor`` by default (:issue:`#2329 <2329>`) - iOS: replace ``enableNetworkPathMonitor`` with a new ``setNetworkMonitoringMode`` API to allow disabling monitoring (:issue:`#2345 <2345>`) - iOS: release artifacts no longer embed bitcode +- api: the ``setOnEngineRunning`` closure now passes the engine's opaque ID so it can be passed to Envoy's C++ APis Bugfixes: diff --git a/library/cc/engine.cc b/library/cc/engine.cc index fadf2458d6..010f742270 100644 --- a/library/cc/engine.cc +++ b/library/cc/engine.cc @@ -16,6 +16,8 @@ StreamClientSharedPtr Engine::streamClient() { return std::make_shared(shared_from_this()); } +envoy_engine_t Engine::handle() { return engine_; } + PulseClientSharedPtr Engine::pulseClient() { return std::make_shared(); } void Engine::terminate() { diff --git a/library/cc/engine.h b/library/cc/engine.h index e1c6768eb0..7b7de9f338 100644 --- a/library/cc/engine.h +++ b/library/cc/engine.h @@ -20,6 +20,8 @@ class Engine : public std::enable_shared_from_this { void terminate(); + envoy_engine_t handle(); + private: Engine(envoy_engine_t engine); diff --git a/library/objective-c/EnvoyEngine.h b/library/objective-c/EnvoyEngine.h index 0129cece17..e9e3f34b58 100644 --- a/library/objective-c/EnvoyEngine.h +++ b/library/objective-c/EnvoyEngine.h @@ -462,7 +462,7 @@ extern const int kEnvoyFailure; @param eventTracker Event tracking interface. @param networkMonitoringMode Configure how the engines observe network reachability. */ -- (instancetype)initWithRunningCallback:(nullable void (^)())onEngineRunning +- (instancetype)initWithRunningCallback:(nullable void (^)(NSInteger))onEngineRunning logger:(nullable void (^)(NSString *))logger eventTracker:(nullable void (^)(EnvoyEvent *))eventTracker networkMonitoringMode:(int)networkMonitoringMode; @@ -586,7 +586,8 @@ extern const int kEnvoyFailure; // Concrete implementation of the `EnvoyEngine` interface. @interface EnvoyEngineImpl : NSObject -@property (nonatomic, copy, nullable) void (^onEngineRunning)(); +@property (nonatomic, copy, nullable) void (^onEngineRunning)(NSInteger); +- (NSInteger)handle; @end diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index 3b583fb2a7..5a022519ce 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -18,7 +18,7 @@ static void ios_on_engine_running(void *context) { @autoreleasepool { EnvoyEngineImpl *engineImpl = (__bridge EnvoyEngineImpl *)context; if (engineImpl.onEngineRunning) { - engineImpl.onEngineRunning(); + engineImpl.onEngineRunning(engineImpl.handle); } } } @@ -435,7 +435,11 @@ @implementation EnvoyEngineImpl { EnvoyNetworkMonitor *_networkMonitor; } -- (instancetype)initWithRunningCallback:(nullable void (^)())onEngineRunning +- (NSInteger)handle { + return (NSInteger)_engineHandle; +} + +- (instancetype)initWithRunningCallback:(nullable void (^)(NSInteger))onEngineRunning logger:(nullable void (^)(NSString *))logger eventTracker:(nullable void (^)(EnvoyEvent *))eventTracker networkMonitoringMode:(int)networkMonitoringMode { diff --git a/library/swift/EngineBuilder.swift b/library/swift/EngineBuilder.swift index 7d5615ebbe..5e76542073 100644 --- a/library/swift/EngineBuilder.swift +++ b/library/swift/EngineBuilder.swift @@ -39,7 +39,7 @@ open class EngineBuilder: NSObject { private var appVersion: String = "unspecified" private var appId: String = "unspecified" private var virtualClusters: String = "[]" - private var onEngineRunning: (() -> Void)? + private var onEngineRunning: ((Int) -> Void)? private var logger: ((String) -> Void)? private var eventTracker: (([String: String]) -> Void)? private(set) var monitoringMode: NetworkMonitoringMode = .pathMonitor @@ -394,11 +394,12 @@ open class EngineBuilder: NSObject { /// Set a closure to be called when the engine finishes its async startup and begins running. /// - /// - parameter closure: The closure to be called. + /// - parameter closure: The closure to be called. Passes an engine handle that can be used + /// to interoperate with Envoy's C++ APIs. /// /// - returns: This builder. @discardableResult - public func setOnEngineRunning(closure: @escaping () -> Void) -> Self { + public func setOnEngineRunning(closure: @escaping (Int) -> Void) -> Self { self.onEngineRunning = closure return self } diff --git a/library/swift/mocks/MockEnvoyEngine.swift b/library/swift/mocks/MockEnvoyEngine.swift index 1f910e9ced..bb360ae6c1 100644 --- a/library/swift/mocks/MockEnvoyEngine.swift +++ b/library/swift/mocks/MockEnvoyEngine.swift @@ -3,7 +3,7 @@ import Foundation /// Mock implementation of `EnvoyEngine`. Used internally for testing the bridging layer & mocking. final class MockEnvoyEngine: NSObject { - init(runningCallback onEngineRunning: (() -> Void)? = nil, logger: ((String) -> Void)? = nil, + init(runningCallback onEngineRunning: ((Int) -> Void)? = nil, logger: ((String) -> Void)? = nil, eventTracker: (([String: String]) -> Void)? = nil, networkMonitoringMode: Int32 = 0) {} /// Closure called when `run(withConfig:)` is called. From 32362661380b78186508d7df95da09d0c6f907ed Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 20:21:39 -0400 Subject: [PATCH 02/14] Update Swift tests and examples Signed-off-by: JP Simard --- examples/swift/hello_world/ViewController.swift | 2 +- test/kotlin/integration/SetLoggerTest.kt | 2 +- test/swift/apps/baseline/ViewController.swift | 2 +- test/swift/apps/experimental/ViewController.swift | 2 +- test/swift/integration/EngineApiTest.swift | 4 +++- test/swift/integration/SetLoggerTest.swift | 2 +- test/swift/integration/StatFlushIntegrationTest.swift | 2 +- 7 files changed, 9 insertions(+), 7 deletions(-) diff --git a/examples/swift/hello_world/ViewController.swift b/examples/swift/hello_world/ViewController.swift index 8676c01002..2618478054 100644 --- a/examples/swift/hello_world/ViewController.swift +++ b/examples/swift/hello_world/ViewController.swift @@ -24,7 +24,7 @@ final class ViewController: UITableViewController { .addPlatformFilter(AsyncDemoFilter.init) // swiftlint:disable:next line_length .addNativeFilter(name: "envoy.filters.http.buffer", typedConfig: "{\"@type\":\"type.googleapis.com/envoy.extensions.filters.http.buffer.v3.Buffer\",\"max_request_bytes\":5242880}") - .setOnEngineRunning { NSLog("Envoy async internal setup completed") } + .setOnEngineRunning { _ in NSLog("Envoy async internal setup completed") } .addStringAccessor(name: "demo-accessor", accessor: { return "PlatformString" }) .setEventTracker { NSLog("Envoy event emitted: \($0)") } .build() diff --git a/test/kotlin/integration/SetLoggerTest.kt b/test/kotlin/integration/SetLoggerTest.kt index 4afcd4cfeb..d52584004e 100644 --- a/test/kotlin/integration/SetLoggerTest.kt +++ b/test/kotlin/integration/SetLoggerTest.kt @@ -106,7 +106,7 @@ class SetLoggerTest { } } .addLogLevel(LogLevel.DEBUG) - .setOnEngineRunning { + .setOnEngineRunning { _ in countDownLatch.countDown() } .build() diff --git a/test/swift/apps/baseline/ViewController.swift b/test/swift/apps/baseline/ViewController.swift index 8676c01002..2618478054 100644 --- a/test/swift/apps/baseline/ViewController.swift +++ b/test/swift/apps/baseline/ViewController.swift @@ -24,7 +24,7 @@ final class ViewController: UITableViewController { .addPlatformFilter(AsyncDemoFilter.init) // swiftlint:disable:next line_length .addNativeFilter(name: "envoy.filters.http.buffer", typedConfig: "{\"@type\":\"type.googleapis.com/envoy.extensions.filters.http.buffer.v3.Buffer\",\"max_request_bytes\":5242880}") - .setOnEngineRunning { NSLog("Envoy async internal setup completed") } + .setOnEngineRunning { _ in NSLog("Envoy async internal setup completed") } .addStringAccessor(name: "demo-accessor", accessor: { return "PlatformString" }) .setEventTracker { NSLog("Envoy event emitted: \($0)") } .build() diff --git a/test/swift/apps/experimental/ViewController.swift b/test/swift/apps/experimental/ViewController.swift index 59644895ed..7af1ce9c23 100644 --- a/test/swift/apps/experimental/ViewController.swift +++ b/test/swift/apps/experimental/ViewController.swift @@ -26,7 +26,7 @@ final class ViewController: UITableViewController { .enableInterfaceBinding(true) // swiftlint:disable:next line_length .addNativeFilter(name: "envoy.filters.http.buffer", typedConfig: "{\"@type\":\"type.googleapis.com/envoy.extensions.filters.http.buffer.v3.Buffer\",\"max_request_bytes\":5242880}") - .setOnEngineRunning { NSLog("Envoy async internal setup completed") } + .setOnEngineRunning { _ in NSLog("Envoy async internal setup completed") } .addStringAccessor(name: "demo-accessor", accessor: { return "PlatformString" }) .setEventTracker { NSLog("Envoy event emitted: \($0)") } .build() diff --git a/test/swift/integration/EngineApiTest.swift b/test/swift/integration/EngineApiTest.swift index d81d626213..ffc10bc34f 100644 --- a/test/swift/integration/EngineApiTest.swift +++ b/test/swift/integration/EngineApiTest.swift @@ -9,7 +9,9 @@ final class EngineApiTest: XCTestCase { let engine = EngineBuilder() .addLogLevel(.debug) .addStatsFlushSeconds(1) - .setOnEngineRunning { + .setOnEngineRunning { engineHandle in + // TODO(jpsim): Update this check when the engine singleton is removed + XCTAssertEqual(engineHandle, 1) engineExpectation.fulfill() } .build() diff --git a/test/swift/integration/SetLoggerTest.swift b/test/swift/integration/SetLoggerTest.swift index f9c7aa729b..161ec23f07 100644 --- a/test/swift/integration/SetLoggerTest.swift +++ b/test/swift/integration/SetLoggerTest.swift @@ -69,7 +69,7 @@ static_resources: loggingExpectation.fulfill() } } - .setOnEngineRunning { + .setOnEngineRunning { _ in engineExpectation.fulfill() } .setEventTracker { event in diff --git a/test/swift/integration/StatFlushIntegrationTest.swift b/test/swift/integration/StatFlushIntegrationTest.swift index 519116a4e4..ba579e352c 100644 --- a/test/swift/integration/StatFlushIntegrationTest.swift +++ b/test/swift/integration/StatFlushIntegrationTest.swift @@ -9,7 +9,7 @@ final class StatFlushIntegrationTest: XCTestCase { let engine = EngineBuilder() .addLogLevel(.debug) .addStatsFlushSeconds(1) - .setOnEngineRunning { + .setOnEngineRunning { _ in engineExpectation.fulfill() } .build() From 8d27f39fdef48f50a2e394e0f1b40e7926a3b5e0 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 20:38:14 -0400 Subject: [PATCH 03/14] Update Java/Kotlin APIs Signed-off-by: JP Simard --- .../envoymobile/engine/types/EnvoyOnEngineRunning.java | 2 +- library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt | 4 ++-- test/kotlin/integration/SetLoggerTest.kt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java index adb7e3d885..3b28895618 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java @@ -2,5 +2,5 @@ /* Interface used to support lambdas being passed from Kotlin for engine setup completion. */ public interface EnvoyOnEngineRunning { - Object invokeOnEngineRunning(); + Object invokeOnEngineRunning(Integer engineHandle); } diff --git a/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt b/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt index 2a29f7be0b..41a957b800 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt @@ -21,7 +21,7 @@ class Custom(val yaml: String) : BaseConfiguration() open class EngineBuilder( private val configuration: BaseConfiguration = Standard() ) { - protected var onEngineRunning: (() -> Unit) = {} + protected var onEngineRunning: ((Int) -> Unit) = {} protected var logger: ((String) -> Unit)? = null protected var eventTracker: ((Map) -> Unit)? = null private var engineType: () -> EnvoyEngine = { @@ -435,7 +435,7 @@ open class EngineBuilder( * * @return this builder. */ - fun setOnEngineRunning(closure: () -> Unit): EngineBuilder { + fun setOnEngineRunning(closure: (Int) -> Unit): EngineBuilder { this.onEngineRunning = closure return this } diff --git a/test/kotlin/integration/SetLoggerTest.kt b/test/kotlin/integration/SetLoggerTest.kt index d52584004e..4afcd4cfeb 100644 --- a/test/kotlin/integration/SetLoggerTest.kt +++ b/test/kotlin/integration/SetLoggerTest.kt @@ -106,7 +106,7 @@ class SetLoggerTest { } } .addLogLevel(LogLevel.DEBUG) - .setOnEngineRunning { _ in + .setOnEngineRunning { countDownLatch.countDown() } .build() From e660c05d9706b873094d83287bb74703f1745029 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 20:39:55 -0400 Subject: [PATCH 04/14] Update Objective-C example Signed-off-by: JP Simard --- examples/objective-c/hello_world/ViewController.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/objective-c/hello_world/ViewController.m b/examples/objective-c/hello_world/ViewController.m index 3a989e42cf..44e25276a2 100644 --- a/examples/objective-c/hello_world/ViewController.m +++ b/examples/objective-c/hello_world/ViewController.m @@ -39,7 +39,7 @@ - (void)startEnvoy { NSLog(@"starting Envoy..."); EngineBuilder *builder = [[EngineBuilder alloc] init]; [builder addLogLevel:LogLevelDebug]; - [builder setOnEngineRunningWithClosure:^{ + [builder setOnEngineRunningWithClosure:^(NSInteger){ NSLog(@"Envoy async internal setup completed"); }]; From b93ff82e4c5e7e5936ab8230abd0cb6b008aab58 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 20:49:10 -0400 Subject: [PATCH 05/14] Fix format Signed-off-by: JP Simard --- examples/objective-c/hello_world/ViewController.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/objective-c/hello_world/ViewController.m b/examples/objective-c/hello_world/ViewController.m index 44e25276a2..8fb1964853 100644 --- a/examples/objective-c/hello_world/ViewController.m +++ b/examples/objective-c/hello_world/ViewController.m @@ -39,7 +39,7 @@ - (void)startEnvoy { NSLog(@"starting Envoy..."); EngineBuilder *builder = [[EngineBuilder alloc] init]; [builder addLogLevel:LogLevelDebug]; - [builder setOnEngineRunningWithClosure:^(NSInteger){ + [builder setOnEngineRunningWithClosure:^(NSInteger) { NSLog(@"Envoy async internal setup completed"); }]; From 03c80bb3215e2abd56aabc7087e08466914a4e85 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 20:56:33 -0400 Subject: [PATCH 06/14] Fix JNI handling by moving handle to `envoy_on_engine_running_f` Signed-off-by: JP Simard --- library/cc/engine.cc | 2 -- library/cc/engine.h | 2 -- library/common/engine.cc | 4 +++- library/common/jni/jni_interface.cc | 4 ++-- library/common/types/c_types.h | 2 +- library/objective-c/EnvoyEngine.h | 1 - library/objective-c/EnvoyEngineImpl.m | 8 ++------ 7 files changed, 8 insertions(+), 15 deletions(-) diff --git a/library/cc/engine.cc b/library/cc/engine.cc index 010f742270..fadf2458d6 100644 --- a/library/cc/engine.cc +++ b/library/cc/engine.cc @@ -16,8 +16,6 @@ StreamClientSharedPtr Engine::streamClient() { return std::make_shared(shared_from_this()); } -envoy_engine_t Engine::handle() { return engine_; } - PulseClientSharedPtr Engine::pulseClient() { return std::make_shared(); } void Engine::terminate() { diff --git a/library/cc/engine.h b/library/cc/engine.h index 7b7de9f338..e1c6768eb0 100644 --- a/library/cc/engine.h +++ b/library/cc/engine.h @@ -20,8 +20,6 @@ class Engine : public std::enable_shared_from_this { void terminate(); - envoy_engine_t handle(); - private: Engine(envoy_engine_t engine); diff --git a/library/common/engine.cc b/library/common/engine.cc index 74b9e34743..59ef1d9f22 100644 --- a/library/common/engine.cc +++ b/library/common/engine.cc @@ -122,7 +122,9 @@ envoy_status_t Engine::main(const std::string config, const std::string log_leve server_->api().randomGenerator()); dispatcher_->drain(server_->dispatcher()); if (callbacks_.on_engine_running != nullptr) { - callbacks_.on_engine_running(callbacks_.context); + // TODO(jpsim): Update when singleton is removed + // reinterpret_cast(this) + callbacks_.on_engine_running(1, callbacks_.context); } }); } // mutex_ diff --git a/library/common/jni/jni_interface.cc b/library/common/jni/jni_interface.cc index 50617b90d4..6b514bb740 100644 --- a/library/common/jni/jni_interface.cc +++ b/library/common/jni/jni_interface.cc @@ -26,7 +26,7 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) { // JniLibrary -static void jvm_on_engine_running(void* context) { +static void jvm_on_engine_running(jlong engine_handle, void* context) { if (context == nullptr) { return; } @@ -37,7 +37,7 @@ static void jvm_on_engine_running(void* context) { jclass jcls_JvmonEngineRunningContext = env->GetObjectClass(j_context); jmethodID jmid_onEngineRunning = env->GetMethodID( jcls_JvmonEngineRunningContext, "invokeOnEngineRunning", "()Ljava/lang/Object;"); - env->CallObjectMethod(j_context, jmid_onEngineRunning); + env->CallObjectMethod(j_context, jmid_onEngineRunning, engine_handle); env->DeleteLocalRef(jcls_JvmonEngineRunningContext); // TODO(goaway): This isn't re-used by other engine callbacks, so it's safe to delete here. diff --git a/library/common/types/c_types.h b/library/common/types/c_types.h index 92d45b12df..31a940a11c 100644 --- a/library/common/types/c_types.h +++ b/library/common/types/c_types.h @@ -406,7 +406,7 @@ typedef void (*envoy_on_exit_f)(void* context); * @param context, contains the necessary state to carry out platform-specific dispatch and * execution. */ -typedef void (*envoy_on_engine_running_f)(void* context); +typedef void (*envoy_on_engine_running_f)(envoy_engine_t engine, void* context); /** * Called when envoy's logger logs data. diff --git a/library/objective-c/EnvoyEngine.h b/library/objective-c/EnvoyEngine.h index e9e3f34b58..646ebab701 100644 --- a/library/objective-c/EnvoyEngine.h +++ b/library/objective-c/EnvoyEngine.h @@ -587,7 +587,6 @@ extern const int kEnvoyFailure; @interface EnvoyEngineImpl : NSObject @property (nonatomic, copy, nullable) void (^onEngineRunning)(NSInteger); -- (NSInteger)handle; @end diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index 5a022519ce..708fb4dfcf 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -12,13 +12,13 @@ #import #endif -static void ios_on_engine_running(void *context) { +static void ios_on_engine_running(envoy_engine_t engine, void *context) { // 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 { EnvoyEngineImpl *engineImpl = (__bridge EnvoyEngineImpl *)context; if (engineImpl.onEngineRunning) { - engineImpl.onEngineRunning(engineImpl.handle); + engineImpl.onEngineRunning(engine); } } } @@ -435,10 +435,6 @@ @implementation EnvoyEngineImpl { EnvoyNetworkMonitor *_networkMonitor; } -- (NSInteger)handle { - return (NSInteger)_engineHandle; -} - - (instancetype)initWithRunningCallback:(nullable void (^)(NSInteger))onEngineRunning logger:(nullable void (^)(NSString *))logger eventTracker:(nullable void (^)(EnvoyEvent *))eventTracker From a57cdc5b3e745d630fc166a366e051b396a488d6 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 21:06:13 -0400 Subject: [PATCH 07/14] Update Kotlin docs Signed-off-by: JP Simard --- docs/root/api/starting_envoy.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/root/api/starting_envoy.rst b/docs/root/api/starting_envoy.rst index 4bcfefae51..b11141f34d 100644 --- a/docs/root/api/starting_envoy.rst +++ b/docs/root/api/starting_envoy.rst @@ -319,10 +319,10 @@ desired. **Example**:: // Kotlin - builder.setOnEngineRunning { /*do something*/ } + builder.setOnEngineRunning { engineHandle -> /* do something */ } // Swift - builder.setOnEngineRunning { engineHandle in /*do something*/ } + builder.setOnEngineRunning { engineHandle in /* do something */ } ~~~~~~~~~~~~~ ``setLogger`` From cf8e9dd9221dbcb948506ce03944ef854e432cc4 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 21:10:17 -0400 Subject: [PATCH 08/14] Update more APIs Signed-off-by: JP Simard --- library/cc/engine_builder.cc | 2 +- library/cc/engine_builder.h | 2 +- library/cc/engine_callbacks.cc | 4 ++-- library/cc/engine_callbacks.h | 2 +- .../java/org/chromium/net/impl/CronetUrlRequestContext.java | 2 +- test/cc/integration/lifetimes_test.cc | 2 +- test/cc/integration/send_headers_test.cc | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/cc/engine_builder.cc b/library/cc/engine_builder.cc index 5d9477f645..3903fa2411 100644 --- a/library/cc/engine_builder.cc +++ b/library/cc/engine_builder.cc @@ -18,7 +18,7 @@ EngineBuilder& EngineBuilder::addLogLevel(LogLevel log_level) { return *this; } -EngineBuilder& EngineBuilder::setOnEngineRunning(std::function closure) { +EngineBuilder& EngineBuilder::setOnEngineRunning(std::function closure) { this->callbacks_->on_engine_running = closure; return *this; } diff --git a/library/cc/engine_builder.h b/library/cc/engine_builder.h index f978fff14c..b6341a7862 100644 --- a/library/cc/engine_builder.h +++ b/library/cc/engine_builder.h @@ -16,7 +16,7 @@ class EngineBuilder { EngineBuilder(); EngineBuilder& addLogLevel(LogLevel log_level); - EngineBuilder& setOnEngineRunning(std::function closure); + EngineBuilder& setOnEngineRunning(std::function closure); EngineBuilder& addGrpcStatsDomain(const std::string& stats_domain); EngineBuilder& addConnectTimeoutSeconds(int connect_timeout_seconds); diff --git a/library/cc/engine_callbacks.cc b/library/cc/engine_callbacks.cc index d76bfa616b..19c50d5898 100644 --- a/library/cc/engine_callbacks.cc +++ b/library/cc/engine_callbacks.cc @@ -5,9 +5,9 @@ namespace Platform { namespace { -void c_on_engine_running(void* context) { +void c_on_engine_running(envoy_engine_t engine, void* context) { auto engine_callbacks = *static_cast(context); - engine_callbacks->on_engine_running(); + engine_callbacks->on_engine_running(engine); } void c_on_exit(void* context) { diff --git a/library/cc/engine_callbacks.h b/library/cc/engine_callbacks.h index 61c1d196f4..05ccff47ce 100644 --- a/library/cc/engine_callbacks.h +++ b/library/cc/engine_callbacks.h @@ -10,7 +10,7 @@ namespace Envoy { namespace Platform { struct EngineCallbacks : public std::enable_shared_from_this { - std::function on_engine_running; + std::function on_engine_running; // unused: // std::function on_exit; diff --git a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java index c189541b0e..a6e9b5e72c 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java @@ -88,7 +88,7 @@ public CronetUrlRequestContext(NativeCronetEngineBuilderImpl builder) { android.os.Process.setThreadPriority(threadPriority); mInitCompleted.open(); Runnable taskToExecuteWhenInitializationIsCompleted = - mInitializationCompleter.getAndSet(() -> {}); + mInitializationCompleter.getAndSet((Integer) -> {}); if (taskToExecuteWhenInitializationIsCompleted != null) { taskToExecuteWhenInitializationIsCompleted.run(); } diff --git a/test/cc/integration/lifetimes_test.cc b/test/cc/integration/lifetimes_test.cc index a91aaf069a..64f097228a 100644 --- a/test/cc/integration/lifetimes_test.cc +++ b/test/cc/integration/lifetimes_test.cc @@ -82,7 +82,7 @@ void sendRequestEndToEnd() { absl::Notification engine_running; auto engine_builder = Platform::EngineBuilder(CONFIG_TEMPLATE); engine = engine_builder.addLogLevel(Platform::LogLevel::debug) - .setOnEngineRunning([&]() { engine_running.Notify(); }) + .setOnEngineRunning([&](envoy_engine_t) { engine_running.Notify(); }) .build(); engine_running.WaitForNotification(); diff --git a/test/cc/integration/send_headers_test.cc b/test/cc/integration/send_headers_test.cc index 0d7482654d..b2d3a3e6db 100644 --- a/test/cc/integration/send_headers_test.cc +++ b/test/cc/integration/send_headers_test.cc @@ -60,7 +60,7 @@ TEST(TestSendHeaders, CanSendHeaders) { absl::Notification engine_running; auto engine_builder = Platform::EngineBuilder(CONFIG_TEMPLATE); engine = engine_builder.addLogLevel(Platform::LogLevel::debug) - .setOnEngineRunning([&]() { engine_running.Notify(); }) + .setOnEngineRunning([&](envoy_engine_t) { engine_running.Notify(); }) .build(); engine_running.WaitForNotification(); From ef1dad576de8343ea587b77ee8e69c779ad12201 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 22:03:12 -0400 Subject: [PATCH 09/14] Fix more Kotlin, Java & C++ Signed-off-by: JP Simard --- examples/java/hello_world/MainActivity.java | 2 +- library/common/jni/jni_interface.cc | 4 +- .../engine/types/EnvoyOnEngineRunning.java | 2 +- .../envoyproxy/envoymobile/EngineBuilder.kt | 4 +- test/common/engine_test.cc | 4 +- test/common/main_interface_test.cc | 32 ++++++------- .../AndroidEnvoyExplicitFlowTest.java | 2 +- .../integration/AndroidEnvoyFlowTest.java | 2 +- .../engine/testing/QuicTestServerTest.java | 2 +- .../AndroidEnvoyExplicitH2FlowTest.java | 2 +- .../net/testing/Http2TestServerTest.java | 2 +- .../envoymobile/EngineBuilderTest.kt | 48 +++++++++---------- 12 files changed, 53 insertions(+), 53 deletions(-) diff --git a/examples/java/hello_world/MainActivity.java b/examples/java/hello_world/MainActivity.java index 5d1ebcd4ac..5b0bbc6579 100644 --- a/examples/java/hello_world/MainActivity.java +++ b/examples/java/hello_world/MainActivity.java @@ -60,7 +60,7 @@ protected void onCreate(Bundle savedInstanceState) { engine = new AndroidEngineBuilder(getApplication()) .addLogLevel(LogLevel.DEBUG) - .setOnEngineRunning(() -> { + .setOnEngineRunning((ignored) -> { Log.d("MainActivity", "Envoy async internal setup completed"); return null; }) diff --git a/library/common/jni/jni_interface.cc b/library/common/jni/jni_interface.cc index 6b514bb740..08926f06b9 100644 --- a/library/common/jni/jni_interface.cc +++ b/library/common/jni/jni_interface.cc @@ -26,7 +26,7 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) { // JniLibrary -static void jvm_on_engine_running(jlong engine_handle, void* context) { +static void jvm_on_engine_running(envoy_engine_t engine_handle, void* context) { if (context == nullptr) { return; } @@ -37,7 +37,7 @@ static void jvm_on_engine_running(jlong engine_handle, void* context) { jclass jcls_JvmonEngineRunningContext = env->GetObjectClass(j_context); jmethodID jmid_onEngineRunning = env->GetMethodID( jcls_JvmonEngineRunningContext, "invokeOnEngineRunning", "()Ljava/lang/Object;"); - env->CallObjectMethod(j_context, jmid_onEngineRunning, engine_handle); + env->CallObjectMethod(j_context, jmid_onEngineRunning, static_cast(engine_handle)); env->DeleteLocalRef(jcls_JvmonEngineRunningContext); // TODO(goaway): This isn't re-used by other engine callbacks, so it's safe to delete here. diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java index 3b28895618..645acb71cf 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyOnEngineRunning.java @@ -2,5 +2,5 @@ /* Interface used to support lambdas being passed from Kotlin for engine setup completion. */ public interface EnvoyOnEngineRunning { - Object invokeOnEngineRunning(Integer engineHandle); + Object invokeOnEngineRunning(long engineHandle); } diff --git a/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt b/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt index 41a957b800..13cf74c976 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt @@ -21,7 +21,7 @@ class Custom(val yaml: String) : BaseConfiguration() open class EngineBuilder( private val configuration: BaseConfiguration = Standard() ) { - protected var onEngineRunning: ((Int) -> Unit) = {} + protected var onEngineRunning: ((Long) -> Unit) = {} protected var logger: ((String) -> Unit)? = null protected var eventTracker: ((Map) -> Unit)? = null private var engineType: () -> EnvoyEngine = { @@ -435,7 +435,7 @@ open class EngineBuilder( * * @return this builder. */ - fun setOnEngineRunning(closure: (Int) -> Unit): EngineBuilder { + fun setOnEngineRunning(closure: (Long) -> Unit): EngineBuilder { this.onEngineRunning = closure return this } diff --git a/test/common/engine_test.cc b/test/common/engine_test.cc index 30465e6666..131fa8aec4 100644 --- a/test/common/engine_test.cc +++ b/test/common/engine_test.cc @@ -70,7 +70,7 @@ TEST_F(EngineTest, EarlyExit) { const std::string level = "debug"; engine_test_context test_context{}; - envoy_engine_callbacks callbacks{[](void* context) -> void { + envoy_engine_callbacks callbacks{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -97,7 +97,7 @@ TEST_F(EngineTest, AccessEngineAfterInitialization) { const std::string level = "debug"; engine_test_context test_context{}; - envoy_engine_callbacks callbacks{[](void* context) -> void { + envoy_engine_callbacks callbacks{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); diff --git a/test/common/main_interface_test.cc b/test/common/main_interface_test.cc index c9c22c615d..88bfcddc2b 100644 --- a/test/common/main_interface_test.cc +++ b/test/common/main_interface_test.cc @@ -126,7 +126,7 @@ Http::ResponseHeaderMapPtr toResponseHeaders(envoy_headers headers) { TEST(MainInterfaceTest, BasicStream) { const std::string level = "debug"; engine_test_context engine_cbs_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -189,7 +189,7 @@ TEST(MainInterfaceTest, BasicStream) { TEST(MainInterfaceTest, SendMetadata) { engine_test_context engine_cbs_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -229,7 +229,7 @@ TEST(MainInterfaceTest, SendMetadata) { TEST(MainInterfaceTest, ResetStream) { engine_test_context engine_cbs_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -301,7 +301,7 @@ TEST(MainInterfaceTest, UsingMainInterfaceWithoutARunningEngine) { TEST(MainInterfaceTest, RegisterPlatformApi) { engine_test_context engine_cbs_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -331,7 +331,7 @@ TEST(MainInterfaceTest, InitEngineReturns1) { // TODO(goaway): return new handle once multiple engine support is in place. // https://github.com/envoyproxy/envoy-mobile/issues/332 engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -354,7 +354,7 @@ TEST(MainInterfaceTest, PreferredNetwork) { TEST(EngineTest, RecordCounter) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -376,7 +376,7 @@ TEST(EngineTest, RecordCounter) { TEST(EngineTest, SetGauge) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -400,7 +400,7 @@ TEST(EngineTest, SetGauge) { TEST(EngineTest, AddToGauge) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -424,7 +424,7 @@ TEST(EngineTest, AddToGauge) { TEST(EngineTest, SubFromGauge) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -450,7 +450,7 @@ TEST(EngineTest, SubFromGauge) { TEST(EngineTest, RecordHistogramValue) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); @@ -478,7 +478,7 @@ TEST(EngineTest, RecordHistogramValue) { TEST(EngineTest, Logger) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* test_context = static_cast(context); test_context->on_engine_running.Notify(); @@ -518,7 +518,7 @@ TEST(EngineTest, Logger) { TEST(EngineTest, EventTrackerRegistersDefaultAPI) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* test_context = static_cast(context); test_context->on_engine_running.Notify(); @@ -553,7 +553,7 @@ TEST(EngineTest, EventTrackerRegistersDefaultAPI) { TEST(EngineTest, EventTrackerRegistersAPI) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* test_context = static_cast(context); test_context->on_engine_running.Notify(); @@ -594,7 +594,7 @@ TEST(EngineTest, EventTrackerRegistersAPI) { TEST(EngineTest, EventTrackerRegistersAssertionFailureRecordAction) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* test_context = static_cast(context); test_context->on_engine_running.Notify(); @@ -634,7 +634,7 @@ TEST(EngineTest, EventTrackerRegistersAssertionFailureRecordAction) { TEST(EngineTest, EventTrackerRegistersEnvoyBugRecordAction) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* test_context = static_cast(context); test_context->on_engine_running.Notify(); @@ -674,7 +674,7 @@ TEST(EngineTest, EventTrackerRegistersEnvoyBugRecordAction) { TEST(MainInterfaceTest, ResetConnectivityState) { engine_test_context test_context{}; - envoy_engine_callbacks engine_cbs{[](void* context) -> void { + envoy_engine_callbacks engine_cbs{[](envoy_engine_t, void* context) -> void { auto* engine_running = static_cast(context); engine_running->on_engine_running.Notify(); diff --git a/test/java/integration/AndroidEnvoyExplicitFlowTest.java b/test/java/integration/AndroidEnvoyExplicitFlowTest.java index 09f2e50675..08a8a63bf9 100644 --- a/test/java/integration/AndroidEnvoyExplicitFlowTest.java +++ b/test/java/integration/AndroidEnvoyExplicitFlowTest.java @@ -62,7 +62,7 @@ public void setUpEngine() throws Exception { Context appContext = ApplicationProvider.getApplicationContext(); engine = new AndroidEngineBuilder(appContext) .addLogLevel(LogLevel.OFF) - .setOnEngineRunning(() -> { + .setOnEngineRunning((ignored) -> { latch.countDown(); return null; }) diff --git a/test/java/integration/AndroidEnvoyFlowTest.java b/test/java/integration/AndroidEnvoyFlowTest.java index 7d1bf250c9..c78e3a583f 100644 --- a/test/java/integration/AndroidEnvoyFlowTest.java +++ b/test/java/integration/AndroidEnvoyFlowTest.java @@ -58,7 +58,7 @@ public void setUpEngine() throws Exception { CountDownLatch latch = new CountDownLatch(1); Context appContext = ApplicationProvider.getApplicationContext(); engine = new AndroidEngineBuilder(appContext) - .setOnEngineRunning(() -> { + .setOnEngineRunning((ignored) -> { latch.countDown(); return null; }) diff --git a/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java index 1c2d630186..4029d1f7cc 100644 --- a/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java +++ b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java @@ -110,7 +110,7 @@ public void setUpEngine() throws Exception { engine = new AndroidEngineBuilder( appContext, new Custom(String.format(CONFIG, QuicTestServer.getServerPort()))) .addLogLevel(LogLevel.WARN) - .setOnEngineRunning(() -> { + .setOnEngineRunning((ignored) -> { latch.countDown(); return null; }) diff --git a/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java b/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java index 1582cabbab..6eb31f4576 100644 --- a/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java +++ b/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java @@ -45,7 +45,7 @@ public void setUpEngine() throws Exception { engine = new AndroidEngineBuilder(appContext) .setTrustChainVerification(ACCEPT_UNTRUSTED) .addLogLevel(LogLevel.DEBUG) - .setOnEngineRunning(() -> { + .setOnEngineRunning((ignored) -> { latch.countDown(); return null; }) diff --git a/test/java/org/chromium/net/testing/Http2TestServerTest.java b/test/java/org/chromium/net/testing/Http2TestServerTest.java index 2ed95a0773..c19a39b27d 100644 --- a/test/java/org/chromium/net/testing/Http2TestServerTest.java +++ b/test/java/org/chromium/net/testing/Http2TestServerTest.java @@ -49,7 +49,7 @@ public void setUpEngine() throws Exception { Context appContext = ApplicationProvider.getApplicationContext(); engine = new AndroidEngineBuilder(appContext) .setTrustChainVerification(ACCEPT_UNTRUSTED) - .setOnEngineRunning(() -> { + .setOnEngineRunning((ignored) -> { latch.countDown(); return null; }) diff --git a/test/kotlin/io/envoyproxy/envoymobile/EngineBuilderTest.kt b/test/kotlin/io/envoyproxy/envoymobile/EngineBuilderTest.kt index 56ba5af789..f05e6425fd 100644 --- a/test/kotlin/io/envoyproxy/envoymobile/EngineBuilderTest.kt +++ b/test/kotlin/io/envoyproxy/envoymobile/EngineBuilderTest.kt @@ -26,7 +26,7 @@ class EngineBuilderTest { engineBuilder.addGrpcStatsDomain("stats.envoyproxy.io") val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.grpcStatsDomain).isEqualTo("stats.envoyproxy.io") + assertThat(engine.envoyConfiguration.grpcStatsDomain).isEqualTo("stats.envoyproxy.io") } @Test @@ -36,7 +36,7 @@ class EngineBuilderTest { engineBuilder.enableAdminInterface() val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.adminInterfaceEnabled).isTrue() + assertThat(engine.envoyConfiguration.adminInterfaceEnabled).isTrue() } @Test @@ -46,7 +46,7 @@ class EngineBuilderTest { engineBuilder.enableHappyEyeballs(true) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.enableHappyEyeballs).isTrue() + assertThat(engine.envoyConfiguration.enableHappyEyeballs).isTrue() } @Test @@ -56,7 +56,7 @@ class EngineBuilderTest { engineBuilder.enableInterfaceBinding(true) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.enableInterfaceBinding).isTrue() + assertThat(engine.envoyConfiguration.enableInterfaceBinding).isTrue() } @Test @@ -66,7 +66,7 @@ class EngineBuilderTest { engineBuilder.addConnectTimeoutSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.connectTimeoutSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.connectTimeoutSeconds).isEqualTo(1234) } @Test @@ -76,7 +76,7 @@ class EngineBuilderTest { engineBuilder.addDNSMinRefreshSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.dnsMinRefreshSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.dnsMinRefreshSeconds).isEqualTo(1234) } @Test @@ -86,7 +86,7 @@ class EngineBuilderTest { engineBuilder.addDNSRefreshSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.dnsRefreshSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.dnsRefreshSeconds).isEqualTo(1234) } @Test @@ -96,8 +96,8 @@ class EngineBuilderTest { engineBuilder.addDNSFailureRefreshSeconds(1234, 5678) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.dnsFailureRefreshSecondsBase).isEqualTo(1234) - assertThat(engine.envoyConfiguration!!.dnsFailureRefreshSecondsMax).isEqualTo(5678) + assertThat(engine.envoyConfiguration.dnsFailureRefreshSecondsBase).isEqualTo(1234) + assertThat(engine.envoyConfiguration.dnsFailureRefreshSecondsMax).isEqualTo(5678) } @Test @@ -107,7 +107,7 @@ class EngineBuilderTest { engineBuilder.addDNSQueryTimeoutSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.dnsQueryTimeoutSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.dnsQueryTimeoutSeconds).isEqualTo(1234) } @Test @@ -117,7 +117,7 @@ class EngineBuilderTest { engineBuilder.addDNSFallbackNameservers(listOf("8.8.8.8")) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.dnsFallbackNameservers.size).isEqualTo(1) + assertThat(engine.envoyConfiguration.dnsFallbackNameservers.size).isEqualTo(1) } @Test @@ -127,7 +127,7 @@ class EngineBuilderTest { engineBuilder.enableDNSFilterUnroutableFamilies(true) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.dnsFilterUnroutableFamilies).isTrue() + assertThat(engine.envoyConfiguration.dnsFilterUnroutableFamilies).isTrue() } @Test @@ -137,7 +137,7 @@ class EngineBuilderTest { engineBuilder.addH2ConnectionKeepaliveIdleIntervalMilliseconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.h2ConnectionKeepaliveIdleIntervalMilliseconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.h2ConnectionKeepaliveIdleIntervalMilliseconds).isEqualTo(1234) } @Test @@ -147,7 +147,7 @@ class EngineBuilderTest { engineBuilder.addH2ConnectionKeepaliveTimeoutSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.h2ConnectionKeepaliveTimeoutSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.h2ConnectionKeepaliveTimeoutSeconds).isEqualTo(1234) } @Test @@ -157,7 +157,7 @@ class EngineBuilderTest { engineBuilder.setMaxConnectionsPerHost(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.maxConnectionsPerHost).isEqualTo(1234) + assertThat(engine.envoyConfiguration.maxConnectionsPerHost).isEqualTo(1234) } @Test @@ -167,7 +167,7 @@ class EngineBuilderTest { engineBuilder.h2ExtendKeepaliveTimeout(true) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.h2ExtendKeepaliveTimeout).isTrue() + assertThat(engine.envoyConfiguration.h2ExtendKeepaliveTimeout).isTrue() } @Test @@ -177,7 +177,7 @@ class EngineBuilderTest { engineBuilder.addH2RawDomains(listOf("h2-raw.domain")) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.h2RawDomains.size).isEqualTo(1) + assertThat(engine.envoyConfiguration.h2RawDomains.size).isEqualTo(1) } @Test @@ -187,7 +187,7 @@ class EngineBuilderTest { engineBuilder.addStatsFlushSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.statsFlushSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.statsFlushSeconds).isEqualTo(1234) } @Test @@ -197,7 +197,7 @@ class EngineBuilderTest { engineBuilder.addStreamIdleTimeoutSeconds(1234) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.streamIdleTimeoutSeconds).isEqualTo(1234) + assertThat(engine.envoyConfiguration.streamIdleTimeoutSeconds).isEqualTo(1234) } @Test @@ -207,7 +207,7 @@ class EngineBuilderTest { engineBuilder.addPerTryIdleTimeoutSeconds(5678) val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.perTryIdleTimeoutSeconds).isEqualTo(5678) + assertThat(engine.envoyConfiguration.perTryIdleTimeoutSeconds).isEqualTo(5678) } @Test @@ -217,7 +217,7 @@ class EngineBuilderTest { engineBuilder.addAppVersion("v1.2.3") val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.appVersion).isEqualTo("v1.2.3") + assertThat(engine.envoyConfiguration.appVersion).isEqualTo("v1.2.3") } @Test @@ -227,7 +227,7 @@ class EngineBuilderTest { engineBuilder.addAppId("com.envoymobile.android") val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.appId).isEqualTo("com.envoymobile.android") + assertThat(engine.envoyConfiguration.appId).isEqualTo("com.envoymobile.android") } @Test @@ -237,7 +237,7 @@ class EngineBuilderTest { engineBuilder.addVirtualClusters("[test]") val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.virtualClusters).isEqualTo("[test]") + assertThat(engine.envoyConfiguration.virtualClusters).isEqualTo("[test]") } @Test @@ -247,6 +247,6 @@ class EngineBuilderTest { engineBuilder.addNativeFilter("name", "config") val engine = engineBuilder.build() as EngineImpl - assertThat(engine.envoyConfiguration!!.nativeFilterChain.size).isEqualTo(1) + assertThat(engine.envoyConfiguration.nativeFilterChain.size).isEqualTo(1) } } From aaa9380801302ef7787c96b27e2fb4ef5d3b05bb Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 22:17:03 -0400 Subject: [PATCH 10/14] Fix more Python, Java & C++ Signed-off-by: JP Simard --- .../java/org/chromium/net/impl/CronetUrlRequestContext.java | 2 +- library/python/engine_builder_shim.cc | 6 +++--- library/python/engine_builder_shim.h | 2 +- library/python/envoy_engine.pyi | 2 +- library/python/envoy_requests/common/engine.py | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java index a6e9b5e72c..6ceddf9738 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java @@ -88,7 +88,7 @@ public CronetUrlRequestContext(NativeCronetEngineBuilderImpl builder) { android.os.Process.setThreadPriority(threadPriority); mInitCompleted.open(); Runnable taskToExecuteWhenInitializationIsCompleted = - mInitializationCompleter.getAndSet((Integer) -> {}); + mInitializationCompleter.getAndSet((Long) -> {}); if (taskToExecuteWhenInitializationIsCompleted != null) { taskToExecuteWhenInitializationIsCompleted.run(); } diff --git a/library/python/engine_builder_shim.cc b/library/python/engine_builder_shim.cc index 2966e59d7f..43c0cca88f 100644 --- a/library/python/engine_builder_shim.cc +++ b/library/python/engine_builder_shim.cc @@ -7,10 +7,10 @@ namespace Python { namespace EngineBuilder { Platform::EngineBuilder& setOnEngineRunningShim(Platform::EngineBuilder& self, - std::function closure) { - return self.setOnEngineRunning([closure]() { + std::function closure) { + return self.setOnEngineRunning([closure](envoy_engine_t engine) { py::gil_scoped_acquire acquire; - closure(); + closure(engine); }); } diff --git a/library/python/engine_builder_shim.h b/library/python/engine_builder_shim.h index 5a2a8fab88..123ba5688f 100644 --- a/library/python/engine_builder_shim.h +++ b/library/python/engine_builder_shim.h @@ -10,7 +10,7 @@ namespace Python { namespace EngineBuilder { Platform::EngineBuilder& setOnEngineRunningShim(Platform::EngineBuilder& self, - std::function closure); + std::function closure); } // namespace EngineBuilder } // namespace Python diff --git a/library/python/envoy_engine.pyi b/library/python/envoy_engine.pyi index 092e17b552..7084e3393c 100644 --- a/library/python/envoy_engine.pyi +++ b/library/python/envoy_engine.pyi @@ -11,7 +11,7 @@ class EngineBuilder: @overload def __init__(self): ... def add_log_level(self, log_level: "LogLevel") -> "EngineBuilder": ... - def set_on_engine_running(self, on_engine_running: Callable[[], None]) -> "EngineBuilder": ... + def set_on_engine_running(self, on_engine_running: Callable[[int], None]) -> "EngineBuilder": ... def add_stats_domain(self, stats_domain: str) -> "EngineBuilder": ... def add_connect_timeout_seconds(self, connect_timeout_seconds: int) -> "EngineBuilder": ... def add_dns_refresh_seconds(self, dns_refresh_seconds: int) -> "EngineBuilder": ... diff --git a/library/python/envoy_requests/common/engine.py b/library/python/envoy_requests/common/engine.py index dc34647ba8..95d5def1ad 100644 --- a/library/python/envoy_requests/common/engine.py +++ b/library/python/envoy_requests/common/engine.py @@ -15,7 +15,7 @@ class Engine: _handle: Optional[EnvoyEngine] = None @classmethod - def build(cls, executor: Executor, set_engine_running: Callable[[], None]): + def build(cls, executor: Executor, set_engine_running: Callable[[int], None]): if cls.config_template_override is None: engine_builder = EngineBuilder() else: @@ -29,7 +29,7 @@ def build(cls, executor: Executor, set_engine_running: Callable[[], None]): atexit.register(lambda: cls._handle.terminate()) @classmethod - def handle(cls, executor: Executor, set_engine_running: Callable[[], None]) -> EnvoyEngine: + def handle(cls, executor: Executor, set_engine_running: Callable[[int], None]) -> EnvoyEngine: if cls._handle is None: cls.build(executor, set_engine_running) else: From 2b5bcff1baf3a282c068b385acc6252362c25bde Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 22:36:08 -0400 Subject: [PATCH 11/14] Fix Cronet Signed-off-by: JP Simard --- .../java/org/chromium/net/impl/CronetUrlRequestContext.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java index 6ceddf9738..2aa89f1be6 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java @@ -83,12 +83,12 @@ public CronetUrlRequestContext(NativeCronetEngineBuilderImpl builder) { builder.threadPriority(THREAD_PRIORITY_BACKGROUND + THREAD_PRIORITY_MORE_FAVORABLE); mUserAgent = builder.getUserAgent(); synchronized (mLock) { - mEngine = builder.createEngine(() -> { + mEngine = builder.createEngine((ignored) -> { mNetworkThread = Thread.currentThread(); android.os.Process.setThreadPriority(threadPriority); mInitCompleted.open(); Runnable taskToExecuteWhenInitializationIsCompleted = - mInitializationCompleter.getAndSet((Long) -> {}); + mInitializationCompleter.getAndSet(() -> {}); if (taskToExecuteWhenInitializationIsCompleted != null) { taskToExecuteWhenInitializationIsCompleted.run(); } From 0a5a45c56381be350cd961e9174837c3e6039161 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 23:01:49 -0400 Subject: [PATCH 12/14] Fix JNI Signed-off-by: JP Simard --- library/common/jni/jni_interface.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/common/jni/jni_interface.cc b/library/common/jni/jni_interface.cc index 08926f06b9..bee516a742 100644 --- a/library/common/jni/jni_interface.cc +++ b/library/common/jni/jni_interface.cc @@ -36,7 +36,7 @@ static void jvm_on_engine_running(envoy_engine_t engine_handle, void* context) { jobject j_context = static_cast(context); jclass jcls_JvmonEngineRunningContext = env->GetObjectClass(j_context); jmethodID jmid_onEngineRunning = env->GetMethodID( - jcls_JvmonEngineRunningContext, "invokeOnEngineRunning", "()Ljava/lang/Object;"); + jcls_JvmonEngineRunningContext, "invokeOnEngineRunning", "(J)Ljava/lang/Object;"); env->CallObjectMethod(j_context, jmid_onEngineRunning, static_cast(engine_handle)); env->DeleteLocalRef(jcls_JvmonEngineRunningContext); From 74722e7f00769e65c459a12b6bda447853c0aaf1 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 23:32:12 -0400 Subject: [PATCH 13/14] Fix Python Signed-off-by: JP Simard --- library/python/gevent_util/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/python/gevent_util/__init__.py b/library/python/gevent_util/__init__.py index f062933f31..9c3abb0a50 100644 --- a/library/python/gevent_util/__init__.py +++ b/library/python/gevent_util/__init__.py @@ -43,7 +43,7 @@ def __init__(self): super().__init__() self.executor = GeventExecutor() - def set_on_engine_running(self, closure: Callable[[], None]) -> EngineBuilder: + def set_on_engine_running(self, closure: Callable[[int], None]) -> EngineBuilder: super().set_on_engine_running(self.executor(closure)) return self From 6271ad14024721115ccd1cb9ae69313044b861a5 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 14 Jun 2022 23:40:17 -0400 Subject: [PATCH 14/14] Revert Python changes I can't get the tests to run locally and they fail on CI. Punting on this for now. Signed-off-by: JP Simard --- library/python/engine_builder_shim.cc | 7 ++++--- library/python/engine_builder_shim.h | 2 +- library/python/envoy_engine.pyi | 2 +- library/python/envoy_requests/common/engine.py | 4 ++-- library/python/gevent_util/__init__.py | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/library/python/engine_builder_shim.cc b/library/python/engine_builder_shim.cc index 43c0cca88f..fe768205b2 100644 --- a/library/python/engine_builder_shim.cc +++ b/library/python/engine_builder_shim.cc @@ -7,10 +7,11 @@ namespace Python { namespace EngineBuilder { Platform::EngineBuilder& setOnEngineRunningShim(Platform::EngineBuilder& self, - std::function closure) { - return self.setOnEngineRunning([closure](envoy_engine_t engine) { + std::function closure) { + // TODO(jpsim): Expose engine handle in the Python API + return self.setOnEngineRunning([closure](envoy_engine_t) { py::gil_scoped_acquire acquire; - closure(engine); + closure(); }); } diff --git a/library/python/engine_builder_shim.h b/library/python/engine_builder_shim.h index 123ba5688f..5a2a8fab88 100644 --- a/library/python/engine_builder_shim.h +++ b/library/python/engine_builder_shim.h @@ -10,7 +10,7 @@ namespace Python { namespace EngineBuilder { Platform::EngineBuilder& setOnEngineRunningShim(Platform::EngineBuilder& self, - std::function closure); + std::function closure); } // namespace EngineBuilder } // namespace Python diff --git a/library/python/envoy_engine.pyi b/library/python/envoy_engine.pyi index 7084e3393c..092e17b552 100644 --- a/library/python/envoy_engine.pyi +++ b/library/python/envoy_engine.pyi @@ -11,7 +11,7 @@ class EngineBuilder: @overload def __init__(self): ... def add_log_level(self, log_level: "LogLevel") -> "EngineBuilder": ... - def set_on_engine_running(self, on_engine_running: Callable[[int], None]) -> "EngineBuilder": ... + def set_on_engine_running(self, on_engine_running: Callable[[], None]) -> "EngineBuilder": ... def add_stats_domain(self, stats_domain: str) -> "EngineBuilder": ... def add_connect_timeout_seconds(self, connect_timeout_seconds: int) -> "EngineBuilder": ... def add_dns_refresh_seconds(self, dns_refresh_seconds: int) -> "EngineBuilder": ... diff --git a/library/python/envoy_requests/common/engine.py b/library/python/envoy_requests/common/engine.py index 95d5def1ad..dc34647ba8 100644 --- a/library/python/envoy_requests/common/engine.py +++ b/library/python/envoy_requests/common/engine.py @@ -15,7 +15,7 @@ class Engine: _handle: Optional[EnvoyEngine] = None @classmethod - def build(cls, executor: Executor, set_engine_running: Callable[[int], None]): + def build(cls, executor: Executor, set_engine_running: Callable[[], None]): if cls.config_template_override is None: engine_builder = EngineBuilder() else: @@ -29,7 +29,7 @@ def build(cls, executor: Executor, set_engine_running: Callable[[int], None]): atexit.register(lambda: cls._handle.terminate()) @classmethod - def handle(cls, executor: Executor, set_engine_running: Callable[[int], None]) -> EnvoyEngine: + def handle(cls, executor: Executor, set_engine_running: Callable[[], None]) -> EnvoyEngine: if cls._handle is None: cls.build(executor, set_engine_running) else: diff --git a/library/python/gevent_util/__init__.py b/library/python/gevent_util/__init__.py index 9c3abb0a50..f062933f31 100644 --- a/library/python/gevent_util/__init__.py +++ b/library/python/gevent_util/__init__.py @@ -43,7 +43,7 @@ def __init__(self): super().__init__() self.executor = GeventExecutor() - def set_on_engine_running(self, closure: Callable[[int], None]) -> EngineBuilder: + def set_on_engine_running(self, closure: Callable[[], None]) -> EngineBuilder: super().set_on_engine_running(self.executor(closure)) return self