Resources returned by getSharedArtifacts not adapted to resource object type

Reported by Cody A.W. Somerville on 2012-09-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
High
Ian Booth

Bug Description

Branches and bugs returned by getSharedArtifacts are not adapted corresponding to resource object type. Instead they are just plain dictionaries.

Curtis Hovey (sinzui) wrote :

I think this relates to bug 1028819 which does not want to export PillarName because it is not a useful class. The method we are working with returns an intermediate object that no callsite wants. Launchpad or its restful adapters need to convert the two collections to their respective times. The easier bug very annoy way to solve this is to introduce a type that is just a pointer to the real object that will require EVERY callsite to make an additional call back to lp to get the proper data. We really do not want to do this.

tags: added: api disclosure sharing
Changed in launchpad:
status: New → Triaged
importance: Undecided → High
Ian Booth (wallyworld) on 2012-09-12
Changed in launchpad:
status: Triaged → In Progress
assignee: nobody → Ian Booth (wallyworld)
Ian Booth (wallyworld) wrote :

What's returned is two lists - a list of branch resource entries and a list of bug resource entries. The items in these lists are serialised bug and branch entry resources. Because the result is not a homogeneous list of a single entry resource type, it's not possible to use an operation_returns_collection_of annotation.

Comparing the result of the getSharedArtifacts method as exists now, with another exported method which does return a heterogeneous list, the only difference I can see in what is returned is that the latter returns a json structure representing a batched result. For getSharedArtifacts, batching is not used.

So getSharedArtifacts returns:

[
{u'resource_type_link': u'http://api.launchpad.dev:8085/devel/#bug_task',
 u'self_link': u'http://api.launchpad.dev:8085/devel/product-name-100001/+bug/16',
 ....etc
 }]
[
{u'resource_type_link': u'http://api.launchpad.dev:8085/devel/#branch',
 u'http://api.launchpad.dev:8085/devel/~thundercat/product-name-100001/branch-100010',
...etc
}]

Compare this to for example IBug.getVisibleLinkedBranches:

{u'total_size': 1, u'start': 0,
 u'entries':
[{u'resource_type_link': u'http://api.launchpad.dev/devel/#bug_branch',
  u'self_link': u'http://api.launchpad.dev/devel/~person-name-100019/product-name-100023/branch-100020/+bug/16',
 ...etc
}]}

The latter has the extra batching attributes, but the format of the list entries is the same - serialised resource entries.

So I'm unsure what the bug is asking for. Yes, the data items in the result lists are dicts, but the items do correspond to a resource object type as can be seen from the above example output.

Ian Booth (wallyworld) wrote :

After further looking into this, it is not trivially feasible to make the lazr restful infrastructure do what we want here. So I've taken the following approach:

1. Unexport the getSharedArtifacts () API. It is used internally to get data to populate the sharing details view and needs to keep it's current semantics.

2. Introduce two replacement exported APIs:

getSharedBugs()
getSharedBranches()

The above return collections of bug and branch resource entries as required.

Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
Changed in launchpad:
status: In Progress → Fix Committed

I appreciate you tackling this Ian.

I hate to be difficult but I'd probably actually prefer the old behaviour over having a method for each different type of artifact.

1. Considering the most likely use case of this functionality, auditing, I think it makes a lot of sense for there to be a single method so that scripts don't have to be updated as privacy is added to additional types of artefacts.
2. It would also be nice to use the same API as the sharing details view to remove the risk of a bug causing discrepancies between the two.
3. There is significant overhead for each API call.

What do you think?

Sure, no problem. I'll re-export the single getSharedArtifacts method,
keeping the new ones as well. That way there's a choice for whatever the
desired use case may be. The branch for this bug hasn't passed QA yet so
the original behaviour is still on production. I'll push through the
branch to re-export before this one hits prod so you won't see any
changed behaviour.

On 15/09/12 05:27, Cody A.W. Somerville wrote:
> I appreciate you tackling this Ian.
>
> I hate to be difficult but I'd probably actually prefer the old
> behaviour over having a method for each different type of artifact.
>
> 1. Considering the most likely use case of this functionality, auditing, I think it makes a lot of sense for there to be a single method so that scripts don't have to be updated as privacy is added to additional types of artefacts.
> 2. It would also be nice to use the same API as the sharing details view to remove the risk of a bug causing discrepancies between the two.
> 3. There is significant overhead for each API call.
>
> What do you think?
>

Curtis Hovey (sinzui) on 2012-09-15
tags: added: qa-ok
removed: qa-needstesting
Deryck Hodge (deryck) on 2012-09-17
Changed in launchpad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers