launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/auditor-for-packageupload into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/auditor-for-packageupload/+merge/116180

Write an AuditorClient class, which matches up to the AuditorServer test fixture. It will connect to the configured auditor instance and send or receive data. I have also added a feature flag 'auditor.enabled' which model/browser code can use to check if they should call into the auditor service.
-- 
https://code.launchpad.net/~stevenk/launchpad/auditor-for-packageupload/+merge/116180
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/auditor-for-packageupload into lp:launchpad.

=== added file 'lib/lp/services/auditor/client.py'
--- lib/lp/services/auditor/client.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/auditor/client.py	2012-07-23 06:58:20 +0000
@@ -0,0 +1,42 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Client that will send and recieve audit logs to an auditor instance."""
+
+__metaclass__ = type
+__all__ = [
+    'AuditorClient',
+    ]
+
+from auditorclient.client import Client
+
+from lp.services.config import config
+from lp.services.enterpriseid import (
+    enterpriseid_to_object,
+    object_to_enterpriseid,
+    )
+
+
+class AuditorClient(Client):
+
+    def __init__(self):
+        super(AuditorClient, self).__init__(
+            config.auditor.host, config.auditor.port)
+
+    def send(self, obj, operation, actorobj, comment=None, details=None):
+        return super(AuditorClient, self).send(
+            object_to_enterpriseid(obj), operation,
+            object_to_enterpriseid(actorobj), comment, details)
+
+    def recieve(self, obj=None, operation=None, actorobj=None, limit=None):
+        if obj:
+            obj = object_to_enterpriseid(obj)
+        if actorobj:
+            actorobj = object_to_enterpriseid(actorobj)
+        logs = super(AuditorClient, self).recieve(
+            obj, operation, actorobj, limit)
+        # Process the actors and objects back from enterprise ids.
+        for entry in logs['log-entries']:
+            entry['actor'] = enterpriseid_to_object(entry['actor'])
+            entry['object'] = enterpriseid_to_object(entry['object'])
+        return logs['log-entries']

=== added file 'lib/lp/services/auditor/tests/test_client.py'
--- lib/lp/services/auditor/tests/test_client.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/auditor/tests/test_client.py	2012-07-23 06:58:20 +0000
@@ -0,0 +1,27 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from lp.services.auditor.client import AuditorClient
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import AuditorLayer
+
+
+class TestAuditorClient(TestCaseWithFactory):
+
+    layer = AuditorLayer
+
+    def test_send_and_recieve(self):
+        # We can use .send() and .recieve() on AuditorClient to log.
+        actor = self.factory.makePerson()
+        pu = self.factory.makePackageUpload()
+        client = AuditorClient()
+        result = client.send(pu, 'packageupload-accepted', actor)
+        self.assertEqual('Operation recorded.', result)
+        result = client.recieve(obj=pu)
+        del result[0]['date'] # Ignore the date.
+        expected = [{
+            u'comment': u'', u'details': u'', u'actor': actor,
+            u'operation': u'packageupload-accepted', u'object': pu}]
+        self.assertContentEqual(expected, result)

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2012-07-19 13:55:15 +0000
+++ lib/lp/services/config/schema-lazr.conf	2012-07-23 06:58:20 +0000
@@ -33,6 +33,11 @@
 run_parts_location: none
 
 
+[auditor]
+host: localhost
+port: none
+
+
 [binaryfile_expire]
 dbuser: binaryfile-expire
 

=== modified file 'lib/lp/services/enterpriseid.py'
--- lib/lp/services/enterpriseid.py	2012-03-22 23:21:24 +0000
+++ lib/lp/services/enterpriseid.py	2012-07-23 06:58:20 +0000
@@ -11,15 +11,9 @@
 
 import os
 
-from lp.registry.model.person import Person
 from lp.services.config import config
 
 
-known_types = {
-    'Person': Person,
-    }
-
-
 def object_to_enterpriseid(obj):
     """Given an object, convert it to SOA Enterprise ID."""
     otype = obj.__class__.__name__
@@ -33,8 +27,14 @@
 
 def enterpriseid_to_object(eid):
     """Given an SOA Enterprise ID, return the object that it references."""
+    from lp.registry.model.person import Person
+    from lp.soyuz.model.queue import PackageUpload
     scheme = eid.split(':')
     if not scheme[0].startswith('lp'):
         raise TypeError
+    known_types = {
+        'PackageUpload': PackageUpload,
+        'Person': Person,
+    }
     klass = known_types[scheme[1]]
     return klass.get(scheme[2])

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-07-17 21:41:42 +0000
+++ lib/lp/services/features/flags.py	2012-07-23 06:58:20 +0000
@@ -313,6 +313,12 @@
      '',
      '',
      ''),
+    ('auditor.enabled',
+     'boolean',
+     'If true, send audit data to an auditor instance.',
+     '',
+     '',
+     ''),
     ])
 
 # The set of all flag names that are documented.

=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py	2012-07-09 12:32:23 +0000
+++ lib/lp/soyuz/browser/queue.py	2012-07-23 06:58:20 +0000
@@ -443,7 +443,7 @@
 
     def queue_action_accept(self, queue_item):
         """Reject the queue item passed."""
-        queue_item.acceptFromQueue()
+        queue_item.acceptFromQueue(user=self.user)
 
     def queue_action_reject(self, queue_item):
         """Accept the queue item passed."""

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2012-07-09 12:32:23 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2012-07-23 06:58:20 +0000
@@ -358,8 +358,9 @@
         """
 
     @export_write_operation()
