launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/launchpadlib-services-974139 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #974139 in launchpadlib : "Add "services" ServiceRoot to launchpadlib"
  https://bugs.launchpad.net/launchpadlib/+bug/974139

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/launchpadlib-services-974139/+merge/101349

== Implementation ==

The changes in this branch allow launchpadlib to be used like so:

lp = Launchpad(...)
service = lp.services['myservice']
service.some_method()

Essentially, ServiceFactory is made a web service collection (requiring a collection_default_content to be defined). The default content method returns all registered services.

The changes mean that lp.services.webservice.services.ServicesLink needed to be removed.

This branch can be landed after lp:~wallyworld/launchpadlib/services-serviceroot

== Tests ==

The TestLaunchpadlib test case was changed to locate the service using the new syntax.
An additional test_registeredServices test was added for the new collection_default_content method.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  versions.cfg
  lib/lp/app/configure.zcml
  lib/lp/app/services.py
  lib/lp/app/interfaces/services.py
  lib/lp/app/tests/test_services.py
  lib/lp/registry/interfaces/webservice.py
  lib/lp/registry/services/tests/test_sharingservice.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/launchpadlib-services-974139/+merge/101349
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/launchpadlib-services-974139 into lp:launchpad.

=== modified file 'lib/lp/app/configure.zcml'
--- lib/lp/app/configure.zcml	2012-02-24 05:14:46 +0000
+++ lib/lp/app/configure.zcml	2012-04-10 10:20:44 +0000
@@ -74,13 +74,5 @@
         <allow
             interface="zope.publisher.interfaces.IPublishTraverse"/>
     </securedutility>
-    <securedutility
-        class="lp.services.webservice.services.ServicesLink"
-        provides="lp.services.webservice.services.IServicesLink">
-        <allow
-            interface="lazr.restful.interfaces.ITopLevelEntryLink"/>
-        <allow
-            interface="lp.services.webapp.interfaces.ICanonicalUrlData"/>
-    </securedutility>
 
 </configure>

=== modified file 'lib/lp/app/interfaces/services.py'
--- lib/lp/app/interfaces/services.py	2012-02-28 04:24:19 +0000
+++ lib/lp/app/interfaces/services.py	2012-04-10 10:20:44 +0000
@@ -12,8 +12,15 @@
     ]
 
 from lazr.restful.declarations import (
+    collection_default_content,
+    export_as_webservice_collection,
     export_as_webservice_entry,
+    export_read_operation,
     exported,
+    operation_for_version,
+    operation_parameters,
+    operation_returns_collection_of,
+    operation_returns_entry,
     )
 from zope.interface import Interface
 from zope.schema import TextLine
@@ -24,6 +31,9 @@
 class IService(Interface):
     """Base interface for services."""
 
