Merge lp:~cr3/checkbox/838123 into lp:checkbox

[Marc Tardif]

[Daniel Manrique (roadmr)]

The bug report has no steps to reproduce so I was unable to check what happens with a borked xml submission file. It looks fine to me, there are no errors in the logs from a normal run.

There's one thing I observed, though: when I try to open the report it looks ugly :( this is because in checkbox.xsl the CHECKBOX_SHARE variable is not substituted and there's stuff like:

href="%(CHECKBOX_SHARE)s/report/styles.css"

if I replace %(CHECKBOX_SHARE)s with the actual path where the files reside, all works well.

This is worth fixing, because there are usually compliments about how nice the report looks, and if this slips by, we won't get those anymore :)

review: Needs Fixing

[Marc Tardif (cr3)]

Good catch, fixed.

review: Needs Resubmitting

[lp:~cr3/checkbox/838123]

1035. By Marc Tardif on 2011-09-08

Replaced environment variables in stylesheet.

[Daniel Manrique (roadmr)]

Great! tested and it's working fine now, approving and merging. Thanks!

review: Approve

=== modified file 'checkbox/report.py'
--- checkbox/report.py	2009-07-22 18:02:57 +0000
+++ checkbox/report.py	2011-09-08 18:23:40 +0000
@@ -16,6 +16,9 @@
 # You should have received a copy of the GNU General Public License
 # along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
 #
+import libxml2
+import posixpath
+
 from xml.dom.minidom import Document, Element, parseString
 
 
@@ -27,13 +30,13 @@
     handlers to understand the formats for dumping and loading actions.
     """
 
-    def __init__(self, name, version=None, stylesheet=None):
+    def __init__(self, name, version=None, stylesheet=None, schema=None):
         self.name = name
         self.version = version
         self.stylesheet = stylesheet
+        self.schema = schema
         self.dumps_table = {}
         self.loads_table = {}
-        self.document = None
 
     def handle_dumps(self, type, handler):
         """
@@ -83,17 +86,17 @@
         Dump the given object which may be a container of any objects
         supported by the reports added to the manager.
         """
-        self.document = Document()
+        document = Document()
 
         if self.stylesheet:
             type = "text/xsl"
-            href = "file://%s" % self.stylesheet
-            style = self.document.createProcessingInstruction("xml-stylesheet",
+            href = "file://%s" % posixpath.abspath(self.stylesheet)
+            style = document.createProcessingInstruction("xml-stylesheet",
                 "type=\"%s\" href=\"%s\"" % (type, href))
-            self.document.appendChild(style)
+            document.appendChild(style)
 
-        node = self.document.createElement(self.name)
-        self.document.appendChild(node)
+        node = document.createElement(self.name)
+        document.appendChild(node)
 
         if self.version:
             node.setAttribute("version", self.version)
@@ -103,8 +106,6 @@
         except KeyError, e:
             raise ValueError, "Unsupported type: %s" % e
 
-        document = self.document
-        self.document = None
         return document
 
     def loads(self, str):
@@ -123,6 +124,32 @@
 
         return ret
 
+    def validate(self, str):
+        """
+        Validate the given string 
+        """
+        if not self.schema:
+            return False
+
+        file = open(self.schema)
+        try:
+            schema = file.read()
+        finally:
+            file.close()
+
+        rngParser = libxml2.relaxNGNewMemParserCtxt(schema, len(schema))
+        rngSchema = rngParser.relaxNGParse()
+        ctxt = rngSchema.relaxNGNewValidCtxt()
+        doc = libxml2.parseDoc(str)
+        is_valid = doc.relaxNGValidateDoc(ctxt)
+
+        # Clean up
+        doc.freeDoc()
+        del rngParser, rngSchema, ctxt
+        libxml2.relaxNGCleanupTypes()
+        libxml2.cleanupParser()
+        return is_valid == 0
+
 
 class Report(object):
     """A convenience for writing reports.