+    @call_with(user=REQUEST_USER)
     @operation_for_version("devel")
-    def acceptFromQueue(logger=None, dry_run=False):
+    def acceptFromQueue(logger=None, dry_run=False, user=None):
         """Call setAccepted, do a syncUpdate, and send notification email.
 
          * Grant karma to people involved with the upload.

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2012-07-11 13:43:32 +0000
+++ lib/lp/soyuz/model/queue.py	2012-07-23 06:58:20 +0000
@@ -50,6 +50,7 @@
 from lp.archiveuploader.tagfiles import parse_tagfile_content
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.auditor.client import AuditorClient
 from lp.services.config import config
 from lp.services.database.bulk import load_referencing
 from lp.services.database.constants import UTC_NOW
@@ -64,6 +65,7 @@
     SQLBase,
     sqlvalues,
     )
+from lp.services.features import getFeatureFlag
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.librarian.interfaces.client import DownloadFailed
 from lp.services.librarian.model import LibraryFileAlias
@@ -625,7 +627,7 @@
         # Give some karma!
         self._giveKarma()
 
-    def acceptFromQueue(self, logger=None, dry_run=False):
+    def acceptFromQueue(self, logger=None, dry_run=False, user=None):
         """See `IPackageUpload`."""
         assert not self.is_delayed_copy, 'Cannot process delayed copies.'
 
@@ -633,6 +635,9 @@
             self._acceptNonSyncFromQueue(logger, dry_run)
         else:
             self._acceptSyncFromQueue()
+        if bool(getFeatureFlag('auditor.enabled')):
+            client = AuditorClient()
+            client.send(self, 'packageupload-accepted', user)
 
     def acceptFromCopy(self):
         """See `IPackageUpload`."""

=== modified file 'lib/lp/testing/layers.py'
--- lib/lp/testing/layers.py	2012-07-05 10:38:03 +0000
+++ lib/lp/testing/layers.py	2012-07-23 06:58:20 +0000
@@ -707,44 +707,6 @@
         pass
 
 
-class AuditorLayer(BaseLayer):
-
-    auditor = AuditorServer()
-
-    _is_setup = False
-
-    @classmethod
-    @profiled
-    def setUp(cls):
-        cls.auditor.setUp()
-        cls.config_fixture.add_section(
-            cls.auditor.config.service_config)
-        cls.appserver_config_fixture.add_section(
-            cls.auditor.config.service_config)
-        cls._is_setup = True
-
-    @classmethod
-    @profiled
-    def tearDown(cls):
-        if not cls._is_setup:
-            return
-        cls.auditor.cleanUp()
-        cls._is_setup = False
-        # Can't pop the config above, so bail here and let the test runner
-        # start a sub-process.
-        raise NotImplementedError
-
-    @classmethod
-    @profiled
-    def testSetUp(cls):
-        pass
-
-    @classmethod
-    @profiled
-    def testTearDown(cls):
-        pass
-
-
 # We store a reference to the DB-API connect method here when we
 # put a proxy in its place.
 _org_connect = None
@@ -1389,6 +1351,42 @@
         disconnect_stores()
 
 
+class AuditorLayer(LaunchpadFunctionalLayer):
+
+    auditor = AuditorServer()
+
+    _is_setup = False
+
+    @classmethod
+    @profiled
+    def setUp(cls):
+        cls.auditor.setUp()
+        cls.config_fixture.add_section(cls.auditor.service_config)
+        cls.appserver_config_fixture.add_section(cls.auditor.service_config)
+        cls._is_setup = True
+
+    @classmethod
+    @profiled
+    def tearDown(cls):
+        if not cls._is_setup:
+            return
+        cls.auditor.cleanUp()
+        cls._is_setup = False
+        # Can't pop the config above, so bail here and let the test runner
+        # start a sub-process.
+        raise NotImplementedError
+
+    @classmethod
+    @profiled
+    def testSetUp(cls):
+        pass
+
+    @classmethod
+    @profiled
+    def testTearDown(cls):
+        pass
+
+
 class GoogleLaunchpadFunctionalLayer(LaunchpadFunctionalLayer,
                                      GoogleServiceLayer):
     """Provides Google service in addition to LaunchpadFunctionalLayer."""

=== modified file 'setup.py'
--- setup.py	2012-07-02 04:21:44 +0000
+++ setup.py	2012-07-23 06:58:20 +0000
@@ -27,6 +27,7 @@
     # used in zcml.
     install_requires=[
         'ampoule',
+        'auditorclient',
         'auditorfixture',
         'BeautifulSoup',
         'bzr',

=== modified file 'versions.cfg'
--- versions.cfg	2012-07-13 01:55:27 +0000
+++ versions.cfg	2012-07-23 06:58:20 +0000
@@ -9,7 +9,8 @@
 anyjson = 0.3.1
 argparse = 1.2.1
 auditor = 0.0.1
-auditorfixture = 0.0.1
+auditorclient = 0.0.1
+auditorfixture = 0.0.2
 BeautifulSoup = 3.1.0.1
 bson = 0.3.2
 # The source for this version of bzr is at lp:~benji/bzr/bug-998040