Merge ~sylvain-pineau/checkbox-ng:xml-removal into checkbox-ng:master

[Sylvain Pineau]

Remove XML/HEXR exporter support from checkbox-ng

Requires: https://code.launchpad.net/~sylvain-pineau/plainbox/+git/plainbox/+merge/326677

[Maciej Kisielewski (kissiel)]

+1.

I found one additional change worth considering. See inline.

review: Approve

[Sylvain Pineau (sylvain-pineau)]

sample_xml -> sample_archive

review: Needs Resubmitting

diff --git a/checkbox_ng/certification.py b/checkbox_ng/certification.py
index c46956a..bccae83 100644
--- a/checkbox_ng/certification.py
+++ b/checkbox_ng/certification.py
@@ -23,7 +23,7 @@
 =============================================================================
 
 This module contains a PlainBox transport that knows how to send the
-certification XML data to the Canonical certification database.
+certification tarball to the Canonical certification database.
 """
 
 from gettext import gettext as _
@@ -41,147 +41,6 @@ import requests
 logger = getLogger("checkbox.ng.certification")
 
 
-class CertificationTransport(TransportBase):
-    """
-    Transport for sending data to certification database.
-     - POSTs data to a http(s) endpoint
-     - Adds a header with a hardware identifier
-     - Payload can be in:
-        * checkbox xml-compatible format.
-          This means it will work best with a stream produced by the xml
-          exporter.
-   """
-
-    def __init__(self, where, options):
-        """
-        Initialize the Certification Transport.
-
-        The options string may contain 'secure_id' which must be
-        a 15-character (or longer)  alphanumeric ID for the system.
-
-        It may also contain a submit_to_hexr boolean, set to 1
-        to enable submission to hexr.
-        """
-        super().__init__(where, options)
-        # Interpret this setting here
-        submit_to_hexr = self.options.get('submit_to_hexr')
-        self._submit_to_hexr = False
-        try:
-            if submit_to_hexr and (submit_to_hexr.lower() in
-                                   ('yes', 'true') or
-                                   int(submit_to_hexr) == 1):
-                self._submit_to_hexr = True
-        except ValueError:
-            # Just leave it at False
-            pass
-        self._secure_id = self.options.get('secure_id')
-        if self._secure_id is not None:
-            self._validate_secure_id(self._secure_id)
-
-    def send(self, data, config=None, session_state=None):
-        """
-        Sends data to the specified server.
-
-        :param data:
-            Data containing the xml dump to be sent to the server. This
-            can be either bytes or a file-like object (BytesIO works fine too).
-            If this is a file-like object, it will be read and streamed "on
-            the fly".
-        :param config:
-             Optional PlainBoxConfig object. If http_proxy and https_proxy
-             values are set in this config object, they will be used to send
-             data via the specified protocols. Note that the transport also
-             honors the http_proxy and https_proxy environment variables.
-             Proxy string format is http://[user:password@]<proxy-ip>:port
-        :param session_state:
-            The session for which this transport is associated with
-            the data being sent (optional)
-        :returns:
-            A dictionary with responses from the server if submission
-            was successful. This should contain an 'id' key, however
-            the server response may change, so the only guarantee
-            we make is that this will be non-False if the server
-            accepted the data. Returns empty dictionary otherwise.
-        :raises requests.exceptions.Timeout:
-            If sending timed out.
-        :raises requests.exceptions.ConnectionError:
-            If connection failed outright.
-        :raises requests.exceptions.HTTPError:
-            If the server returned a non-success result code
-        """
-        proxies = None
-        if config and config.environment is not Unset:
-            proxies = {
-                proto[:-len("_proxy")]: config.environment[proto]
-                for proto in ['http_proxy', 'https_proxy']
-                if proto in config.environment
-            }
-        # Find the effective value of secure_id:
-        # - use the configuration object (if available)
-        # - override with secure_id= option (if defined)
-        secure_id = None
-        if config is not None and hasattr(config, 'secure_id'):
-            secure_id = config.secure_id
-        if self._secure_id is not None:
-            secure_id = self._secure_id
-        if secure_id is None:
-            raise InvalidSecureIDError(_("Secure ID not specified"))
-        self._validate_secure_id(secure_id)
-        logger.debug(
-            _("Sending to %s, Secure ID is %s"), self.url, secure_id)
-        headers = {"X_HARDWARE_ID": secure_id}
-        # Similar handling for submit_to_hexr
-        submit_to_hexr = False
-        if config is not None and hasattr(config, 'submit_to_hexr'):
-            submit_to_hexr = config.submit_to_hexr
-            logger.debug(_("submit_to_hexr set to %s by config"),
-                         submit_to_hexr)
-        if self._submit_to_hexr:
-            submit_to_hexr = self._submit_to_hexr
-            logger.debug(_("submit_to_hexr set to %s by UI"), submit_to_hexr)
-        # We could always set this header since hexr will only process a value
-        # of 'True', but this avoids injecting that extraneous knowledge into
-        # the tests.
-        # Note that hexr will only process a submission with this header's
-        # value set to 'True', so this boolean conversion should be ok.
-        if submit_to_hexr:
-            headers["X-Share-With-HEXR"] = submit_to_hexr
-
-        # Requests takes care of properly handling a file-like data.
-        form_payload = {"data": data}
-        try:
-            response = requests.post(
-                self.url, files=form_payload, headers=headers, proxies=proxies)
-        except requests.exceptions.Timeout as exc:
-            raise TransportError(
-                _("Request to {0} timed out: {1}").format(self.url, exc))
-        except requests.exceptions.InvalidSchema as exc:
-            raise TransportError(
-                _("Invalid destination URL: {0}").format(exc))
-        except requests.exceptions.ConnectionError as exc:
-            raise TransportError(
-                _("Unable to connect to {0}: {1}").format(self.url, exc))
-        if response is not None:
-            try:
-                # This will raise HTTPError for status != 20x
-                response.raise_for_status()
-            except requests.exceptions.RequestException as exc:
-                raise TransportError(str(exc))
-            logger.debug("Success! Server said %s", response.text)
-            try:
-                return response.json()
-            except Exception as exc:
-                raise TransportError(str(exc))
-
-        # ISessionStateTransport.send must return dictionary
-        return {}
-
-    def _validate_secure_id(self, secure_id):
-        if not re.match(SECURE_ID_PATTERN, secure_id):
-            raise InvalidSecureIDError(
-                _("secure_id must be 15-character (or more) alphanumeric string"))
-
-
 class SubmissionServiceTransport(TransportBase):
     """
     Transport for sending data to certification database.
