From 7f8bbcc15537144e48b184e3d47e2003ed291678 Mon Sep 17 00:00:00 2001
From: david-sarah <david-sarah@jacaranda.org>
Date: Sun, 20 Nov 2011 23:24:26 +0000
Subject: [PATCH] Use a private/drop_upload_dircap file instead of the
 [drop_upload]upload.dircap option in tahoe.cfg. Fail if the upload.dircap
 option is used, or options are missing. Also updates tests and docs. fixes
 #1593

---
 docs/frontends/drop-upload.rst       | 20 ++++++++---------
 src/allmydata/client.py              | 28 ++++++++++++-----------
 src/allmydata/node.py                | 19 +++++++++++-----
 src/allmydata/scripts/create_node.py |  4 ++--
 src/allmydata/test/test_client.py    | 33 ++++++++++++++++------------
 5 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/docs/frontends/drop-upload.rst b/docs/frontends/drop-upload.rst
index 02746882..1b6b0937 100644
--- a/docs/frontends/drop-upload.rst
+++ b/docs/frontends/drop-upload.rst
@@ -20,8 +20,8 @@ Tahoe-LAFS Summit in June 2011, and is not currently in as mature a state as
 the other frontends (web, CLI, FTP and SFTP). This means that you probably
 should not keep important data in the upload directory, and should not rely
 on all changes to files in the local directory to result in successful uploads.
-There might be incompatible changes to how the feature is configured in
-future versions. There is even the possibility that it may be abandoned, for
+There might be (and have been) incompatible changes to how the feature is
+configured. There is even the possibility that it may be abandoned, for
 example if unsolveable reliability issues are found.
 
 We are very interested in feedback on how well this feature works for you, and
@@ -42,15 +42,8 @@ gateway's ``tahoe.cfg`` file.
 
 ``enabled = (boolean, optional)``
 
-    If this is ``True``, drop-upload will be enabled (provided that the
-    ``upload.dircap`` and ``local.directory`` fields are also set). The
-    default value is ``False``.
-
-``upload.dircap = (directory writecap)``
-
-    This is a writecap pointing to an existing mutable directory to be used
-    as the target of uploads. It will start with ``URI:DIR2:``, and cannot
-    include an alias or path.
+    If this is ``True``, drop-upload will be enabled. The default value is
+    ``False``.
 
 ``local.directory = (UTF-8 path)``
 
@@ -59,6 +52,11 @@ gateway's ``tahoe.cfg`` file.
     in UTF-8 regardless of the system's filesystem encoding. Relative paths
     will be interpreted starting from the node's base directory.
 
+In addition, the file  ``private/drop_upload_dircap`` must contain a
+writecap pointing to an existing mutable directory to be used as the target
+of uploads. It will start with ``URI:DIR2:``, and cannot include an alias
+or path.
+
 After setting the above fields and starting or restarting the gateway,
 you can confirm that the feature is working by copying a file into the
 local directory. Then, use the WUI or CLI to check that it has appeared
diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 813ee395..b78fd7b9 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -26,6 +26,7 @@ from allmydata.interfaces import IStatsProducer, RIStubClient, \
                                  SDMF_VERSION, MDMF_VERSION
 from allmydata.nodemaker import NodeMaker
 from allmydata.blacklist import Blacklist
+from allmydata.node import OldConfigOptionError
 
 
 KiB=1024
@@ -439,19 +440,20 @@ class Client(node.Node, pollmixin.PollMixin):
 
     def init_drop_uploader(self):
         if self.get_config("drop_upload", "enabled", False, boolean=True):
-            upload_dircap = self.get_config("drop_upload", "upload.dircap", None)
-            local_dir_utf8 = self.get_config("drop_upload", "local.directory", None)
-
-            if upload_dircap and local_dir_utf8:
-                try:
-                    from allmydata.frontends import drop_upload
-                    s = drop_upload.DropUploader(self, upload_dircap, local_dir_utf8)
-                    s.setServiceParent(self)
-                    s.startService()
-                except Exception, e:
-                    self.log("couldn't start drop-uploader: %r", args=(e,))
-            else:
-                self.log("couldn't start drop-uploader: upload.dircap or local.directory not specified")
+            if self.get_config("drop_upload", "upload.dircap", None):
+                raise OldConfigOptionError("The [drop_upload]upload.dircap option is no longer supported; please "
+                                           "put the cap in a 'private/drop_upload_dircap' file, and delete this option.")
+
+            upload_dircap = self.get_or_create_private_config("drop_upload_dircap")
+            local_dir_utf8 = self.get_config("drop_upload", "local.directory")
+
+            try:
+                from allmydata.frontends import drop_upload
+                s = drop_upload.DropUploader(self, upload_dircap, local_dir_utf8)
+                s.setServiceParent(self)
+                s.startService()
+            except Exception, e:
+                self.log("couldn't start drop-uploader: %r", args=(e,))
 
     def _check_hotline(self, hotline_file):
         if os.path.exists(hotline_file):
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index eefce3d6..0b843650 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -52,6 +52,9 @@ class OldConfigError(Exception):
                 "See docs/historical/configuration.rst."
                 % "\n".join([quote_output(fname) for fname in self.args[0]]))
 
+class OldConfigOptionError(Exception):
+    pass
+
 
 class Node(service.MultiService):
     # this implements common functionality of both Client nodes and Introducer
@@ -201,21 +204,27 @@ class Node(service.MultiService):
         privname = os.path.join(self.basedir, "private", name)
         open(privname, "w").write(value.strip())
 
