Merge pull request #1986 from Expensify/main

libstuff/SLog.cpp

@@ -76,6 +76,7 @@ static const set<string> PARAMS_WHITELIST = {
     "nvpName",
     "policyAccountID",
     "policyID",
+    "reimbursementEntryID",
     "reportID",
     "requestID",
     "requestTimestamp",
@@ -89,7 +90,8 @@ static const set<string> PARAMS_WHITELIST = {
     "transactionID",
     "type",
     "userID",
-    "secondaryLogin"
+    "secondaryLogin",
+    "walletBankAccountID"
 };
 
 string addLogParams(string&& message, const STable& params) {

sqlitecluster/SQLiteClusterMessenger.cpp

@@ -130,10 +130,10 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket,
     // that needs serialization, and if so, we serialize that as well.
     if (command.httpsRequests.size()) {
         request["httpsRequests"] = command.serializeHTTPSRequests();
-        string serializedData = command.serializeData();
-        if (serializedData.size()) {
-            request["serializedData"] = move(serializedData);
-        }
+    }
+    string serializedData = command.serializeData();
+    if (serializedData.size()) {
+        request["serializedData"] = move(serializedData);
     }
 
     request.nameValueMap["ID"] = command.id;

test/clustertest/testplugin/TestPlugin.cpp

@@ -1,5 +1,6 @@
 #include "TestPlugin.h"
 
+#include <dlfcn.h>
 #include <iostream>
 #include <sys/file.h>
 #include <string.h>
@@ -102,7 +103,8 @@ unique_ptr<BedrockCommand> BedrockPlugin_TestPlugin::getCommand(SQLiteCommand&&
         "prepeekpostprocesscommand",
         "preparehandler",
         "testquery",
-        "testPostProcessTimeout"
+        "testPostProcessTimeout",
+        "EscalateSerializedData"
     };
     for (auto& cmdName : supportedCommands) {
         if (SStartsWith(baseCommand.request.methodLine, cmdName)) {
@@ -119,6 +121,19 @@ TestPluginCommand::TestPluginCommand(SQLiteCommand&& baseCommand, BedrockPlugin_
 {
 }
 
+string TestPluginCommand::serializeData() const {
+    if (SStartsWith(request.methodLine, "EscalateSerializedData")) {
+        return serializedDataString;
+    }
+    return "";
+}
+
+void TestPluginCommand::deserializeData(const string& data) {
+    if (SStartsWith(request.methodLine, "EscalateSerializedData")) {
+        serializedDataString = data;
+    }
+}
+
 TestPluginCommand::~TestPluginCommand()
 {
     if (request.methodLine == "testescalate") {
@@ -200,6 +215,15 @@ bool TestPluginCommand::peek(SQLite& db) {
         }
         response.content = "this is a test response";
         return true;
+    } else if (SStartsWith(request.methodLine, "EscalateSerializedData")) {
+        // Only set this if it's blank. The intention is that it will be blank on a follower, but already set by
+        // serialize/deserialize on the leader.
+        if (serializedDataString.empty()) {
+            serializedDataString = _plugin->server.args["-nodeName"] + ":" + _plugin->server.args["-testName"];
+        }
+        // On followers, this immediately escalates to leader.
+        // On leader, it immediately skips to `process`.
+        return false;
     } else if (SStartsWith(request.methodLine, "broadcastwithtimeouts")) {
         // First, send a `broadcastwithtimeouts` which will generate a new command and broadcast that to peers.
         SData subCommand("storeboradcasttimeouts");
@@ -391,6 +415,10 @@ void TestPluginCommand::process(SQLite& db) {
             querySize += nq.size();
         }
         response["QuerySize"] = to_string(querySize);
+    } else if (SStartsWith(request.methodLine,"EscalateSerializedData")) {
+        // We want to return the data that was serialized and escalated, and also our own nodename, to verify it does not match
+        // the node that serialized the data.
+        response.content = _plugin->server.args["-nodeName"] + ":" + serializedDataString;
     } else if (SStartsWith(request.methodLine, "sendrequest")) {
         // This flag makes us pass through the response we got from the server, rather than returning 200 if every
         // response we got from the server was < 400. I.e., if the server returns 202, or 304, or anything less than

test/clustertest/testplugin/TestPlugin.h

@@ -53,10 +53,14 @@ class TestPluginCommand : public BedrockCommand {
     virtual void reset(BedrockCommand::STAGE stage) override;
     bool shouldEnableOnPrepareNotification(const SQLite& db, void (**handler)(SQLite& _db, int64_t tableID)) override;
 
+  virtual string serializeData() const override;
+  virtual void deserializeData(const string& data) override;
+
   private:
     BedrockPlugin_TestPlugin& plugin() { return static_cast<BedrockPlugin_TestPlugin&>(*_plugin); }
 
     bool pendingResult;
     string chainedHTTPResponseContent;
     string urls;
+    string serializedDataString;
 };

test/clustertest/tests/EscalateTest.cpp

@@ -1,10 +1,12 @@
+#include "libstuff/libstuff.h"
 #include <libstuff/SData.h>
 #include <test/clustertest/BedrockClusterTester.h>
 
 struct EscalateTest : tpunit::TestFixture {
     EscalateTest() : tpunit::TestFixture("Escalate", BEFORE_CLASS(EscalateTest::setup),
                                                      AFTER_CLASS(EscalateTest::teardown),
                                                      TEST(EscalateTest::test),
+                                                     TEST(EscalateTest::testSerializedData),
                                                      TEST(EscalateTest::socketReuse)) { }
 
     BedrockClusterTester* tester = nullptr;
@@ -46,6 +48,26 @@ struct EscalateTest : tpunit::TestFixture {
         SFileDelete(cmd["tempFile"]);
     }
 
+    void testSerializedData()
+    {
+        // We're going to escalate from follower 1.
+        BedrockTester& brtester = tester->getTester(1);
+        SData cmd("EscalateSerializedData");
+        SData result = brtester.executeWaitMultipleData({cmd})[0];
+
+        // Parse the results, which should contain the node name we escalated from,
+        // the node name we escalated to, and the test name.
+        auto resultComponenets = SParseList(result.content, ':');
+
+        // Validate the results.
+        ASSERT_EQUAL("cluster_node_0", resultComponenets.front());
+        resultComponenets.pop_front();
+        ASSERT_EQUAL("cluster_node_1", resultComponenets.front());
+        resultComponenets.pop_front();
+        ASSERT_EQUAL("Escalate", resultComponenets.front());
+        resultComponenets.pop_front();
+    }
+
     // Note: see this PR: https://github.com/Expensify/Bedrock/pull/1308 for the reasoning behind this test.
     void socketReuse()
     {