@@ -197,9 +56,6 @@ class SubmissionServiceTransport(TransportBase):
 
         The options string may contain 'secure_id' which must be
         a 15-character (or longer)  alphanumeric ID for the system.
-
-        It may also contain a submit_to_hexr boolean, set to 1
-        to enable submission to hexr.
         """
         super().__init__(where, options)
         self._secure_id = self.options.get('secure_id')
@@ -211,7 +67,7 @@ class SubmissionServiceTransport(TransportBase):
         Sends data to the specified server.
 
         :param data:
-            Data containing the xml dump to be sent to the server. This
+            Data containing the session dump to be sent to the server. This
             can be either bytes or a file-like object (BytesIO works fine too).
             If this is a file-like object, it will be read and streamed "on
             the fly".
diff --git a/checkbox_ng/launcher/subcommands.py b/checkbox_ng/launcher/subcommands.py
index af8ce9c..ce5a2e6 100644
--- a/checkbox_ng/launcher/subcommands.py
+++ b/checkbox_ng/launcher/subcommands.py
@@ -98,19 +98,8 @@ class Submit(Command):
         if ctx.args.staging:
             url = ('https://certification.staging.canonical.com/'
                    'api/v1/submission/{}/'.format(ctx.args.secure_id))
-        if ctx.args.submission.endswith('xml'):
-            from checkbox_ng.certification import CertificationTransport
-            transport_cls = CertificationTransport
-            mode = 'r'
-            enc = 'utf-8'
-            url = ('https://certification.canonical.com/'
-                   'submissions/submit/')
-            if ctx.args.staging:
-                url = ('https://certification.staging.canonical.com/'
-                       'submissions/submit/')
-        else:
-            from checkbox_ng.certification import SubmissionServiceTransport
-            transport_cls = SubmissionServiceTransport
+        from checkbox_ng.certification import SubmissionServiceTransport
+        transport_cls = SubmissionServiceTransport
         transport = transport_cls(url, options_string)
         try:
             with open(ctx.args.submission, mode, encoding=enc) as subm_file:
@@ -587,24 +576,24 @@ class Launcher(Command, MainLoopStage):
             self.launcher.reports['1_text_to_screen'] = {
                 'transport': 'stdout', 'exporter': 'text', 'forced': 'yes'}
         elif report == 'certification':
-            self.launcher.exporters['hexr'] = {
-                'unit': 'com.canonical.plainbox::hexr'}
+            self.launcher.exporters['tar'] = {
+                'unit': 'com.canonical.plainbox::tar'}
             self.launcher.transports['c3'] = {
-                'type': 'certification',
+                'type': 'submission-service',
                 'secure_id': self.launcher.transports.get('c3', {}).get(
                     'secure_id', None)}
             self.launcher.reports['upload to certification'] = {
-                'transport': 'c3', 'exporter': 'hexr'}
+                'transport': 'c3', 'exporter': 'tar'}
         elif report == 'certification-staging':
-            self.launcher.exporters['hexr'] = {
-                'unit': 'com.canonical.plainbox::hexr'}
+            self.launcher.exporters['tar'] = {
+                'unit': 'com.canonical.plainbox::tar'}
             self.launcher.transports['c3-staging'] = {
-                'type': 'certification',
+                'type': 'submission-service',
                 'secure_id': self.launcher.transports.get('c3', {}).get(
                     'secure_id', None),
                 'staging': 'yes'}
             self.launcher.reports['upload to certification-staging'] = {
-                'transport': 'c3-staging', 'exporter': 'hexr'}
+                'transport': 'c3-staging', 'exporter': 'tar'}
         elif report == 'submission_files':
             # LP:1585326 maintain isoformat but removing ':' chars that cause
             # issues when copying files.
@@ -612,7 +601,7 @@ class Launcher(Command, MainLoopStage):
             timestamp = datetime.datetime.utcnow().strftime(isoformat)
             if not os.path.exists(self.base_dir):
                 os.makedirs(self.base_dir)
-            for exporter, file_ext in [('hexr', '.xml'), ('html', '.html'),
+            for exporter, file_ext in [('html', '.html'),
                                        ('junit', '.junit.xml'),
                                        ('xlsx', '.xlsx'), ('tar', '.tar.xz')]:
                 path = os.path.join(self.base_dir, ''.join(
@@ -652,22 +641,6 @@ class Launcher(Command, MainLoopStage):
         elif tr_type == 'stream':
             self.transports[transport] = cls(
                 self.launcher.transports[transport]['stream'])
-        elif tr_type == 'certification':
-            if self.launcher.transports[transport].get('staging', False):
-                url = ('https://certification.staging.canonical.com/'
-                       'submissions/submit/')
-            else:
-                url = ('https://certification.canonical.com/'
-                       'submissions/submit/')
-            secure_id = self.launcher.transports[transport].get(
-                'secure_id', None)
-            if not secure_id and self.is_interactive:
-                secure_id = input(self.C.BLUE(_('Enter secure-id:')))
-            if secure_id:
-                options = "secure_id={}".format(secure_id)
-            else:
-                options = ""
-            self.transports[transport] = cls(url, options)
         elif tr_type == 'submission-service':
             secure_id = self.launcher.transports[transport].get(
                 'secure_id', None)
diff --git a/checkbox_ng/test_certification.py b/checkbox_ng/test_certification.py
index e2d558e..2c61bab 100644
--- a/checkbox_ng/test_certification.py
+++ b/checkbox_ng/test_certification.py
@@ -1,9 +1,10 @@
 # This file is part of Checkbox.
 #
-# Copyright 2012, 2013 Canonical Ltd.
+# Copyright 2012, 2017 Canonical Ltd.
 # Written by:
 #   Zygmunt Krynicki <[e-mail]>
 #   Daniel Manrique <[e-mail]>
+#   Sylvain Pineau <[e-mail]>
 #
 # Checkbox is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3,
@@ -37,10 +38,10 @@ from plainbox.vendor.mock import MagicMock
 from requests.exceptions import ConnectionError, InvalidSchema, HTTPError
 import requests
 
-from checkbox_ng.certification import CertificationTransport
+from checkbox_ng.certification import SubmissionServiceTransport
 
 
-class CertificationTransportTests(TestCase):
+class SubmissionServiceTransportTests(TestCase):
 
     # URL are just here to exemplify, since we mock away all network access,
     # they're not really used.
@@ -51,39 +52,34 @@ class CertificationTransportTests(TestCase):
     valid_option_string = "secure_id={}".format(valid_secure_id)
 
     def setUp(self):
-        self.sample_xml = BytesIO(resource_string(
-            "plainbox", "test-data/xml-exporter/example-data.xml"
+        self.sample_archive = BytesIO(resource_string(
+            "plainbox", "test-data/tar-exporter/example-data.tar.xz"
         ))
         self.patcher = mock.patch('requests.post')
         self.mock_requests = self.patcher.start()
 
     def test_parameter_parsing(self):
         # Makes sense since I'm overriding the base class's constructor.
-        transport = CertificationTransport(
+        transport = SubmissionServiceTransport(
             self.valid_url, self.valid_option_string)
         self.assertEqual(self.valid_url, transport.url)
         self.assertEqual(self.valid_secure_id,
                          transport.options['secure_id'])
 
-    def test_submit_to_hexr_interpretation(self):
-        transport = CertificationTransport(
-            self.valid_url, "submit_to_hexr=1")
-        self.assertTrue(transport._submit_to_hexr is True)
-
     def test_invalid_length_secure_id_are_rejected(self):
         length = 14
         dummy_id = "a" * length
         option_string = "secure_id={}".format(dummy_id)
         with self.assertRaises(InvalidSecureIDError):
-            CertificationTransport(self.valid_url, option_string)
+            SubmissionServiceTransport(self.valid_url, option_string)
 
     def test_invalid_characters_in_secure_id_are_rejected(self):
         option_string = "secure_id=aA0#"
         with self.assertRaises(InvalidSecureIDError):
-            CertificationTransport(self.valid_url, option_string)
+            SubmissionServiceTransport(self.valid_url, option_string)
 
     def test_invalid_url(self):
-        transport = CertificationTransport(
+        transport = SubmissionServiceTransport(
             self.invalid_url, self.valid_option_string)
         dummy_data = BytesIO(b"some data to send")
         requests.post.side_effect = InvalidSchema
@@ -92,12 +88,11 @@ class CertificationTransportTests(TestCase):
             result = transport.send(dummy_data)
             self.assertIsNotNone(result)
         requests.post.assert_called_with(
-            self.invalid_url, files={'data': dummy_data},
-            headers={'X_HARDWARE_ID': self.valid_secure_id}, proxies=None)
+            self.invalid_url, data=dummy_data, proxies=None)
 
     @mock.patch('checkbox_ng.certification.logger')
     def test_valid_url_cant_connect(self, mock_logger):
-        transport = CertificationTransport(
+        transport = SubmissionServiceTransport(
             self.unreachable_url, self.valid_option_string)
         dummy_data = BytesIO(b"some data to send")
         requests.post.side_effect = ConnectionError
@@ -105,37 +100,20 @@ class CertificationTransportTests(TestCase):
             result = transport.send(dummy_data)
             self.assertIsNotNone(result)
         requests.post.assert_called_with(self.unreachable_url,
-                                         files={'data': dummy_data},
-                                         headers={'X_HARDWARE_ID':
-                                                  self.valid_secure_id},
-                                         proxies=None)
-
-    @mock.patch('checkbox_ng.certification.logger')
-    def test_share_with_hexr_header_sent(self, mock_logger):
-        transport = CertificationTransport(
-            self.valid_url, self.valid_option_string+",submit_to_hexr=1")
-        dummy_data = BytesIO(b"some data to send")
-        result = transport.send(dummy_data)
-        self.assertIsNotNone(result)
-        requests.post.assert_called_with(self.valid_url,
-                                         files={'data': dummy_data},
-                                         headers={'X_HARDWARE_ID':
-                                                  self.valid_secure_id,
-                                                  'X-Share-With-HEXR':
-                                                  True},
+                                         data=dummy_data,
                                          proxies=None)
 
     def test_send_success(self):
-        transport = CertificationTransport(
+        transport = SubmissionServiceTransport(
             self.valid_url, self.valid_option_string)
         requests.post.return_value = MagicMock(name='response')
         requests.post.return_value.status_code = 200
         requests.post.return_value.text = '{"id": 768}'
-        result = transport.send(self.sample_xml)
+        result = transport.send(self.sample_archive)
         self.assertTrue(result)
 
     def test_send_failure(self):
-        transport = CertificationTransport(
+        transport = SubmissionServiceTransport(
             self.valid_url, self.valid_option_string)
         requests.post.return_value = MagicMock(name='response')
         requests.post.return_value.status_code = 412
@@ -145,7 +123,7 @@ class CertificationTransportTests(TestCase):
         requests.post.return_value.raise_for_status = MagicMock(
             side_effect=HTTPError)
         with self.assertRaises(TransportError):
-            transport.send(self.sample_xml)
+            transport.send(self.sample_archive)
 
     def proxy_test(self, environment, proxies):
         test_environment = environment
@@ -153,7 +131,7 @@ class CertificationTransportTests(TestCase):
         test_config = PlainBoxConfig()
         test_config.environment = test_environment
 
-        transport = CertificationTransport(
+        transport = SubmissionServiceTransport(
             self.valid_url, self.valid_option_string)
         dummy_data = BytesIO(b"some data to send")
 
@@ -163,8 +141,7 @@ class CertificationTransportTests(TestCase):
         result = transport.send(dummy_data, config=test_config)
         self.assertTrue(result)
         requests.post.assert_called_with(
-            self.valid_url, files={'data': dummy_data},
-            headers={'X_HARDWARE_ID': self.valid_secure_id},
+            self.valid_url, data=dummy_data,
             proxies=test_proxies)
 
     def test_set_only_one_proxy(self):
diff --git a/contrib/com.canonical.certification.PlainBox1.service b/contrib/com.canonical.certification.PlainBox1.service
deleted file mode 100644
index a40cf65..0000000
--- a/contrib/com.canonical.certification.PlainBox1.service
+++ /dev/null
@@ -1,3 +0,0 @@
-[D-BUS Service]
-Name=com.canonical.certification.PlainBox1
-Exec=/usr/bin/checkbox service
diff --git a/docs/conf.py b/docs/conf.py
index 970bfc7..58b6dba 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -24,9 +24,7 @@ else:
     # Inject mock modules so that we can build the
     # documentation without having the real stuff available
     from plainbox.vendor import mock
-    for mod_name in ['lxml', 'xlsxwriter', 'dbus', 'dbus.lowlevel',
-                     'dbus.exceptions', 'dbus._compat', 'dbus.service',
-                     '_dbus_bindings']:
+    for mod_name in ['xlsxwriter']:
         sys.modules[mod_name] = mock.Mock()
         print("Mocked {}".format(mod_name))
 try:
diff --git a/docs/index.rst b/docs/index.rst
index 844f14f..ea4a5df 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -31,24 +31,6 @@ or newer.
 
     $ sudo add-apt-repository ppa:hardware-certification/public && sudo apt-get update && sudo apt-get install checkbox-ng
 
-Running stable release update tests
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-Checkbox has special support for running stable release updates tests in an
-automated manner. This runs all the jobs from the *sru.test plan* and sends the
-results to the certification website.
-
-To run SRU tests you will need to know the so-called :term:`Secure ID` of the
-device you are testing. Once you know that all you need to do is run:
-
-.. code-block:: bash
-
-    $ checkbox sru $secure_id submission.xml
-
-The second argument, submission.xml, is a name of the fallback file that is
-only created when sending the data to the certification website fails to work
-for any reason.
-
 Table of contents
 =================
 
diff --git a/docs/launcher-tutorial.rst b/docs/launcher-tutorial.rst
index 74dcf0d..d6dd71d 100644
--- a/docs/launcher-tutorial.rst
+++ b/docs/launcher-tutorial.rst
@@ -99,7 +99,7 @@ number of the stock ones. In launchers version 1 there are 4 stock reports you
 may use:
 
     * ``text`` - print results as text on standard output
-    * ``submission_files`` - write ``html``, ``xlsx``, ``json`` and ``xml``
+    * ``submission_files`` - write ``html``, ``xlsx``, ``json`` and ``tar.xz``
       files to ``$XDG_DATA_HOME`` directory (or to ``~/.local/share/`` if
       ``$XDG_DATA_HOME`` is not defined.
     * ``certification`` - send results to certification site
@@ -485,7 +485,7 @@ report to standard output.
     exporter = text
 
 2) Interactive testing of FooBar project. Report should be uploaded to the
-staging version of certification site and saved to /tmp/submission.xml
+staging version of certification site and saved to /tmp/submission.tar.xz
 
 ::
 
@@ -512,15 +512,12 @@ staging version of certification site and saved to /tmp/submission.xml
 
     [transport:local_file]
     type = file
-    path = /tmp/submission.xml
-
-    [exporter:xml]
-    unit = com.canonical.plainbox::hexr
+    path = /tmp/submission.tar.xz
 
     [report:c3-staging]
-    transport = outfile
-    exporter = xml
+    transport = certification
+    exporter = tar
 
     [report:file]
     transport = local_file
-    exporter = xml
+    exporter = tar
diff --git a/docs/stack.rst b/docs/stack.rst
index 7e8a6aa..8e36841 100644
--- a/docs/stack.rst
+++ b/docs/stack.rst
@@ -71,11 +71,9 @@ Component Descriptions
 | Checkbox (CLI)         | - The python command-line interface   | Application |
 |                        |                                       |             |
 |                        |   - the text user interface           |             |
-|                        |   - the SRU testing command           |             |
 |                        |                                       |             |
 |                        | - Additional certification APIs       |             |
 |                        |                                       |             |
-|                        |   - sending data to Launchpad         |             |
 |                        |   - sending data to HEXR              |             |
 +------------------------+---------------------------------------+-------------+
 | Client Certification   | - canonical-certification-client      | Provider    |
@@ -107,7 +105,7 @@ Component Descriptions
 |                        |   - Trusted launcher                  |             |
 |                        |   - Dependency resolver               |             |
 |                        |   - Command line handling             |             |
-|                        |   - The XML, HTML and XSLX exporters  |             |
+|                        |   - The HTML and XSLX exporters       |             |
 |                        |   - and more...                       |             |
 |                        |                                       |             |
 |                        | - Provider development toolkit        |             |
diff --git a/requirements/deb-core.txt b/requirements/deb-core.txt
index 5806499..18a7ef6 100644
--- a/requirements/deb-core.txt
+++ b/requirements/deb-core.txt
@@ -1,2 +1 @@
 python3-guacamole
-python3-lxml
diff --git a/requirements/deb-dbus.txt b/requirements/deb-dbus.txt
deleted file mode 100644
index a1d5df0..0000000
--- a/requirements/deb-dbus.txt
+++ /dev/null
@@ -1 +0,0 @@
-python3-dbus
diff --git a/requirements/pip-dbus.txt b/requirements/pip-dbus.txt
deleted file mode 100644
index 43e3c72..0000000
--- a/requirements/pip-dbus.txt
+++ /dev/null
@@ -1 +0,0 @@
-funcsigs==0.3
diff --git a/requirements/rpm-dbus.txt b/requirements/rpm-dbus.txt
deleted file mode 100644
index a1d5df0..0000000
--- a/requirements/rpm-dbus.txt
+++ /dev/null
@@ -1 +0,0 @@
-python3-dbus
diff --git a/setup.py b/setup.py
index 2b7bfaa..ac86e70 100755
--- a/setup.py
+++ b/setup.py
@@ -70,8 +70,6 @@ setup(
             'checkbox-cli=checkbox_ng.launcher.checkbox_cli:main',
         ],
         'plainbox.transport': [
-            'certification='
-            'checkbox_ng.certification:CertificationTransport',
             'submission-service='
             'checkbox_ng.certification:SubmissionServiceTransport',
         ],