[tor-commits] [stem/master] Revisions for prior SETCONF and RESETCONF additions

atagar at torproject.org atagar at torproject.org
Sun Jul 8 20:14:44 UTC 2012


commit f9b370c455d4128aabe73702a7a445dfe17d4a2f
Author: Damian Johnson <atagar at torproject.org>
Date:   Sat Jul 7 18:50:17 2012 -0700

    Revisions for prior SETCONF and RESETCONF additions
    
    Several revisions to the prior changes, the most prominent of which is to use
    an API more similar to arm's. The SETCONF and RESETCONF methods are
    functionally identical except how they handle undefined values, so there's
    little reason to provide separate methods in our controller. Just using an
    optional 'reset' arg instead.
    
    One thing that arm got wrong (and so did stem) is that our method to change
    multiple options should accept a dict rather than a tuple list. Order *might*
    matter for the accursed HiddenService options, but for the 99.9% use case a
    tuple list is a very strange argument to accept for this.
    
    This also includes some testing fixes, for instance...
    * Testing when we have a list argument (for instance, setting our ExitPolicy).
    * Failing our test if one of the calls that should fail actually succeeds.
    * Reverting the configuration options that we change after the test.
    * An identical test for context sensitive options was added to both the getconf
      and setconf test. Definitely a good use case to check and it could belong
      with either setconf or getconf, but doing the same test twice is pointless.
---
 stem/control.py                  |  114 +++++++++++++----------------
 stem/response/__init__.py        |   13 ++--
 test/integ/control/controller.py |  150 +++++++++++++++-----------------------
 test/unit/response/singleline.py |    4 +-
 4 files changed, 119 insertions(+), 162 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index 7eca284..f377cd3 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -581,9 +581,7 @@ class Controller(BaseController):
       return default if default != UNDEFINED else None
     
     entries = self.get_conf_map(param, default, multiple)
-    
-    if entries == default: return default
-    else: return _case_insensitive_lookup(entries, param)
+    return _case_insensitive_lookup(entries, param, default)
   
   def get_conf_map(self, param, default = UNDEFINED, multiple = True):
     """
@@ -666,46 +664,22 @@ class Controller(BaseController):
       if default != UNDEFINED: return default
       else: raise exc
   
-  def _set_conf(self, params, command="SETCONF"):
-    controlitems = [command]
-    
-    for key, value in params:
-      if type(value) in (list, tuple):
-        controlitems.extend(["%s=\"%s\"" % (key, val.strip()) for val in value])
-      elif value:
-        controlitems.append("%s=\"%s\"" % (key, value.strip()))
-      else:
-        controlitems.append(key)
-    
-    response = self.msg(" ".join(controlitems))
-    stem.response.convert("SINGLELINE", response)
-    
-    if response.is_ok():
-      return True
-    elif response.code == "552":
-      if response.message.startswith("Unrecognized option: Unknown option '"):
-        key = response.message[37:response.message.find("\'", 37)]
-        raise stem.socket.InvalidArguments(response.code, response.message, [key])
-      raise stem.socket.InvalidRequest(response.code, response.message)
-    elif response.code in ("513", "553"):
-      raise stem.socket.InvalidRequest(response.code, response.message)
-    else:
-      raise stem.socket.ProtocolError("%s returned unexpected status code" % command)
-  
-  def set_conf(self, *params):
+  def set_option(self, param, value, reset = False):
     """
-    Changes the configuration of one or more configuration variables using the
-    control socket.
+    Changes the value of a tor configuration option via either a SETCONF or
+    RESETCONF query. Both behave identically unless our value is None, in which
+    case SETCONF sets the value to 0 or NULL, and RESETCONF returns it to its
+    default value.
     
-    :param list,dict params: Can be one of...
+    Our value can be any of the following...
     
-      * a list containing tuples of configuration keys (string) and their corresponding values (string or list of strings)
-      * a dict containing config key-value pairs
+    * a string to set a single value
+    * a list of strings to set a series of values (for instance the ExitPolicy)
+    * None to either set the value to 0/NULL or reset it to its default
     
-    Or
-    
-    :param str key: configuration key
-    :param str value: configuration value
+    :param str param: configuration option to be set
+    :param str,list value: value to set the parameter to
+    :param bool reset: issues a SETCONF if False, and RESETCONF if True
     
     :raises:
       :class:`stem.socket.ControllerError` if the call fails