@@ -133,12 +160,12 @@
     """
 
     def _create_element(self, name, parent):
-        element = self._manager.document.createElement(name)
+        element = parent.ownerDocument.createElement(name)
         parent.appendChild(element)
         return element
 
     def _create_text_node(self, text, parent):
-        text_node = self._manager.document.createTextNode(text)
+        text_node = parent.ownerDocument.createTextNode(text)
         parent.appendChild(text_node)
         return text_node
 

=== modified file 'checkbox/tests/report.py'
--- checkbox/tests/report.py	2009-02-16 15:48:24 +0000
+++ checkbox/tests/report.py	2011-09-08 18:23:40 +0000
@@ -33,7 +33,7 @@
         self._manager.handle_loads("default", self.loads_default)
 
     def dumps_test(self, obj, parent):
-        text_node = self._manager.document.createTextNode(obj)
+        text_node = parent.ownerDocument.createTextNode(obj)
         parent.appendChild(text_node)
 
     def loads_test(self, node):

=== modified file 'data/whitelists/default.whitelist'
--- data/whitelists/default.whitelist	2011-09-01 10:55:45 +0000
+++ data/whitelists/default.whitelist	2011-09-08 18:23:40 +0000
@@ -61,6 +61,7 @@
 info/gcov_attachment
 info/modprobe_attachment
 info/modules_attachment
+info/sysfs_attachment
 info/sysctl_attachment
 info/udev_attachment
 __input__

=== modified file 'debian/changelog'
--- debian/changelog	2011-09-06 21:44:47 +0000
+++ debian/changelog	2011-09-08 18:23:40 +0000
@@ -2,6 +2,7 @@
 
   [Marc Tardif]
   * Only connecting hyper text widgets once (LP: #827904)
+  * Validating the submission.xml (LP: #838123)
 
  -- Daniel Manrique <[e-mail]>  Tue, 06 Sep 2011 17:42:57 -0400
 

=== modified file 'debian/control'
--- debian/control	2011-08-25 15:05:30 +0000
+++ debian/control	2011-09-08 18:23:40 +0000
@@ -13,7 +13,7 @@
 Architecture: all
 Replaces: hwtest (<< 0.1-0ubuntu12)
 Provides: hwtest
-Depends: ${misc:Depends}, ${python:Depends}, debconf, udev
+Depends: ${misc:Depends}, ${python:Depends}, debconf, python-libxml2, udev
 Recommends: dpkg (>= 1.13), lsb-release, pm-utils, python-apport, python-apt, python-dateutil, python-gst0.10
 Suggests: checkbox-cli | checkbox-gtk, bonnie++, bootchart, bzr, cvs, ethtool, flex, fwts, git-core, hdparm, lshw, make, nmap, obexd-client, python-pexpect, smartmontools, sox, stress, wodim
 Conflicts: hwtest (<< 0.1-0ubuntu12)

=== modified file 'jobs/info.txt.in'
--- jobs/info.txt.in	2011-08-24 19:17:05 +0000
+++ jobs/info.txt.in	2011-09-08 18:23:40 +0000
@@ -37,6 +37,15 @@
 plugin: attachment
 command: find /etc/sysctl.* -name \*.conf | xargs cat
 
+name: info/sysfs_attachment
+plugin: attachment
+command:
+ for i in `udevadm info --export-db | sed -n 's/^P: //p'`; do
+  echo "P: $i"
+  udevadm info --attribute-walk --path=/sys$i 2>/dev/null | sed -n 's/    ATTR{\(.*\)}=="\(.*\)"/A: \1=\2/p'
+  echo
+ done
+
 name: info/udev_attachment
 plugin: attachment
 command: udevadm info --export-db

=== modified file 'plugins/launchpad_prompt.py'
--- plugins/launchpad_prompt.py	2011-02-02 16:57:29 +0000
+++ plugins/launchpad_prompt.py	2011-09-08 18:23:40 +0000
@@ -46,7 +46,7 @@
         self.persist = persist.root_at("launchpad_prompt")
 
     def launchpad_report(self, report):
-        self._launchpad_report = report
+        self.report = report
 
     def prompt_exchange(self, interface):
         if self.persist and self.persist.has("email"):
@@ -66,7 +66,7 @@
                 for error in errors:
                     self._manager.reactor.fire("prompt-error", interface, error)
 
-                url = "file://%s" % posixpath.abspath(self._launchpad_report)
+                url = "file://%s" % posixpath.abspath(self.report)
 
                 email = interface.show_entry(_("""\
 The following report has been generated for submission to the Launchpad \

