From 4a3de26a886684c1a56e104b7f8b63fee80e04e7 Mon Sep 17 00:00:00 2001 From: John Knox Date: Mon, 11 Feb 2019 14:01:31 -0800 Subject: [PATCH] Add requestResponse handler for incoming calls Summary: Flipper exposes a call() api to plugins which lets them call their sdk component, and it returns a promise with the response. Currently this is done by sending a fireAndForget request, noting the id of the request, and then receiving fireAndForget requests and matching up the ids to give the result back to the right plugin promise. Instead, it will be simpler to use rsocket requestResponse, instead of fireAndForget, which is for this exact use case. This diff adds a requestResponse handler to the SDK, so that it can deal with such requests and respond accordingly, while preserving the current functionality if it receives a fireAndForget. So this part is backwards compatible and should be safe to land in isolation. A later diff will change the desktop app to use requestResponse, which may not be backwards compatible, so that will have to be deployed more carefully. Reviewed By: passy Differential Revision: D13974049 fbshipit-source-id: b371d94a86b1f186375161ed8f2242a462ce418f --- android/src/main/cpp/sonar.cpp | 4 +- .../CppBridge/FlipperCppBridgingConnection.mm | 4 +- .../CppBridge/FlipperCppBridgingResponder.h | 2 +- .../CppBridge/FlipperCppBridgingResponder.mm | 6 +- iOS/FlipperKitTests/FlipperClientTests.mm | 43 +- .../FireAndForgetBasedFlipperResponder.h | 48 +++ xplat/Flipper/FlipperClient.cpp | 63 ++- xplat/Flipper/FlipperClient.h | 4 +- xplat/Flipper/FlipperConnection.h | 2 +- xplat/Flipper/FlipperConnectionImpl.h | 24 +- xplat/Flipper/FlipperConnectionManager.h | 15 +- .../Flipper/FlipperConnectionManagerImpl.cpp | 96 ++++- xplat/Flipper/FlipperResponderImpl.h | 38 +- xplat/FlipperTests/FlipperClientTests.cpp | 368 +++++++++--------- .../FlipperResponderImplTests.cpp | 53 +++ 15 files changed, 486 insertions(+), 284 deletions(-) create mode 100644 xplat/Flipper/FireAndForgetBasedFlipperResponder.h create mode 100644 xplat/FlipperTests/FlipperResponderImplTests.cpp diff --git a/android/src/main/cpp/sonar.cpp b/android/src/main/cpp/sonar.cpp index 5e31678f0..018197045 100644 --- a/android/src/main/cpp/sonar.cpp +++ b/android/src/main/cpp/sonar.cpp @@ -174,8 +174,8 @@ class JFlipperConnectionImpl : public jni::HybridClass receiver) { auto global = make_global(receiver); - _connection->receive(std::move(method), [global] (const folly::dynamic& params, std::unique_ptr responder) { - global->receive(params, std::move(responder)); + _connection->receive(std::move(method), [global] (const folly::dynamic& params, std::shared_ptr responder) { + global->receive(params, responder); }); } diff --git a/iOS/FlipperKit/CppBridge/FlipperCppBridgingConnection.mm b/iOS/FlipperKit/CppBridge/FlipperCppBridgingConnection.mm index ab236f69c..16f939cfe 100644 --- a/iOS/FlipperKit/CppBridge/FlipperCppBridgingConnection.mm +++ b/iOS/FlipperKit/CppBridge/FlipperCppBridgingConnection.mm @@ -34,10 +34,10 @@ - (void)receive:(NSString *)method withBlock:(SonarReceiver)receiver { const auto lambda = [receiver](const folly::dynamic &message, - std::unique_ptr responder) { + std::shared_ptr responder) { @autoreleasepool { FlipperCppBridgingResponder *const objCResponder = - [[FlipperCppBridgingResponder alloc] initWithCppResponder:std::move(responder)]; + [[FlipperCppBridgingResponder alloc] initWithCppResponder:responder]; receiver(facebook::cxxutils::convertFollyDynamicToId(message), objCResponder); } }; diff --git a/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.h b/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.h index 2039a1d74..eba546481 100644 --- a/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.h +++ b/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.h @@ -14,5 +14,5 @@ that forwards messages to the underlying C++ responder. This class allows pure Objective-C plugins to send messages to the underlying responder. */ @interface FlipperCppBridgingResponder : NSObject -- (instancetype)initWithCppResponder:(std::unique_ptr)responder; +- (instancetype)initWithCppResponder:(std::shared_ptr)responder; @end diff --git a/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.mm b/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.mm index 9df9bbf5e..539887bbf 100644 --- a/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.mm +++ b/iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.mm @@ -10,17 +10,17 @@ #import @implementation FlipperCppBridgingResponder { - std::unique_ptr responder_; + std::shared_ptr responder_; } -- (instancetype)initWithCppResponder:(std::unique_ptr)responder +- (instancetype)initWithCppResponder:(std::shared_ptr)responder { if (!responder) { return nil; } if (self = [super init]) { - responder_ = std::move(responder); + responder_ = responder; } return self; diff --git a/iOS/FlipperKitTests/FlipperClientTests.mm b/iOS/FlipperKitTests/FlipperClientTests.mm index 3cb52c613..da17f9899 100644 --- a/iOS/FlipperKitTests/FlipperClientTests.mm +++ b/iOS/FlipperKitTests/FlipperClientTests.mm @@ -14,9 +14,11 @@ #import #import #import +#import #import #import #import +#import @interface FlipperClientTests : XCTestCase @@ -34,7 +36,7 @@ FlipperClient *objcClient; client = new facebook::flipper::FlipperClient(std::unique_ptr{socket}, state); objcClient = [[FlipperClient alloc] initWithCppClient:client]; - + } - (void)tearDown { @@ -43,13 +45,13 @@ FlipperClient *objcClient; } - (void)testGetPlugin { - + BlockBasedSonarPlugin *cat = [[BlockBasedSonarPlugin alloc] initIdentifier:@"cat" connect:nil disconnect:nil]; BlockBasedSonarPlugin *dog = [[BlockBasedSonarPlugin alloc] initIdentifier:@"dog" connect:nil disconnect:nil]; - + [objcClient addPlugin:cat]; [objcClient addPlugin:dog]; - + NSObject *retrievedPlugin = [objcClient pluginWithIdentifier:@"cat"]; XCTAssertEqual(retrievedPlugin, cat); retrievedPlugin = [objcClient pluginWithIdentifier:@"dog"]; @@ -64,9 +66,15 @@ FlipperClient *objcClient; [objcClient removePlugin:cat]; folly::dynamic message = folly::dynamic::object("id", 1)("method", "getPlugins"); - socket->callbacks->onMessageReceived(message); - folly::dynamic expected = folly::dynamic::object("id", 1)("success", folly::dynamic::object("plugins", folly::dynamic::array())); - XCTAssertEqual(socket->messages.back(), expected); + + std::vector successes = std::vector(); + std::vector errors = std::vector(); + std::unique_ptr responder = std::make_unique(&successes, &errors); + socket->callbacks->onMessageReceived(message, std::move(responder)); + folly::dynamic expected = folly::dynamic::object("plugins", folly::dynamic::array()); + XCTAssertEqual(successes.size(), 1); + XCTAssertEqual(errors.size(), 0); + XCTAssertEqual(successes[0], expected); } - (void) testPluginActivatedInBackgroundMode { @@ -109,7 +117,9 @@ FlipperClient *objcClient; [objcClient start]; folly::dynamic messageInit = folly::dynamic::object("method", "init")("params", folly::dynamic::object("plugin", "cat")); - socket->callbacks->onMessageReceived(messageInit); + std::unique_ptr responder = std::make_unique(); + + socket->callbacks->onMessageReceived(messageInit, std::move(responder)); XCTAssertTrue(pluginConnected); [objcClient stop]; XCTAssertFalse(pluginConnected); @@ -173,9 +183,12 @@ FlipperClient *objcClient; [objcClient start]; folly::dynamic messageInit = folly::dynamic::object("method", "init")("params", folly::dynamic::object("plugin", "PluginIdentifier")); - socket->callbacks->onMessageReceived(messageInit); + std::unique_ptr responder1 = std::make_unique(); + socket->callbacks->onMessageReceived(messageInit, std::move(responder1)); folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")("params", folly::dynamic::object("api", "PluginIdentifier")("method", "MethodName")); - socket->callbacks->onMessageReceived(message); + std::unique_ptr responder2 = std::make_unique(); + + socket->callbacks->onMessageReceived(message, std::move(responder2)); XCTAssertTrue(isCalled); } @@ -194,7 +207,8 @@ FlipperClient *objcClient; [objcClient start]; folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")("params", folly::dynamic::object("api", "PluginIdentifier")("method", "MethodName")); - socket->callbacks->onMessageReceived(message); + std::unique_ptr responder = std::make_unique(); + socket->callbacks->onMessageReceived(message, std::move(responder)); XCTAssertTrue(isCalled); } @@ -215,9 +229,14 @@ FlipperClient *objcClient; [objcClient start]; folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")("params", folly::dynamic::object("api", "PluginIdentifier")("method", "MethodName")); + std::vector successes = std::vector(); + std::vector errors = std::vector(); + std::unique_ptr responder = std::make_unique(&successes, &errors); - XCTAssertNoThrow(socket->callbacks->onMessageReceived(message)); // This will call + XCTAssertNoThrow(socket->callbacks->onMessageReceived(message, std::move(responder))); XCTAssertTrue(isCalled); + XCTAssertEqual(successes.size(), 0); + XCTAssertEqual(errors.size(), 1); } @end diff --git a/xplat/Flipper/FireAndForgetBasedFlipperResponder.h b/xplat/Flipper/FireAndForgetBasedFlipperResponder.h new file mode 100644 index 000000000..d2a599a51 --- /dev/null +++ b/xplat/Flipper/FireAndForgetBasedFlipperResponder.h @@ -0,0 +1,48 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. + */ +#pragma once + +#include "FlipperResponder.h" +#include "FlipperConnectionManager.h" +#include + +namespace facebook { +namespace flipper { + +/* Responder for responding to legacy flipper applications. + Originally, flipper desktop used fireAndForget for all messages, so calling + the SDK would send a fire and forget message, to which the SDK would respond + with another one, with an id field that flipper uses to map it to the + original request. This Responder should be used when such requests are + received. + */ +class FireAndForgetBasedFlipperResponder : public FlipperResponder { + public: + FireAndForgetBasedFlipperResponder( + FlipperConnectionManager* socket, + int64_t responseID) + : socket_(socket), responseID_(responseID) {} + + void success(const folly::dynamic& response) const override { + const folly::dynamic message = + folly::dynamic::object("id", responseID_)("success", response); + socket_->sendMessage(message); + } + + void error(const folly::dynamic& response) const override { + const folly::dynamic message = + folly::dynamic::object("id", responseID_)("error", response); + socket_->sendMessage(message); + } + + private: + FlipperConnectionManager* socket_; + int64_t responseID_; +}; + +} // namespace flipper +} // namespace facebook diff --git a/xplat/Flipper/FlipperClient.cpp b/xplat/Flipper/FlipperClient.cpp index aec308f8a..d2c00f536 100644 --- a/xplat/Flipper/FlipperClient.cpp +++ b/xplat/Flipper/FlipperClient.cpp @@ -10,6 +10,7 @@ #include #include #include "ConnectionContextStore.h" +#include "FireAndForgetBasedFlipperResponder.h" #include "FlipperConnectionImpl.h" #include "FlipperConnectionManagerImpl.h" #include "FlipperResponderImpl.h" @@ -154,18 +155,17 @@ void FlipperClient::onDisconnected() { }); } -void FlipperClient::onMessageReceived(const dynamic& message) { - performAndReportError([this, &message]() { +void FlipperClient::onMessageReceived( + const dynamic& message, + std::unique_ptr uniqueResponder) { + // Convert to shared pointer so we can hold on to it while passing it to the plugin, and still use it + // to respond with an error if we catch an exception. + std::shared_ptr responder = std::move(uniqueResponder); + try { std::lock_guard lock(mutex_); const auto& method = message["method"]; const auto& params = message.getDefault("params"); - std::unique_ptr responder; - if (message.find("id") != message.items().end()) { - responder.reset( - new FlipperResponderImpl(socket_.get(), message["id"].getInt())); - } - if (method == "getPlugins") { dynamic identifiers = dynamic::array(); for (const auto& elem : plugins_) { @@ -179,9 +179,12 @@ void FlipperClient::onMessageReceived(const dynamic& message) { if (method == "init") { const auto identifier = params["plugin"].getString(); if (plugins_.find(identifier) == plugins_.end()) { - throw std::out_of_range( - "plugin " + identifier + " not found for method " + - method.getString()); + std::string errorMessage = "Plugin " + identifier + + " not found for method " + method.getString(); + log(errorMessage); + responder->error(folly::dynamic::object("message", errorMessage)( + "name", "PluginNotFound")); + return; } const auto plugin = plugins_.at(identifier); if (!plugin.get()->runInBackground()) { @@ -196,9 +199,12 @@ void FlipperClient::onMessageReceived(const dynamic& message) { if (method == "deinit") { const auto identifier = params["plugin"].getString(); if (plugins_.find(identifier) == plugins_.end()) { - throw std::out_of_range( - "plugin " + identifier + " not found for method " + - method.getString()); + std::string errorMessage = "Plugin " + identifier + + " not found for method " + method.getString(); + log(errorMessage); + responder->error(folly::dynamic::object("message", errorMessage)( + "name", "PluginNotFound")); + return; } const auto plugin = plugins_.at(identifier); if (!plugin.get()->runInBackground()) { @@ -210,23 +216,42 @@ void FlipperClient::onMessageReceived(const dynamic& message) { if (method == "execute") { const auto identifier = params["api"].getString(); if (connections_.find(identifier) == connections_.end()) { - throw std::out_of_range( - "connection " + identifier + " not found for method " + - method.getString()); + std::string errorMessage = "Connection " + identifier + + " not found for method " + method.getString(); + log(errorMessage); + responder->error(folly::dynamic::object("message", errorMessage)( + "name", "ConnectionNotFound")); + return; } const auto& conn = connections_.at(params["api"].getString()); conn->call( params["method"].getString(), params.getDefault("params"), - std::move(responder)); + responder); return; } dynamic response = dynamic::object("message", "Received unknown method: " + method); responder->error(response); - }); + } catch (std::exception& e) { + log(std::string("Error: ") + e.what()); + if (responder) { + responder->error(dynamic::object("message", e.what())( + "stacktrace", callstack())("name", e.what())); + } + } catch (...) { + log("Unknown error suppressed in FlipperClient"); + if (responder) { + responder->error(dynamic::object( + "message", + "Unknown error during " + message["method"] + ". " + + folly::toJson(message))("stacktrace", callstack())( + "name", "Unknown")); + } + } } + std::string FlipperClient::callstack() { #if __APPLE__ // For some iOS apps, __Unwind_Backtrace symbol wasn't found in sandcastle diff --git a/xplat/Flipper/FlipperClient.h b/xplat/Flipper/FlipperClient.h index a5eb9f87b..abf10065f 100644 --- a/xplat/Flipper/FlipperClient.h +++ b/xplat/Flipper/FlipperClient.h @@ -71,7 +71,9 @@ class FlipperClient : public FlipperConnectionManager::Callbacks { void onDisconnected() override; - void onMessageReceived(const folly::dynamic& message) override; + void onMessageReceived( + const folly::dynamic& message, + std::unique_ptr) override; void addPlugin(std::shared_ptr plugin); diff --git a/xplat/Flipper/FlipperConnection.h b/xplat/Flipper/FlipperConnection.h index 71cc532ed..7c9f36214 100644 --- a/xplat/Flipper/FlipperConnection.h +++ b/xplat/Flipper/FlipperConnection.h @@ -23,7 +23,7 @@ with corresponding identifiers. class FlipperConnection { public: using FlipperReceiver = std::function< - void(const folly::dynamic&, std::unique_ptr)>; + void(const folly::dynamic&, std::shared_ptr)>; virtual ~FlipperConnection() {} diff --git a/xplat/Flipper/FlipperConnectionImpl.h b/xplat/Flipper/FlipperConnectionImpl.h index e7da56808..891d23ad6 100644 --- a/xplat/Flipper/FlipperConnectionImpl.h +++ b/xplat/Flipper/FlipperConnectionImpl.h @@ -1,17 +1,16 @@ -/* - * Copyright (c) 2018-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ - #pragma once -#include "FlipperConnection.h" -#include "FlipperConnectionManager.h" #include #include +#include "FlipperConnection.h" +#include "FlipperConnectionManager.h" +#include "Log.h" namespace facebook { namespace flipper { @@ -24,11 +23,14 @@ class FlipperConnectionImpl : public FlipperConnection { void call( const std::string& method, const folly::dynamic& params, - std::unique_ptr responder) { + std::shared_ptr responder) { if (receivers_.find(method) == receivers_.end()) { - throw std::out_of_range("receiver " + method + " not found."); + std::string errorMessage = "Receiver " + method + " not found."; + log("Error: " + errorMessage); + responder->error(folly::dynamic::object("message", errorMessage)); + return; } - receivers_.at(method)(params, std::move(responder)); + receivers_.at(method)(params, responder); } void send(const std::string& method, const folly::dynamic& params) override { diff --git a/xplat/Flipper/FlipperConnectionManager.h b/xplat/Flipper/FlipperConnectionManager.h index 617cb2b0f..96ff689a2 100644 --- a/xplat/Flipper/FlipperConnectionManager.h +++ b/xplat/Flipper/FlipperConnectionManager.h @@ -1,14 +1,13 @@ -/* - * Copyright (c) 2018-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ - #pragma once #include +#include "FlipperResponder.h" namespace facebook { namespace flipper { @@ -56,7 +55,9 @@ class FlipperConnectionManager::Callbacks { virtual void onDisconnected() = 0; - virtual void onMessageReceived(const folly::dynamic& message) = 0; + virtual void onMessageReceived( + const folly::dynamic& message, + std::unique_ptr) = 0; }; } // namespace flipper diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp index 858f519e2..491f4dcc1 100644 --- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp +++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp @@ -16,8 +16,11 @@ #include #include #include "ConnectionContextStore.h" +#include "FireAndForgetBasedFlipperResponder.h" +#include "FlipperResponderImpl.h" #include "FlipperStep.h" #include "Log.h" +#include "yarpl/Single.h" #define WRONG_THREAD_EXIT_MSG \ "ERROR: Aborting flipper initialization because it's not running in the flipper thread." @@ -30,6 +33,8 @@ static constexpr int maxPayloadSize = 0xFFFFFF; namespace facebook { namespace flipper { +rsocket::Payload toRSocketPayload(dynamic data); + class ConnectionEvents : public rsocket::RSocketConnectionEvents { private: FlipperConnectionManagerImpl* websocket_; @@ -72,7 +77,54 @@ class Responder : public rsocket::RSocketResponder { rsocket::Payload request, rsocket::StreamId streamId) { const auto payload = request.moveDataToString(); - websocket_->callbacks_->onMessageReceived(folly::parseJson(payload)); + std::unique_ptr responder; + auto message = folly::parseJson(payload); + if (message.find("id") != message.items().end()) { + auto id = message["id"].getInt(); + responder = + std::make_unique(websocket_, id); + } + + websocket_->callbacks_->onMessageReceived( + folly::parseJson(payload), std::move(responder)); + } + + std::shared_ptr> + handleRequestResponse(rsocket::Payload request, rsocket::StreamId streamId) { + const auto requestString = request.moveDataToString(); + + auto dynamicSingle = yarpl::single::Single::create( + [payload = std::move(requestString), this](auto observer) { + auto responder = std::make_unique(observer); + websocket_->callbacks_->onMessageReceived( + folly::parseJson(payload), std::move(responder)); + }); + + auto rsocketSingle = yarpl::single::Single::create( + [payload = std::move(requestString), dynamicSingle, this]( + auto observer) { + observer->onSubscribe( + yarpl::single::SingleSubscriptions::empty()); + dynamicSingle->subscribe( + [observer, this](folly::dynamic d) { + websocket_->connectionEventBase_->runInEventBaseThread( + [observer, d]() { + try { + observer->onSuccess(toRSocketPayload(d)); + + } catch (std::exception& e) { + log(e.what()); + observer->onError(e); + } + }); + }, + [observer, this](folly::exception_wrapper e) { + websocket_->connectionEventBase_->runInEventBaseThread( + [observer, e]() { observer->onError(e); }); + }); + }); + + return rsocketSingle; } }; @@ -236,24 +288,18 @@ void FlipperConnectionManagerImpl::setCallbacks(Callbacks* callbacks) { void FlipperConnectionManagerImpl::sendMessage(const folly::dynamic& message) { flipperEventBase_->add([this, message]() { - std::string json = folly::toJson(message); - rsocket::Payload payload = rsocket::Payload(json); - auto payloadLength = payload.data->computeChainDataLength(); - - DCHECK_LE(payloadLength, maxPayloadSize); - if (payloadLength > maxPayloadSize) { - auto logMessage = - std::string( - "Error: Skipping sending message larger than max rsocket payload: ") + - json; - log(logMessage); + try { + rsocket::Payload payload = toRSocketPayload(message); + if (client_) { + client_->getRequester() + ->fireAndForget(std::move(payload)) + ->subscribe([]() {}); + } + } catch (std::length_error& e) { + // Skip sending messages that are too large. + log(e.what()); return; } - if (client_) { - client_->getRequester() - ->fireAndForget(std::move(payload)) - ->subscribe([]() {}); - } }); } @@ -341,5 +387,21 @@ bool FlipperConnectionManagerImpl::isRunningInOwnThread() { return flipperEventBase_->isInEventBaseThread(); } +rsocket::Payload toRSocketPayload(dynamic data) { + std::string json = folly::toJson(data); + rsocket::Payload payload = rsocket::Payload(json); + auto payloadLength = payload.data->computeChainDataLength(); + + DCHECK_LE(payloadLength, maxPayloadSize); + if (payloadLength > maxPayloadSize) { + auto logMessage = + std::string( + "Error: Skipping sending message larger than max rsocket payload: ") + + json; + throw new std::length_error(logMessage); + } + return payload; +} + } // namespace flipper } // namespace facebook diff --git a/xplat/Flipper/FlipperResponderImpl.h b/xplat/Flipper/FlipperResponderImpl.h index 52438402a..0ab23bbdb 100644 --- a/xplat/Flipper/FlipperResponderImpl.h +++ b/xplat/Flipper/FlipperResponderImpl.h @@ -1,40 +1,42 @@ -/* - * Copyright (c) 2018-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ - #pragma once -#include "FlipperResponder.h" -#include "FlipperConnectionManager.h" +#include #include +#include +#include "FlipperConnectionManager.h" +#include "FlipperResponder.h" namespace facebook { namespace flipper { +/* Responder to encapsulate yarpl observables and hide them from flipper core + + * plugins */ class FlipperResponderImpl : public FlipperResponder { public: - FlipperResponderImpl(FlipperConnectionManager* socket, int64_t responseID) - : socket_(socket), responseID_(responseID) {} + FlipperResponderImpl( + std::shared_ptr> + downstreamObserver) + : downstreamObserver_(downstreamObserver) {} void success(const folly::dynamic& response) const override { - const folly::dynamic message = - folly::dynamic::object("id", responseID_)("success", response); - socket_->sendMessage(message); + const folly::dynamic message = folly::dynamic::object("success", response); + downstreamObserver_->onSuccess(message); } void error(const folly::dynamic& response) const override { - const folly::dynamic message = - folly::dynamic::object("id", responseID_)("error", response); - socket_->sendMessage(message); + const folly::dynamic message = folly::dynamic::object("error", response); + downstreamObserver_->onSuccess(message); } private: - FlipperConnectionManager* socket_; - int64_t responseID_; + std::shared_ptr> + downstreamObserver_; }; } // namespace flipper diff --git a/xplat/FlipperTests/FlipperClientTests.cpp b/xplat/FlipperTests/FlipperClientTests.cpp index 9acbbeabe..7e2826f2f 100644 --- a/xplat/FlipperTests/FlipperClientTests.cpp +++ b/xplat/FlipperTests/FlipperClientTests.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -17,9 +18,32 @@ namespace test { using folly::dynamic; -auto state = std::make_shared(); +class FlipperClientTest : public ::testing::Test { + protected: + std::unique_ptr client; + FlipperConnectionManagerMock* socket; + std::shared_ptr state; -TEST(FlipperClientTests, testSaneMocks) { + std::vector successes; + std::vector failures; + + void SetUp() override { + successes.clear(); + failures.clear(); + + state.reset(new FlipperState()); + + socket = new FlipperConnectionManagerMock; + client = std::make_unique( + std::unique_ptr{socket}, state); + } + + std::unique_ptr getResponder() { + return std::make_unique(&successes, &failures); + } +}; + +TEST_F(FlipperClientTest, testSaneMocks) { FlipperConnectionManagerMock socket; socket.start(); EXPECT_TRUE(socket.isOpen()); @@ -30,82 +54,61 @@ TEST(FlipperClientTests, testSaneMocks) { EXPECT_EQ(plugin.identifier(), "Test"); } -TEST(FlipperClientTests, testGetPlugins) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - client.start(); +TEST_F(FlipperClientTest, testGetPlugins) { + client->start(); - client.addPlugin(std::make_shared("Cat")); - client.addPlugin(std::make_shared("Dog")); + client->addPlugin(std::make_shared("Cat")); + client->addPlugin(std::make_shared("Dog")); dynamic message = dynamic::object("id", 1)("method", "getPlugins"); - socket->callbacks->onMessageReceived(message); + socket->callbacks->onMessageReceived(message, getResponder()); - dynamic expected = dynamic::object("id", 1)( - "success", dynamic::object("plugins", dynamic::array("Cat", "Dog"))); - EXPECT_EQ(socket->messages.back(), expected); + dynamic expected = dynamic::object("plugins", dynamic::array("Cat", "Dog")); + EXPECT_EQ(successes[0], expected); + EXPECT_EQ(failures.size(), 0); } -TEST(FlipperClientTests, testGetPlugin) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testGetPlugin) { const auto catPlugin = std::make_shared("Cat"); - client.addPlugin(catPlugin); + client->addPlugin(catPlugin); const auto dogPlugin = std::make_shared("Dog"); - client.addPlugin(dogPlugin); + client->addPlugin(dogPlugin); - EXPECT_EQ(catPlugin, client.getPlugin("Cat")); - EXPECT_EQ(dogPlugin, client.getPlugin("Dog")); + EXPECT_EQ(catPlugin, client->getPlugin("Cat")); + EXPECT_EQ(dogPlugin, client->getPlugin("Dog")); } -TEST(FlipperClientTests, testGetPluginWithDowncast) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testGetPluginWithDowncast) { const auto catPlugin = std::make_shared("Cat"); - client.addPlugin(catPlugin); - EXPECT_EQ(catPlugin, client.getPlugin("Cat")); + client->addPlugin(catPlugin); + EXPECT_EQ(catPlugin, client->getPlugin("Cat")); } -TEST(FlipperClientTests, testRemovePlugin) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - client.start(); +TEST_F(FlipperClientTest, testRemovePlugin) { + client->start(); auto plugin = std::make_shared("Test"); - client.addPlugin(plugin); - client.removePlugin(plugin); + client->addPlugin(plugin); + client->removePlugin(plugin); dynamic message = dynamic::object("id", 1)("method", "getPlugins"); - socket->callbacks->onMessageReceived(message); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(message, getResponder()); - dynamic expected = dynamic::object("id", 1)( - "success", dynamic::object("plugins", dynamic::array())); - EXPECT_EQ(socket->messages.back(), expected); + dynamic expected = dynamic::object("plugins", dynamic::array()); + EXPECT_EQ(successes[0], expected); + EXPECT_EQ(failures.size(), 0); } -TEST(FlipperClientTests, testStartStop) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - - client.start(); +TEST_F(FlipperClientTest, testStartStop) { + client->start(); EXPECT_TRUE(socket->isOpen()); - client.stop(); + client->stop(); EXPECT_FALSE(socket->isOpen()); } -TEST(FlipperClientTests, testConnectDisconnect) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testConnectDisconnect) { bool pluginConnected = false; const auto connectionCallback = [&](std::shared_ptr conn) { pluginConnected = true; @@ -113,23 +116,20 @@ TEST(FlipperClientTests, testConnectDisconnect) { const auto disconnectionCallback = [&]() { pluginConnected = false; }; auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback); - client.addPlugin(plugin); + client->addPlugin(plugin); - client.start(); + client->start(); dynamic messageInit = dynamic::object("method", "init")( "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageInit); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); EXPECT_TRUE(pluginConnected); - client.stop(); + client->stop(); EXPECT_FALSE(pluginConnected); } -TEST(FlipperClientTests, testInitDeinit) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testInitDeinit) { bool pluginConnected = false; const auto connectionCallback = [&](std::shared_ptr conn) { pluginConnected = true; @@ -138,37 +138,42 @@ TEST(FlipperClientTests, testInitDeinit) { auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback); - client.start(); - client.addPlugin(plugin); + client->start(); + client->addPlugin(plugin); EXPECT_FALSE(pluginConnected); dynamic expected = dynamic::object("method", "refreshPlugins"); EXPECT_EQ(socket->messages.front(), expected); - dynamic messageInit = dynamic::object("method", "init")( - "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageInit); - EXPECT_TRUE(pluginConnected); + { + dynamic messageInit = dynamic::object("method", "init")( + "params", dynamic::object("plugin", "Test")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); + EXPECT_TRUE(pluginConnected); + } - dynamic messageDeinit = dynamic::object("method", "deinit")( - "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageDeinit); - EXPECT_FALSE(pluginConnected); + { + dynamic messageDeinit = dynamic::object("method", "deinit")( + "params", dynamic::object("plugin", "Test")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageDeinit, getResponder()); + EXPECT_FALSE(pluginConnected); + } - dynamic messageReinit = dynamic::object("method", "init")( - "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageReinit); - EXPECT_TRUE(pluginConnected); + { + dynamic messageReinit = dynamic::object("method", "init")( + "params", dynamic::object("plugin", "Test")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageReinit, getResponder()); + EXPECT_TRUE(pluginConnected); + } - client.stop(); + client->stop(); EXPECT_FALSE(pluginConnected); } -TEST(FlipperClientTests, testRemovePluginWhenConnected) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testRemovePluginWhenConnected) { bool pluginConnected = false; const auto connectionCallback = [&](std::shared_ptr conn) { pluginConnected = true; @@ -177,75 +182,76 @@ TEST(FlipperClientTests, testRemovePluginWhenConnected) { auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback); - client.addPlugin(plugin); - client.start(); - client.removePlugin(plugin); + client->addPlugin(plugin); + client->start(); + client->removePlugin(plugin); EXPECT_FALSE(pluginConnected); dynamic expected = dynamic::object("method", "refreshPlugins"); EXPECT_EQ(socket->messages.back(), expected); } -TEST(FlipperClientTests, testUnhandleableMethod) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testUnhandleableMethod) { auto plugin = std::make_shared("Test"); - client.addPlugin(plugin); + client->addPlugin(plugin); - dynamic messageInit = dynamic::object("method", "init")( - "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageInit); + { + dynamic messageInit = dynamic::object("method", "init")( + "params", dynamic::object("plugin", "Test")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); + } - dynamic messageExecute = dynamic::object("id", 1)("method", "unexpected"); - socket->callbacks->onMessageReceived(messageExecute); + { + dynamic messageExecute = dynamic::object("id", 1)("method", "unexpected"); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageExecute, getResponder()); + } - dynamic expected = dynamic::object("id", 1)( - "error", - dynamic::object("message", "Received unknown method: unexpected")); - EXPECT_EQ(socket->messages.back(), expected); + dynamic expected = + dynamic::object("message", "Received unknown method: unexpected"); + EXPECT_EQ(failures[0], expected); + EXPECT_EQ(successes.size(), 0); } -TEST(FlipperClientTests, testExecute) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - client.start(); +TEST_F(FlipperClientTest, testExecute) { + client->start(); const auto connectionCallback = [](std::shared_ptr conn) { const auto receiver = [](const dynamic& params, - std::unique_ptr responder) { + std::shared_ptr responder) { dynamic payload = dynamic::object("message", "yes_i_hear_u"); responder->success(payload); }; conn->receive("plugin_can_u_hear_me", receiver); }; auto plugin = std::make_shared("Test", connectionCallback); - client.addPlugin(plugin); + client->addPlugin(plugin); - dynamic messageInit = dynamic::object("method", "init")( - "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageInit); + { + dynamic messageInit = dynamic::object("method", "init")( + "params", dynamic::object("plugin", "Test")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); + } - dynamic messageUnexpected = dynamic::object("id", 1)("method", "execute")( - "params", - dynamic::object("api", "Test")("method", "plugin_can_u_hear_me")); - socket->callbacks->onMessageReceived(messageUnexpected); + { + dynamic messageUnexpected = dynamic::object("id", 1)("method", "execute")( + "params", + dynamic::object("api", "Test")("method", "plugin_can_u_hear_me")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageUnexpected, getResponder()); + } - dynamic expected = dynamic::object("id", 1)( - "success", dynamic::object("message", "yes_i_hear_u")); - EXPECT_EQ(socket->messages.back(), expected); + dynamic expected = dynamic::object("message", "yes_i_hear_u"); + EXPECT_EQ(successes[0], expected); + EXPECT_EQ(failures.size(), 0); } -TEST(FlipperClientTests, testExecuteWithParams) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testExecuteWithParams) { const auto connectionCallback = [&](std::shared_ptr conn) { const auto receiver = [](const dynamic& params, - std::unique_ptr responder) { + std::shared_ptr responder) { const auto& first = params["first"].asString(); const auto& second = params["second"].asString(); std::map m{{"dog", "woof"}, {"cat", "meow"}}; @@ -255,62 +261,56 @@ TEST(FlipperClientTests, testExecuteWithParams) { conn->receive("animal_sounds", receiver); }; auto plugin = std::make_shared("Test", connectionCallback); - client.addPlugin(plugin); + client->addPlugin(plugin); - dynamic messageInit = dynamic::object("method", "init")( - "params", dynamic::object("plugin", "Test")); - socket->callbacks->onMessageReceived(messageInit); + { + dynamic messageInit = dynamic::object("method", "init")( + "params", dynamic::object("plugin", "Test")); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); + } - dynamic messageExecute = dynamic::object("id", 1)("method", "execute")( - "params", - dynamic::object("api", "Test")("method", "animal_sounds")( - "params", dynamic::object("first", "dog")("second", "cat"))); - socket->callbacks->onMessageReceived(messageExecute); + { + dynamic messageExecute = dynamic::object("id", 1)("method", "execute")( + "params", + dynamic::object("api", "Test")("method", "animal_sounds")( + "params", dynamic::object("first", "dog")("second", "cat"))); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageExecute, getResponder()); + } - dynamic expected = dynamic::object("id", 1)( - "success", dynamic::object("dog", "woof")("cat", "meow")); - EXPECT_EQ(socket->messages.back(), expected); + dynamic expected = dynamic::object("dog", "woof")("cat", "meow"); + EXPECT_EQ(successes[0], expected); + EXPECT_EQ(failures.size(), 0); } -TEST(FlipperClientTests, testExceptionUnknownPlugin) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - client.start(); +TEST_F(FlipperClientTest, testExceptionUnknownPlugin) { + client->start(); dynamic messageInit = dynamic::object("method", "init")( "params", dynamic::object("plugin", "Unknown")); - socket->callbacks->onMessageReceived(messageInit); - EXPECT_EQ( - socket->messages.back()["error"]["message"], - "plugin Unknown not found for method init"); - EXPECT_EQ( - socket->messages.back()["error"]["name"], - "plugin Unknown not found for method init"); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); + + auto failure = failures[0]; + EXPECT_EQ(failure["message"], "Plugin Unknown not found for method init"); + EXPECT_EQ(failure["name"], "PluginNotFound"); } -TEST(FlipperClientTests, testExceptionUnknownApi) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - client.start(); +TEST_F(FlipperClientTest, testExceptionUnknownApi) { + client->start(); dynamic messageInit = dynamic::object("method", "execute")( "params", dynamic::object("api", "Unknown")); - socket->callbacks->onMessageReceived(messageInit); + auto responder = std::make_shared(); + socket->callbacks->onMessageReceived(messageInit, getResponder()); + auto failure = failures[0]; EXPECT_EQ( - socket->messages.back()["error"]["message"], - "connection Unknown not found for method execute"); - EXPECT_EQ( - socket->messages.back()["error"]["name"], - "connection Unknown not found for method execute"); + failure["message"], "Connection Unknown not found for method execute"); + EXPECT_EQ(failure["name"], "ConnectionNotFound"); } -TEST(FlipperClientTests, testBackgroundPluginActivated) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testBackgroundPluginActivated) { bool pluginConnected = false; const auto connectionCallback = [&](std::shared_ptr conn) { pluginConnected = true; @@ -319,18 +319,14 @@ TEST(FlipperClientTests, testBackgroundPluginActivated) { auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback, true); - client.addPlugin(plugin); - client.start(); + client->addPlugin(plugin); + client->start(); EXPECT_TRUE(pluginConnected); - client.stop(); + client->stop(); EXPECT_FALSE(pluginConnected); } -TEST(FlipperClientTests, testNonBackgroundPluginNotActivated) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testNonBackgroundPluginNotActivated) { bool pluginConnected = false; const auto connectionCallback = [&](std::shared_ptr conn) { pluginConnected = true; @@ -339,18 +335,14 @@ TEST(FlipperClientTests, testNonBackgroundPluginNotActivated) { auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback, false); - client.addPlugin(plugin); - client.start(); + client->addPlugin(plugin); + client->start(); EXPECT_FALSE(pluginConnected); - client.stop(); + client->stop(); EXPECT_FALSE(pluginConnected); } -TEST(FlipperClientTests, testCrashInDidConnectDisConnectIsSuppressed) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - +TEST_F(FlipperClientTest, testCrashInDidConnectDisConnectIsSuppressed) { const auto connectionCallback = [&](std::shared_ptr conn) { throw std::runtime_error("Runtime Error in test"); }; @@ -360,19 +352,15 @@ TEST(FlipperClientTests, testCrashInDidConnectDisConnectIsSuppressed) { auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback, true); - client.addPlugin(plugin); + client->addPlugin(plugin); - EXPECT_NO_FATAL_FAILURE(client.start()); - EXPECT_NO_FATAL_FAILURE(client.stop()); + EXPECT_NO_FATAL_FAILURE(client->start()); + EXPECT_NO_FATAL_FAILURE(client->stop()); } -TEST( - FlipperClientTests, +TEST_F( + FlipperClientTest, testNonStandardCrashInDidConnectDisConnectIsSuppressed) { - auto socket = new FlipperConnectionManagerMock; - FlipperClient client( - std::unique_ptr{socket}, state); - const auto connectionCallback = [&](std::shared_ptr conn) { throw "Non standard exception"; }; @@ -380,10 +368,10 @@ TEST( auto plugin = std::make_shared( "Test", connectionCallback, disconnectionCallback, true); - client.addPlugin(plugin); + client->addPlugin(plugin); - EXPECT_NO_FATAL_FAILURE(client.start()); - EXPECT_NO_FATAL_FAILURE(client.stop()); + EXPECT_NO_FATAL_FAILURE(client->start()); + EXPECT_NO_FATAL_FAILURE(client->stop()); } } // namespace test diff --git a/xplat/FlipperTests/FlipperResponderImplTests.cpp b/xplat/FlipperTests/FlipperResponderImplTests.cpp new file mode 100644 index 000000000..62ef4fae3 --- /dev/null +++ b/xplat/FlipperTests/FlipperResponderImplTests.cpp @@ -0,0 +1,53 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. + */ +#include +#include "yarpl/Flowable.h" +#include "yarpl/Single.h" +#include "yarpl/single/SingleTestObserver.h" + +#include +#include + +namespace facebook { +namespace flipper { +namespace test { + +using folly::dynamic; + +TEST(FlipperResponderImplTest, testSuccessWrapper) { + auto dynamicSingle = + yarpl::single::Single::create([](auto observer) mutable { + observer->onSubscribe(yarpl::single::SingleSubscriptions::empty()); + auto responder = std::make_shared(observer); + responder->success(folly::dynamic::object("my", "object")); + }); + auto to = yarpl::single::SingleTestObserver::create(); + dynamicSingle->subscribe(to); + + to->awaitTerminalEvent(); + auto output = to->getOnSuccessValue(); + EXPECT_EQ(output["success"]["my"], "object"); +} + +TEST(FlipperResponderImplTest, testErrorWrapper) { + auto dynamicSingle = + yarpl::single::Single::create([](auto observer) mutable { + observer->onSubscribe(yarpl::single::SingleSubscriptions::empty()); + auto responder = std::make_shared(observer); + responder->error(folly::dynamic::object("my", "object")); + }); + auto to = yarpl::single::SingleTestObserver::create(); + dynamicSingle->subscribe(to); + + to->awaitTerminalEvent(); + auto output = to->getOnSuccessValue(); + EXPECT_EQ(output["error"]["my"], "object"); +} + +} // namespace test +} // namespace flipper +} // namespace facebook