Merge lp:~jpds/launchpad/fix_517839 into lp:launchpad

[Jonathan Davies]

Exported countries and DistributionMirror.country's via the API.

[Jonathan Davies (jpds)]

= Summary =

As per bug #517839, we should export the country attribute of DistributionMirror's. This is to compliment bug #361650 later on.

= Pre-implementation details =

I spoke to Curtis, who said that the simplest fix would be to register a canonical URL for country. I have done this by exporting countries as +countries/$CC.

= Tests =

Newly exported Country's can be tested with:

bin/test -vvct webservice/xx-country.txt

And DistributionMirror changes:

bin/test -vvct webservice/xx-distribution-mirror.txt

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/canonical/launchpad/browser/launchpad.py
  lib/lp/registry/interfaces/distributionmirror.py
  lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
  lib/lp/services/worlddata/browser/
  lib/lp/services/worlddata/configure.zcml
  lib/lp/services/worlddata/stories/
  lib/lp/services/worlddata/browser/__init__.py
  lib/lp/services/worlddata/browser/configure.zcml
  lib/lp/services/worlddata/browser/country.py
  lib/lp/services/worlddata/interfaces/country.py
  lib/lp/services/worlddata/model/country.py
  lib/lp/services/worlddata/stories/webservice/
  lib/lp/services/worlddata/stories/webservice/xx-country.txt

[Tim Penhey (thumper)]

Mostly small style issues.

> === added file 'lib/lp/services/worlddata/browser/country.py'
> --- lib/lp/services/worlddata/browser/country.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/services/worlddata/browser/country.py 2010-02-20 15:12:23 +0000
> @@ -0,0 +1,10 @@
> +# Copyright 2009 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).

It is 2010 now :-)

> +
> +from lp.services.worlddata.interfaces.country import ICountrySet
> +
> +from canonical.launchpad.webapp import (
> + GetitemNavigation)

This can fit on one line.

2 blank lines between module level bits.

> +
> +class CountrySetNavigation(GetitemNavigation):
> + usedfor = ICountrySet
>
> === modified file 'lib/lp/services/worlddata/interfaces/country.py'
> --- lib/lp/services/worlddata/interfaces/country.py 2009-06-25 04:06:00 +0000
> +++ lib/lp/services/worlddata/interfaces/country.py 2010-02-20 15:12:23 +0000
> @@ -20,26 +20,32 @@
> from canonical.launchpad.validators.name import valid_name
> from canonical.launchpad import _
>
> +from lazr.restful.declarations import (
> + export_as_webservice_collection, collection_default_content,
> + export_read_operation, export_as_webservice_entry, exported,
> + operation_parameters, operation_returns_entry)
> +
> class ICountry(Interface):
> """The country description."""
> + export_as_webservice_entry(plural_name='countries')
>
> id = Int(
> title=_('Country ID'), required=True, readonly=True,
> )
> - iso3166code2 = TextLine( title=_('iso3166code2'), required=True,
> - readonly=True)
> - iso3166code3 = TextLine( title=_('iso3166code3'), required=True,
> - readonly=True)
> - name = TextLine(
> + iso3166code2 = exported(TextLine( title=_('iso3166code2'), required=True,
> + readonly=True))
> + iso3166code3 = exported(TextLine( title=_('iso3166code3'), required=True,
> + readonly=True))

These could be formatted a little nicer :)