=== modified file 'plugins/launchpad_report.py'
--- plugins/launchpad_report.py	2011-08-08 13:13:20 +0000
+++ plugins/launchpad_report.py	2011-09-08 18:23:40 +0000
@@ -17,7 +17,8 @@
 # along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
 #
 import os
-import posixpath
+
+from gettext import gettext as _
 
 from checkbox.lib.safe import safe_make_directory
 
@@ -31,6 +32,9 @@
     # Filename where submission information is cached.
     filename = Path(default="%(checkbox_data)s/submission.xml")
 
+    # XML Schema
+    schema = Path(default="%(checkbox_share)s/report/hardware-1_0.rng")
+
     # XSL Stylesheet
     stylesheet = Path(default="%(checkbox_share)s/report/checkbox.xsl")
 
@@ -67,10 +71,13 @@
     def report_attachments(self, attachments):
         for attachment in attachments:
             name = attachment["name"]
-            if "dmi" in name:
+            if "sysfs_attachment" in name:
+                self._report["hardware"]["sysfs-attributes"] = attachment["data"]
+
+            elif "dmi_attachment" in name:
                 self._report["hardware"]["dmi"] = attachment["data"]
 
-            elif "udev" in name:
+            elif "udev_attachment" in name:
                 self._report["hardware"]["udev"] = attachment["data"]
 
             else:
@@ -123,24 +130,24 @@
             self._report["questions"].append(question)
 
     def report(self):
-        # Convert stylesheet to report directory
+        # Prepare the payload and attach it to the form
+        stylesheet_path = os.path.join(
+            os.path.dirname(self.filename),
+            os.path.basename(self.stylesheet))
+        report_manager = LaunchpadReportManager(
+            "system", "1.0", stylesheet_path, self.schema)
+        payload = report_manager.dumps(self._report).toprettyxml("")
+        if not report_manager.validate(payload):
+            self._manager.reactor.fire("report-error", _("""\
+The generated report seems to have validation errors,
+so it might not be processed by Launchpad."""))
+
+        # Write the report
         stylesheet_data = open(self.stylesheet).read() % os.environ
-        stylesheet_path = posixpath.join(
-            posixpath.dirname(self.filename),
-            posixpath.basename(self.stylesheet))
         open(stylesheet_path, "w").write(stylesheet_data)
-
-        # Prepare the payload and attach it to the form
-        stylesheet_path = posixpath.abspath(stylesheet_path)
-        report_manager = LaunchpadReportManager("system", "1.0", stylesheet_path)
-        payload = report_manager.dumps(self._report).toprettyxml("")
-
-        directory = posixpath.dirname(self.filename)
+        directory = os.path.dirname(self.filename)
         safe_make_directory(directory)
-
-        file = open(self.filename, "w")
-        file.write(payload)
-        file.close()
+        open(self.filename, "w").write(payload)
 
         self._manager.reactor.fire("launchpad-report", self.filename)
 

=== modified file 'plugins/report_prompt.py'
--- plugins/report_prompt.py	2010-02-22 15:16:05 +0000
+++ plugins/report_prompt.py	2011-09-08 18:23:40 +0000
@@ -30,9 +30,16 @@
         self._manager.reactor.call_on("prompt-report", self.prompt_report)
 
     def prompt_report(self, interface):