+    export_as_webservice_entry(
+        'service', publish_web_link=False, as_of='beta')
+
     name = exported(
         TextLine(
             title=_('Name'),
@@ -34,4 +44,18 @@
 class IServiceFactory(Interface):
     """Interface representing a factory used to access named services."""
 
-    export_as_webservice_entry(publish_web_link=False, as_of='beta')
+    export_as_webservice_collection(IService)
+
+    @collection_default_content()
+    @operation_returns_collection_of(IService)
+    @export_read_operation()
+    @operation_for_version("beta")
+    def registeredServices():
+        """Return all the registered services."""
+
+    @operation_parameters(name=TextLine(required=True))
+    @operation_returns_entry(IService)
+    @export_read_operation()
+    @operation_for_version("beta")
+    def getByName(name):
+        """Lookup a service by name."""

=== modified file 'lib/lp/app/services.py'
--- lib/lp/app/services.py	2012-02-23 10:13:48 +0000
+++ lib/lp/app/services.py	2012-04-10 10:20:44 +0000
@@ -8,7 +8,10 @@
     'ServiceFactory',
     ]
 
-from zope.component import getUtility
+from zope.component import (
+    getAllUtilitiesRegisteredFor,
+    getUtility,
+    )
 from zope.interface import implements
 
 from lp.app.interfaces.services import (
@@ -19,9 +22,9 @@
 
 
 class ServiceFactory(Navigation):
-    """Creates a named service.
+    """Provides access to named services.
 
-    Services are traversed via urls of the form /services/<name>
+    Services are traversed via urls of the form /+services/<name>
     Implementation classes are registered as named zope utilities.
     """
 
@@ -31,7 +34,10 @@
         super(ServiceFactory, self).__init__(None)
 
     def traverse(self, name):
-        return self.getService(name)
-
-    def getService(self, service_name):
-        return getUtility(IService, service_name)
+        return self.getByName(name)
+
+    def getByName(self, name):
+        return getUtility(IService, name)
+
+    def registeredServices(self):
+        return getAllUtilitiesRegisteredFor(IService)

=== modified file 'lib/lp/app/tests/test_services.py'
--- lib/lp/app/tests/test_services.py	2012-02-28 04:24:19 +0000
+++ lib/lp/app/tests/test_services.py	2012-04-10 10:20:44 +0000
@@ -43,5 +43,12 @@
         self.registerUtility(fake_service, IService, "fake")
         context, view, request = test_traverse(
             'https://launchpad.dev/api/devel/+services/fake')
-        self.assertEqual(getUtility(IServiceFactory), context)
-        self.assertEqual(fake_service, view)
+        self.assertEqual(getUtility(IService, "fake"), context)
+
+    def test_registeredServices(self):
+        # Test the ServiceFactory registeredServices method.
+        login(ANONYMOUS)
+        service_factory = getUtility(IServiceFactory)
+        self.assertEqual(
+            [service_factory.getByName('sharing')],
+            service_factory.registeredServices())

=== modified file 'lib/lp/registry/interfaces/webservice.py'
--- lib/lp/registry/interfaces/webservice.py	2012-03-22 23:21:24 +0000
+++ lib/lp/registry/interfaces/webservice.py	2012-04-10 10:20:44 +0000
@@ -29,6 +29,7 @@
     'IProductSet',
     'IProjectGroup',
     'IProjectGroupSet',
+    'IService',
     'IServiceFactory',
     'ISharingService',
     'ISSHKey',
@@ -43,7 +44,10 @@
 # XXX: JonathanLange 2010-11-09 bug=673083: Legacy work-around for circular
 # import bugs.  Break this up into a per-package thing.
 from lp import _schema_circular_imports
-from lp.app.interfaces.services import IServiceFactory
+from lp.app.interfaces.services import (
+    IService,
+    IServiceFactory,
+    )
 from lp.registry.interfaces.commercialsubscription import (
     ICommercialSubscription,
     )

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-04-06 17:28:25 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-04-10 10:20:44 +0000
@@ -758,10 +758,7 @@
     def setUp(self):
         super(TestLaunchpadlib, self).setUp()
         self.launchpad = self.factory.makeLaunchpadService(person=self.owner)
-        # XXX 2012-02-23 wallyworld bug 681767
-        # Launchpadlib can't do relative url's
-        self.service = self.launchpad.load(
-            '%s/+services/sharing' % self.launchpad._root_uri)
+        self.service = self.launchpad.services['sharing']
         flag = FeatureFixture(WRITE_FLAG)
         flag.setUp()
         self.addCleanup(flag.cleanUp)

=== removed file 'lib/lp/services/webservice/services.py'
--- lib/lp/services/webservice/services.py	2012-02-23 10:13:48 +0000
+++ lib/lp/services/webservice/services.py	1970-01-01 00:00:00 +0000
@@ -1,32 +0,0 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""A class for the top-level link to the services factory."""
-
-__metaclass__ = type
-__all__ = [
-    'IServicesLink',
-    'ServicesLink',
-    ]
-
-from lazr.restful.interfaces import ITopLevelEntryLink
-from zope.interface import implements
-
-from lp.app.interfaces.services import IServiceFactory
-from lp.services.webapp.interfaces import ICanonicalUrlData
-
-
-class IServicesLink(ITopLevelEntryLink, ICanonicalUrlData):
-    """A marker interface."""
-
-
-class ServicesLink:
-    """The top-level link to the services factory."""
-    implements(IServicesLink)
-
-    link_name = 'services'
-    entry_type = IServiceFactory
-
-    inside = None
-    path = 'services'
-    rootsite = 'api'

=== modified file 'versions.cfg'
--- versions.cfg	2012-04-06 17:28:25 +0000
+++ versions.cfg	2012-04-10 10:20:44 +0000
@@ -35,7 +35,7 @@
 Jinja2 = 2.2
 keyring = 0.6.2
 kombu = 2.1.1
-launchpadlib = 1.9.12
+launchpadlib = 1.9.13
 lazr.amqp = 0.1
 lazr.authentication = 0.1.1
 lazr.batchnavigator = 1.2.10