Add unit test for client foolscap config flag.

src/allmydata/client.py

@@ -466,7 +466,7 @@ def create_introducer_clients(config, main_tub, _introducer_factory=None):
     return introducer_clients
 
 
-def create_storage_farm_broker(config, default_connection_handlers, foolscap_connection_handlers, tub_options, introducer_clients):
+def create_storage_farm_broker(config: _Config, default_connection_handlers, foolscap_connection_handlers, tub_options, introducer_clients):
     """
     Create a StorageFarmBroker object, for use by Uploader/Downloader
     (and everybody else who wants to use storage servers)

src/allmydata/storage_client.py

@@ -89,6 +89,7 @@ from allmydata.storage.http_client import (
     ClientException as HTTPClientException, StorageClientMutables,
     ReadVector, TestWriteVectors, WriteVector, TestVector, ClientException
 )
+from .node import _Config
 
 ANONYMOUS_STORAGE_NURLS = "anonymous-storage-NURLs"
 
@@ -199,7 +200,7 @@ class StorageFarmBroker(service.MultiService):
             self,
             permute_peers,
             tub_maker,
-            node_config,
+            node_config: _Config,
             storage_client_config=None,
     ):
         service.MultiService.__init__(self)
@@ -274,6 +275,16 @@ class StorageFarmBroker(service.MultiService):
             in self.storage_client_config.storage_plugins.items()
         })
 
+    @staticmethod
+    def _should_we_use_http(node_config: _Config, announcement: dict) -> bool:
+        """
+        Given an announcement dictionary and config, return whether we should
+        connect to storage server over HTTP.
+        """
+        return not node_config.get_config(
+            "client", "force_foolscap", default=True, boolean=True,
+        ) and len(announcement.get(ANONYMOUS_STORAGE_NURLS, [])) > 0
+
     @log_call(
         action_type=u"storage-client:broker:make-storage-server",
         include_args=["server_id"],
@@ -299,9 +310,7 @@ class StorageFarmBroker(service.MultiService):
             "pub-{}".format(str(server_id, "ascii")),  # server_id is v0-<key> not pub-v0-key .. for reasons?
         )
 
-        if not self.node_config.get_config(
+        if self._should_we_use_http(self.node_config, server["ann"]):
-                "client", "force_foolscap", default=True, boolean=True,
-        ) and len(server["ann"].get(ANONYMOUS_STORAGE_NURLS, [])) > 0:
             s = HTTPNativeStorageServer(
                 server_id,
                 server["ann"],

src/allmydata/test/test_storage_client.py

@@ -94,10 +94,13 @@ from allmydata.storage_client import (
     StorageFarmBroker,
     _FoolscapStorage,
     _NullStorage,
+    ANONYMOUS_STORAGE_NURLS
 )
 from ..storage.server import (
     StorageServer,
 )
+from ..client import config_from_string
+
 from allmydata.interfaces import (
     IConnectionStatus,
     IStorageServer,
@@ -739,3 +742,42 @@ storage:
 
         yield done
         self.assertTrue(done.called)
+
+    def test_should_we_use_http_default(self):
+        """Default is to not use HTTP; this will change eventually"""
+        basedir = self.mktemp()
+        node_config = config_from_string(basedir, "", "")
+        announcement = {ANONYMOUS_STORAGE_NURLS: ["pb://..."]}
+        self.assertFalse(
+            StorageFarmBroker._should_we_use_http(node_config, announcement)
+        )
+        self.assertFalse(
+            StorageFarmBroker._should_we_use_http(node_config, {})
+        )
+
+    def test_should_we_use_http(self):
+        """
+        If HTTP is allowed, it will only be used if the announcement includes
+        some NURLs.
+        """
+        basedir = self.mktemp()
+
+        no_nurls = {}
+        empty_nurls = {ANONYMOUS_STORAGE_NURLS: []}
+        has_nurls = {ANONYMOUS_STORAGE_NURLS: ["pb://.."]}
+
+        for force_foolscap, announcement, expected_http_usage in [
+                ("false", no_nurls, False),
+                ("false", empty_nurls, False),
+                ("false", has_nurls, True),
+                ("true", empty_nurls, False),
+                ("true", no_nurls, False),
+                ("true", has_nurls, False),
+        ]:
+            node_config = config_from_string(
+                basedir, "", f"[client]\nforce_foolscap = {force_foolscap}"
+            )
+            self.assertEqual(
+                StorageFarmBroker._should_we_use_http(node_config, announcement),
+                expected_http_usage
+            )