@@ -713,28 +687,24 @@ class Controller(BaseController):
       :class:`stem.socket.InvalidRequest` if the configuration setting is impossible or if there's a syntax error in the configuration values
     """
     
-    if len(params) == 2:
-      arg = [params]
-    elif len(params) == 1:
-      if type(params[0]) == dict:
-        arg = params[0].items()
-      else:
-        arg = params[0]
-    else:
-      raise TypeError("set_conf expected 1 or 2 arguments, got %d", len(args))
-    
-    self._set_conf(arg, "SETCONF")
+    self.set_options({param: value}, reset)
   
-  def reset_conf(self, params):
+  def set_options(self, params, reset = False):
     """
-    Resets the configuration of one or more configuration variables using the
-    control socket.
+    Changes multiple tor configurations, in a similar fashion to
+    :func:`stem.control.Controller.set_option`. For example...
+    
+    ::
     
-    :param str,list,dict params: Can be one of...
+      my_controller.set_options({
+        "Nickname", "caerSidi",
+        "ExitPolicy": ["accept *:80", "accept *:443", "reject *:*"],
+        "ContactInfo": "caerSidi-exit at someplace.com",
+        "Log": None,
+      })
     
-      * a single configuration key (string)
-      * a list of configuration keys (list of strings) to be reset and/or tuples of configuration keys (string) and their corresponding values (string or list of strings)
-      * a dict containing configuration key/value pairs.
+    :param dict params: mapping of configuration options to the values we're setting it to
+    :param bool reset: issues a SETCONF if False, and RESETCONF if True
     
     :raises:
       :class:`stem.socket.ControllerError` if the call fails
@@ -742,14 +712,30 @@ class Controller(BaseController):
       :class:`stem.socket.InvalidRequest` if the configuration setting is impossible or if there's a syntax error in the configuration values
     """
     
-    if type(params) == str:
-      arg = [(params, None)]
-    elif type(params) == dict:
-      arg = params.items()
-    else:
-      arg = map(lambda item: (item, None) if type(item) == str else item, params)
+    # constructs the SETCONF or RESETCONF query
+    query_comp = ["RESETCONF" if reset else "SETCONF"]
     
-    self._set_conf(arg, "RESETCONF")
+    for param, value in params.items():
+      if isinstance(value, str):
+        query_comp.append("%s=\"%s\"" % (param, value.strip()))
+      elif value:
+        query_comp.extend(["%s=\"%s\"" % (param, val.strip()) for val in value])
+      else:
+        query_comp.append(param)
+    
+    response = self.msg(" ".join(query_comp))
+    stem.response.convert("SINGLELINE", response)
+    
+    if not response.is_ok():
+      if response.code == "552":
+        if response.message.startswith("Unrecognized option: Unknown option '"):
+          key = response.message[37:response.message.find("\'", 37)]
+          raise stem.socket.InvalidArguments(response.code, response.message, [key])
+        raise stem.socket.InvalidRequest(response.code, response.message)
+      elif response.code in ("513", "553"):
+        raise stem.socket.InvalidRequest(response.code, response.message)
+      else:
+        raise stem.socket.ProtocolError("%s returned unexpected status code" % command)
 
 def _case_insensitive_lookup(entries, key, default = UNDEFINED):
   """
diff --git a/stem/response/__init__.py b/stem/response/__init__.py
index 2ae0b82..1d96988 100644
--- a/stem/response/__init__.py
+++ b/stem/response/__init__.py
@@ -421,11 +421,12 @@ def _get_quote_indeces(line, escaped):
 
 class SingleLineResponse(ControlMessage):
   """
-  A reply that contains only a single line
+  Reply to a request that performs an action rather than querying data. These
+  requests only contain a single line, which is 'OK' if successful, and a
+  description of the problem if not.
   
-  :var dict entries:
-    mapping between the queried options (string) and their values (string/list
-    of strings)
+  :var str code: status code for our line
+  :var str message: content of the line
   """
   
   def is_ok(self, strict = False):
@@ -433,7 +434,7 @@ class SingleLineResponse(ControlMessage):
     Checks if the response code is "250". If strict is True, checks if the
     response is "250 OK"
     