> + name = exported(TextLine(
> title=_('Country name'), required=True,
> constraint=valid_name,
> - )
> - title = Title(
> + ))
> + title = exported(Title(
> title=_('Country title'), required=True,
> - )
> - description = Description(
> + ))
> + description = exported(Description(
> title=_('Description'), required=True,
> - )
> + ))
>
> continent = Attribute("The Continent where this country is located.")
> languages = Attribute("An iterator over languages that are spoken in "
> @@ -48,6 +54,7 @@
>
> class ICountrySet(Interface):
> """A container for countries."""
> + export_as_webservice_collection(ICountry)
>
> def __getitem__(key):
> """Get a country."""
> @@ -55,6 +62,24 @@
> def __iter__():
> """Iterate through the countries in this set."""
>
> + @operation_parameters(
> + name=TextLine(title=_("Name"), required=True))
> + @operation_...

review: Needs Fixing

[Tim Penhey (thumper)]

Thanks for the update. Good to go now.

review: Approve

=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
--- lib/canonical/launchpad/browser/launchpad.py	2010-02-18 17:00:54 +0000
+++ lib/canonical/launchpad/browser/launchpad.py	2010-02-23 19:47:16 +0000
@@ -92,6 +92,7 @@
     ISourcePackageNameSet)
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.blueprints.interfaces.sprint import ISprintSet
+from lp.services.worlddata.interfaces.country import ICountrySet
 from lp.translations.interfaces.translationgroup import (
     ITranslationGroupSet)
 from lp.translations.interfaces.translationimportqueue import (
@@ -548,6 +549,7 @@
         '+code': IBazaarApplication,
         '+code-imports': ICodeImportSet,
         'codeofconduct': ICodeOfConductSet,
+        '+countries': ICountrySet,
         'distros': IDistributionSet,
         '+hwdb': IHWDBApplication,
         'karmaaction': IKarmaActionSet,

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-02-22 17:11:52 +0000
+++ lib/canonical/launchpad/security.py	2010-02-23 19:47:16 +0000
@@ -119,6 +119,7 @@
 from lp.answers.interfaces.faqtarget import IFAQTarget
 from lp.answers.interfaces.question import IQuestion
 from lp.answers.interfaces.questiontarget import IQuestionTarget
+from lp.services.worlddata.interfaces.country import ICountry
 
 
 class AuthorizationBase:
@@ -840,6 +841,11 @@
     usedfor = IDistroSeries
 
 
+class ViewCountry(AnonymousAuthorization):
+    """Anyone can view a Country."""
+    usedfor = ICountry
+
+
 class SeriesDrivers(AuthorizationBase):
     """Drivers can approve or decline features and target bugs.
 

=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
--- lib/lp/registry/interfaces/distributionmirror.py	2010-02-08 20:26:19 +0000
+++ lib/lp/registry/interfaces/distributionmirror.py	2010-02-23 19:47:16 +0000
@@ -32,7 +32,7 @@
 from lazr.enum import DBEnumeratedType, DBItem
 from lazr.restful.declarations import (
     export_as_webservice_entry, export_read_operation, exported)
-from lazr.restful.fields import Reference
+from lazr.restful.fields import Reference, ReferenceChoice
 
 from canonical.launchpad import _
 from canonical.launchpad.fields import (
@@ -40,6 +40,7 @@
 from canonical.launchpad.validators.name import name_validator
 from canonical.launchpad.validators import LaunchpadValidationError
 from canonical.launchpad.webapp.menu import structured
+from lp.services.worlddata.interfaces.country import ICountry
 
 
 # The number of hours before we bother probing a mirror again
@@ -353,9 +354,11 @@
     speed = exported(Choice(
         title=_('Link Speed'), required=True, readonly=False,
         vocabulary=MirrorSpeed))
-    country = Choice(
-        title=_('Location'), required=True, readonly=False,
-        vocabulary='CountryName')
+    country = exported(ReferenceChoice(
+        title=_('Location'), description=_(
+            "The country in which this mirror is based."),
+        required=True, readonly=False,
+        vocabulary='CountryName', schema=ICountry))
     content = exported(Choice(
         title=_('Content'), required=True, readonly=False,
         description=_(

=== modified file 'lib/lp/registry/stories/webservice/xx-distribution-mirror.txt'
--- lib/lp/registry/stories/webservice/xx-distribution-mirror.txt	2010-02-22 17:47:59 +0000
+++ lib/lp/registry/stories/webservice/xx-distribution-mirror.txt	2010-02-23 19:47:16 +0000
@@ -12,6 +12,7 @@
     >>> canonical_archive_json = webservice.get(canonical_archive['self_link']).jsonBody()
     >>> pprint_entry(canonical_archive_json)
     content: u'Archive'
+    country_link: u'http://.../+countries/GB'
     date_created: u'2006-10-16T18:31:43.434567+00:00'
     date_reviewed: None
     description: None
@@ -38,6 +39,7 @@
     >>> canonical_releases_json = webservice.get(canonical_releases['self_link']).jsonBody()
     >>> pprint_entry(canonical_releases_json)
     content: u'CD Image'
+    country_link: u'http://.../+countries/GB'
     date_created: u'2006-10-16T18:31:43.434567+00:00'
     date_reviewed: None
     description: None
@@ -126,6 +128,7 @@
     ...     archive_404_mirror['self_link']).jsonBody()
     >>> pprint_entry(response)
     content: u'Archive'
+    country_link: u'http://.../+countries/FR'
     date_created: u'2006-10-16T18:31:43.438573+00:00'
     date_reviewed: None
     description: None
@@ -190,7 +193,10 @@
 
 While others can be set with the appropriate authorization:
 
+    >>> greenland = webservice.named_get("/+countries",
+    ...     "getByCode", code="GL").jsonBody()
     >>> patch = {
+    ...     u'country_link': greenland['self_link'],
     ...     u'status' : 'Unofficial',
     ...     u'whiteboard' : u'This mirror is too shiny to be true'
     ... }
@@ -203,7 +209,23 @@
     ...     canonical_releases['self_link'], 'application/json', dumps(patch)).jsonBody()
     >>> pprint_entry(response)
     content: u'CD Image'
-    ...
+    country_link: u'http://.../+countries/GL'
+    date_created: u'2006-10-16T18:31:43.434567+00:00'
+    date_reviewed: None
+    description: None
+    displayname: None
+    distribution_link: u'http://.../ubuntu'
+    enabled: True
+    ftp_base_url: None
+    http_base_url: u'http://releases.ubuntu.com/'
+    name: u'canonical-releases'
+    official_candidate: True
+    owner_link: u'http://.../~mark'
+    resource_type_link: u'http://.../#distribution_mirror'
+    reviewer_link: None
+    rsync_base_url: None
+    self_link: u'http://.../ubuntu/+mirror/canonical-releases'
+    speed: u'100 Mbps'
     status: u'Unofficial'
     whiteboard: u'This mirror is too shiny to be true'
 

=== modified file 'lib/lp/registry/stories/webservice/xx-distribution.txt'
--- lib/lp/registry/stories/webservice/xx-distribution.txt	2010-02-05 18:53:28 +0000
+++ lib/lp/registry/stories/webservice/xx-distribution.txt	2010-02-23 19:47:16 +0000
@@ -123,6 +123,7 @@
     ...     name='canonical-releases').jsonBody()
     >>> pprint_entry(canonical_releases)
     content: u'CD Image'
+    country_link: u'http://.../+countries/GB'
     date_created: u'2006-10-16T18:31:43.434567+00:00'
     date_reviewed: None
     description: None

=== added directory 'lib/lp/services/worlddata/browser'
=== added file 'lib/lp/services/worlddata/browser/__init__.py'
=== added file 'lib/lp/services/worlddata/browser/configure.zcml'
--- lib/lp/services/worlddata/browser/configure.zcml	1970-01-01 00:00:00 +0000
+++ lib/lp/services/worlddata/browser/configure.zcml	2010-02-23 19:47:16 +0000
@@ -0,0 +1,28 @@
+<!-- Copyright 2010 Canonical Ltd.  This software is licensed under the 
+     GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<configure
+    xmlns="http://namespaces.zope.org/zope"
+    xmlns:browser="http://namespaces.zope.org/browser"
+    xmlns:i18n="http://namespaces.zope.org/i18n"
+    xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
+    i18n_domain="launchpad">
+
+    <browser:url
+        for="lp.services.worlddata.interfaces.country.ICountry"
+        path_expression="iso3166code2"
+        parent_utility="lp.services.worlddata.interfaces.country.ICountrySet"
+        rootsite="mainsite" />
+
+    <browser:url
+        for="lp.services.worlddata.interfaces.country.ICountrySet"
+        path_expression="string:+countries"
+        parent_utility="canonical.launchpad.interfaces.ILaunchpadRoot"
+        rootsite="mainsite" />
+
+    <browser:navigation
+        module="lp.services.worlddata.browser.country"
+        classes="CountrySetNavigation" />
+
+</configure>

=== added file 'lib/lp/services/worlddata/browser/country.py'
--- lib/lp/services/worlddata/browser/country.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/worlddata/browser/country.py	2010-02-23 19:47:16 +0000
@@ -0,0 +1,10 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from lp.services.worlddata.interfaces.country import ICountrySet
+
+from canonical.launchpad.webapp import GetitemNavigation
+
+
+class CountrySetNavigation(GetitemNavigation):
+    usedfor = ICountrySet

=== modified file 'lib/lp/services/worlddata/configure.zcml'
--- lib/lp/services/worlddata/configure.zcml	2009-09-17 14:45:59 +0000
+++ lib/lp/services/worlddata/configure.zcml	2010-02-23 19:47:16 +0000
@@ -55,4 +55,6 @@
         factory="lp.translations.browser.language.LanguageSetBreadcrumb"
         permission="zope.Public"/>
 
+    <include package=".browser"/>
+
 </configure>

=== modified file 'lib/lp/services/worlddata/interfaces/country.py'
--- lib/lp/services/worlddata/interfaces/country.py	2009-06-25 04:06:00 +0000
+++ lib/lp/services/worlddata/interfaces/country.py	2010-02-23 19:47:16 +0000
@@ -20,26 +20,35 @@
 from canonical.launchpad.validators.name import valid_name
 from canonical.launchpad import _
 
+from lazr.restful.declarations import (
+    export_as_webservice_collection, collection_default_content,
+    export_read_operation, export_as_webservice_entry, exported,
+    operation_parameters, operation_returns_entry)
+
+
 class ICountry(Interface):
     """The country description."""
+    export_as_webservice_entry(plural_name='countries')
 
     id = Int(
-            title=_('Country ID'), required=True, readonly=True,
-            )
-    iso3166code2 = TextLine( title=_('iso3166code2'), required=True,
-                             readonly=True)
-    iso3166code3 = TextLine( title=_('iso3166code3'), required=True,
-                             readonly=True)
-    name = TextLine(
+        title=_('Country ID'), required=True, readonly=True,
+        )
+    iso3166code2 = exported(
+        TextLine(title=_('iso3166code2'), required=True,
+            readonly=True))
+    iso3166code3 = exported(
+        TextLine(title=_('iso3166code3'), required=True,
+            readonly=True))
+    name = exported(
+        TextLine(
             title=_('Country name'), required=True,
-            constraint=valid_name,
-            )
-    title = Title(
-            title=_('Country title'), required=True,
-            )
-    description = Description(
-            title=_('Description'), required=True,
-            )
+            constraint=valid_name))
+    title = exported(
+        Title(
+            title=_('Country title'), required=True))
+    description = exported(
+        Description(
+            title=_('Description'), required=True))
 
     continent = Attribute("The Continent where this country is located.")
     languages = Attribute("An iterator over languages that are spoken in "
@@ -48,6 +57,7 @@
 
 class ICountrySet(Interface):
     """A container for countries."""
+    export_as_webservice_collection(ICountry)
 
     def __getitem__(key):
         """Get a country."""
@@ -55,6 +65,24 @@
     def __iter__():
         """Iterate through the countries in this set."""
 
+    @operation_parameters(
+        name=TextLine(title=_("Name"), required=True))
+    @operation_returns_entry(ICountry)
+    @export_read_operation()
+    def getByName(name):
+        """Return a country by its name."""
+
+    @operation_parameters(
+        code=TextLine(title=_("Code"), required=True))
+    @operation_returns_entry(ICountry)
+    @export_read_operation()
+    def getByCode(code):
+        """Return a country by its code."""
+
+    @collection_default_content()
+    def getCountries():
+        """Return a collection of countries."""
+
 
 class IContinent(Interface):
     """See IContinent."""

=== modified file 'lib/lp/services/worlddata/model/country.py'
--- lib/lp/services/worlddata/model/country.py	2009-06-25 04:06:00 +0000
+++ lib/lp/services/worlddata/model/country.py	2010-02-23 19:47:16 +0000
@@ -13,6 +13,7 @@
 from canonical.database.constants import DEFAULT
 from canonical.database.sqlbase import SQLBase
 from canonical.launchpad.interfaces import NotFoundError
+from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.services.worlddata.interfaces.country import (
     ICountry, ICountrySet, IContinent)
 
@@ -57,6 +58,17 @@
         for row in Country.select():
             yield row
 
+    def getByName(self, name):
+        """See `ICountrySet`."""
+        return IStore(Country).find(Country, name=name).one()
+
+    def getByCode(self, code):
+        """See `ICountrySet`."""
+        return IStore(Country).find(Country, iso3166code2=code).one()
+
+    def getCountries(self):
+        """See `ICountrySet`."""
+        return IStore(Country).find(Country).order_by(Country.iso3166code2)
 
 class Continent(SQLBase):
     """See IContinent."""

=== added directory 'lib/lp/services/worlddata/stories'
=== added directory 'lib/lp/services/worlddata/stories/webservice'
=== added file 'lib/lp/services/worlddata/stories/webservice/xx-country.txt'
--- lib/lp/services/worlddata/stories/webservice/xx-country.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/services/worlddata/stories/webservice/xx-country.txt	2010-02-23 19:47:16 +0000
@@ -0,0 +1,72 @@
+= Countries =
+
+At the top level we provide the collection of all countries.
+
+    >>> countries = webservice.get("/+countries").jsonBody()
+    >>> for entry in countries['entries']:
+    ...     print entry['self_link']
+    http://.../+countries/AD
+    http://.../+countries/AE
+    http://.../+countries/AF
+    http://.../+countries/AG
+    http://.../+countries/AI
+
+And for every country we publish most of its attributes.
+
+    >>> from lazr.restful.testing.webservice import pprint_entry
+    >>> country = countries['entries'][0]
+    >>> andorra = webservice.get(country['self_link']).jsonBody()
+    >>> pprint_entry(andorra)
+    description: None
+    iso3166code2: u'AD'
+    iso3166code3: u'AND'
+    name: u'Andorra'
+    resource_type_link: u'http://.../#country'
+    self_link: u'http://.../+countries/AD'
+    title: u'Principality of Andorra'
+
+Make sure that invalid countries return 404s and not OOPSes.
+
+    >>> bogus_country = "http://api.launchpad.dev/beta/+countries/bogus"
+    >>> print webservice.get(bogus_country)
+    HTTP/1.1 404 Not Found
+    ...
+    Traceback (most recent call last):
+    ...
+    NotFound: Object: ..., name: u'bogus'
+    <BLANKLINE>
+
+== Country Custom Operations ==
+
+"getByName" returns a country for the given name.
+
+    >>> uk = webservice.named_get(
+    ...     '/+countries', 'getByName',
+    ...     name='United Kingdom').jsonBody()
+    >>> print uk['self_link']
+    http://.../+countries/GB
+
+Ensure that unknown/non-existent countries return a None and not an OOPS:
+
+    >>> bogus_country_by_name = webservice.named_get(
+    ...     '/+countries', 'getByName',
+    ...     name='Klingon Land').jsonBody()
+    >>> print bogus_country_by_name
+    None
+
+
+"getByCode" returns a country for the given code.
+
+    >>> au = webservice.named_get(
+    ...     '/+countries', 'getByCode',
+    ...     code='AU').jsonBody()
+    >>> print au['self_link']
+    http://.../+countries/AU
+
+Ensure that unknown/non-existent country codes return a None and not an OOPS:
+
+    >>> bogus_country_by_code = webservice.named_get(
+    ...     '/+countries', 'getByCode',
+    ...     code='TEST').jsonBody()
+    >>> print bogus_country_by_code
+    None