From c2ed2484d9a97e140c73d091216b175e757087b9 Mon Sep 17 00:00:00 2001 From: Lorenzo Blasa Date: Wed, 17 Aug 2022 09:18:20 -0700 Subject: [PATCH] Expose a send method with a string params Summary: For C++, folly::dynamic is used throughout. On iOS and Android though, Flipper goes through multiple conversions to get to a folly::dynamic only to ultimately obtain a JSON string from it. Let's take a look at Android: There are multiple types like FlipperObject, FlipperArray that wrap around a JSONObject. When data needs to be sent: 1. The JSONObject is asked for its string representation. 2. The string representation is then parsed by folly to construct the folly::dynamic instance. 3. The step above involves an extra boundary cross through JNI. 4. Ultimately, a socket or ws connection does not understand folly::dynamic so we then get a JSON string representation from it. 5. Data is sent. As described above, for big enough objects, this represents an issue. So, the idea of this change, is to allow plugins to send a JSON string instead. This will remove a few serialisation/deserialisation steps from the process. *Note: this API is not currently used by anything so there's no impact to existing plugins.* Changelog: expose a send method that accept a string as params Reviewed By: LukeDefeo Differential Revision: D38741582 fbshipit-source-id: 78e0acd80fc8c97378ee986cbaf377078996ed60 --- android/src/main/cpp/sonar.cpp | 5 +++++ .../flipper/android/FlipperConnectionImpl.java | 7 +++++++ .../flipper/core/FlipperConnection.java | 6 ++++++ .../flipper/testing/FlipperConnectionMock.java | 13 +++++++++++++ xplat/Flipper/FlipperConnection.h | 9 ++++++++- xplat/Flipper/FlipperConnectionImpl.h | 18 ++++++++++++++++++ xplat/Flipper/FlipperConnectionManager.h | 6 ++++++ xplat/Flipper/FlipperConnectionManagerImpl.cpp | 14 ++++++++++++++ xplat/Flipper/FlipperConnectionManagerImpl.h | 2 ++ .../FlipperConnectionManagerMock.h | 4 ++++ xplat/FlipperTestLib/FlipperConnectionMock.h | 6 ++++++ 11 files changed, 89 insertions(+), 1 deletion(-) diff --git a/android/src/main/cpp/sonar.cpp b/android/src/main/cpp/sonar.cpp index bcc043292..5d7208d69 100644 --- a/android/src/main/cpp/sonar.cpp +++ b/android/src/main/cpp/sonar.cpp @@ -514,6 +514,7 @@ class JFlipperConnectionImpl registerHybrid({ makeNativeMethod("sendObject", JFlipperConnectionImpl::sendObject), makeNativeMethod("sendArray", JFlipperConnectionImpl::sendArray), + makeNativeMethod("sendString", JFlipperConnectionImpl::sendString), makeNativeMethod("reportError", JFlipperConnectionImpl::reportError), makeNativeMethod( "reportErrorWithMetadata", @@ -522,6 +523,10 @@ class JFlipperConnectionImpl }); } + void sendString(const std::string method, const std::string params) { + _connection->send(std::move(method), std::move(params)); + } + void sendObject( const std::string method, jni::alias_ref json) { diff --git a/android/src/main/java/com/facebook/flipper/android/FlipperConnectionImpl.java b/android/src/main/java/com/facebook/flipper/android/FlipperConnectionImpl.java index d804771d8..7dbb86667 100644 --- a/android/src/main/java/com/facebook/flipper/android/FlipperConnectionImpl.java +++ b/android/src/main/java/com/facebook/flipper/android/FlipperConnectionImpl.java @@ -40,10 +40,17 @@ class FlipperConnectionImpl implements FlipperConnection { sendArray(method, params); } + @Override + public void send(String method, String params) { + sendString(method, params); + } + public native void sendObject(String method, FlipperObject params); public native void sendArray(String method, FlipperArray params); + public native void sendString(String method, String params); + @Override public native void reportErrorWithMetadata(String reason, String stackTrace); diff --git a/android/src/main/java/com/facebook/flipper/core/FlipperConnection.java b/android/src/main/java/com/facebook/flipper/core/FlipperConnection.java index c8db6c9b3..ef48bfeb0 100644 --- a/android/src/main/java/com/facebook/flipper/core/FlipperConnection.java +++ b/android/src/main/java/com/facebook/flipper/core/FlipperConnection.java @@ -26,6 +26,12 @@ public interface FlipperConnection { */ void send(String method, FlipperArray params); + /** + * Call a remote method on the Flipper desktop application, passing an optional JSON string as a + * parameter. + */ + void send(String method, String message); + /** Report client error with reason and stacktrace as an argument */ void reportErrorWithMetadata(String reason, String stackTrace); diff --git a/android/src/main/java/com/facebook/flipper/testing/FlipperConnectionMock.java b/android/src/main/java/com/facebook/flipper/testing/FlipperConnectionMock.java index cf31a4912..3ae3d17d5 100644 --- a/android/src/main/java/com/facebook/flipper/testing/FlipperConnectionMock.java +++ b/android/src/main/java/com/facebook/flipper/testing/FlipperConnectionMock.java @@ -47,6 +47,19 @@ public class FlipperConnectionMock implements FlipperConnection { paramList.add(params); } + @Override + public void send(String method, String params) { + final List paramList; + if (sent.containsKey(method)) { + paramList = sent.get(method); + } else { + paramList = new ArrayList<>(); + sent.put(method, paramList); + } + + paramList.add(params); + } + @Override public void reportErrorWithMetadata(String reason, String stackTrace) { errors.add(new Throwable(reason)); diff --git a/xplat/Flipper/FlipperConnection.h b/xplat/Flipper/FlipperConnection.h index 974032b7b..12cd22f46 100644 --- a/xplat/Flipper/FlipperConnection.h +++ b/xplat/Flipper/FlipperConnection.h @@ -27,12 +27,19 @@ class FlipperConnection { virtual ~FlipperConnection() {} /** - Invoke a method on the Flipper desktop plugin with with a matching identifier. + Invoke a method on the Flipper desktop plugin with a matching identifier. */ virtual void send( const std::string& method, const folly::dynamic& params) = 0; + /** + Invoke a method on the Flipper desktop plugin with a matching + identifier. + Note: The `message` argument is expected to contain a valid JSON. + */ + virtual void send(const std::string& method, const std::string& params) = 0; + /** Report an error to the Flipper desktop app */ diff --git a/xplat/Flipper/FlipperConnectionImpl.h b/xplat/Flipper/FlipperConnectionImpl.h index e9578480f..e0b646ae7 100644 --- a/xplat/Flipper/FlipperConnectionImpl.h +++ b/xplat/Flipper/FlipperConnectionImpl.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include "FlipperConnection.h" #include "FlipperConnectionManager.h" @@ -52,6 +53,23 @@ class FlipperConnectionImpl : public FlipperConnection { socket_->sendMessage(message); } + void send(const std::string& method, const std::string& params) override { + std::stringstream ss; + ss << "{" + "\"method\": \"execute\"," + "\"params\": {" + "\"api\": \"" + << name_ + << "\"," + "\"method\": \"" + << method + << "\"," + "\"params\":" + << params << "}}"; + + socket_->sendMessage(ss.str()); + } + void error(const std::string& message, const std::string& stacktrace) override { socket_->sendMessage(folly::dynamic::object( diff --git a/xplat/Flipper/FlipperConnectionManager.h b/xplat/Flipper/FlipperConnectionManager.h index cb9bd97f8..1387791ff 100644 --- a/xplat/Flipper/FlipperConnectionManager.h +++ b/xplat/Flipper/FlipperConnectionManager.h @@ -54,6 +54,12 @@ class FlipperConnectionManager { */ virtual void sendMessage(const folly::dynamic& message) = 0; + /** + Send message to the ws server. + Note: The `message` argument is expected to contain a valid JSON. + */ + virtual void sendMessage(const std::string& message) = 0; + /** Handler for connection and message receipt from the ws server. The callbacks should be set before a connection is established. diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp index eee230582..10d5537c0 100644 --- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp +++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp @@ -341,6 +341,20 @@ void FlipperConnectionManagerImpl::sendMessage(const folly::dynamic& message) { }); } +void FlipperConnectionManagerImpl::sendMessage(const std::string& message) { + flipperScheduler_->schedule([this, message]() { + try { + if (client_) { + client_->send(message, []() {}); + } + } catch (std::length_error& e) { + // Skip sending messages that are too large. + log(e.what()); + return; + } + }); +} + void FlipperConnectionManagerImpl::onMessageReceived( const folly::dynamic& message, std::unique_ptr responder) { diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.h b/xplat/Flipper/FlipperConnectionManagerImpl.h index e1653dee1..3a586130b 100644 --- a/xplat/Flipper/FlipperConnectionManagerImpl.h +++ b/xplat/Flipper/FlipperConnectionManagerImpl.h @@ -42,6 +42,8 @@ class FlipperConnectionManagerImpl : public FlipperConnectionManager { void sendMessage(const folly::dynamic& message) override; + void sendMessage(const std::string& message) override; + void onMessageReceived( const folly::dynamic& message, std::unique_ptr responder) override; diff --git a/xplat/FlipperTestLib/FlipperConnectionManagerMock.h b/xplat/FlipperTestLib/FlipperConnectionManagerMock.h index ca365be58..51f57ba92 100644 --- a/xplat/FlipperTestLib/FlipperConnectionManagerMock.h +++ b/xplat/FlipperTestLib/FlipperConnectionManagerMock.h @@ -40,6 +40,10 @@ class FlipperConnectionManagerMock : public FlipperConnectionManager { messages.push_back(message); } + void sendMessage(const std::string& message) override { + messages.push_back(folly::parseJson(message)); + } + void setCertificateProvider( const std::shared_ptr provider) override{}; diff --git a/xplat/FlipperTestLib/FlipperConnectionMock.h b/xplat/FlipperTestLib/FlipperConnectionMock.h index 93d83a6cd..d662bf290 100644 --- a/xplat/FlipperTestLib/FlipperConnectionMock.h +++ b/xplat/FlipperTestLib/FlipperConnectionMock.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include #include @@ -22,6 +23,11 @@ class FlipperConnectionMock : public FlipperConnection { sent_message_history[method].push(params); } + void send(const std::string& method, const std::string& params) override { + sent_[method] = folly::parseJson(params); + sent_message_history[method].push(params); + } + void receive(const std::string& method, const FlipperReceiver& receiver) override { receivers_[method] = receiver;