-    def get_or_create_private_config(self, name, default):
+    def get_or_create_private_config(self, name, default=_None):
         """Try to get the (string) contents of a private config file (which
         is a config file that resides within the subdirectory named
         'private'), and return it. Any leading or trailing whitespace will be
         stripped from the data.
 
-        If the file does not exist, try to create it using default, and
-        then return the value that was written. If 'default' is a string,
-        use it as a default value. If not, treat it as a 0-argument callable
-        which is expected to return a string.
+        If the file does not exist, and default is not given, report an error.
+        If the file does not exist and a default is specified, try to create
+        it using that default, and then return the value that was written.
+        If 'default' is a string, use it as a default value. If not, treat it
+        as a zero-argument callable that is expected to return a string.
         """
         privname = os.path.join(self.basedir, "private", name)
         try:
             value = fileutil.read(privname)
         except EnvironmentError:
+            if os.path.exists(privname):
+                raise
+            if default is _None:
+                raise MissingConfigEntry("The required configuration file %s is missing."
+                                         % (quote_output(privname),))
             if isinstance(default, basestring):
                 value = default
             else:
diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py
index 294f6dd8..7e9dcf79 100644
--- a/src/allmydata/scripts/create_node.py
+++ b/src/allmydata/scripts/create_node.py
@@ -155,8 +155,8 @@ def create_node(config, out=sys.stdout, err=sys.stderr):
     c.write("[drop_upload]\n")
     c.write("# Shall this node automatically upload files created or modified in a local directory?\n")
     c.write("enabled = false\n")
-    c.write("# This must be a mutable directory writecap.\n")
-    c.write("upload.dircap =\n")
+    c.write("# To specify the target of uploads, a mutable directory writecap URI must be placed\n"
+            "# in 'private/drop_upload_dircap'.\n")
     c.write("local.directory = ~/drop_upload\n")
     c.write("\n")
 
diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py
index 26af5118..14c9c791 100644
--- a/src/allmydata/test/test_client.py
+++ b/src/allmydata/test/test_client.py
@@ -3,7 +3,7 @@ from twisted.trial import unittest
 from twisted.application import service
 
 import allmydata
-from allmydata.node import OldConfigError
+from allmydata.node import OldConfigError, OldConfigOptionError, MissingConfigEntry
 from allmydata import client
 from allmydata.storage_client import StorageFarmBroker
 from allmydata.util import base32, fileutil
@@ -191,13 +191,24 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase):
                   "[storage]\n" +
                   "enabled = false\n" +
                   "[drop_upload]\n" +
-                  "enabled = true\n" +
-                  "upload.dircap = " + upload_dircap + "\n" +
-                  "local.directory = " + local_dir_utf8 + "\n")
+                  "enabled = true\n")
 
         basedir1 = "test_client.Basic.test_create_drop_uploader1"
         os.mkdir(basedir1)
+        fileutil.write(os.path.join(basedir1, "tahoe.cfg"),
+                       config + "local.directory = " + local_dir_utf8 + "\n")
+        self.failUnlessRaises(MissingConfigEntry, client.Client, basedir1)
+
         fileutil.write(os.path.join(basedir1, "tahoe.cfg"), config)
+        fileutil.write(os.path.join(basedir1, "private", "drop_upload_dircap"), "URI:DIR2:blah")
+        self.failUnlessRaises(MissingConfigEntry, client.Client, basedir1)
+
+        fileutil.write(os.path.join(basedir1, "tahoe.cfg"),
+                       config + "upload.dircap = " + upload_dircap + "\n")
+        self.failUnlessRaises(OldConfigOptionError, client.Client, basedir1)
+
+        fileutil.write(os.path.join(basedir1, "tahoe.cfg"),
+                       config + "local.directory = " + local_dir_utf8 + "\n")
         c1 = client.Client(basedir1)
         uploader = c1.getServiceNamed('drop-upload')
         self.failUnless(isinstance(uploader, MockDropUploader), uploader)
@@ -213,21 +224,15 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase):
 
         basedir2 = "test_client.Basic.test_create_drop_uploader2"
         os.mkdir(basedir2)
+        os.mkdir(os.path.join(basedir2, "private"))
         fileutil.write(os.path.join(basedir2, "tahoe.cfg"),
                        BASECONFIG +
                        "[drop_upload]\n" +
-                       "enabled = true\n")
+                       "enabled = true\n" +
+                       "local.directory = " + local_dir_utf8 + "\n")
+        fileutil.write(os.path.join(basedir2, "private", "drop_upload_dircap"), "URI:DIR2:blah")
         c2 = client.Client(basedir2)
         self.failUnlessRaises(KeyError, c2.getServiceNamed, 'drop-upload')
-        self.failIf([True for arg in mock_log_msg.call_args_list if "Boom" in repr(arg)],
-                    mock_log_msg.call_args_list)
-        self.failUnless([True for arg in mock_log_msg.call_args_list if "upload.dircap or local.directory not specified" in repr(arg)],
-                        mock_log_msg.call_args_list)
-
-        basedir3 = "test_client.Basic.test_create_drop_uploader3"
-        os.mkdir(basedir3)
-        fileutil.write(os.path.join(basedir3, "tahoe.cfg"), config)
-        client.Client(basedir3)
         self.failUnless([True for arg in mock_log_msg.call_args_list if "Boom" in repr(arg)],
                         mock_log_msg.call_args_list)
 
-- 
2.45.2