From 4646451de6c51442881c744ffc07c14176ae20be Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Wed, 18 Feb 2009 16:23:01 -0700
Subject: [PATCH] change StorageServer to take nodeid in the constructor,
 instead of assigning it later, since it's cleaner and because the original
 problem (Tubs not being ready until later) went away

---
 src/allmydata/client.py            |  2 +-
 src/allmydata/storage/server.py    | 17 ++++-------------
 src/allmydata/test/no_network.py   | 12 ++++++------
 src/allmydata/test/test_mutable.py |  3 +--
 src/allmydata/test/test_storage.py | 18 ++++++------------
 5 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 085b2917..a77bda4f 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -162,7 +162,7 @@ class Client(node.Node, pollmixin.PollMixin):
             reserved = 0
         discard = self.get_config("storage", "debug_discard", False,
                                   boolean=True)
-        ss = StorageServer(storedir,
+        ss = StorageServer(storedir, self.nodeid,
                            reserved_space=reserved,
                            discard_storage=discard,
                            readonly_storage=readonly,
diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py
index fdc2a1a6..a6580169 100644
--- a/src/allmydata/storage/server.py
+++ b/src/allmydata/storage/server.py
@@ -42,10 +42,13 @@ class StorageServer(service.MultiService, Referenceable):
     implements(RIStorageServer, IStatsProducer)
     name = 'storage'
 
-    def __init__(self, storedir, reserved_space=0,
+    def __init__(self, storedir, nodeid, reserved_space=0,
                  discard_storage=False, readonly_storage=False,
                  stats_provider=None):
         service.MultiService.__init__(self)
+        assert isinstance(nodeid, str)
+        assert len(nodeid) == 20
+        self.my_nodeid = nodeid
         self.storedir = storedir
         sharedir = os.path.join(storedir, "shares")
         fileutil.make_dirs(sharedir)
@@ -125,18 +128,6 @@ class StorageServer(service.MultiService, Referenceable):
             kwargs["facility"] = "tahoe.storage"
         return log.msg(*args, **kwargs)
 
-    def setNodeID(self, nodeid):
-        # somebody must set this before any slots can be created or leases
-        # added
-        self.my_nodeid = nodeid
-
-    def startService(self):
-        service.MultiService.startService(self)
-        if self.parent:
-            nodeid = self.parent.nodeid # 20 bytes, binary
-            assert len(nodeid) == 20
-            self.setNodeID(nodeid)
-
     def _clean_incomplete(self):
         fileutil.rm_dir(self.incomingdir)
 
diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py
index 1380720b..e6bee787 100644
--- a/src/allmydata/test/no_network.py
+++ b/src/allmydata/test/no_network.py
@@ -152,7 +152,7 @@ class NoNetworkGrid(service.MultiService):
             serverdir = os.path.join(basedir, "servers",
                                      idlib.shortnodeid_b2a(serverid))
             fileutil.make_dirs(serverdir)
-            ss = StorageServer(serverdir)
+            ss = StorageServer(serverdir, serverid)
             self.add_server(i, serverid, ss)
 
         for i in range(num_clients):
@@ -181,11 +181,11 @@ class NoNetworkGrid(service.MultiService):
             self.clients.append(c)
 
     def add_server(self, i, serverid, ss):
-        # TODO: ss.setServiceParent(self), but first remove the goofy
-        # self.parent.nodeid from Storage.startService . At the moment,
-        # Storage doesn't really need to be startService'd, but it will in
-        # the future.
-        ss.setNodeID(serverid)
+        # TODO: ss.setServiceParent(self), but first deal with the fact that
+        # all StorageServers are named 'storage'. At the moment, Storage
+        # doesn't really need to be startService'd, but it will in the
+        # future.
+        #ss.setServiceParent(self)
         self.servers_by_number[i] = ss
         self.servers_by_id[serverid] = wrap(ss, "storage")
         self.all_servers = frozenset(self.servers_by_id.items())
diff --git a/src/allmydata/test/test_mutable.py b/src/allmydata/test/test_mutable.py
index 7da3c018..16b63279 100644
--- a/src/allmydata/test/test_mutable.py
+++ b/src/allmydata/test/test_mutable.py
@@ -1804,8 +1804,7 @@ class LessFakeClient(FakeClient):
         for peerid in self._peerids:
             peerdir = os.path.join(basedir, idlib.shortnodeid_b2a(peerid))
             make_dirs(peerdir)
-            ss = StorageServer(peerdir)
-            ss.setNodeID(peerid)
+            ss = StorageServer(peerdir, peerid)
             lw = LocalWrapper(ss)
             self._connections[peerid] = lw
         self.nodeid = "fakenodeid"
diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index e8721b03..f2c9ac5b 100644
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -236,9 +236,8 @@ class Server(unittest.TestCase):
 
     def create(self, name, reserved_space=0, klass=StorageServer):
         workdir = self.workdir(name)
-        ss = klass(workdir, reserved_space=reserved_space,
+        ss = klass(workdir, "\x00" * 20, reserved_space=reserved_space,
                    stats_provider=FakeStatsProvider())
-        ss.setNodeID("\x00" * 20)
         ss.setServiceParent(self.sparent)
         return ss
 
@@ -586,8 +585,7 @@ class Server(unittest.TestCase):
 
     def test_readonly(self):
         workdir = self.workdir("test_readonly")
-        ss = StorageServer(workdir, readonly_storage=True)
-        ss.setNodeID("\x00" * 20)
+        ss = StorageServer(workdir, "\x00" * 20, readonly_storage=True)
         ss.setServiceParent(self.sparent)
 
         already,writers = self.allocate(ss, "vid", [0,1,2], 75)
@@ -606,8 +604,7 @@ class Server(unittest.TestCase):
     def test_discard(self):
         # discard is really only used for other tests, but we test it anyways
         workdir = self.workdir("test_discard")
-        ss = StorageServer(workdir, discard_storage=True)
-        ss.setNodeID("\x00" * 20)
+        ss = StorageServer(workdir, "\x00" * 20, discard_storage=True)
         ss.setServiceParent(self.sparent)
 
         canary = FakeCanary()
@@ -626,8 +623,7 @@ class Server(unittest.TestCase):
 
     def test_advise_corruption(self):
         workdir = self.workdir("test_advise_corruption")
-        ss = StorageServer(workdir, discard_storage=True)
-        ss.setNodeID("\x00" * 20)
+        ss = StorageServer(workdir, "\x00" * 20, discard_storage=True)
         ss.setServiceParent(self.sparent)
 
         si0_s = base32.b2a("si0")
@@ -685,9 +681,8 @@ class MutableServer(unittest.TestCase):
 
     def create(self, name):
         workdir = self.workdir(name)
-        ss = StorageServer(workdir)
+        ss = StorageServer(workdir, "\x00" * 20)
         ss.setServiceParent(self.sparent)
-        ss.setNodeID("\x00" * 20)
         return ss
 
     def test_create(self):
@@ -1235,8 +1230,7 @@ class Stats(unittest.TestCase):
 
     def create(self, name):
         workdir = self.workdir(name)
-        ss = StorageServer(workdir)
-        ss.setNodeID("\x00" * 20)
+        ss = StorageServer(workdir, "\x00" * 20)
         ss.setServiceParent(self.sparent)
         return ss
 
-- 
2.45.2