Merge branch 'plugin-api' into semantic-review

Conflicts:
	debexpo/importer/importer.py (because of fca4b947)

debexpo/importer/importer.py

@@ -252,7 +252,7 @@ class Importer(object):
         if not os.path.isfile(self.changes_file):
             self._fail('Cannot find changes file')
 
-    def _create_db_entries(self, qa):
+    def _create_db_entries(self):
         """
         Create entries in the Database for the package upload.
         """
@@ -291,22 +291,17 @@ class Importer(object):
         except KeyError:
             closes = None
 
-        # TODO: fix these magic numbers
-        if qa.stop():
-            qa_status = 1
-        else:
-            qa_status = 0
-
         maintainer_matches = re.compile(r'(.*) <(.*)>').match(self.changes['Changed-By'])
         maintainer = maintainer_matches.group(2)
 
-        package_version = PackageVersion(package=package, version=self.changes['Version'],
-            section=section, distribution=self.changes['Distribution'], qa_status=qa_status,
+        # FIXME: qa_status (might be removed from the model)
+        self.package_version = PackageVersion(package=package, version=self.changes['Version'],
+            section=section, distribution=self.changes['Distribution'], qa_status=1,
             component=component, priority=self.changes.get_priority(), closes=closes,
             uploaded=datetime.now(), maintainer=maintainer)
-        meta.session.add(package_version)
+        meta.session.add(self.package_version)
 
-        source_package = SourcePackage(package_version=package_version)
+        source_package = SourcePackage(package_version=self.package_version)
         meta.session.add(source_package)
 
         binary_package = None
@@ -327,7 +322,6 @@ class Importer(object):
             if file.endswith('.deb'):
                 binary_package = BinaryPackage(package_version=package_version, arch=file[:-4].split('_')[-1])
                 meta.session.add(binary_package)
-
                 meta.session.add(PackageFile(filename=filename, binary_package=binary_package, size=size, md5sum=sum))
             else:
                 meta.session.add(PackageFile(filename=filename, source_package=source_package, size=size, md5sum=sum))
@@ -335,11 +329,6 @@ class Importer(object):
         meta.session.commit()
         log.warning("Finished adding PackageFile objects.")
 
-        # Add PackageInfo objects to the database for the package_version
-        for result in qa.result:
-            meta.session.add(PackageInfo(package_version=package_version, from_plugin=result.from_plugin,
-                outcome=result.outcome, rich_data=result.data, severity=result.severity))
-
         # Commit all changes to the database
         meta.session.commit()
         log.debug('Committed package data to the database')
@@ -392,6 +381,15 @@ class Importer(object):
 
         return None
 
+    def _run_plugins(self, stage):
+        plugins = Plugins(stage,
+                          self.changes,
+                          self.changes_file,
+                          self.package_version,
+                          user_id=self.user_id)
+        log.debug('Running plugins of type: %s' % type)
+        plugins.run_plugins()
+
     def main(self):
         """
         Actually start the import of the package.
@@ -489,12 +487,12 @@ class Importer(object):
             # by doing nothing here for that case
 
         # Run post-upload plugins.
-        post_upload = Plugins('post-upload', self.changes, self.changes_file,
-            user_id=self.user_id)
-        if post_upload.stop():
-            log.critical('post-upload plugins failed')
-            self._remove_changes()
-            sys.exit(1)
+        #post_upload = Plugins('post-upload', self.changes, self.changes_file,
+        #    user_id=self.user_id)
+        #if post_upload.stop():
+        #    log.critical('post-upload plugins failed')
+        #    self._remove_changes()
+        #    sys.exit(1)
 
         # Check whether a post-upload plugin has got the orig tarball from somewhere.
         if not orig_file_found and not filecheck.is_native_package(self.changes):
@@ -526,9 +524,11 @@ class Importer(object):
         if not os.access(pylons.config['debexpo.repository'], os.W_OK):
             self._fail('debexpo.repository is not writeable')
 
-        qa = Plugins('qa', self.changes, self.changes_file, user_id=self.user_id)
-        if qa.stop():
-            self._reject('QA plugins failed the package')
+        # Create th§e database rows
+        self._create_db_entries()
+
+        # Run QA plugins
+        self._run_plugins('qa')
 
         # Loop through parent directories in the target installation directory to make sure they
         # all exist. If not, create them.
@@ -545,15 +545,13 @@ class Importer(object):
             shutil.move(file, os.path.join(destdir, file))
 
         self._remove_temporary_files()
-        # Create the database rows
-        self._create_db_entries(qa)
 
         # Execute post-successful-upload plugins
         f = open(self.changes_file)
         changes_contents = f.read()
         f.close()
-        Plugins('post-successful-upload', self.changes, self.changes_file,
-            changes_contents=changes_contents)
+        #Plugins('post-successful-upload', self.changes, self.changes_file,
+        #    changes_contents=changes_contents)
 
         # Remove the changes file
         self._remove_changes()

debexpo/lib/plugins.py

@@ -46,6 +46,8 @@ import traceback
 from debian import deb822
 import pylons
 
+from debexpo.model import meta
+
 log = logging.getLogger(__name__)
 
 # Different plugin stages and their options.
@@ -66,7 +68,7 @@ plugin_stages = {
 
 class Plugins(object):
 
-    def __init__(self, type, changes, changes_file, **kw):
+    def __init__(self, type, changes, changes_file, package_version, **kw):
         """
         Class constructor. Sets class attributes and then runs the plugins.
 
@@ -86,15 +88,16 @@ class Plugins(object):
         self.type = type.replace('-', '_')
         self.changes = changes
         self.changes_file = changes_file
-        self.result = None
+        self.package_version = package_version
+        self.result_objects = []
         self.tempdir = None
         self.kw = kw
 
         # Run the plugins.
         if type in plugin_stages:
             self.conf = plugin_stages[type]
-            log.debug('Running plugins of type: %s' % type)
-            self.result = self._run_plugins()
+        else:
+            return
 
     def _import_plugin(self, name):
         """
@@ -167,7 +170,7 @@ class Plugins(object):
             shutil.rmtree(self.tempdir)
             os.chdir(self.oldcurdir)
 
-    def _run_plugins(self):
+    def run_plugins(self):
         """
         Look in the config file and run the plugins.
         """
@@ -175,11 +178,10 @@ class Plugins(object):
         log.debug("Getting plugins with key (repr): %s", repr(key))
         plugins = self.config.get(key)
         log.debug("Using these plugins: %s", plugins)
-        result = []
 
         if not plugins:
-            log.debug("Returning result: %s", result)
-            return result
+            log.debug("Returning result: %s", self.result_objects)
+            return self.result_objects
 
         # Look at whether the plugins need extracting.
         if 'extract' in self.conf and self.conf['extract']:
@@ -204,28 +206,28 @@ class Plugins(object):
 
             # The 'plugin' object points to the class containing the actual plugin/test
             if hasattr(module, 'plugin'):
-                p = getattr(module, 'plugin')(name=plugin, changes=self.changes, \
-                    changes_file=self.changes_file, tempdir=self.tempdir)
-
-                for item in self.kw:
-                    setattr(p, item, self.kw[item])
+                p = getattr(module, 'plugin')
+                plugin_instance = p(self.package_version,
+                  name=plugin,
+                  changes=self.changes,
+                  changes_file=self.changes_file,
+                  tempdir=self.tempdir,
+                  **self.kw)
 
                 try:
-                    result.extend(p.run())
+                    plugin_instance.run()
                 except Exception:
                     log.debug("Something wrong happened while running the plugin '%s': %s" % (plugin, traceback.format_exc()))
 
-        if self.conf['extract']:
-            self._cleanup()
+                else:
+                    for obj in plugin_instance.db_objects:
+                        meta.session.add(obj)
 
-        return result
 
-    def stop(self):
-        """
-        Returns whether the importer should stop.
-        """
-        for result in self.result:
-            if result.stop():
-                return True
+            meta.session.commit()
+            log.debug("Added plugin result objects to the database.")
+
+        if self.conf['extract']:
+            self._cleanup()
 
-        return False
+        return self.result_objects

debexpo/model/__init__.py

@@ -62,7 +62,7 @@ def import_all_models():
     from debexpo.model import binary_packages, package_files, packages, source_packages, \
         user_metrics, package_comments, package_info, package_versions, user_countries, \
         users, package_subscriptions, user_upload_key, password_reset, sponsor_metrics, \
-        data_store
+        data_store, plugin_results
 
 class OrmObject(object):
     """

debexpo/model/plugin_results.py

+# -*- coding: utf-8 -*-
+#
+#   plugin_results.py — plugin results table model
+#
+#   This file is part of debexpo - https://alioth.debian.org/projects/debexpo/
+#
+#   Copyright © 2012 Clément Schreiner <clement@mux.me>
+#
+#   Permission is hereby granted, free of charge, to any person
+#   obtaining a copy of this software and associated documentation
+#   files (the "Software"), to deal in the Software without
+#   restriction, including without limitation the rights to use,
+#   copy, modify, merge, publish, distribute, sublicense, and/or sell
+#   copies of the Software, and to permit persons to whom the
+#   Software is furnished to do so, subject to the following
+#   conditions:
+#
+#   The above copyright notice and this permission notice shall be
+#   included in all copies or substantial portions of the Software.
+#
+#   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+#   EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+#   OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+#   NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+#   HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+#   WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+#   FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+#   OTHER DEALINGS IN THE SOFTWARE.
+
+"""
+Holds plugin_results table model.
+"""
+
+from sqlalchemy import orm
+import sqlalchemy as sa
+
+from debexpo.model import meta, OrmObject
+from debexpo.model.package_versions import PackageVersion
+
+t_plugin_results = sa.Table('plugin_results', meta.metadata,
+                            sa.Column('plugin', sa.types.String(200), primary_key=True),
+                            sa.Column('package_version_id', sa.types.Integer,
+                                      sa.ForeignKey('package_versions.id'),
+                                      primary_key=True),
+                            sa.Column('id', sa.types.Integer, primary_key=True),
+                            sa.Column('key', sa.types.String(200), primary_key=True),
+                            sa.Column('value', sa.types.Text()),
+                           )
+
+class PluginResult(OrmObject):
+    foreign = ['package_info']
+
+plugin_result_mapper = orm.mapper(PluginResult,
+    t_plugin_results,
+    polymorphic_on = t_plugin_results.c.plugin,
+    polymorphic_identity = 'plugin_results', \
+        properties = {
+            'plugin_result' : orm.relation(PackageVersion,
+                                           backref='plugin_results'),
+        }
+    )
+

debexpo/plugins/__init__.py

@@ -37,114 +37,32 @@ __license__ = 'MIT'
 
 from debexpo.lib import constants
 
+
 class BasePlugin(object):
-    """
-    The class all other plugins should extend.
-    """
+    result_model = None
 
-    def __init__(self, **kw):
-        """
-        Class constructor. Sets class attributes depending on the arguments of the
-        constructor.
+    def __init__(self, package_version, **kw):
+        self._db_objects = []
+        self.package_version = package_version
 
-        ``kw``
-            Values to assign to class attributes.
-        """
         for key in kw:
             setattr(self, key, kw[key])
 
     def run(self):
-        """
-        Runs all the tests in the self.tests list.
-        """
-        self.result = []
         for method in dir(self):
             if method.startswith('test'):
                 getattr(self, method)()
 
-        return self.result
-
-    def passed(self, outcome, data, severity):
-        """
-        Adds a PluginResult for a passed test to the result list.
-
-        ``outcome``
-            Outcome tag of the test.
-
-        ``data``
-            Resulting data from the plugin, like more details about the process.
-
-        ``severity``
-            Severity of the result.
-        """
-        self.result.append(PluginResult(from_plugin=self.name, outcome=outcome,
-            data=data, severity=severity))
-
-    def failed(self, outcome, data, severity):
-        """
-        Adds a PluginResult for a failed test to the result list.
-
-        ``outcome``
-            Outcome tag of the test.
-
-        ``data``
-            Resulting data from the plugin, like more details about the process.
-
-        ``severity``
-            Severity of the result.
-
-        """
-        self.result.append(PluginResult(from_plugin=self.name, outcome=outcome,
-            data=data, severity=severity))
-
-    def info(self, outcome, data):
-        """
-        Adds a PluginResult for an info test to the result list.
-
-        ``outcome``
-            Outcome tag of the test.
-
-        ``data``
-            Resulting data from the plugin, like more detail about the process.
-        """
-        self.result.append(PluginResult(from_plugin=self.name, outcome=outcome,
-            data=data, severity=constants.PLUGIN_SEVERITY_INFO))
-
-class PluginResult(object):
-    """
-    The class tests should return to provide details about a test.
-    """
-
-    def __init__(self, from_plugin, outcome, data, severity):
-        """
-        Class constructor. Sets important fields.
-
-        ``from_plugin``
-            Name of the plugin the test was carried out in.
-
-        ``outcome``
-            Outcome of the test.
-
-        ``data``
-            More details of the test.
-
-        ``severity``
-            Severity of the result.
-        """
-        self.from_plugin = from_plugin
-        self.outcome = outcome
-        self.data = data
-        self.severity = severity
-
-    def failed(self):
-        """
-        Returns whether the test failed.
-        """
-        return self.severity > constants.PLUGIN_SEVERITY_INFO
+    def _add_db_objects(self, *db_objects):
+        self._db_objects.extend(db_objects)
 
-    def stop(self):
-        """
-        Returns whether the process should stop after the test.
-        """
-        return self.severity >= constants.PLUGIN_SEVERITY_CRITICAL
+    def _add_data(self, id=0, **data):
+        result_data = [self.result_model(package_version_id = self.package_version.id,
+                                        id = id,
+                                        key = key,
+                                        value = data[key]) for key in data]
+        self._add_db_objects(*result_data)
 
+    @property
+    def db_objects(self):
+        return self._db_objects

debexpo/plugins/buildsystem.py

@@ -48,10 +48,20 @@ from debian import deb822
 
 from debexpo.lib import constants
 from debexpo.plugins import BasePlugin
+from debexpo.models.plugin_results import PluginResult, plugin_result_mapper
+
+from sqlalchemy import orm
 
 log = logging.getLogger(__name__)
 
+class BuildSystemResult(PluginResult):
+    pass
+
+orm.mapper(BuildSystemResult, inherits=plugin_result_mapper,
+           polymorphic_identity = 'buildsystem_result')
+
 class BuildSystemPlugin(BasePlugin):
+    model = BuildSystemResult
 
     def test_build_system(self):
         """
@@ -61,16 +71,12 @@ class BuildSystemPlugin(BasePlugin):
 
         dsc = deb822.Dsc(file(self.changes.get_dsc()))
 
-        data = {}
-        severity = constants.PLUGIN_SEVERITY_INFO
-
         build_depends = dsc.get('Build-Depends', '')
 
         if 'cdbs' in build_depends:
-            outcome = "Package uses CDBS"
-            data["build-system"] = "cdbs"
+            self._add_data(buildsystem = 'cdbs')
         elif 'debhelper' in build_depends:
-            data["build-system"] = "debhelper"
+            self._add_data(buildsystem = 'debhelper')
 
             # Retrieve the debhelper compat level
             compatpath = os.path.join(self.tempdir, "extracted/debian/compat")
@@ -78,22 +84,14 @@ class BuildSystemPlugin(BasePlugin):
                 with open(compatpath, "rb") as f:
                     compat_level = int(f.read().strip())
             except IOError:
-                compat_level = None
+                compat_level = 'unknown'
 
-            data["compat-level"] = compat_level
+            self._add_data(compat-level = compat_level)
 
             # Warn on old compatibility levels
             if compat_level is None or compat_level <= 4:
-                outcome = "Package uses debhelper with an old compatibility level"
-                severity = constants.PLUGIN_SEVERITY_WARNING
-            else:
-                outcome = "Package uses debhelper"
+                self._add_data(severity = constants.PLUGIN_SEVERITY_WARNING)
         else:
-            outcome = "Package uses an unknown build system"
-            data["build-system"] = "unknown"
-            severity = constants.PLUGIN_SEVERITY_WARNING
-
-        self.failed(outcome, data, severity)
-
-
+            self._add_data(build-system = 'unknown',
+                           severity = constants.PLUGIN_SEVERITY_WARNING)
 plugin = BuildSystemPlugin

debexpo/plugins/debianqa.py

@@ -50,16 +50,23 @@ from debexpo.plugins import BasePlugin
 
 log = logging.getLogger(__name__)
 
+class DebianqaResult(PluginResult):
+    @property
+    def in_debian(self):
+        return True if self.get('in_debian') == 'true' else False
+
 class DebianPlugin(BasePlugin):
+    result_model = DebianqaResult
 
+    @property
     def _in_debian(self):
         try:
             self.qa_page = urllib2.urlopen('http://packages.qa.debian.org/%s' % self.changes['Source'])
         except urllib2.HTTPError:
-            self.in_debian = False
+           return False
         else:
-            self.in_debian = True
             self.parsed_qa = lxml.etree.fromstring(self.qa_page.read())
+            return = True
 
     def _qa_xpath(self, query, item = None):
         """Perform the xpath query on the given item"""
@@ -76,12 +83,7 @@ class DebianPlugin(BasePlugin):
         """
         log.debug('Testing whether the package is in Debian already')
 
-        if self.in_debian:
-            self.outcome = "Package is already in Debian"
-        else:
-            self.outcome = "Package is not in Debian"
-
-        self.data["in-debian"] = self.in_debian
+        self._add_result(in-debian = 'true' if self._in_debian else 'false')
 
     def _test_last_upload(self):
         """

debexpo/plugins/native.py

@@ -43,10 +43,20 @@ import logging
 
 from debexpo.lib import constants, filesystem
 from debexpo.plugins import BasePlugin
+from debexpo.model.plugin_results import PluginResult, plugin_result_mapper
+
+from sqlalchemy import orm
 
 log = logging.getLogger(__name__)
 
+class NativeResult(PluginResult):
+    pass
+
+orm.mapper(NativeResult, inherits=plugin_result_mapper,
+           polymorphic_identity = 'native')
+
 class NativePlugin(BasePlugin):
+    result_model = NativeResult
 
     def test_native(self):
         """
@@ -61,9 +71,10 @@ class NativePlugin(BasePlugin):
             # Most uploads will not be native, and especially on mentors, a native
             # package is almost probably in error.
             log.warning('Package is native')
-            self.failed('Package is native', {"native": True}, constants.PLUGIN_SEVERITY_WARNING)
+            self._add_data(native = 'true')
         else:
             log.debug('Package is not native')
-            self.passed('Package is not native', {"native": False}, constants.PLUGIN_SEVERITY_INFO)
+            self._add_data(native = 'false',
+                            severity = constants.PLUGIN_SEVERITY_INFO)
 
 plugin = NativePlugin