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,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
+There might be (and have been) incompatible changes to how the feature is
-future versions. There is even the possibility that it may be abandoned, for
+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
+    If this is ``True``, drop-upload will be enabled. The default value is
-    ``upload.dircap`` and ``local.directory`` fields are also set). The
+    ``False``.
-    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.
 
 ``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

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,10 +440,13 @@ 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)
+            if self.get_config("drop_upload", "upload.dircap", None):
-            local_dir_utf8 = self.get_config("drop_upload", "local.directory", 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")
 
-            if upload_dircap and local_dir_utf8:
             try:
                 from allmydata.frontends import drop_upload
                 s = drop_upload.DropUploader(self, upload_dircap, local_dir_utf8)
@@ -450,8 +454,6 @@ class Client(node.Node, pollmixin.PollMixin):
                 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")
 
     def _check_hotline(self, hotline_file):
         if os.path.exists(hotline_file):

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
+        If the file does not exist, and default is not given, report an error.
-        then return the value that was written. If 'default' is a string,
+        If the file does not exist and a default is specified, try to create
-        use it as a default value. If not, treat it as a 0-argument callable
+        it using that default, and then return the value that was written.
-        which is expected to return a string.
+        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:

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("# To specify the target of uploads, a mutable directory writecap URI must be placed\n"
-    c.write("upload.dircap =\n")
+            "# in 'private/drop_upload_dircap'.\n")
     c.write("local.directory = ~/drop_upload\n")
     c.write("\n")
 

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" +
+                  "enabled = true\n")
-                  "upload.dircap = " + upload_dircap + "\n" +
-                  "local.directory = " + local_dir_utf8 + "\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)