-    :param bool strict: If True, check if the message is "250 OK"
+    :param bool strict: checks for a "250 OK" message if True
     
     :returns:
       * If strict is False: True if the response code is "250", False otherwise
@@ -452,5 +453,5 @@ class SingleLineResponse(ControlMessage):
     elif len(content) == 0:
       raise stem.socket.ProtocolError("Received empty response")
     else:
-      self.code, self.delimiter, self.message = content[0]
+      self.code, _, self.message = content[0]
 
diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py
index eb2386c..b7eea7e 100644
--- a/test/integ/control/controller.py
+++ b/test/integ/control/controller.py
@@ -193,104 +193,72 @@ class TestController(unittest.TestCase):
       self.assertEqual("la-di-dah", controller.get_conf("", "la-di-dah"))
       self.assertEqual({}, controller.get_conf_map("", "la-di-dah"))
       self.assertEqual({}, controller.get_conf_map([], "la-di-dah"))
-      
-      # context-sensitive keys
-      tmpdir = tempfile.mkdtemp()
-      keys = [
-          ("HiddenServiceDir", tmpdir),
-          ("HiddenServicePort", "17234 127.0.0.1:17235")
-          ]
-      controller.set_conf(keys)
-      self.assertEqual(tmpdir, controller.get_conf("HiddenServiceDir"))
-      self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("HiddenServicePort"))
-      shutil.rmtree(tmpdir)
   
-  def test_setconf(self):
+  def test_set_options(self):
     """
-    Exercises Controller.set_conf with valid and invalid requests.
+    Exercises set_option() and set_options() with valid and invalid requests.
     """
     
-    runner = test.runner.get_runner()
-    
-    with runner.get_tor_controller() as controller:
-      # Single key, valid and invalid
-      connlimit = int(controller.get_conf("ConnLimit"))
-      controller.set_conf("connlimit", str(connlimit - 1))
-      self.assertEqual(connlimit - 1, int(controller.get_conf("ConnLimit")))
-      try:
-        controller.set_conf("invalidkeyboo", "abcde")
-      except stem.socket.InvalidArguments, exc:
-        self.assertEqual(["invalidkeyboo"], exc.arguments)
-      
-      settings = {
-          "connlimit": str(connlimit - 2),
-          "contactinfo": "stem at testing"
-          }
-      controller.set_conf(settings)
-      self.assertEqual(connlimit - 2, int(controller.get_conf("ConnLimit")))
-      self.assertEqual("stem at testing", controller.get_conf("contactinfo"))
-      
-      settings["bombay"] = "vadapav"
-      try:
-        controller.set_conf(settings)
-      except stem.socket.InvalidArguments, exc:
-        self.assertEqual(["bombay"], exc.arguments)
-      
-      tmpdir = tempfile.mkdtemp()
-      settings = [
-          ("HiddenServiceDir", tmpdir),
-          ("HiddenServicePort", "17234 127.0.0.1:17235")
-          ]
-      controller.set_conf(settings)
-      self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("hiddenserviceport"))
-      self.assertEqual(tmpdir, controller.get_conf("hiddenservicedir"))
-      shutil.rmtree(tmpdir)
-  
-  def test_resetconf(self):
-    """
-    Exercises Controller.reset_conf with valid and invalid requests.
-    """
+    if test.runner.require_control(self): return
     
     runner = test.runner.get_runner()
+    tmpdir = tempfile.mkdtemp()
     
     with runner.get_tor_controller() as controller:
-      # Single valid key
-      controller.set_conf("contactinfo", "stem at testing")
-      self.assertEqual("stem at testing", controller.get_conf("contactinfo"))
-      controller.reset_conf("contactinfo")
-      self.assertEqual(None, controller.get_conf("contactinfo"))
-      
-      # Invalid key
       try:
