openinfra/refstack
Code Issues Proposed changes

Add verification_status field to test

Browse Source 
This patch adds the ability for Foundation admins
to mark a test that has been associated to a target program and
guideline as 'verified' so that the test can not be deleted or updated.

Updated error messages to reflect that products/vendors can not
be deleted if a test is associated.

Change-Id: Ifa7cc2eff9d2bce9129ec1738adebfa3a0a4966f
Implements-Spec: https://review.openstack.org/#/c/343954
This commit is contained in:
Paul Van Eck
2016-09-27 16:24:01 -07:00
parent 96c8679314
commit 02307f3a1e
9 changed files with 193 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
refstack/api/constants.py
View File

@@ -20,6 +20,7 @@ END_DATE = 'end_date'
CPID = 'cpid'
PAGE = 'page'
SIGNED = 'signed'
VERIFICATION_STATUS = 'verification_status'
OPENID = 'openid'
USER_PUBKEYS = 'pubkeys'
@@ -50,6 +51,10 @@ USER_OPENID = 'user_openid'
USER = 'user'
SHARED_TEST_RUN = 'shared'
# Test verification statuses
TEST_NOT_VERIFIED = 0
TEST_VERIFIED = 1
# Roles
ROLE_USER = 'user'
ROLE_OWNER = 'owner'

15
refstack/api/controllers/products.py
View File

@@ -19,6 +19,7 @@ import json
import uuid
from oslo_config import cfg
from oslo_db.exception import DBReferenceError
from oslo_log import log
import pecan
from pecan.secure import secure
@@ -112,8 +113,11 @@ class VersionsController(validation.BaseRestControllerWithValidation):
not api_utils.check_user_is_foundation_admin()):
pecan.abort(403, 'Forbidden.')
db.delete_product_version(version_id)
try:
db.delete_product_version(version_id)
except DBReferenceError:
pecan.abort(400, 'Unable to delete. There are still tests '
'associated to this product version.')
pecan.response.status = 204
@@ -262,6 +266,9 @@ class ProductsController(validation.BaseRestControllerWithValidation):
if (not api_utils.check_user_is_foundation_admin() and
not api_utils.check_user_is_product_admin(id)):
pecan.abort(403, 'Forbidden.')
db.delete_product(id)
try:
db.delete_product(id)
except DBReferenceError:
pecan.abort(400, 'Unable to delete. There are still tests '
'associated to versions of this product.')
pecan.response.status = 204

48
refstack/api/controllers/results.py
View File