+        def report_error(e):
+            self._manager.reactor.fire("prompt-error", interface, e)
+
+        event_id = self._manager.reactor.call_on("report-error", report_error)
+
         if interface.direction != PREV:
             interface.show_progress(_("Building report..."),
                 self._manager.reactor.fire, "report")
 
+        self._manager.reactor.cancel_call(event_id)
+
 
 factory = ReportPrompt

=== added file 'report/hardware-1_0.rng'
--- report/hardware-1_0.rng	1970-01-01 00:00:00 +0000
+++ report/hardware-1_0.rng	2011-09-08 18:23:40 +0000
@@ -0,0 +1,533 @@
+<?xml version="1.0" ?>
+<grammar xmlns="http://relaxng.org/ns/structure/1.0"
+         datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"
+         xmlns:a="http://launchpad.net/annotation">
+    <start>
+        <element name="system">
+            <attribute name="version">
+                <value>1.0</value>
+            </attribute>
+          <interleave>
+            <element name="summary">
+                <ref name="summarySection"/>
+            </element>
+            <element name="hardware">
+                <ref name="hardwareSection"/>
+            </element>
+            <element name="software">
+                <ref name="softwareSection"/>
+            </element>
+            <element name="questions">
+                <ref name="questionsSection"/>
+            </element>
+            <optional>
+                <element name="context">
+                    <ref name="contextSection"/>
+                </element>
+            </optional>
+          </interleave>
+        </element>
+    </start>
+
+    <define name="summarySection">
+        <interleave>
+            <element name="live_cd">
+                <attribute name="value">
+                    <ref name="booleanValue"/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="system_id">
+                <attribute name="value">
+                    <text/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="distribution">
+                <attribute name="value">
+                    <text/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="distroseries">
+                <attribute name="value">
+                    <text/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="architecture">
+                <attribute name="value">
+                    <text/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="private">
+                <attribute name="value">
+                    <ref name="booleanValue"/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="contactable">
+                <attribute name="value">
+                    <ref name="booleanValue"/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="date_created">
+                <attribute name="value">
+                    <data type="dateTime"/>
+                </attribute>
+                <empty/>
+            </element>
+            <element name="client">
+                <attribute name="name">
+                    <text/>
+                </attribute>
+                <attribute name="version">
+                    <text/>
+                </attribute>
+                <empty/>
+                <zeroOrMore>
+                    <element name="plugin">
+                        <attribute name="name">
+                            <text/>
+                        </attribute>
+                        <attribute name="version">
+                            <text/>
+                        </attribute>
+                    </element>
+                </zeroOrMore>
+            </element>
+            <optional>
+                <element name="kernel-release">
+                    <attribute name="value">
+                        <text/>
+                    </attribute>
+                </element>
+            </optional>
+        </interleave>
+    </define>
+
+    <define name="hardwareSection">
+        <interleave>
+            <choice>
+                <element name="hal">
+                    <attribute name="version">
+                        <text/>
+                    </attribute>
+                    <oneOrMore>
+                        <element name="device">
+                            <attribute name="id">
+                                <data type="integer">
+                                    <except>
+                                        <value/>
+                                    </except>
+                                </data>
+                            </attribute>
+                            <attribute name="udi">
+                                <text/>
+                            </attribute>
+                            <optional>
+                                <attribute name="parent">
+                                   <data type="integer"/>
+                                </attribute>
+                            </optional>
+                            <!-- XXX: Abel Deuring 2007-12-07:
+                                 specify a set of required properties? -->
+                            <oneOrMore>
+                                <ref name="property"/>
+                            </oneOrMore>
+                        </element>
+                    </oneOrMore>
+                </element>
+                <group>
+                    <interleave>
+                        <element name="udev">
+                            <text/>
+                        </element>
+                        <element name="dmi">
+                            <text/>
+                        </element>
+                        <element name="sysfs-attributes">
+                            <zeroOrMore>
+                                <text/>
+                            </zeroOrMore>
+                        </element>
+                    </interleave>
+                </group>
+            </choice>
+            <element name="processors">
+                <oneOrMore>
+                    <element name="processor">
+                        <attribute name="id">
+                            <data type="integer">
+                                <except>
+                                    <value/>
+                                </except>
+                            </data>
+                        </attribute>
+                        <attribute name="name">
+                            <text/>
+                        </attribute>
+                         <oneOrMore>
+                             <ref name="property"/>
+                         </oneOrMore>
+                    </element>
+                </oneOrMore>
+            </element>
+            <optional>
+                <element name="aliases">
+                    <zeroOrMore>
+                        <element name="alias">
+                            <attribute name="target">
+                                <text/>
+                            </attribute>
+                            <interleave>
+                                <element name="vendor">
+                                    <text/>
+                                </element>
+                                <element name="model">
+                                    <text/>
+                                </element>
+                            </interleave>
+                        </element>
+                    </zeroOrMore>
+                </element>
+            </optional>
+        </interleave>
+    </define>
+
+    <define name="softwareSection">
+        <interleave>
+            <element name="lsbrelease">
+                <!-- XXX: Abel Deuring 2007-12-07:
+                     specify a more restrictive set of allowed
+                     and/or required properties?
+                -->
+                <oneOrMore>
+                    <ref name="property"/>
+                </oneOrMore>
+            </element>
+            <optional>
+                <element name="packages">
+                    <zeroOrMore>
+                        <element name="package">
+                            <attribute name="name"/>
+                            <attribute name="id">
+                                <data type="integer">
+                                    <except>
+                                        <value/>
+                                    </except>
+                                </data>
+                            </attribute>
+                            <oneOrMore>
+                                <ref name="property"/>
+                            </oneOrMore>
+                        </element>
+                    </zeroOrMore>
+                </element>
+            </optional>
+            <optional>
+                <element name="xorg">
+                    <attribute name="version">
+                        <text/>
+                    </attribute>
+                    <zeroOrMore>
+                        <element name="driver">
+                            <optional>
+                                <attribute name="device">
+                                    <data type="integer"/>
+                                </attribute>
+                            </optional>
+                            <attribute name="name">
+                                <text/>
+                            </attribute>
+                            <optional>
+                                <attribute name="version">
+                                    <text/>
+                                </attribute>
+                            </optional>
+                            <attribute name="class">
+                                <text/>
+                            </attribute>
+                        </element>
+                    </zeroOrMore>
+                </element>
+            </optional>
+        </interleave>
+    </define>
+
+    <define name="questionsSection">
+        <zeroOrMore>
+            <element name="question">
+                <attribute name="name">
+                    <text/>
+                </attribute>
+                <a:comment>
+                    The attribute "plugin" must be set, if the question
+                    is generated by a plugin.
+                </a:comment>
+                <optional>
+                    <attribute name="plugin">
+                        <text/>
+                    </attribute>
+                </optional>
+                <interleave>
+                    <optional>
+                        <element name="command">
+                            <text/>
+                        </element>
+                    </optional>
+                    <choice>
+                        <interleave>
+                            <element name="answer">
+                                <attribute name="type">
+                                    <value>multiple_choice</value>
+                                </attribute>
+                                <optional>
+                                    <attribute name="unit">
+                                        <text/>
+                                    </attribute>
+                                </optional>
+                                <text/>
+                            </element>
+                            <element name="answer_choices">
+                                <oneOrMore>
+                                    <ref name="value"/>
+                                </oneOrMore>
+                            </element>
+                        </interleave>
+                        <element name="answer">
+                            <attribute name="type">
+                                <value>measurement</value>
+                            </attribute>
+                            <optional>
+                                <attribute name="unit">
+                                    <text/>
+                                </attribute>
+                            </optional>
+                            <data type="decimal"/>
+                        </element>
+                    </choice>
+                    <zeroOrMore>
+                        <element name="target">
+                            <attribute name="id">
+                                <text/>
+                            </attribute>
+                            <interleave>
+                                <zeroOrMore>
+                                    <element name="driver">
+                                        <text/>
+                                    </element>
+                                </zeroOrMore>
+                            </interleave>
+                        </element>
+                    </zeroOrMore>
+                    <optional>
+                        <element name="comment">
+                            <text/>
+                        </element>
+                    </optional>
+                </interleave>
+            </element>
+        </zeroOrMore>
+    </define>
+
+    <define name="contextSection">
+        <zeroOrMore>
+            <element name="info">
+                <attribute name="command">
+                    <text/>
+                </attribute>
+                <text/>
+            </element>
+        </zeroOrMore>
+    </define>
+
+    <a:comment>
+        convenience for Python code: 'True'/'False' for boolean values
+        instead of 'true'/'false' as defined by
+        http://www.w3.org/2001/XMLSchema-datatypes .
+    </a:comment>
+    <define name="booleanValue">
+        <choice>
+            <value>True</value>
+            <value>False</value>
+        </choice>
+    </define>
+
+    <define name="propertyAndValueContent">
+        <a:comment>
+            Allowed types and values:
+            The dbus... data types are used for HAL properties; the data 
+            types are specified in 
+            http://dbus.freedesktop.org/doc/dbus-specification.html
+            The other data types are Python data types, defined in
+            http://docs.python.org/lib/types.html
+        </a:comment>
+        <choice>
+            <group>
+                <attribute name="type">
+                    <choice>
+                        <value>dbus.Boolean</value>
+                        <value>bool</value>
+                    </choice>
+                </attribute>
+                <ref name="booleanValue"/>
+            </group>
+            <group>
+                <attribute name="type">
+                    <choice>
+                        <value>dbus.String</value>
+                        <value>dbus.UTF8String</value>
+                        <value>str</value>
+                    </choice>
+                </attribute>
+                <text/>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.Byte</value>
+                </attribute>
+                <data type="unsignedByte">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.Int16</value>
+                </attribute>
+                <data type="short">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.Int32</value>
+                </attribute>
+                <data type="int">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.Int64</value>
+                </attribute>
+                <data type="long">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.UInt16</value>
+                </attribute>
+                <data type="unsignedShort">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.UInt32</value>
+                </attribute>
+                <data type="unsignedInt">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>dbus.UInt64</value>
+                </attribute>
+                <data type="unsignedLong">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>int</value>
+                </attribute>
+                <data type="long">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <value>long</value>
+                </attribute>
+                <data type="integer">
+                    <except>
+                        <value/>
+                    </except>
+                </data>
+            </group>
+            <group>
+                <attribute name="type">
+                    <choice>
+                        <value>dbus.Double</value>
+                        <value>float</value>
+                    </choice>
+                </attribute>
+                <data type="decimal"/>
+            </group>
+            <group>
+                <attribute name="type">
+                    <choice>
+                        <value>dbus.Array</value>
+                        <value>list</value>
+                    </choice>
+                </attribute>
+                <zeroOrMore>
+                    <element name="value">
+                        <ref name="propertyAndValueContent"/>
+                    </element>
+                </zeroOrMore>
+            </group>
+            <group>
+                <attribute name="type">
+                    <choice>
+                        <value>dbus.Dictionary</value>
+                        <value>dict</value>
+                    </choice>
+                </attribute>
+                <zeroOrMore>
+                    <element name="value">
+                        <attribute name="name">
+                            <text/>
+                        </attribute>
+                        <ref name="propertyAndValueContent"/>
+                    </element>
+                </zeroOrMore>
+            </group>
+         </choice>
+     </define>
+
+    <define name="property">
+        <element name="property">
+            <attribute name="name">
+                <text/>
+            </attribute>
+            <ref name="propertyAndValueContent"/>
+       </element>
+    </define>
+
+    <define name="value">
+        <element name="value">
+            <ref name="propertyAndValueContent"/>
+        </element>
+    </define>
+</grammar>