Merge lp:~thomir-deactivatedaccount/autopilot/fix-server-side-param-matching into lp:autopilot

[Thomi Richards]

Fix parameter matching for special characters.

Make the parameter matching match the deficiencies in xpathselect more closely.

[lp:~thomir-deactivatedaccount/autopilot/fix-server-side-param-matching]

307. By Thomi Richards on 2013-08-08

fix typo and pep8 issues.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:307
http://jenkins.qa.ubuntu.com/job/autopilot-ci/197/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-amd64-ci/125
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-armhf-ci/125

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-ci/197/rebuild

review: Approve (continuous-integration)

[lp:~thomir-deactivatedaccount/autopilot/fix-server-side-param-matching]

308. By Thomi Richards on 2013-08-08

Added initial test, fixed function to return a boolean, rather than a re.Match object.

309. By Thomi Richards on 2013-08-08

Added tests, fixed pep8 issues.

310. By Thomi Richards on 2013-08-08

remember to save files before comitting

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:310
http://jenkins.qa.ubuntu.com/job/autopilot-ci/199/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-amd64-ci/127
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-armhf-ci/127

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-ci/199/rebuild

review: Approve (continuous-integration)

[Christopher Lee (veebers)]

Awesome, looks good.

review: Approve

=== modified file 'autopilot/introspection/dbus.py'
--- autopilot/introspection/dbus.py	2013-08-07 03:18:32 +0000
+++ autopilot/introspection/dbus.py	2013-08-08 20:34:31 +0000
@@ -29,6 +29,7 @@
 
 from contextlib import contextmanager
 import logging
+import re
 from testtools.matchers import Equals
 from time import sleep
 from uuid import uuid4
@@ -391,7 +392,13 @@
 
         first_param = ''
         for k, v in kwargs.iteritems():
-            if isinstance(v, str) and '_' not in k:
+            # LP Bug 1209029: The XPathSelect protocol does not allow all valid
+            # node names or values. We need to decide here whether the filter
+            # parameters are going to work on the backend or not. If not, we
+            # just do the processing client-side. See the
+            # _is_valid_server_side_filter_param function (below) for the
+            # specific requirements.
+            if _is_valid_server_side_filter_param(k, v):
                 first_param = '[{}={}]'.format(k, v)
                 kwargs.pop(k)
                 break
@@ -550,6 +557,17 @@
             self.__refresh_on_attribute = True
 
 
+def _is_valid_server_side_filter_param(key, value):
+    """Return True if the key and value parameters are valid for server-side
+    processing.
+
+    """
+    return (
+        isinstance(value, str) and
+        re.match(r'^[a-zA-Z0-9_\-]+( [a-zA-Z0-9_\-])*$', key) is not None and
+        re.match(r'^[a-zA-Z0-9_\-]+( [a-zA-Z0-9_\-])*$', value) is not None)
+
+
 class _CustomEmulatorMeta(IntrospectableObjectMetaclass):
 
     def __new__(cls, name, bases, d):

=== modified file 'autopilot/tests/unit/test_introspection_features.py'
--- autopilot/tests/unit/test_introspection_features.py	2013-07-22 03:38:23 +0000
+++ autopilot/tests/unit/test_introspection_features.py	2013-08-08 20:34:31 +0000
@@ -20,8 +20,13 @@
 
 from testtools import TestCase
 from testtools.matchers import Equals, NotEquals
-
-from autopilot.introspection.dbus import CustomEmulatorBase
+from testscenarios import TestWithScenarios
+
+
+from autopilot.introspection.dbus import (
+    CustomEmulatorBase,
+    _is_valid_server_side_filter_param,
+)
 
 
 class IntrospectionFeatureTests(TestCase):
@@ -55,3 +60,23 @@
             pass
 
         self.assertThat(MyEmulatorBase._id, NotEquals(MyEmulatorBase2._id))
+
+
+class ServerSideParamMatchingTests(TestWithScenarios, TestCase):
+
+    """Tests for the server side matching decision function."""
+
+    scenarios = [
+        ('should work', dict(key='keyname', value='value', result=True)),
+        ('invalid key', dict(key='k  e', value='value', result=False)),
+        ('invalid value', dict(key='key', value='v  e', result=False)),
+        ('invalid value2', dict(key='key', value='v?e', result=False)),
+        ('invalid value3', dict(key='key', value='1/2', result=False)),
+        ('invalid value type', dict(key='key', value=False, result=False)),
+    ]
+
+    def test_valid_server_side_param(self):
+        self.assertThat(
+            _is_valid_server_side_filter_param(self.key, self.value),
+            Equals(self.result)
+        )