@@ -76,6 +76,10 @@ class MetadataController(rest.RestController):
@pecan.expose('json')
def post(self, test_id, key):
"""Save value for key in test run metadata."""
test = db.get_test(test_id)
if test['verification_status'] == const.TEST_VERIFIED:
pecan.abort(403, 'Can not add/alter a new metadata key for a '
'verified test run.')
db.save_test_meta_item(test_id, key, pecan.request.body)
pecan.response.status = 201
@@ -84,6 +88,10 @@ class MetadataController(rest.RestController):
@pecan.expose('json')
def delete(self, test_id, key):
"""Delete key from test run metadata."""
test = db.get_test(test_id)
if test['verification_status'] == const.TEST_VERIFIED:
pecan.abort(403, 'Can not delete a metadata key for a '
'verified test run.')
db.delete_test_meta_item(test_id, key)
pecan.response.status = 204
@@ -104,7 +112,8 @@ class ResultsController(validation.BaseRestControllerWithValidation):
test_info = db.get_test(
test_id, allowed_keys=['id', 'cpid', 'created_at',
'duration_seconds', 'meta',
'product_version_id']
'product_version_id',
'verification_status']
)
else:
test_info = db.get_test(test_id)
@@ -143,6 +152,10 @@ class ResultsController(validation.BaseRestControllerWithValidation):
@api_utils.check_permissions(level=const.ROLE_OWNER)
def delete(self, test_id):
"""Delete test run."""
test = db.get_test(test_id)
if test['verification_status'] == const.TEST_VERIFIED:
pecan.abort(403, 'Can not delete a verified test run.')
db.delete_test(test_id)
pecan.response.status = 204
@@ -162,7 +175,8 @@ class ResultsController(validation.BaseRestControllerWithValidation):
const.START_DATE,
const.END_DATE,
const.CPID,
const.SIGNED
const.SIGNED,
const.VERIFICATION_STATUS
]
filters = api_utils.parse_input_params(expected_input_params)
@@ -205,7 +219,14 @@ class ResultsController(validation.BaseRestControllerWithValidation):
def put(self, test_id, **kw):
"""Update a test result."""
test_info = {'id': test_id}
is_foundation_admin = api_utils.check_user_is_foundation_admin()
if 'product_version_id' in kw:
test = db.get_test(test_id)
if test['verification_status'] == const.TEST_VERIFIED:
pecan.abort(403, 'Can not update product_version_id for a '
'verified test run.')
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'])
@@ -218,13 +239,34 @@ class ResultsController(validation.BaseRestControllerWithValidation):
# 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']
if 'verification_status' in kw:
if not is_foundation_admin:
pecan.abort(403, 'You do not have permission to change a '
'verification status.')
if kw['verification_status'] not in (0, 1):
pecan.abort(400, 'Invalid verification_status value: %d' %
kw['verification_status'])
# Check pre-conditions are met to mark a test verified.
if (kw['verification_status'] == 1 and
not (db.get_test_meta_key(test_id, 'target') and
db.get_test_meta_key(test_id, 'guideline') and
db.get_test_meta_key(test_id, const.SHARED_TEST_RUN))):
pecan.abort(403, 'In order to mark a test verified, the '
'test must be shared and have been '
'associated to a guideline and target '
'program.')
test_info['verification_status'] = kw['verification_status']
test = db.update_test(test_info)
pecan.response.status = 201
return test

7
refstack/api/controllers/vendors.py
View File

@@ -19,6 +19,7 @@ import json
import six
from oslo_config import cfg
from oslo_db.exception import DBReferenceError
from oslo_log import log
import pecan
from pecan import rest
@@ -196,7 +197,11 @@ class VendorsController(validation.BaseRestControllerWithValidation):
pecan.abort(403, 'Forbidden.')
_check_is_not_foundation(vendor_id)
db.delete_organization(vendor_id)
try:
db.delete_organization(vendor_id)
except DBReferenceError:
pecan.abort(400, 'Unable to delete. There are still tests '
'associated to products for this vendor.')
pecan.response.status = 204
@secure(api_utils.is_authenticated)

28
refstack/db/migrations/alembic/versions/59df512e82f_add_verification_status.py Normal file
View File

@@ -0,0 +1,28 @@
"""Add verification_status field to test.
Revision ID: 59df512e82f
Revises: 23843be3da52
Create Date: 2016-09-26 11:51:08.955006
"""
# revision identifiers, used by Alembic.
revision = '59df512e82f'
down_revision = '23843be3da52'
MYSQL_CHARSET = 'utf8'
from alembic import op
import sqlalchemy as sa
def upgrade():
"""Upgrade DB."""
op.add_column('test', sa.Column('verification_status',
sa.Integer,
nullable=False,
default=0))
def downgrade():
"""Downgrade DB."""
op.drop_column('test', 'verification_status')

13
refstack/db/sqlalchemy/api.py
View File

