simplify _tub_portlocation helper

src/allmydata/node.py

@@ -510,10 +510,16 @@ def _convert_tub_port(s):
     return s
 
 
-def _tub_portlocation(config, cfg_tubport, cfg_location):
+def _tub_portlocation(config):
-    # return None, or tuple of (port, location)
+    """
-    tubport_disabled = False
+    :returns: None or tuple of (port, location) for the main tub based
+        on the given configuration. May raise ValueError or PrivacyError
+        if there are problems with the config
+    """
+    cfg_tubport = config.get_config("node", "tub.port", None)
+    cfg_location = config.get_config("node", "tub.location", None)
     reveal_ip = config.get_config("node", "reveal-IP-address", True, boolean=True)
+    tubport_disabled = False
 
     if cfg_tubport is not None:
         cfg_tubport = cfg_tubport.strip()
@@ -592,9 +598,7 @@ def create_main_tub(config, tub_options,
                     default_connection_handlers, foolscap_connection_handlers,
                     i2p_provider, tor_provider,
                     handler_overrides={}, cert_filename="node.pem"):
-    cfg_tubport = config.get_config("node", "tub.port", None)
+    portlocation = _tub_portlocation(config)
-    cfg_location = config.get_config("node", "tub.location", None)
-    portlocation = _tub_portlocation(config, cfg_tubport, cfg_location)
 
     certfile = config.get_private_path("node.pem")  # FIXME? "node.pem" was the CERTFILE option/thing
     tub = create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,

src/allmydata/test/test_connections.py

@@ -443,29 +443,15 @@ class Privacy(unittest.TestCase):
             str(ctx.exception),
             "tub.location uses AUTO",
         )
-        return
-        n = FakeNode(BASECONFIG+"[node]\nreveal-IP-address = false\n")
-        n._portnumfile = "missing"
-        n.check_privacy()
-        e = self.assertRaises(PrivacyError, n.get_tub_portlocation,
-                              None, "AUTO")
-        self.assertEqual(str(e), "tub.location uses AUTO")
-
-        n = FakeNode(BASECONFIG+"[node]\nreveal-IP-address = false\n")
-        n._portnumfile = "missing"
-        n.check_privacy()
-        e = self.assertRaises(PrivacyError, n.get_tub_portlocation,
-                              None, "AUTO,tcp:hostname:1234")
-        self.assertEqual(str(e), "tub.location uses AUTO")
 
     def test_tub_location_tcp(self):
         config = config_from_string(
-            BASECONFIG + "[node]\nreveal-IP-address = false\n",
+            BASECONFIG + "[node]\nreveal-IP-address = false\ntub.location=tcp:hostname:1234\n",
             "fake.port",
             "no-basedir",
         )
         with self.assertRaises(PrivacyError) as ctx:
-            _tub_portlocation(config, None, "tcp:hostname:1234")
+            _tub_portlocation(config)
         self.assertEqual(
             str(ctx.exception),
             "tub.location includes tcp: hint",
@@ -473,13 +459,13 @@ class Privacy(unittest.TestCase):
 
     def test_tub_location_legacy_tcp(self):
         config = config_from_string(
-            BASECONFIG + "[node]\nreveal-IP-address = false\n",
+            BASECONFIG + "[node]\nreveal-IP-address = false\ntub.location=hostname:1234\n",
             "fake.port",
             "no-basedir",
         )
 
         with self.assertRaises(PrivacyError) as ctx:
-            _tub_portlocation(config, None, "hostname:1234")
+            _tub_portlocation(config)
 
         self.assertEqual(
             str(ctx.exception),

src/allmydata/test/test_node.py

@@ -332,12 +332,15 @@ class TestMissingPorts(unittest.TestCase):
             "allmydata.util.iputil.allocate_tcp_port",
             return_value=999,
         )
-        config = read_config(self.basedir, "portnum")
+        config_data = (
+            "[node]\n"
+            "tub.port = tcp:777\n"
+            "tub.location = AUTO\n"
+        )
+        config = config_from_string(config_data, self.basedir, "portnum")
 
         with get_addr, alloc_port:
-            cfg_tubport = "tcp:777"
+            tubport, tublocation = _tub_portlocation(config)
-            cfg_location = "AUTO"
-            tubport, tublocation = _tub_portlocation(config, cfg_tubport, cfg_location)
         self.assertEqual(tubport, "tcp:777")
         self.assertEqual(tublocation, "tcp:LOCAL:777")
 
@@ -353,12 +356,13 @@ class TestMissingPorts(unittest.TestCase):
             "allmydata.util.iputil.allocate_tcp_port",
             return_value=999,
         )
-        config = read_config(self.basedir, "portnum")
+        config_data = (
+            "[node]\n"
+        )
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with get_addr, alloc_port:
-            cfg_tubport = None
+            tubport, tublocation = _tub_portlocation(config)
-            cfg_location = None
-            tubport, tublocation = _tub_portlocation(config, cfg_tubport, cfg_location)
         self.assertEqual(tubport, "tcp:999")
         self.assertEqual(tublocation, "tcp:LOCAL:999")
 
@@ -374,12 +378,14 @@ class TestMissingPorts(unittest.TestCase):
             "allmydata.util.iputil.allocate_tcp_port",
             return_value=999,
         )
-        config = read_config(self.basedir, "portnum")
+        config_data = (
+            "[node]\n"
+            "tub.location = tcp:HOST:888,AUTO\n"
+        )
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with get_addr, alloc_port:
-            cfg_tubport = None
+            tubport, tublocation = _tub_portlocation(config)
-            cfg_location = "tcp:HOST:888,AUTO"
-            tubport, tublocation = _tub_portlocation(config, cfg_tubport, cfg_location)
         self.assertEqual(tubport, "tcp:999")
         self.assertEqual(tublocation, "tcp:HOST:888,tcp:LOCAL:999")
 
@@ -395,12 +401,15 @@ class TestMissingPorts(unittest.TestCase):
             "allmydata.util.iputil.allocate_tcp_port",
             return_value=999,
         )
-        config = read_config(self.basedir, "portnum")
+        config_data = (
+            "[node]\n"
+            "tub.port = disabled\n"
+            "tub.location = disabled\n"
+        )
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with get_addr, alloc_port:
-            cfg_tubport = "disabled"
+            res = _tub_portlocation(config)
-            cfg_location = "disabled"
-            res = _tub_portlocation(config, cfg_tubport, cfg_location)
         self.assertTrue(res is None)
 
     def test_empty_tub_port(self):
@@ -414,7 +423,7 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config, "", None)
+            _tub_portlocation(config)
         self.assertIn(
             "tub.port must not be empty",
             str(ctx.exception)
@@ -431,7 +440,7 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config, None, "")
+            _tub_portlocation(config)
         self.assertIn(
             "tub.location must not be empty",
             str(ctx.exception)
@@ -449,7 +458,7 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config, "disabled", "not_disabled")
+            _tub_portlocation(config)
         self.assertIn(
             "tub.port is disabled, but not tub.location",
             str(ctx.exception)
@@ -467,7 +476,7 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config, "not_disabled", "disabled")
+            _tub_portlocation(config)
         self.assertIn(
             "tub.location is disabled, but not tub.port",
             str(ctx.exception)