From 96c867931427f3b8c0ced7e626916b94a74a3f01 Mon Sep 17 00:00:00 2001 From: Paul Van Eck Date: Wed, 7 Sep 2016 18:06:23 -0700 Subject: [PATCH] Add ability to associate product version to test A product_version_id field was added to the test table. Users can now associate product versions to a test via a REST PUT request to the results endpoint. Change-Id: I75815533d2a0bda60b6b1bab112c3658bc685fad Paritally-Implements-Spec: https://review.openstack.org/#/c/332260/ --- refstack/api/controllers/results.py | 32 +++++++++- refstack/api/utils.py | 12 +++- refstack/db/api.py | 8 +++ .../23843be3da52_add_product_version_id.py | 28 ++++++++ refstack/db/sqlalchemy/api.py | 22 ++++++- refstack/db/sqlalchemy/models.py | 6 +- refstack/tests/api/test_results.py | 64 +++++++++++++++++++ refstack/tests/unit/test_api.py | 3 +- refstack/tests/unit/test_api_utils.py | 6 ++ refstack/tests/unit/test_db.py | 15 +++++ 10 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 refstack/db/migrations/alembic/versions/23843be3da52_add_product_version_id.py diff --git a/refstack/api/controllers/results.py b/refstack/api/controllers/results.py index 6c24615e..96fe2f89 100644 --- a/refstack/api/controllers/results.py +++ b/refstack/api/controllers/results.py @@ -103,7 +103,8 @@ class ResultsController(validation.BaseRestControllerWithValidation): if user_role in (const.ROLE_FOUNDATION, const.ROLE_OWNER): test_info = db.get_test( test_id, allowed_keys=['id', 'cpid', 'created_at', - 'duration_seconds', 'meta'] + 'duration_seconds', 'meta', + 'product_version_id'] ) else: test_info = db.get_test(test_id) @@ -198,3 +199,32 @@ class ResultsController(validation.BaseRestControllerWithValidation): pecan.abort(400) return page + + @api_utils.check_permissions(level=const.ROLE_OWNER) + @pecan.expose('json') + def put(self, test_id, **kw): + """Update a test result.""" + test_info = {'id': test_id} + if 'product_version_id' in kw: + if kw['product_version_id']: + # Verify that the user is a member of the product's vendor. + version = db.get_product_version(kw['product_version_id']) + is_vendor_admin = ( + api_utils + .check_user_is_product_admin(version['product_id']) + ) + else: + # No product vendor to check membership for, so just set + # is_vendor_admin to True. + is_vendor_admin = True + kw['product_version_id'] = None + is_foundation_admin = api_utils.check_user_is_foundation_admin() + + if not is_vendor_admin and not is_foundation_admin: + pecan.abort(403, 'Forbidden.') + + test_info['product_version_id'] = kw['product_version_id'] + + test = db.update_test(test_info) + pecan.response.status = 201 + return test diff --git a/refstack/api/utils.py b/refstack/api/utils.py index 89f1aefa..fe8fc5bd 100644 --- a/refstack/api/utils.py +++ b/refstack/api/utils.py @@ -249,8 +249,16 @@ def check_owner(test_id): """Check that user has access to specified test run as owner.""" if not is_authenticated(): return False - user = db.get_test_meta_key(test_id, const.USER) - return user and user == get_user_id() + + test = db.get_test(test_id) + # If the test is owned by a product. + if test.get('product_version_id'): + version = db.get_product_version(test['product_version_id']) + return check_user_is_product_admin(version['product_id']) + # Otherwise, check the user ownership. + else: + user = db.get_test_meta_key(test_id, const.USER) + return user and user == get_user_id() def check_permissions(level): diff --git a/refstack/db/api.py b/refstack/db/api.py index ae434660..1726fba8 100644 --- a/refstack/db/api.py +++ b/refstack/db/api.py @@ -64,6 +64,14 @@ def delete_test(test_id): return IMPL.delete_test(test_id) +def update_test(test_info): + """Update test from the given test_info dictionary. + + :param test_info: The test + """ + return IMPL.update_test(test_info) + + def get_test_results(test_id): """Get all passed tempest tests for a specified test run. diff --git a/refstack/db/migrations/alembic/versions/23843be3da52_add_product_version_id.py b/refstack/db/migrations/alembic/versions/23843be3da52_add_product_version_id.py new file mode 100644 index 00000000..9c3fc176 --- /dev/null +++ b/refstack/db/migrations/alembic/versions/23843be3da52_add_product_version_id.py @@ -0,0 +1,28 @@ +"""Add product_version_id column to test. + +Revision ID: 23843be3da52 +Revises: 35bf54e2c13c +Create Date: 2016-07-30 18:15:52.429610 +""" + +# revision identifiers, used by Alembic. +revision = '23843be3da52' +down_revision = '35bf54e2c13c' +MYSQL_CHARSET = 'utf8' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + """Upgrade DB.""" + op.add_column('test', sa.Column('product_version_id', sa.String(36), + nullable=True)) + op.create_foreign_key('fk_test_prod_version_id', 'test', 'product_version', + ['product_version_id'], ['id']) + + +def downgrade(): + """Downgrade DB.""" + op.drop_constraint('fk_test_prod_version_id', 'test', type_="foreignkey") + op.drop_column('test', 'product_version_id') diff --git a/refstack/db/sqlalchemy/api.py b/refstack/db/sqlalchemy/api.py index a355db85..9e240dd3 100644 --- a/refstack/db/sqlalchemy/api.py +++ b/refstack/db/sqlalchemy/api.py @@ -158,6 +158,24 @@ def delete_test(test_id): raise NotFound('Test result %s not found' % test_id) +def update_test(test_info): + """Update test from the given test_info dictionary.""" + session = get_session() + _id = test_info.get('id') + test = session.query(models.Test).filter_by(id=_id).first() + if test is None: + raise NotFound('Test result with id %s not found' % _id) + + keys = ['product_version_id'] + for key in keys: + if key in test_info: + setattr(test, key, test_info[key]) + + with session.begin(): + test.save(session=session) + return _to_dict(test) + + def get_test_meta_key(test_id, key, default=None): """Get metadata value related to specified test run.""" session = get_session() @@ -616,8 +634,8 @@ def get_product_version(product_version_id, allowed_keys=None): .filter_by(id=product_version_id).first() ) if version is None: - raise NotFound('Version with id "%s" not found' % id) - return _to_dict(version) + raise NotFound('Version with id "%s" not found' % product_version_id) + return _to_dict(version, allowed_keys=allowed_keys) def get_product_versions(product_id, allowed_keys=None): diff --git a/refstack/db/sqlalchemy/models.py b/refstack/db/sqlalchemy/models.py index 71a6aae5..4a4d964a 100644 --- a/refstack/db/sqlalchemy/models.py +++ b/refstack/db/sqlalchemy/models.py @@ -59,6 +59,9 @@ class Test(BASE, RefStackBase): # pragma: no cover duration_seconds = sa.Column(sa.Integer, nullable=False) results = orm.relationship('TestResults', backref='test') meta = orm.relationship('TestMeta', backref='test') + product_version_id = sa.Column(sa.String(36), + sa.ForeignKey('product_version.id'), + nullable=True, unique=False) @property def _extra_keys(self): @@ -74,7 +77,8 @@ class Test(BASE, RefStackBase): # pragma: no cover @property def default_allowed_keys(self): """Default keys.""" - return 'id', 'created_at', 'duration_seconds', 'meta' + return ('id', 'created_at', 'duration_seconds', 'meta', + 'product_version_id') class TestResults(BASE, RefStackBase): # pragma: no cover diff --git a/refstack/tests/api/test_results.py b/refstack/tests/api/test_results.py index 59aa430a..4c102fd8 100644 --- a/refstack/tests/api/test_results.py +++ b/refstack/tests/api/test_results.py @@ -15,11 +15,14 @@ import json import uuid +import mock from oslo_config import fixture as config_fixture import six import webtest.app +from refstack.api import constants as api_const from refstack.api import validators +from refstack import db from refstack.tests import api FAKE_TESTS_RESULT = { @@ -79,6 +82,67 @@ class TestResultsEndpoint(api.FunctionalTest): self.URL, params=results) + @mock.patch('refstack.api.utils.check_owner') + @mock.patch('refstack.api.utils.check_user_is_foundation_admin') + @mock.patch('refstack.api.utils.get_user_id', return_value='test-open-id') + def test_put(self, mock_user, mock_check_foundation, mock_check_owner): + """Test results endpoint with put request.""" + results = json.dumps(FAKE_TESTS_RESULT) + test_response = self.post_json(self.URL, params=results) + url = self.URL + test_response.get('test_id') + + user_info = { + 'openid': 'test-open-id', + 'email': 'foo@bar.com', + 'fullname': 'Foo Bar' + } + db.user_save(user_info) + + fake_product = { + 'name': 'product name', + 'description': 'product description', + 'product_type': api_const.CLOUD, + } + + # Create a product + product_response = self.post_json('/v1/products/', + params=json.dumps(fake_product)) + # Create a product version + version_url = '/v1/products/' + product_response['id'] + '/versions/' + version_response = self.post_json(version_url, + params=json.dumps({'version': '1'})) + + # Test Foundation admin can put. + mock_check_foundation.return_value = True + body = {'product_version_id': version_response['id']} + self.put_json(url, params=json.dumps(body)) + get_response = self.get_json(url) + self.assertEqual(version_response['id'], + get_response['product_version_id']) + + # Test when product_version_id is None. + body = {'product_version_id': None} + self.put_json(url, params=json.dumps(body)) + get_response = self.get_json(url) + self.assertIsNone(get_response['product_version_id']) + + # Check test owner can put. + mock_check_foundation.return_value = False + mock_check_owner.return_value = True + body = {'product_version_id': version_response['id']} + self.put_json(url, params=json.dumps(body)) + get_response = self.get_json(url) + self.assertEqual(version_response['id'], + get_response['product_version_id']) + + # Test unauthorized put. + mock_check_foundation.return_value = False + mock_check_owner.return_value = False + self.assertRaises(webtest.app.AppError, + self.put_json, + url, + params=json.dumps(body)) + def test_get_one(self): """Test get request.""" results = json.dumps(FAKE_TESTS_RESULT) diff --git a/refstack/tests/unit/test_api.py b/refstack/tests/unit/test_api.py index 85fe6d0e..2779ad7f 100644 --- a/refstack/tests/unit/test_api.py +++ b/refstack/tests/unit/test_api.py @@ -140,7 +140,8 @@ class ResultsControllerTestCase(BaseControllerTestCase): mock_get_test_res.assert_called_once_with('fake_arg') mock_get_test.assert_called_once_with( 'fake_arg', allowed_keys=['id', 'cpid', 'created_at', - 'duration_seconds', 'meta'] + 'duration_seconds', 'meta', + 'product_version_id'] ) @mock.patch('refstack.db.store_results') diff --git a/refstack/tests/unit/test_api_utils.py b/refstack/tests/unit/test_api_utils.py index c0bdf35e..c318b5d6 100644 --- a/refstack/tests/unit/test_api_utils.py +++ b/refstack/tests/unit/test_api_utils.py @@ -336,16 +336,19 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch('refstack.api.utils.check_user_is_foundation_admin') @mock.patch('pecan.abort', side_effect=exc.HTTPError) @mock.patch('refstack.db.get_test_meta_key') + @mock.patch('refstack.db.get_test') @mock.patch.object(api_utils, 'is_authenticated') @mock.patch.object(api_utils, 'get_user_id') def test_check_get_user_role(self, mock_get_user_id, mock_is_authenticated, + mock_get_test, mock_get_test_meta_key, mock_pecan_abort, mock_check_foundation): # Check user level mock_check_foundation.return_value = False mock_get_test_meta_key.return_value = None + mock_get_test.return_value = {} self.assertEqual(const.ROLE_USER, api_utils.get_user_role('fake_test')) api_utils.enforce_permissions('fake_test', const.ROLE_USER) self.assertRaises(exc.HTTPError, api_utils.enforce_permissions, @@ -409,10 +412,12 @@ class APIUtilsTestCase(base.BaseTestCase): @mock.patch('refstack.api.utils.check_user_is_foundation_admin') @mock.patch('pecan.abort', side_effect=exc.HTTPError) @mock.patch('refstack.db.get_test_meta_key') + @mock.patch('refstack.db.get_test') @mock.patch.object(api_utils, 'is_authenticated') @mock.patch.object(api_utils, 'get_user_id') def test_check_permissions(self, mock_get_user_id, mock_is_authenticated, + mock_get_test, mock_get_test_meta_key, mock_pecan_abort, mock_foundation_check): @@ -437,6 +442,7 @@ class APIUtilsTestCase(base.BaseTestCase): private_test = 'fake_test' mock_get_user_id.return_value = 'fake_openid' + mock_get_test.return_value = {} mock_get_test_meta_key.side_effect = lambda *args: { (public_test, const.USER): None, (private_test, const.USER): 'fake_openid', diff --git a/refstack/tests/unit/test_db.py b/refstack/tests/unit/test_db.py index 3d7f5b13..ec20e1e1 100644 --- a/refstack/tests/unit/test_db.py +++ b/refstack/tests/unit/test_db.py @@ -254,6 +254,21 @@ class DBBackendTestCase(base.BaseTestCase): .first.return_value = None self.assertRaises(api.NotFound, db.delete_test, 'fake_id') + @mock.patch.object(api, 'get_session') + @mock.patch.object(api, '_to_dict', side_effect=lambda x: x) + def test_update_test(self, mock_to_dict, mock_get_session): + session = mock_get_session.return_value + mock_test = mock.Mock() + session.query.return_value.filter_by.return_value\ + .first.return_value = mock_test + + test_info = {'product_version_id': '123'} + api.update_test(test_info) + + mock_get_session.assert_called_once_with() + mock_test.save.assert_called_once_with(session=session) + session.begin.assert_called_once_with() + @mock.patch('refstack.db.sqlalchemy.api.models') @mock.patch.object(api, 'get_session') def test_get_test_meta_key(self, mock_get_session, mock_models):