@@ -166,7 +166,7 @@ def update_test(test_info):
if test is None:
raise NotFound('Test result with id %s not found' % _id)
keys = ['product_version_id']
keys = ['product_version_id', 'verification_status']
for key in keys:
if key in test_info:
setattr(test, key, test_info[key])
@@ -238,6 +238,11 @@ def _apply_filters_for_query(query, filters):
if cpid:
query = query.filter(models.Test.cpid == cpid)
verification_status = filters.get(api_const.VERIFICATION_STATUS)
if verification_status:
query = query.filter(models.Test.verification_status ==
verification_status)
signed = api_const.SIGNED in filters
# If we only want to get the user's test results.
if signed:
@@ -442,6 +447,12 @@ def delete_organization(organization_id):
"""delete organization by id."""
session = get_session()
with session.begin():
product_ids = (session
.query(models.Product.id)
.filter_by(organization_id=organization_id))
(session.query(models.ProductVersion).
filter(models.ProductVersion.product_id.in_(product_ids)).
delete(synchronize_session=False))
(session.query(models.Product).
filter_by(organization_id=organization_id).
delete(synchronize_session=False))

3
refstack/db/sqlalchemy/models.py
View File

@@ -62,6 +62,7 @@ class Test(BASE, RefStackBase): # pragma: no cover
product_version_id = sa.Column(sa.String(36),
sa.ForeignKey('product_version.id'),
nullable=True, unique=False)
verification_status = sa.Column(sa.Integer, nullable=False, default=0)
@property
def _extra_keys(self):
@@ -78,7 +79,7 @@ class Test(BASE, RefStackBase): # pragma: no cover
def default_allowed_keys(self):
"""Default keys."""
return ('id', 'created_at', 'duration_seconds', 'meta',
'product_version_id')
'product_version_id', 'verification_status')
class TestResults(BASE, RefStackBase): # pragma: no cover

61
refstack/tests/api/test_results.py
View File

