Merge lp:~wgrant/launchpad/export-structural-subscriptions-bug-392515 into lp:launchpad

[William Grant]

[r=noideayet][ui=none] (wgrant) Export structural subscriptions in the webservice API.

[William Grant (wgrant)]

= Summary =
Structural subscriptions want to be exported through the API. This is the last of four branches: the actual interface decorations.

== Proposed fix ==
Decorate IStructuralSubscription and IStructuralSubscriptionTarget to export all relevant methods and attributes.

== Implementation details ==
The export of ISST.getSubscriptions is deliberately somewhat limited. Only one level of bug subscription is used at the moment, and I hope the level setup will change, so I thought it best not to expose those details.

Apart from that, there should be nothing unobvious here.

== Tests ==
I've added lib/canonical/launchpad/pagetests/webservice/xx-structuralsubscription.txt

 $ bin/test -vvt xx-structuralsubscription.txt

[Gavin Panella (allenap)]

Hi William,

This is a very nice patch. I have a few questions/comments, but they're small.

Gavin.

> === modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
> --- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-08-28 07:34:44 +0000
> +++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-09-09 13:04:34 +0000
> @@ -22,6 +22,8 @@
> patch_collection_return_type, patch_plain_parameter_type,
> patch_choice_parameter_type, patch_reference_property)
>
> +from canonical.launchpad.interfaces.structuralsubscription import (
> + IStructuralSubscription, IStructuralSubscriptionTarget)
> from lp.bugs.interfaces.bug import IBug
> from lp.bugs.interfaces.bugbranch import IBugBranch
> from lp.bugs.interfaces.bugnomination import IBugNomination
> @@ -278,3 +280,11 @@
> IPackageUpload['pocket'].vocabulary = PackagePublishingPocket
> patch_reference_property(IPackageUpload, 'distroseries', IDistroSeries)
> patch_reference_property(IPackageUpload, 'archive', IArchive)
> +
> +# IStructuralSubscription
> +patch_reference_property(
> + IStructuralSubscription, 'target', IStructuralSubscriptionTarget)
> +
> +patch_reference_property(
> + IStructuralSubscriptionTarget, 'parent_subscription_target',
> + IStructuralSubscriptionTarget)
>
> === modified file 'lib/canonical/launchpad/interfaces/structuralsubscription.py'
> --- lib/canonical/launchpad/interfaces/structuralsubscription.py 2009-08-25 11:08:42 +0000
> +++ lib/canonical/launchpad/interfaces/structuralsubscription.py 2009-09-09 13:04:34 +0000
> @@ -24,6 +24,14 @@
> from canonical.launchpad import _
> from canonical.launchpad.fields import (
> ParticipatingPersonChoice, PublicPersonChoice)
> +from lp.registry.interfaces.person import IPerson
> +
> +from lazr.restful.declarations import (
> + REQUEST_USER, call_with, exported, export_as_webservice_entry,
> + export_factory_operation, export_read_operation, export_write_operation,
> + operation_parameters, operation_returns_collection_of,
> + operation_returns_entry, webservice_error)
> +from lazr.restful.fields import Reference
>
>
> class BugNotificationLevel(DBEnumeratedType):
> @@ -84,6 +92,7 @@
>
> class IStructuralSubscription(Interface):
> """A subscription to a Launchpad structure."""
> + export_as_webservice_entry()
>
> id = Int(title=_('ID'), readonly=True, required=True)
> product = Int(title=_('Product'), required=False, readonly=True)
> @@ -96,13 +105,13 @@
> title=_('Distribution series'), required=False, readonly=True)
> sourcepackagename = Int(
> title=_('Source package name'), required=False, readonly=True)
> - subscriber = ParticipatingPersonChoice(
> + subscriber = exported(ParticipatingPersonChoice(
> title=_('Subscriber'), required=True, vocabulary='ValidPersonOrTeam',
> - readonly=True, description=_("The person subscribed."))
> - subscribed_by = PublicPersonChoice(
> + readonly=True, description=_("The person subscribed.")))
> + subscribed_by = exported(PublicPersonChoice(
> title=_('Subscribed by'), required=True,
> vocabulary='ValidP...

review: Approve (code)

[William Grant (wgrant)]

Hi Gavin,

On Wed, 2009-09-09 at 13:35 +0000, Gavin Panella wrote:
> Review: Approve code
> Hi William,
>
> This is a very nice patch. I have a few questions/comments, but they're small.

Thanks for the review.

As discussed on IRC, I've adjusted the parameter descriptions to
indicate the behaviour of omitted arguments.

At intellectronica's suggestion, I've also unexported
ISST.parent_subscription_target for now (it is considered an
implementation detail, and we can do something much nicer later).

Diff from 9230 to 9233 attached.

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2009-08-28 07:34:44 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2009-09-09 11:28:03 +0000
@@ -22,6 +22,8 @@
     patch_collection_return_type, patch_plain_parameter_type,
     patch_choice_parameter_type, patch_reference_property)
 
+from canonical.launchpad.interfaces.structuralsubscription import (
+    IStructuralSubscription, IStructuralSubscriptionTarget)
 from lp.bugs.interfaces.bug import IBug
 from lp.bugs.interfaces.bugbranch import IBugBranch
 from lp.bugs.interfaces.bugnomination import IBugNomination
@@ -278,3 +280,11 @@
 IPackageUpload['pocket'].vocabulary = PackagePublishingPocket
 patch_reference_property(IPackageUpload, 'distroseries', IDistroSeries)
 patch_reference_property(IPackageUpload, 'archive', IArchive)
+
+# IStructuralSubscription
+patch_reference_property(
+    IStructuralSubscription, 'target', IStructuralSubscriptionTarget)
+
+patch_reference_property(
+    IStructuralSubscriptionTarget, 'parent_subscription_target',
+    IStructuralSubscriptionTarget)

=== modified file 'lib/canonical/launchpad/interfaces/structuralsubscription.py'
--- lib/canonical/launchpad/interfaces/structuralsubscription.py	2009-08-25 11:08:42 +0000
+++ lib/canonical/launchpad/interfaces/structuralsubscription.py	2009-08-25 11:26:58 +0000
@@ -24,6 +24,14 @@
 from canonical.launchpad import _
 from canonical.launchpad.fields import (
     ParticipatingPersonChoice, PublicPersonChoice)
+from lp.registry.interfaces.person import IPerson
+
+from lazr.restful.declarations import (
+    REQUEST_USER, call_with, exported, export_as_webservice_entry,
+    export_factory_operation, export_read_operation, export_write_operation,
+    operation_parameters, operation_returns_collection_of,
+    operation_returns_entry, webservice_error)
+from lazr.restful.fields import Reference
 
 
 class BugNotificationLevel(DBEnumeratedType):
@@ -84,6 +92,7 @@
 
 class IStructuralSubscription(Interface):
     """A subscription to a Launchpad structure."""
+    export_as_webservice_entry()
 
     id = Int(title=_('ID'), readonly=True, required=True)
     product = Int(title=_('Product'), required=False, readonly=True)
@@ -96,13 +105,13 @@
         title=_('Distribution series'), required=False, readonly=True)
     sourcepackagename = Int(
         title=_('Source package name'), required=False, readonly=True)
-    subscriber = ParticipatingPersonChoice(
+    subscriber = exported(ParticipatingPersonChoice(
         title=_('Subscriber'), required=True, vocabulary='ValidPersonOrTeam',
-        readonly=True, description=_("The person subscribed."))
-    subscribed_by = PublicPersonChoice(
+        readonly=True, description=_("The person subscribed.")))
+    subscribed_by = exported(PublicPersonChoice(
         title=_('Subscribed by'), required=True,
         vocabulary='ValidPersonOrTeam', readonly=True,
-        description=_("The person creating the subscription."))
+        description=_("The person creating the subscription.")))
     bug_notification_level = Choice(
         title=_("Bug notification level"), required=True,
         vocabulary=BugNotificationLevel,
@@ -115,19 +124,30 @@
         default=BlueprintNotificationLevel.NOTHING,
         description=_("The volume and type of blueprint notifications "
                       "this subscription will generate."))
-    date_created = Datetime(
+    date_created = exported(Datetime(
         title=_("The date on which this subscription was created."),
-        required=False)
-    date_last_updated = Datetime(
+        required=False, readonly=True))
+    date_last_updated = exported(Datetime(
         title=_("The date on which this subscription was last updated."),
-        required=False)
+        required=False, readonly=True))
 
-    target = Attribute("The structure to which this subscription belongs.")
+    target = exported(Reference(
+        schema=Interface, # IStructuralSubscriptionTarget
+        required=True, readonly=True,
+        title=_("The structure to which this subscription belongs.")))
 
 
 class IStructuralSubscriptionTarget(Interface):
     """A Launchpad Structure allowing users to subscribe to it."""
+    export_as_webservice_entry()
 
+    # We don't really want to expose the level details yet. Only
+    # BugNotificationLevel.COMMENTS is used at this time.
+    @call_with(
+        min_bug_notification_level=BugNotificationLevel.COMMENTS,
+        min_blueprint_notification_level=BlueprintNotificationLevel.NOTHING)
+    @operation_returns_collection_of(IStructuralSubscription)
+    @export_read_operation()
     def getSubscriptions(min_bug_notification_level,
                          min_blueprint_notification_level):
         """Return all the subscriptions with the specified levels.
@@ -140,8 +160,10 @@
         :return: A sequence of `IStructuralSubscription`.
         """
 
-    parent_subscription_target = Attribute(
-        "The target's parent, or None if one doesn't exist.")
+    parent_subscription_target = exported(Reference(
+        schema=Interface, # IStructuralSubscriptionTarget
+        required=True, readonly=True,
+        title=_("The target's parent, or None if one doesn't exist.")))
 
     def addSubscription(subscriber, subscribed_by):
         """Add a subscription for this structure.
@@ -154,6 +176,11 @@
         :return: The new subscription.
         """
 
+    @operation_parameters(
+        subscriber=Reference(
+            schema=IPerson, title=_('Person'), required=False))
+    @call_with(subscribed_by=REQUEST_USER)
+    @export_factory_operation(IStructuralSubscription, [])
     def addBugSubscription(subscriber, subscribed_by):
         """Add a bug subscription for this structure.
 
@@ -166,6 +193,11 @@
         :return: The new bug subscription.
         """
 
+    @operation_parameters(
+        subscriber=Reference(
+            schema=IPerson, title=_('Person'), required=False))
+    @call_with(unsubscribed_by=REQUEST_USER)
+    @export_write_operation()
     def removeBugSubscription(subscriber, unsubscribed_by):
         """Remove a subscription to bugs from this structure.
 
@@ -177,6 +209,9 @@
         :unsubscribed_by: The IPerson removing the subscription.
         """
 
+    @operation_parameters(person=Reference(schema=IPerson))
+    @operation_returns_entry(IStructuralSubscription)
+    @export_read_operation()
     def getSubscription(person):
         """Return the subscription for `person`, if it exists."""
 
@@ -209,7 +244,9 @@
 
     Raised when an error occurred trying to delete a
     structural subscription."""
+    webservice_error(400)
 
 
 class UserCannotSubscribePerson(Exception):
     """User does not have permission to subscribe the person or team."""
+    webservice_error(401)

=== added file 'lib/canonical/launchpad/pagetests/webservice/xx-structuralsubscription.txt'
--- lib/canonical/launchpad/pagetests/webservice/xx-structuralsubscription.txt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/pagetests/webservice/xx-structuralsubscription.txt	2009-08-27 04:16:41 +0000
@@ -0,0 +1,151 @@
+= Structural Subscriptions =
+
+Structural subscriptions can be obtained from any target: a project,
+project series, project group, distribution, distribution series or
+distribution source package.
+
+    >>> login('[e-mail]')
+    >>> eric_db = factory.makePerson(name='eric')
+    >>> michael_db = factory.makePerson(name='michael')
+    >>> pythons_db = factory.makeTeam(name='pythons', owner=michael_db)
+    >>> pythons_db.addMember(eric_db, michael_db)
+
+    >>> fooix_db = factory.makeProduct(name='fooix', owner=eric_db)
+    >>> fooix01_db = fooix_db.newSeries(eric_db, '0.1', 'Series 0.1')
+    >>> logout()
+
+We can list the structural subscriptions on a target using the
+getSubscriptions named operation. There are none just yet.
+
+    >>> from lazr.restful.testing.webservice import (
+    ...     pprint_collection, pprint_entry)
+    >>> subscriptions = webservice.named_get(
+    ...     '/fooix', 'getSubscriptions').jsonBody()
+    >>> pprint_collection(subscriptions)
+    start: None
+    total_size: 0
+    ---
+
+Now Eric subscribes to Fooix's bug notifications.
+
+    >>> from canonical.launchpad.testing.pages import webservice_for_person
+    >>> from canonical.launchpad.webapp.interfaces import OAuthPermission
+    >>> eric_webservice = webservice_for_person(
+    ...     eric_db, permission=OAuthPermission.WRITE_PRIVATE)
+
+    >>> print eric_webservice.named_post(
+    ...     '/fooix', 'addBugSubscription')
+    HTTP/1.1 201 Created
+    ...
+    Location: http://.../fooix/+subscription/eric
+    ...
+
+    >>> subscriptions = webservice.named_get(
+    ...     '/fooix', 'getSubscriptions').jsonBody()
+    >>> pprint_collection(subscriptions)
+    start: 0
+    total_size: 1
+    ---
+    date_created: u'...'
+    date_last_updated: u'...'
+    resource_type_link: u'http://.../#structural_subscription'
+    self_link: u'http://.../fooix/+subscription/eric'
+    subscribed_by_link: u'http://.../~eric'
+    subscriber_link: u'http://.../~eric'
+    target_link: u'http://.../fooix'
+    ---
+
+He can examine his subscription directly.
+
+    >>> pprint_entry(eric_webservice.named_get(
+    ...     '/fooix', 'getSubscription',
+    ...     person=webservice.getAbsoluteUrl('/~eric')).jsonBody())
+    date_created: u'...'
+    date_last_updated: u'...'
+    resource_type_link: u'http://.../#structural_subscription'
+    self_link: u'http://.../fooix/+subscription/eric'
+    subscribed_by_link: u'http://.../~eric'
+    subscriber_link: u'http://.../~eric'
+    target_link: u'http://.../fooix'
+
+If the subscription doesn't exist, None will be returned.
+
+    >>> print webservice.named_get(
+    ...     '/fooix', 'getSubscription',
+    ...     person=webservice.getAbsoluteUrl('/~michael')).jsonBody()
+    None
+
+Eric can remove his subscription through the webservice.
+
+    >>> print eric_webservice.named_post(
+    ...     '/fooix', 'removeBugSubscription')
+    HTTP/1.1 200 Ok...
+
+    >>> print webservice.named_get(
+    ...     '/fooix', 'getSubscription',
+    ...     person=webservice.getAbsoluteUrl('/~eric')).jsonBody()
+    None
+
+Teams can be subscribed by passing in the team as an argument. Eric
+tries this.
+
+    >>> print eric_webservice.named_post(
+    ...     '/fooix', 'addBugSubscription',
+    ...     subscriber=webservice.getAbsoluteUrl('/~pythons'))
+    HTTP/1.1 401 Unauthorized
+    ...
+    UserCannotSubscribePerson: eric does not have permission to subscribe pythons.
+    <BLANKLINE>
+
+Oops, Eric isn't a team admin. Eric gets Michael to try, since he is an
+admin by virtue of his ownership.
+
+    >>> michael_webservice = webservice_for_person(
+    ...     michael_db, permission=OAuthPermission.WRITE_PRIVATE)
+
+    >>> print michael_webservice.named_post(
+    ...     '/fooix', 'addBugSubscription',
+    ...     subscriber=webservice.getAbsoluteUrl('/~pythons'))
+    HTTP/1.1 201 Created
+    ...
+    Location: http://.../fooix/+subscription/pythons
+    ...
+
+    >>> subscriptions = webservice.named_get(
+    ...     '/fooix', 'getSubscriptions').jsonBody()
+    >>> pprint_collection(subscriptions)
+    start: 0
+    total_size: 1
+    ---
+    date_created: u'...'
+    date_last_updated: u'...'
+    resource_type_link: u'http://.../#structural_subscription'
+    self_link: u'http://.../fooix/+subscription/pythons'
+    subscribed_by_link: u'http://.../~michael'
+    subscriber_link: u'http://.../~pythons'
+    target_link: u'http://.../fooix'
+    ---
+
+Eric can't unsubscribe the team either.
+
+    >>> print eric_webservice.named_post(
+    ...     '/fooix', 'removeBugSubscription',
+    ...     subscriber=webservice.getAbsoluteUrl('/~pythons'))
+    HTTP/1.1 401 Unauthorized
+    ...
+    UserCannotSubscribePerson: eric does not have permission to unsubscribe pythons.
+    <BLANKLINE>
+
+Michael can, though.
+
+    >>> print michael_webservice.named_post(
+    ...     '/fooix', 'removeBugSubscription',
+    ...     subscriber=webservice.getAbsoluteUrl('/~pythons'))
+    HTTP/1.1 200 Ok...
+
+    >>> subscriptions = webservice.named_get(
+    ...     '/fooix', 'getSubscriptions').jsonBody()
+    >>> pprint_collection(subscriptions)
+    start: None
+    total_size: 0
+    ---