launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06549
[Merge] lp:~wallyworld/launchpad/more-accesspolicy-service-3 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/more-accesspolicy-service-3 into lp:launchpad with lp:~wallyworld/launchpad/more-accesspolicy-service-2 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/more-accesspolicy-service-3/+merge/95168
== Implementation ==
Part 3 of the work to implement the +sharing page
Provide service implementation for getPillarObservers() and addPillarObserver() methods using the new access policy model code.
The +sharing page now works in that the sharing picker can be used to grant access. When the page is first opened, any existing observers are displayed.
Also, rename the product methods to pillar.
TODO:
- add deletePillarObserver implementation
- add security permissions (atm, anyone can see and amend the data)
== Tests ==
Add new test case: TestAccessPolicyService
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/test_product.py
lib/lp/registry/interfaces/accesspolicyservice.py
lib/lp/registry/javascript/disclosure/pillar_sharing_view.js
lib/lp/registry/javascript/disclosure/tests/test_pillar_sharing_view.html
lib/lp/registry/javascript/disclosure/tests/test_pillar_sharing_view.js
lib/lp/registry/services/accesspolicyservice.py
lib/lp/registry/services/tests/test_accesspolicyservice.py
lib/lp/registry/templates/product-sharing.pt
--
https://code.launchpad.net/~wallyworld/launchpad/more-accesspolicy-service-3/+merge/95168
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/more-accesspolicy-service-3 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/browser/product.py 2012-02-29 13:21:20 +0000
@@ -2461,7 +2461,7 @@
@property
def observer_data(self):
- return self._getAccessPolicyService().getProductObservers(self.context)
+ return self._getAccessPolicyService().getPillarObservers(self.context)
def initialize(self):
super(ProductSharingView, self).initialize()
=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/browser/tests/test_product.py 2012-02-29 13:21:20 +0000
@@ -433,5 +433,5 @@
self.assertIsNotNone(cache.objects.get('access_policies'))
self.assertIsNotNone(cache.objects.get('sharing_permissions'))
aps = getUtility(IService, 'accesspolicy')
- observers = aps.getProductObservers(product)
+ observers = aps.getPillarObservers(product)
self.assertTrue(observers, cache.objects.get('observer_data'))
=== modified file 'lib/lp/registry/interfaces/accesspolicyservice.py'
--- lib/lp/registry/interfaces/accesspolicyservice.py 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/interfaces/accesspolicyservice.py 2012-02-29 13:21:20 +0000
@@ -13,22 +13,21 @@
from zope.schema import Choice
from lazr.restful.declarations import (
+ call_with,
export_as_webservice_entry,
export_read_operation,
export_write_operation,
operation_for_version,
operation_parameters,
+ REQUEST_USER,
)
from lazr.restful.fields import Reference
from lp import _
from lp.app.interfaces.services import IService
-from lp.registry.enums import (
- AccessPolicyType,
- SharingPermission,
- )
+from lp.registry.enums import AccessPolicyType
from lp.registry.interfaces.person import IPerson
-from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.pillar import IPillar
class IAccessPolicyService(IService):
@@ -46,26 +45,32 @@
@export_read_operation()
@operation_parameters(
- product=Reference(IProduct, title=_('Product'), required=True))
- @operation_for_version('devel')
- def getProductObservers(product):
- """Return people/teams who can see product artifacts."""
-
- @export_write_operation()
- @operation_parameters(
- product=Reference(IProduct, title=_('Product'), required=True),
- observer=Reference(IPerson, title=_('Observer'), required=True),
- access_policy=Choice(vocabulary=AccessPolicyType),
- sharing_permission=Choice(vocabulary=SharingPermission))
- @operation_for_version('devel')
- def addProductObserver(product, observer, access_policy,
- sharing_permission):
- """Add an observer with the access policy to a product."""
-
- @export_write_operation()
- @operation_parameters(
- product=Reference(IProduct, title=_('Product'), required=True),
- observer=Reference(IPerson, title=_('Observer'), required=True))
- @operation_for_version('devel')
- def deleteProductObserver(product, observer):
- """Remove an observer from a product."""
+ pillar=Reference(IPillar, title=_('Pillar'), required=True))
+ @operation_for_version('devel')
+ def getPillarObservers(pillar):
+ """Return people/teams who can see pillar artifacts."""
+
+ @export_write_operation()
+ @call_with(user=REQUEST_USER)
+ @operation_parameters(
+ pillar=Reference(IPillar, title=_('Pillar'), required=True),
+ observer=Reference(IPerson, title=_('Observer'), required=True),
+ access_policy_type=Choice(vocabulary=AccessPolicyType))
+ @operation_for_version('devel')
+ def addPillarObserver(pillar, observer, access_policy_type, user):
+ """Add an observer with the access policy to a pillar."""
+
+ @export_write_operation()
+ @operation_parameters(
+ pillar=Reference(IPillar, title=_('Pillar'), required=True),
+ observer=Reference(IPerson, title=_('Observer'), required=True),
+ access_policy_type=Choice(vocabulary=AccessPolicyType))
+ @operation_for_version('devel')
+ def deletePillarObserver(pillar, observer, access_policy_type):
+ """Remove an observer from a pillar.
+
+ :param pillar: the pillar from which to remove access
+ :param observer: the person or team to remove
+ :param access_policy_type: if None, remove all access, otherwise just
+ remove the specified access_policy
+ """
=== renamed file 'lib/lp/registry/javascript/disclosure/product_sharing.js' => 'lib/lp/registry/javascript/disclosure/pillar_sharing_view.js'
--- lib/lp/registry/javascript/disclosure/product_sharing.js 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/javascript/disclosure/pillar_sharing_view.js 2012-02-29 13:21:20 +0000
@@ -10,11 +10,11 @@
var namespace = Y.namespace('lp.registry.disclosure.sharing');
-function ProductSharingView(config) {
- ProductSharingView.superclass.constructor.apply(this, arguments);
+function PillarSharingView(config) {
+ PillarSharingView.superclass.constructor.apply(this, arguments);
}
-ProductSharingView.ATTRS = {
+PillarSharingView.ATTRS = {
lp_client: {
value: new Y.lp.client.Launchpad()
},
@@ -28,7 +28,7 @@
}
};
-Y.extend(ProductSharingView, Y.Widget, {
+Y.extend(PillarSharingView, Y.Widget, {
initializer: function(config) {
var vocab = 'ValidPillarOwner';
@@ -155,7 +155,7 @@
*/
performRemoveObserver: function(delete_link, person_uri) {
var error_handler = new Y.lp.client.ErrorHandler();
- var product_uri = LP.cache.context.self_link;
+ var pillar_uri = LP.cache.context.self_link;
var self = this;
var y_config = {
on: {
@@ -167,12 +167,12 @@
failure: error_handler.getFailureHandler()
},
parameters: {
- product: product_uri,
+ pillar: pillar_uri,
observer: person_uri
}
};
this.get('lp_client').named_post(
- '/+services/accesspolicy', 'deleteProductObserver', y_config);
+ '/+services/accesspolicy', 'deletePillarObserver', y_config);
},
/**
@@ -219,7 +219,7 @@
*/
saveSharingSelection: function(selection_result) {
var error_handler = new Y.lp.client.ErrorHandler();
- var product_uri = LP.cache.context.self_link;
+ var pillar_uri = LP.cache.context.self_link;
var person_uri = Y.lp.client.normalize_uri(selection_result.api_uri);
person_uri = Y.lp.client.get_absolute_uri(person_uri);
var self = this;
@@ -233,19 +233,18 @@
failure: error_handler.getFailureHandler()
},
parameters: {
- product: product_uri,
+ pillar: pillar_uri,
observer: person_uri,
- access_policy: selection_result.access_policy,
- sharing_permission: 'All'
+ access_policy_type: selection_result.access_policy
}
};
this.get('lp_client').named_post(
- '/+services/accesspolicy', 'addProductObserver', y_config);
+ '/+services/accesspolicy', 'addPillarObserver', y_config);
}
});
-ProductSharingView.NAME = 'productSharingView';
-namespace.ProductSharingView = ProductSharingView;
+PillarSharingView.NAME = 'pillarSharingView';
+namespace.PillarSharingView = PillarSharingView;
}, "0.1", { "requires": [
'node', 'lp.client', 'lp.mustache', 'lazr.picker', 'lp.app.picker',
=== renamed file 'lib/lp/registry/javascript/disclosure/tests/test_product_sharing_view.html' => 'lib/lp/registry/javascript/disclosure/tests/test_pillar_sharing_view.html'
--- lib/lp/registry/javascript/disclosure/tests/test_product_sharing_view.html 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_pillar_sharing_view.html 2012-02-29 13:21:20 +0000
@@ -7,7 +7,7 @@
<html>
<head>
- <title>Product Sharing View Tests</title>
+ <title>Pillar Sharing View Tests</title>
<!-- YUI and test setup -->
<script type="text/javascript"
@@ -62,10 +62,10 @@
src="../../../../../../build/js/lp/registry/disclosure/disclosure_person_picker.js"></script>
<!-- The module under test. -->
- <script type="text/javascript" src="../product_sharing.js"></script>
+ <script type="text/javascript" src="../pillar_sharing_view.js"></script>
<!-- The test suite -->
- <script type="text/javascript" src="test_product_sharing_view.js"></script>
+ <script type="text/javascript" src="test_pillar_sharing_view.js"></script>
<script id="test-fixture" type="text/x-template">
<table id='observer-table'></table>
=== renamed file 'lib/lp/registry/javascript/disclosure/tests/test_product_sharing_view.js' => 'lib/lp/registry/javascript/disclosure/tests/test_pillar_sharing_view.js'
--- lib/lp/registry/javascript/disclosure/tests/test_product_sharing_view.js 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/javascript/disclosure/tests/test_pillar_sharing_view.js 2012-02-29 13:21:20 +0000
@@ -15,7 +15,7 @@
window.LP = {
links: {},
cache: {
- context: {self_link: "~product" },
+ context: {self_link: "~pillar" },
observer_data: [
{'name': 'fred', 'display_name': 'Fred Bloggs',
'role': '(Maintainer)', web_link: '~fred',
@@ -50,7 +50,7 @@
_create_Widget: function(cfg) {
var ns = Y.lp.registry.disclosure.sharing;
- return new ns.ProductSharingView(cfg);
+ return new ns.PillarSharingView(cfg);
},
test_library_exists: function () {
@@ -62,7 +62,7 @@
test_widget_can_be_instantiated: function() {
this.view = this._create_Widget();
Y.Assert.isInstanceOf(
- Y.lp.registry.disclosure.sharing.ProductSharingView,
+ Y.lp.registry.disclosure.sharing.PillarSharingView,
this.view,
"Observer table failed to be instantiated");
},
@@ -131,7 +131,7 @@
'/api/devel/+services/accesspolicy',
mockio.last_request.url);
Y.Assert.areEqual(
- 'ws.op=deleteProductObserver&product=~product' +
+ 'ws.op=deletePillarObserver&pillar=~pillar' +
'&observer=~fred',
mockio.last_request.config.data);
mockio.last_request.successJSON({});
@@ -197,15 +197,13 @@
person_uri = Y.lp.client.get_absolute_uri(person_uri);
var expected_url;
expected_url = Y.lp.client.append_qs(
- expected_url, 'ws.op', 'addProductObserver');
+ expected_url, 'ws.op', 'addPillarObserver');
expected_url = Y.lp.client.append_qs(
- expected_url, 'product', '~product');
+ expected_url, 'pillar', '~pillar');
expected_url = Y.lp.client.append_qs(
expected_url, 'observer', person_uri);
expected_url = Y.lp.client.append_qs(
- expected_url, 'access_policy', 'Policy 2');
- expected_url = Y.lp.client.append_qs(
- expected_url, 'sharing_permission', 'All');
+ expected_url, 'access_policy_type', 'Policy 2');
Y.Assert.areEqual(expected_url, mockio.last_request.config.data);
mockio.last_request.successJSON({
'resource_type_link': 'entity',
=== modified file 'lib/lp/registry/services/accesspolicyservice.py'
--- lib/lp/registry/services/accesspolicyservice.py 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/services/accesspolicyservice.py 2012-02-29 13:21:20 +0000
@@ -18,8 +18,11 @@
AccessPolicyType,
SharingPermission,
)
+from lp.registry.interfaces.accesspolicy import (
+ IAccessPolicySource,
+ IAccessPolicyGrantSource,
+ )
from lp.registry.interfaces.accesspolicyservice import IAccessPolicyService
-from lp.registry.interfaces.person import IPersonSet
class AccessPolicyService:
@@ -61,40 +64,59 @@
sharing_permissions.append(item)
return sharing_permissions
- def getProductObservers(self, product):
+ def getPillarObservers(self, pillar):
"""See `IAccessPolicyService`."""
- # TODO - replace this sample data with something real
+
+ # Currently support querying for sharing_permission = ALL
+ # TODO - support querying for sharing_permission = SOME
+
+ policies = getUtility(IAccessPolicySource).findByPillar([pillar])
+ policy_grant_source = getUtility(IAccessPolicyGrantSource)
+ grants = policy_grant_source.findByPolicy(policies)
+
result = []
+ person_by_id = {}
request = get_current_web_service_request()
- personset = getUtility(IPersonSet)
- for id in range(1, 4):
- person = personset.get(id)
- resource = EntryResource(person, request)
- person_data = resource.toDataForJSON()
- permissions = {
- 'PUBLICSECURITY': 'SOME',
- 'EMBARGOEDSECURITY': 'ALL'
- }
- if id > 2:
- permissions['USERDATA'] = 'SOME'
- person_data['permissions'] = permissions
+ for g in grants:
+ if not g.grantee.id in person_by_id:
+ resource = EntryResource(g.grantee, request)
+ person_data = resource.toDataForJSON()
+ person_data['permissions'] = {}
+ person_by_id[g.grantee.id] = person_data
+ person_data = person_by_id[g.grantee.id]
+ person_data['permissions'][g.policy.type.name] = (
+ SharingPermission.ALL.name)
result.append(person_data)
return result
- def addProductObserver(self, product, observer, access_policy,
- sharing_permission):
+ def addPillarObserver(self, pillar, observer, access_policy_type, user):
"""See `IAccessPolicyService`."""
- # TODO - implement this
+
+ # Create a pillar access policy if one doesn't exist.
+ policy_source = getUtility(IAccessPolicySource)
+ pillar_access_policy = [(pillar, access_policy_type)]
+ policies = list(policy_source.find(pillar_access_policy))
+ if len(policies) == 0:
+ [policy] = policy_source.create(pillar_access_policy)
+ else:
+ policy = policies[0]
+ # We have a policy, create the grant if it doesn't exist.
+ policy_grant_source = getUtility(IAccessPolicyGrantSource)
+ grants = list(policy_grant_source.find([(policy, observer)]))
+ if len(grants) == 0:
+ policy_grant_source.grant([(policy, observer, user)])
+
+ # Return observer data to the caller.
request = get_current_web_service_request()
resource = EntryResource(observer, request)
person_data = resource.toDataForJSON()
permissions = {
- access_policy.name: sharing_permission.name,
+ access_policy_type.name: SharingPermission.ALL.name,
}
person_data['permissions'] = permissions
return person_data
- def deleteProductObserver(self, product, observer):
+ def deletePillarObserver(self, pillar, observer, access_policy_type):
"""See `IAccessPolicyService`."""
# TODO - implement this
pass
=== modified file 'lib/lp/registry/services/tests/test_accesspolicyservice.py'
--- lib/lp/registry/services/tests/test_accesspolicyservice.py 2012-02-28 04:24:19 +0000
+++ lib/lp/registry/services/tests/test_accesspolicyservice.py 2012-02-29 13:21:20 +0000
@@ -5,9 +5,16 @@
import simplejson
+from lazr.restful import EntryResource
+from lazr.restful.utils import get_current_web_service_request
from zope.component import getUtility
-from lp.registry.enums import AccessPolicyType
+from lp.app.interfaces.services import IService
+from lp.registry.enums import AccessPolicyType, SharingPermission
+from lp.registry.interfaces.accesspolicy import (
+ IAccessPolicyGrantSource,
+ IAccessPolicySource,
+ )
from lp.registry.services.accesspolicyservice import AccessPolicyService
from lp.services.webapp.interfaces import ILaunchpadRoot
from lp.services.webapp.publisher import canonical_url
@@ -15,10 +22,70 @@
TestCaseWithFactory,
WebServiceTestCase,
)
-from lp.testing.layers import AppServerLayer
+from lp.testing.layers import AppServerLayer, DatabaseFunctionalLayer
from lp.testing.pages import LaunchpadWebServiceCaller
+class TestAccessPolicyService(TestCaseWithFactory):
+ """Tests for the AccessPolicyService."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestAccessPolicyService, self).setUp()
+ self.service = getUtility(IService, 'accesspolicy')
+
+ def _makeObserverData(self, observer):
+ # Unpack an observer into it's attributes and add in permissions.
+ request = get_current_web_service_request()
+ resource = EntryResource(observer, request)
+ observer_data = resource.toDataForJSON()
+ observer_data['permissions'] = {
+ AccessPolicyType.PROPRIETARY.name: SharingPermission.ALL.name}
+ return observer_data
+
+ def _test_getPillarObservers(self, pillar):
+ """getPillarObservers returns the expected data."""
+ access_policy = self.factory.makeAccessPolicy(pillar=pillar)
+ grantee = self.factory.makePerson()
+ self.factory.makeAccessPolicyGrant(access_policy, grantee)
+ [observer] = self.service.getPillarObservers(pillar)
+ person_data = self._makeObserverData(grantee)
+ self.assertContentEqual(person_data, observer)
+
+ def test_getProductObservers(self):
+ product = self.factory.makeProduct()
+ self._test_getPillarObservers(product)
+
+ def test_getDistroObservers(self):
+ distro = self.factory.makeDistribution()
+ self._test_getPillarObservers(distro)
+
+ def _test_addPillarObserver(self, pillar):
+ """addPillarObservers works and returns the expected data."""
+ observer = self.factory.makePerson()
+ access_policy_type = AccessPolicyType.USERDATA
+ user = self.factory.makePerson()
+ observer_data = self.service.addPillarObserver(
+ pillar, observer, access_policy_type, user)
+ [policy] = getUtility(IAccessPolicySource).findByPillar([pillar])
+ policy_grant_source = getUtility(IAccessPolicyGrantSource)
+ [grant] = policy_grant_source.findByPolicy([policy])
+ self.assertEqual(user, grant.grantor)
+ self.assertEqual(observer, grant.grantee)
+ self.assertEqual(policy, grant.policy)
+ expected_observer_data = self._makeObserverData(observer)
+ self.assertContentEqual(expected_observer_data, observer_data)
+
+ def test_addProductObserver(self):
+ product = self.factory.makeProduct()
+ self._test_addPillarObserver(product)
+
+ def test_addDistroObserver(self):
+ distro = self.factory.makeDistribution()
+ self._test_addPillarObserver(distro)
+
+
class ApiTestMixin:
"""Common tests for launchpadlib and webservice."""
=== modified file 'lib/lp/registry/templates/product-sharing.pt'
--- lib/lp/registry/templates/product-sharing.pt 2012-02-29 13:21:19 +0000
+++ lib/lp/registry/templates/product-sharing.pt 2012-02-29 13:21:20 +0000
@@ -15,7 +15,7 @@
function(Y) {
Y.on('domready', function() {
var config = ${view/json_sharing_picker_config}
- var view_widget = new Y.lp.registry.disclosure.sharing.ProductSharingView(config);
+ var view_widget = new Y.lp.registry.disclosure.sharing.PillarSharingView(config);
view_widget.render();
});
});
@@ -30,17 +30,6 @@
</p>
<table id="observer-table" class="disclosure listing">
- <thead>
- <tr>
- <th colspan="2" style="width: 33%">User or Team</th>
- <th colspan="2" style="width: ">Sharing <span class="help">
- (<a href="/+help-registry/sharing.html"
- target="help" class="js-action help">help</a>)</span>
- </th>
- <th colspan="1" style="width: ">Shared items</th>
- </tr>
- </thead>
- <tbody id="observer-table-body"></tbody>
</table>
</div>