@@ -89,7 +89,8 @@ class TestResultsEndpoint(api.FunctionalTest):
"""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')
test_id = test_response.get('test_id')
url = self.URL + test_id
user_info = {
'openid': 'test-open-id',
@@ -126,6 +127,39 @@ class TestResultsEndpoint(api.FunctionalTest):
get_response = self.get_json(url)
self.assertIsNone(get_response['product_version_id'])
# Test when test verification preconditions are not met.
body = {'verification_status': api_const.TEST_VERIFIED}
put_response = self.put_json(url, expect_errors=True,
params=json.dumps(body))
self.assertEqual(403, put_response.status_code)
# Share the test run.
db.save_test_meta_item(test_id, api_const.SHARED_TEST_RUN, True)
put_response = self.put_json(url, expect_errors=True,
params=json.dumps(body))
self.assertEqual(403, put_response.status_code)
# Now associate guideline and target program. Now we should be
# able to mark a test verified.
db.save_test_meta_item(test_id, 'target', 'platform')
db.save_test_meta_item(test_id, 'guideline', '2016.01.json')
put_response = self.put_json(url, params=json.dumps(body))
self.assertEqual(api_const.TEST_VERIFIED,
put_response['verification_status'])
# Unshare the test, and check that we can mark it not verified.
db.delete_test_meta_item(test_id, api_const.SHARED_TEST_RUN)
body = {'verification_status': api_const.TEST_NOT_VERIFIED}
put_response = self.put_json(url, params=json.dumps(body))
self.assertEqual(api_const.TEST_NOT_VERIFIED,
put_response['verification_status'])
# Test when verification_status value is invalid.
body = {'verification_status': 111}
put_response = self.put_json(url, expect_errors=True,
params=json.dumps(body))
self.assertEqual(400, put_response.status_code)
# Check test owner can put.
mock_check_foundation.return_value = False
mock_check_owner.return_value = True
@@ -135,6 +169,12 @@ class TestResultsEndpoint(api.FunctionalTest):
self.assertEqual(version_response['id'],
get_response['product_version_id'])
# Test non-Foundation user can't change verification_status.
body = {'verification_status': 1}
put_response = self.put_json(url, expect_errors=True,
params=json.dumps(body))
self.assertEqual(403, put_response.status_code)
# Test unauthorized put.
mock_check_foundation.return_value = False
mock_check_owner.return_value = False
@@ -287,3 +327,22 @@ class TestResultsEndpoint(api.FunctionalTest):
url = '/v1/results?end_date=1000-01-01 12:00:00'
filtering_results = self.get_json(url)
self.assertEqual([], filtering_results['results'])
@mock.patch('refstack.api.utils.check_owner')
def test_delete(self, mock_check_owner):
results = json.dumps(FAKE_TESTS_RESULT)
test_response = self.post_json(self.URL, params=results)
test_id = test_response.get('test_id')
url = self.URL + test_id
mock_check_owner.return_value = True
# Test can't delete verified test run.
db.update_test({'id': test_id, 'verification_status': 1})
resp = self.delete(url, expect_errors=True)
self.assertEqual(403, resp.status_code)
# Test can delete verified test run.
db.update_test({'id': test_id, 'verification_status': 0})
resp = self.delete(url, expect_errors=True)
self.assertEqual(204, resp.status_code)

30
refstack/tests/unit/test_api.py
View File

@@ -141,7 +141,8 @@ class ResultsControllerTestCase(BaseControllerTestCase):
mock_get_test.assert_called_once_with(
'fake_arg', allowed_keys=['id', 'cpid', 'created_at',
'duration_seconds', 'meta',
'product_version_id']
'product_version_id',
'verification_status']
)
@mock.patch('refstack.db.store_results')
@@ -248,7 +249,8 @@ class ResultsControllerTestCase(BaseControllerTestCase):
const.START_DATE,
const.END_DATE,
const.CPID,
const.SIGNED
const.SIGNED,
const.VERIFICATION_STATUS,
]
page_number = 1
total_pages_number = 10
@@ -286,13 +288,21 @@ class ResultsControllerTestCase(BaseControllerTestCase):
db_get_test.assert_called_once_with(page_number, per_page, filters)
@mock.patch('refstack.db.get_test')
@mock.patch('refstack.db.delete_test')
def test_delete(self, mock_db_delete):
def test_delete(self, mock_db_delete, mock_get_test):
self.mock_get_user_role.return_value = const.ROLE_OWNER
self.controller.delete('test_id')
self.assertEqual(204, self.mock_response.status)
# Verified test deletion attempt should raise error.
mock_get_test.return_value = {'verification_status':
const.TEST_VERIFIED}
self.assertRaises(webob.exc.HTTPError,
self.controller.delete, 'test_id')
self.mock_get_user_role.return_value = const.ROLE_USER
self.mock_abort.side_effect = webob.exc.HTTPError()
self.assertRaises(webob.exc.HTTPError,
self.controller.delete, 'test_id')
@@ -634,9 +644,13 @@ class MetadataControllerTestCase(BaseControllerTestCase):
self.mock_get_user_role.return_value = const.ROLE_FOUNDATION
self.assertEqual(42, self.controller.get_one('test_id', 'user'))
@mock.patch('refstack.db.get_test')
@mock.patch('refstack.db.save_test_meta_item')
def test_post(self, mock_save_test_meta_item):
def test_post(self, mock_save_test_meta_item, mock_get_test):
self.mock_get_user_role.return_value = const.ROLE_OWNER
mock_get_test.return_value = {
'verification_status': const.TEST_NOT_VERIFIED
}
# Test trying to post a valid key.
self.controller.post('test_id', 'shared')
@@ -654,9 +668,13 @@ class MetadataControllerTestCase(BaseControllerTestCase):
self.assertRaises(webob.exc.HTTPError,
self.controller.post, 'test_id', 'shared')
@mock.patch('refstack.db.get_test')
@mock.patch('refstack.db.delete_test_meta_item')
def test_delete(self, mock_delete_test_meta_item):
def test_delete(self, mock_delete_test_meta_item, mock_get_test):
self.mock_get_user_role.return_value = const.ROLE_OWNER
mock_get_test.return_value = {
'verification_status': const.TEST_NOT_VERIFIED
}
self.controller.delete('test_id', 'shared')
self.assertEqual(204, self.mock_response.status)
mock_delete_test_meta_item.assert_called_once_with('test_id', 'shared')