Launchpad itself

Merge lp:~james-w/launchpad/export-code-import into lp:launchpad

  1. export-code-import
  2. Merge into devel
Proposed by James Westby
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/export-code-import
Merge into: lp:launchpad
Diff against target: 184 lines (+28/-16)
6 files modified
lib/canonical/launchpad/doc/webservice-marshallers.txt (+5/-3)
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+4/-0)
lib/lp/code/interfaces/codeimport.py (+6/-3)
lib/lp/registry/interfaces/distroseries.py (+6/-5)
lib/lp/registry/interfaces/productseries.py (+2/-1)
lib/lp/registry/interfaces/projectgroup.py (+5/-4)
To merge this branch: bzr merge lp:~james-w/launchpad/export-code-import
Reviewer Review Type Date Requested Status
Curtis Hovey (community) Approve
Review via email: mp+22175@code.launchpad.net

Commit message

Various attributes are now exported correctly so that they can be used by launchpadlib.

Description of the change

Fix various exports to use ReferenceChoice.

When exporting something that is a link to another exported
object you must use a lazr.restful field, rather than a plain
zope one.

This is because lazr.restful has to know that it is a link to
an exported object so that it can have special behaviour.

Currently when you don't do this you don't get an error until
you come to access it with launchpadlib.

What happens is that the tales that writes out the wadl just
considers it to be a plain attribute, and so writes out
<wadl attribute name> == <field name>.

However, when the marshaller comes to write out the JSON for the
object that it will find there it sees that it is an exported
object and so writes out the URL instead. As it does this though
it gives it name of <field name> + "_link", which doesn't match
the wadl.

Therefore the webservice doesn't behave as stated and the client
falls over.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi James.

We talked IRC and agreed that this branch does fix the specific issues, but not the root cause. You are already working on a fix for the root cause that will not require us to rewrite the existing tests...which gave us a false sense that the wadl and json were in agreement.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/doc/webservice-marshallers.txt'
--- lib/canonical/launchpad/doc/webservice-marshallers.txt 2010-03-24 16:01:58 +0000
+++ lib/canonical/launchpad/doc/webservice-marshallers.txt 2010-03-25 20:49:29 +0000
@@ -29,13 +29,16 @@
29string is a URL corresponding to a vocabulary item, the marshaller29string is a URL corresponding to a vocabulary item, the marshaller
30returns that item. Otherwise it raises a ValueError.30returns that item. Otherwise it raises a ValueError.
3131
32 >>> from canonical.launchpad.interfaces import IPerson
32 >>> from zope.component import getMultiAdapter33 >>> from zope.component import getMultiAdapter
33 >>> from zope.schema import Choice34 >>> from lazr.restful.fields import ReferenceChoice
34 >>> from lazr.restful.interfaces import IFieldMarshaller35 >>> from lazr.restful.interfaces import IFieldMarshaller
35 >>> from lazr.restful import EntryResource36 >>> from lazr.restful import EntryResource
3637
37 # Bind the field, to resolve the vocabulary name.38 # Bind the field, to resolve the vocabulary name.
38 >>> field = Choice(__name__='some_person', vocabulary='ValidPersonOrTeam')39 >>> field = ReferenceChoice(
40 ... __name__='some_person', vocabulary='ValidPersonOrTeam',
41 ... schema=IPerson)
39 >>> field = field.bind(None)42 >>> field = field.bind(None)
40 >>> marshaller = getMultiAdapter((field, request), IFieldMarshaller)43 >>> marshaller = getMultiAdapter((field, request), IFieldMarshaller)
41 >>> verifyObject(IFieldMarshaller, marshaller)44 >>> verifyObject(IFieldMarshaller, marshaller)
@@ -49,7 +52,6 @@
49 >>> person.name52 >>> person.name
50 u'salgado'53 u'salgado'
5154
52 >>> from canonical.launchpad.interfaces import IPerson
53 >>> ubuntu_team = marshaller.marshall_from_json_data(55 >>> ubuntu_team = marshaller.marshall_from_json_data(
54 ... "http://api.launchpad.dev/beta/~ubuntu-team")56 ... "http://api.launchpad.dev/beta/~ubuntu-team")
55 >>> ubuntu_team.name57 >>> ubuntu_team.name
5658
=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2010-03-18 17:30:14 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2010-03-25 20:49:29 +0000
@@ -294,6 +294,7 @@
294 IDistroSeries, 'getPackageUploads', 'archive', IArchive)294 IDistroSeries, 'getPackageUploads', 'archive', IArchive)
295patch_collection_return_type(295patch_collection_return_type(
296 IDistroSeries, 'getPackageUploads', IPackageUpload)296 IDistroSeries, 'getPackageUploads', IPackageUpload)
297patch_reference_property(IDistroSeries, 'parent_series', IDistroSeries)
297298
298# IDistroArchSeries299# IDistroArchSeries
299patch_reference_property(IDistroArchSeries, 'main_archive', IArchive)300patch_reference_property(IDistroArchSeries, 'main_archive', IArchive)
@@ -374,3 +375,6 @@
374375
375# IBugTracker376# IBugTracker
376patch_reference_property(IBugTracker, 'owner', IPerson)377patch_reference_property(IBugTracker, 'owner', IPerson)
378
379# IProductSeries
380patch_reference_property(IProductSeries, 'product', IProduct)
377381
=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py 2010-03-18 17:30:14 +0000
+++ lib/lp/code/interfaces/codeimport.py 2010-03-25 20:49:29 +0000
@@ -22,9 +22,11 @@
22from canonical.launchpad.fields import PublicPersonChoice, URIField22from canonical.launchpad.fields import PublicPersonChoice, URIField
23from canonical.launchpad.validators import LaunchpadValidationError23from canonical.launchpad.validators import LaunchpadValidationError
24from lp.code.enums import CodeImportReviewStatus, RevisionControlSystems24from lp.code.enums import CodeImportReviewStatus, RevisionControlSystems
25from lp.code.interfaces.branch import IBranch
2526
26from lazr.restful.declarations import (27from lazr.restful.declarations import (
27 export_as_webservice_entry, exported)28 export_as_webservice_entry, exported)
29from lazr.restful.fields import ReferenceChoice
2830
2931
30def validate_cvs_root(cvsroot):32def validate_cvs_root(cvsroot):
@@ -70,10 +72,11 @@
70 title=_("Date Created"), required=True, readonly=True)72 title=_("Date Created"), required=True, readonly=True)
7173
72 branch = exported(74 branch = exported(
73 Choice(75 ReferenceChoice(
74 title=_('Branch'), required=True, readonly=True,76 title=_('Branch'), required=True, readonly=True,
75 vocabulary='Branch',77 vocabulary='Branch', schema=IBranch,
76 description=_("The Bazaar branch produced by the import system.")))78 description=_("The Bazaar branch produced by the "
79 "import system.")))
7780
78 registrant = PublicPersonChoice(81 registrant = PublicPersonChoice(
79 title=_('Registrant'), required=True, readonly=True,82 title=_('Registrant'), required=True, readonly=True,
8083
=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py 2010-03-15 15:01:48 +0000
+++ lib/lp/registry/interfaces/distroseries.py 2010-03-25 20:49:29 +0000
@@ -25,7 +25,7 @@
25 export_factory_operation, export_read_operation, exported,25 export_factory_operation, export_read_operation, exported,
26 operation_parameters, operation_returns_collection_of,26 operation_parameters, operation_returns_collection_of,
27 operation_returns_entry, rename_parameters_as, webservice_error)27 operation_returns_entry, rename_parameters_as, webservice_error)
28from lazr.restful.fields import Reference28from lazr.restful.fields import Reference, ReferenceChoice
2929
30from canonical.launchpad import _30from canonical.launchpad import _
31from canonical.launchpad.fields import (31from canonical.launchpad.fields import (
@@ -43,6 +43,7 @@
43from lp.bugs.interfaces.bugtarget import (43from lp.bugs.interfaces.bugtarget import (
44 IBugTarget, IHasBugs, IHasOfficialBugTags)44 IBugTarget, IHasBugs, IHasOfficialBugTags)
45from lp.registry.interfaces.milestone import IHasMilestones, IMilestone45from lp.registry.interfaces.milestone import IHasMilestones, IMilestone
46from lp.registry.interfaces.person import IPerson
46from lp.registry.interfaces.role import IHasOwner47from lp.registry.interfaces.role import IHasOwner
47from lp.registry.interfaces.series import ISeriesMixin, SeriesStatus48from lp.registry.interfaces.series import ISeriesMixin, SeriesStatus
48from lp.registry.interfaces.sourcepackage import ISourcePackage49from lp.registry.interfaces.sourcepackage import ISourcePackage
@@ -193,23 +194,23 @@
193 datereleased = exported(194 datereleased = exported(
194 Datetime(title=_("Date released")))195 Datetime(title=_("Date released")))
195 parent_series = exported(196 parent_series = exported(
196 Choice(197 ReferenceChoice(
197 title=_("Parent series"),198 title=_("Parent series"),
198 description=_("The series from which this one was branched."),199 description=_("The series from which this one was branched."),
199 required=True,200 required=True, schema=Interface, # Really IDistroSeries, see below
200 vocabulary='DistroSeries'))201 vocabulary='DistroSeries'))
201 owner = exported(202 owner = exported(
202 PublicPersonChoice(title=_("Owner"), vocabulary='ValidOwner'))203 PublicPersonChoice(title=_("Owner"), vocabulary='ValidOwner'))
203 date_created = exported(204 date_created = exported(
204 Datetime(title=_("The date this series was registered.")))205 Datetime(title=_("The date this series was registered.")))
205 driver = exported(206 driver = exported(
206 Choice(207 ReferenceChoice(
207 title=_("Driver"),208 title=_("Driver"),
208 description=_(209 description=_(
209 "The person or team responsible for decisions about features "210 "The person or team responsible for decisions about features "
210 "and bugs that will be targeted to this series of the "211 "and bugs that will be targeted to this series of the "
211 "distribution."),212 "distribution."),
212 required=False, vocabulary='ValidPersonOrTeam'))213 required=False, vocabulary='ValidPersonOrTeam', schema=IPerson))
213 changeslist = exported(214 changeslist = exported(
214 TextLine(215 TextLine(
215 title=_("E-mail changes to"), required=True,216 title=_("E-mail changes to"), required=True,
216217
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2010-03-18 13:27:47 +0000
+++ lib/lp/registry/interfaces/productseries.py 2010-03-25 20:49:29 +0000
@@ -99,7 +99,8 @@
99 id = Int(title=_('ID'))99 id = Int(title=_('ID'))
100100
101 product = exported(101 product = exported(
102 Choice(title=_('Project'), required=True, vocabulary='Product'),102 ReferenceChoice(title=_('Project'), required=True,
103 vocabulary='Product', schema=Interface), # really IProduct
103 exported_as='project')104 exported_as='project')
104105
105 status = exported(106 status = exported(
106107
=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
--- lib/lp/registry/interfaces/projectgroup.py 2010-03-24 14:32:20 +0000
+++ lib/lp/registry/interfaces/projectgroup.py 2010-03-25 20:49:29 +0000
@@ -15,7 +15,7 @@
15 ]15 ]
1616
17from zope.interface import Interface, Attribute17from zope.interface import Interface, Attribute
18from zope.schema import Bool, Choice, Datetime, Int, Object, Text, TextLine18from zope.schema import Bool, Datetime, Int, Object, Text, TextLine
1919
20from canonical.launchpad import _20from canonical.launchpad import _
21from canonical.launchpad.fields import (21from canonical.launchpad.fields import (
@@ -25,6 +25,7 @@
25 IHasBranchVisibilityPolicy)25 IHasBranchVisibilityPolicy)
26from lp.code.interfaces.hasbranches import IHasBranches, IHasMergeProposals26from lp.code.interfaces.hasbranches import IHasBranches, IHasMergeProposals
27from lp.bugs.interfaces.bugtarget import IHasBugs, IHasOfficialBugTags27from lp.bugs.interfaces.bugtarget import IHasBugs, IHasOfficialBugTags
28from lp.bugs.interfaces.bugtracker import IBugTracker
28from lp.registry.interfaces.karma import IKarmaContext29from lp.registry.interfaces.karma import IKarmaContext
29from canonical.launchpad.interfaces.launchpad import (30from canonical.launchpad.interfaces.launchpad import (
30 IHasAppointedDriver, IHasDrivers, IHasIcon, IHasLogo, IHasMugshot)31 IHasAppointedDriver, IHasDrivers, IHasIcon, IHasLogo, IHasMugshot)
@@ -45,7 +46,7 @@
45from canonical.launchpad.fields import (46from canonical.launchpad.fields import (
46 IconImageUpload, LogoImageUpload, MugshotImageUpload, PillarNameField)47 IconImageUpload, LogoImageUpload, MugshotImageUpload, PillarNameField)
4748
48from lazr.restful.fields import CollectionField, Reference49from lazr.restful.fields import CollectionField, Reference, ReferenceChoice
49from lazr.restful.declarations import (50from lazr.restful.declarations import (
50 collection_default_content, export_as_webservice_collection,51 collection_default_content, export_as_webservice_collection,
51 export_as_webservice_entry, export_read_operation, exported,52 export_as_webservice_entry, export_read_operation, exported,
@@ -236,8 +237,8 @@
236 "reviewed.")))237 "reviewed.")))
237238
238 bugtracker = exported(239 bugtracker = exported(
239 Choice(title=_('Bug Tracker'), required=False,240 ReferenceChoice(title=_('Bug Tracker'), required=False,
240 vocabulary='BugTracker',241 vocabulary='BugTracker', schema=IBugTracker,
241 description=_(242 description=_(
242 "The bug tracker the projects in this project group use.")),243 "The bug tracker the projects in this project group use.")),
243 exported_as="bug_tracker")244 exported_as="bug_tracker")