-        controller.reset_conf(("invalidkeyboo", "abcde"))
-      except stem.socket.InvalidArguments, exc:
-        self.assertEqual(["invalidkeyboo"], exc.arguments)
-      
-      # Multiple keys, list & dict
-      settings = {
-          "connlimit": "314",
-          "contactinfo": "stem at testing"
-          }
-      controller.reset_conf(settings)
-      self.assertEqual("314", controller.get_conf("ConnLimit"))
-      self.assertEqual("stem at testing", controller.get_conf("contactinfo"))
-      
-      settings = [
-          ("connlimit", "786"),
-          ("contactinfo", "stem testing")
-          ]
-      controller.reset_conf(settings)
-      self.assertEqual("786", controller.get_conf("ConnLimit"))
-      self.assertEqual("stem testing", controller.get_conf("contactinfo"))
-      
-      # context-sensitive keys
-      tmpdir = tempfile.mkdtemp()
-      settings = [
-          ("HiddenServiceDir", tmpdir),
-          ("HiddenServicePort", "17234 127.0.0.1:17235")
-          ]
-      controller.reset_conf(settings)
-      self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("hiddenserviceport"))
-      self.assertEqual(tmpdir, controller.get_conf("hiddenservicedir"))
-      shutil.rmtree(tmpdir)
+        # successfully set a single option
+        connlimit = int(controller.get_conf("ConnLimit"))
+        controller.set_option("connlimit", str(connlimit - 1))
+        self.assertEqual(connlimit - 1, int(controller.get_conf("ConnLimit")))
+        
+        # successfully set a single list option
+        exit_policy = ["accept *:7777", "reject *:*"]
+        controller.set_option("ExitPolicy", exit_policy)
+        self.assertEqual(exit_policy, controller.get_conf("ExitPolicy", multiple = True))
+        
+        # fail to set a single option
+        try:
+          controller.set_option("invalidkeyboo", "abcde")
+          self.fail()
+        except stem.socket.InvalidArguments, exc:
+          self.assertEqual(["invalidkeyboo"], exc.arguments)
+        
+        # successfully sets multiple config options
+        controller.set_options({
+          "connlimit": str(connlimit - 2),
+          "contactinfo": "stem at testing",
+        })
+        
+        self.assertEqual(connlimit - 2, int(controller.get_conf("ConnLimit")))
+        self.assertEqual("stem at testing", controller.get_conf("contactinfo"))
+        
+        # fail to set multiple config options
+        try:
+          controller.set_options({
+            "contactinfo": "stem at testing",
+            "bombay": "vadapav",
+          })
+          self.fail()
+        except stem.socket.InvalidArguments, exc:
+          self.assertEqual(["bombay"], exc.arguments)
+        
+        # context-sensitive keys
+        controller.set_options({
+          "HiddenServiceDir": tmpdir,
+          "HiddenServicePort": "17234 127.0.0.1:17235",
+        })
+        
+        self.assertEqual(tmpdir, controller.get_conf("HiddenServiceDir"))
+        self.assertEqual("17234 127.0.0.1:17235", controller.get_conf("HiddenServicePort"))
+      finally:
+        # reverts configuration changes
+        controller.set_options({
+          "ExitPolicy": "reject *:*",
+          "ConnLimit": None,
+          "ContactInfo": None,
+          "HiddenServiceDir": None,
+          "HiddenServicePort": None,
+        }, reset = True)
+        
+        shutil.rmtree(tmpdir)
 
diff --git a/test/unit/response/singleline.py b/test/unit/response/singleline.py
index 8781bf9..aa9c915 100644
--- a/test/unit/response/singleline.py
+++ b/test/unit/response/singleline.py
@@ -1,5 +1,5 @@
 """
-Unit tests for the stem.response.getconf.GetConfResponse class.
+Unit tests for the stem.response.SingleLineResponse class.
 """
 
 import unittest
@@ -16,6 +16,7 @@ class TestSingleLineResponse(unittest.TestCase):
     message = mocking.get_message("552 NOTOK")
     stem.response.convert("SINGLELINE", message)
     self.assertEqual(False, message.is_ok())
+    
     message = mocking.get_message("250 KK")
     stem.response.convert("SINGLELINE", message)
     self.assertEqual(True, message.is_ok())
@@ -23,6 +24,7 @@ class TestSingleLineResponse(unittest.TestCase):
     message = mocking.get_message("250 OK")
     stem.response.convert("SINGLELINE", message)
     self.assertEqual(True, message.is_ok(True))
+    
     message = mocking.get_message("250 HMM")
     stem.response.convert("SINGLELINE", message)
     self.assertEqual(False, message.is_ok(True))





More information about the tor-commits mailing list