Add missing unit tests

This patch adds additional unit tests for updating, listing,
uploading and deleteing of artifacts.

Plus several tests were added for trusted auth middleware.

Finally some dead code was removed from the system.

Change-Id: Ia570d02251946b0a9c28b34678239109a125abcf
This commit is contained in:
Mike Fedosin 2017-05-28 21:01:38 +03:00
parent 7da6ebf6b2
commit 9682a0036f
8 changed files with 346 additions and 28 deletions

View File

@ -266,7 +266,7 @@ class ArtifactsController(api_versioning.VersionedResource):
@supported_versions(min_ver='1.0')
@log_request_progress
def list(self, req, type_name, filters, marker=None, limit=None,
def list(self, req, type_name, filters=None, marker=None, limit=None,
sort=None, latest=False):
"""List available artifacts.

View File

@ -14,7 +14,6 @@
import operator
import threading
import uuid
from oslo_config import cfg
from oslo_db import exception as db_exception
@ -100,13 +99,6 @@ def drop_db():
models.unregister_models(engine)
def clear_db_env():
"""Unset global configuration variables for database.
"""
global _FACADE
_FACADE = None
def create(context, values, session):
return _create_or_update(context, None, values, session)
@ -138,15 +130,9 @@ def _create_or_update(context, artifact_id, values, session):
with session.begin():
_drop_protected_attrs(models.Artifact, values)
if artifact_id is None:
if 'type_name' not in values:
msg = _('Type name must be set.')
raise exception.BadRequest(msg)
# create new artifact
artifact = models.Artifact()
if 'id' not in values:
artifact.id = str(uuid.uuid4())
else:
artifact.id = values.pop('id')
artifact.id = values.pop('id')
artifact.created_at = timeutils.utcnow()
else:
# update the existing artifact

View File

@ -287,7 +287,7 @@ class BaseArtifact(base.VersionedObject):
else:
filters.extend([('visibility', 'public')])
if len(cls.list(context, filters)) > 0:
msg = _("Artifact with this name and version is already "
msg = _("Artifact with this name and version already "
"exists for this owner.")
raise exception.Conflict(msg)
else:
@ -695,10 +695,6 @@ class BaseArtifact(base.VersionedObject):
msg = _("Only {'status': %s} is allowed in a request "
"for reactivation.") % cls.STATUS.ACTIVE
raise exception.BadRequest(msg)
if af.status != cls.STATUS.DEACTIVATED:
raise exception.InvalidStatusTransition(
orig=af.status, new=cls.STATUS.ACTIVE
)
LOG.info("Parameters validation for artifact %(artifact)s "
"reactivate passed for request %(request)s.",
{'artifact': af.id, 'request': context.request_id})

View File

@ -130,6 +130,22 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
self.assertRaises(exc.GlareException, self.controller.delete,
self.req, 'sample_artifact', self.artifact['id'])
@mock.patch('glare.common.store_api.delete_blob',
side_effect=exc.NotFound)
def test_delete_blob_not_found(self, mocked_delete):
# Upload a file to blob dict
self.controller.upload_blob(
self.req, 'sample_artifact', self.artifact['id'],
'dict_of_blobs/blob',
BytesIO(b'a' * 100), 'application/octet-stream')
# Despite the exception artifact should be deleted successfully
self.controller.delete(self.req, 'sample_artifact',
self.artifact['id'])
self.assertRaises(exc.NotFound, self.controller.show,
self.req, 'sample_artifact', self.artifact['id'])
self.assertEqual(2, mocked_delete.call_count)
@mock.patch('glare.common.store_api.delete_blob',
side_effect=store_api.delete_blob)
def test_delayed_delete(self, mocked_delete):

View File

@ -58,15 +58,27 @@ class TestArtifactList(base.BaseTestArtifactAPI):
# (filter_name, filter_value)
# List all artifacts
res = self.controller.list(self.req, 'sample_artifact', [])
res = self.controller.list(self.req, 'sample_artifact')
self.assertEqual(7, len(res['artifacts']))
self.assertEqual('sample_artifact', res['type_name'])
# List all artifacts as an anonymous. Only public artifacts are visible
anon_req = self.get_fake_request(user=self.users['anonymous'])
res = self.controller.list(anon_req, 'sample_artifact')
self.assertEqual(1, len(res['artifacts']))
self.assertIn(arts[4], res['artifacts'])
# Filter by name
filters = [('name', 'art1')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(5, len(res['artifacts']))
filters = [('name', 'in:art2,art3')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(2, len(res['artifacts']))
for i in (5, 6):
self.assertIn(arts[i], res['artifacts'])
# Filter by string_required
filters = [('string_required', 'str1')]
res = self.controller.list(self.req, 'sample_artifact', filters)
@ -81,6 +93,12 @@ class TestArtifactList(base.BaseTestArtifactAPI):
for i in (0, 2, 4):
self.assertIn(arts[i], res['artifacts'])
filters = [('int1', 'in:5,6')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(4, len(res['artifacts']))
for i in (0, 1, 2, 4):
self.assertIn(arts[i], res['artifacts'])
# Filter by float1
filters = [('float1', '5.0')]
res = self.controller.list(self.req, 'sample_artifact', filters)
@ -146,13 +164,18 @@ class TestArtifactList(base.BaseTestArtifactAPI):
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact', filters)
# Filter by nonexistent field leads to BadRequest
filters = [('NONEXISTENT', 'something')]
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact', filters)
def test_list_marker_and_limit(self):
# Create artifacts
art_list = [
self.controller.create(
self.req, 'sample_artifact',
{'name': 'name%s' % i,
'version': '1.0',
'version': '%d.0' % i,
'tags': ['tag%s' % i],
'int1': 1024 + i,
'float1': 123.456,
@ -203,6 +226,22 @@ class TestArtifactList(base.BaseTestArtifactAPI):
marker=marker, limit=2, sort=sort)
self.assertEqual([art_list[0]], result['artifacts'])
# paginate by version in desc order with limit 2
sort = [('version', 'desc')]
result = self.controller.list(self.req, 'sample_artifact', filters=(),
limit=2, sort=sort)
self.assertEqual(art_list[4:2:-1], result['artifacts'])
marker = result['next_marker']
result = self.controller.list(self.req, 'sample_artifact', filters=(),
marker=marker, limit=2, sort=sort)
self.assertEqual(art_list[2:0:-1], result['artifacts'])
marker = result['next_marker']
result = self.controller.list(self.req, 'sample_artifact', filters=(),
marker=marker, limit=2, sort=sort)
self.assertEqual([art_list[0]], result['artifacts'])
def test_list_version(self):
values = [
{'name': 'art1', 'version': '0.0.1'},
@ -395,6 +434,16 @@ class TestArtifactList(base.BaseTestArtifactAPI):
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact', filters)
# Filter by nonexistent dict leads to BadRequest
filters = [('NOTEXIST.one', 'eq:1')]
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact', filters)
# Test with TypeError
filters = [('dict_of_int.1', 'lala')]
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact', filters)
# Return artifacts that contain key 'aa' in 'list_of_str'
filters = [('list_of_str', 'eq:aa')]
res = self.controller.list(self.req, 'sample_artifact', filters)
@ -428,6 +477,58 @@ class TestArtifactList(base.BaseTestArtifactAPI):
for i in (0, 1, 2, 3):
self.assertIn(arts[i], res['artifacts'])
def test_filter_by_tags(self):
values = [
{'name': 'name1', 'tags': ['tag1', 'tag2']},
{'name': 'name2', 'tags': ['tag1', 'tag3']},
{'name': 'name3', 'tags': ['tag1']},
{'name': 'name4', 'tags': ['tag2']},
{'name': 'name5', 'tags': ['tag4']},
{'name': 'name6', 'tags': ['tag4', 'tag5']},
]
arts = [self.controller.create(self.req, 'sample_artifact', val)
for val in values]
filters = [('tags', 'tag1')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(3, len(res['artifacts']))
for i in (0, 1, 2):
self.assertIn(arts[i], res['artifacts'])
filters = [('tags', 'tag1,tag2')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(1, len(res['artifacts']))
self.assertIn(arts[0], res['artifacts'])
filters = [('tags', 'NOT_A_TAG')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(0, len(res['artifacts']))
filters = [('tags-any', 'tag1')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(3, len(res['artifacts']))
for i in (0, 1, 2):
self.assertIn(arts[i], res['artifacts'])
filters = [('tags-any', 'tag1,NOT_A_TAG')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(3, len(res['artifacts']))
for i in (0, 1, 2):
self.assertIn(arts[i], res['artifacts'])
filters = [('tags-any', 'tag2,tag5')]
res = self.controller.list(self.req, 'sample_artifact', filters)
self.assertEqual(3, len(res['artifacts']))
for i in (0, 3, 5):
self.assertIn(arts[i], res['artifacts'])
# Filtering by tags with operators leads to BadRequest
for f in ('tags', 'tags-any'):
filters = [(f, 'eq:tag1')]
self.assertRaises(
exc.BadRequest, self.controller.list,
self.req, 'sample_artifact', filters)
def test_list_and_sort_fields(self):
amount = 7
# Create a bunch of artifacts for list sorting tests
@ -459,6 +560,22 @@ class TestArtifactList(base.BaseTestArtifactAPI):
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact',
[], sort=[("NONEXISTENT", "desc")])
# sort by wrong direction
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact',
[], sort=[("name", "WRONG_DIR")])
# For performance sake sorting by more than one custom field
# is forbidden. Nevertheless, sorting by several basic field are
# absolutely fine.
# List of basic fields is located in glare/db/sqlalchemy/api.py as
# BASE_ARTIFACT_PROPERTIES tuple.
sort = [("int1", "desc"), ("float1", "desc")]
self.assertRaises(exc.BadRequest, self.controller.list,
self.req, 'sample_artifact',
[], sort=sort)
# sort with non-sortable fields
for name, field in sample_artifact.SampleArtifact.fields.items():
for sort_dir in ['asc', 'desc']:
@ -466,4 +583,4 @@ class TestArtifactList(base.BaseTestArtifactAPI):
self.assertRaises(
exc.BadRequest, self.controller.list,
self.req, 'sample_artifact',
[], sort=[(field, sort_dir)])
[], sort=[(name, sort_dir)])

View File

@ -12,9 +12,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from six import BytesIO
from uuid import uuid4
from glare.common import exception as exc
from glare.db import artifact_api
from glare.tests.unit import base
@ -43,11 +45,92 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
self.assertEqual({'tag1', 'tag2'}, set(res['tags']))
self.assertEqual({'k': 'v'}, res['metadata'])
def test_update_replace_values(self):
changes = [
{'op': 'replace', 'path': '/int1', 'value': 1},
{'op': 'replace', 'path': '/float1', 'value': 1.0},
{'op': 'replace', 'path': '/str1', 'value': 'Test'},
{'op': 'replace', 'path': '/list_of_int', 'value': [0, 1]},
{'op': 'replace', 'path': '/dict_of_str', 'value': {'k': 'v'}},
]
res = self.update_with_values(changes)
self.assertEqual(1, res['int1'])
self.assertEqual(1.0, res['float1'])
self.assertEqual('Test', res['str1'])
self.assertEqual([0, 1], res['list_of_int'])
self.assertEqual({'k': 'v'}, res['dict_of_str'])
changes = [
{'op': 'replace', 'path': '/int1', 'value': 2},
{'op': 'replace', 'path': '/float1', 'value': 2.0},
{'op': 'replace', 'path': '/str1', 'value': 'New_Test'},
{'op': 'replace', 'path': '/list_of_int/1', 'value': 4},
{'op': 'replace', 'path': '/dict_of_str/k', 'value': 'new_val'},
]
res = self.update_with_values(changes)
self.assertEqual(2, res['int1'])
self.assertEqual(2.0, res['float1'])
self.assertEqual('New_Test', res['str1'])
self.assertEqual([0, 4], res['list_of_int'])
self.assertEqual({'k': 'new_val'}, res['dict_of_str'])
def test_update_no_artifact_type(self):
changes = [{'op': 'replace', 'path': '/name', 'value': 'new_name'}]
self.update_with_values(
changes, exc_class=exc.NotFound, art_type='wrong_type')
def test_update_name_version(self):
# Create additional artifacts
values = {'name': 'ttt', 'version': '2.0'}
self.controller.create(self.req, 'sample_artifact', values)
values = {'name': 'ddd', 'version': '1.0'}
self.controller.create(self.req, 'sample_artifact', values)
# This name/version is already taken
changes = [{'op': 'replace', 'path': '/version',
'value': '2.0'}]
self.assertRaises(exc.Conflict, self.update_with_values, changes)
changes = [{'op': 'replace', 'path': '/name',
'value': 'ddd'}]
self.assertRaises(exc.Conflict, self.update_with_values, changes)
# Test coercing
# name
changes = [{'op': 'replace', 'path': '/name', 'value': True}]
res = self.update_with_values(changes)
self.assertEqual('True', res['name'])
changes = [{'op': 'replace', 'path': '/name', 'value': 1.0}]
res = self.update_with_values(changes)
self.assertEqual('1.0', res['name'])
# version
changes = [{'op': 'replace', 'path': '/version', 'value': 2.0}]
res = self.update_with_values(changes)
self.assertEqual('2.0.0', res['version'])
changes = [{'op': 'replace', 'path': '/version', 'value': '1-alpha'}]
res = self.update_with_values(changes)
self.assertEqual('1.0.0-alpha', res['version'])
def test_update_deleted_artifact(self):
# Enable delayed delete
self.config(delayed_delete=True)
# Delete artifact and check its status
self.controller.delete(self.req, 'sample_artifact',
self.sample_artifact['id'])
art = self.controller.show(self.req, 'sample_artifact',
self.sample_artifact['id'])
self.assertEqual('deleted', art['status'])
changes = [{'op': 'replace', 'path': '/int1',
'value': 1}]
self.assertRaises(exc.Forbidden, self.update_with_values, changes)
changes = [{'op': 'replace', 'path': '/name',
'value': 'new'}]
self.assertRaises(exc.Forbidden, self.update_with_values, changes)
def test_update_lists(self):
changes = [{'op': 'replace', 'path': '/list_of_str',
'value': ['val1', 'val2']}]
@ -86,6 +169,10 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
'value': [['a']]}]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'remove', 'path': '/list_of_str/-',
'value': 'val3'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
def test_update_dicts(self):
changes = [{'op': 'replace', 'path': '/dict_of_str',
'value': {'k1': 'v1', 'k2': 'v2'}}]
@ -176,6 +263,12 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
'value': {('a' + str(i)): 'a' for i in range(256)}}]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'replace', 'path': '/int1', 'value': 'aaa'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'replace', 'path': '/float1', 'value': 'aaa'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
def test_update_artifact_not_existing_field(self):
changes = [{'op': 'replace', 'path': '/wrong_field', 'value': 'a'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
@ -183,6 +276,13 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
changes = [{'op': 'replace', 'path': '/', 'value': 'a'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
def test_update_artifact_remove_field(self):
changes = [{'op': 'remove', 'path': '/name'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'remove', 'path': '/list_of_int/10'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
def test_update_artifact_blob(self):
changes = [{'op': 'replace', 'path': '/blob', 'value': 'a'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
@ -254,10 +354,8 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
'value': 'wrong_value'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'replace', 'path': '/status',
'value': 'deactivated'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
# It's forbidden to activate artifact until required_on_activate field
# 'string_required' is set
changes = [{'op': 'replace', 'path': '/status',
'value': 'active'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
@ -266,11 +364,53 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
'value': None}]
self.update_with_values(changes, exc_class=exc.BadRequest)
# It's forbidden to deactivate drafted artifact
changes = [{'op': 'replace', 'path': '/status',
'value': 'deactivated'}]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'replace', 'path': '/string_required',
'value': 'some_string'}]
res = self.update_with_values(changes)
self.assertEqual('some_string', res['string_required'])
# It's forbidden to change artifact status with other fields in
# one request
changes = [
{'op': 'replace', 'path': '/name', 'value': 'new_name'},
{'op': 'replace', 'path': '/status', 'value': 'active'}
]
self.update_with_values(changes, exc_class=exc.BadRequest)
# It's impossible to activate the artifact when it has 'saving' blobs
self.controller.upload_blob(
self.req, 'sample_artifact', self.sample_artifact['id'], 'blob',
BytesIO(b'aaa'), 'application/octet-stream')
self.sample_artifact = self.controller.show(
self.req, 'sample_artifact', self.sample_artifact['id'])
# Change status of the blob to 'saving'
self.sample_artifact['blob']['status'] = 'saving'
artifact_api.ArtifactAPI().update_blob(
self.req.context, self.sample_artifact['id'],
{'blob': self.sample_artifact['blob']})
self.sample_artifact = self.controller.show(
self.req, 'sample_artifact', self.sample_artifact['id'])
self.assertEqual('saving', self.sample_artifact['blob']['status'])
# Now activating of the artifact leads to Conflict
changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}]
self.assertRaises(exc.Conflict, self.update_with_values, changes)
# Reverting status of the blob to active again
self.sample_artifact['blob']['status'] = 'active'
artifact_api.ArtifactAPI().update_blob(
self.req.context, self.sample_artifact['id'],
{'blob': self.sample_artifact['blob']})
self.sample_artifact = self.controller.show(
self.req, 'sample_artifact', self.sample_artifact['id'])
self.assertEqual('active', self.sample_artifact['blob']['status'])
changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}]
res = self.update_with_values(changes)
self.assertEqual('active', res['status'])
@ -279,6 +419,14 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
res = self.update_with_values(changes)
self.assertEqual('active', res['status'])
# It's forbidden to change artifact status with other fields in
# one request
changes = [
{'op': 'replace', 'path': '/string_mutable', 'value': 'str'},
{'op': 'replace', 'path': '/status', 'value': 'deactivated'}
]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'replace', 'path': '/status',
'value': 'deactivated'}]
res = self.update_with_values(changes)
@ -289,6 +437,14 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
res = self.update_with_values(changes)
self.assertEqual('deactivated', res['status'])
# It's forbidden to change artifact status with other fields in
# one request
changes = [
{'op': 'replace', 'path': '/name', 'value': 'new_name'},
{'op': 'replace', 'path': '/status', 'value': 'active'}
]
self.update_with_values(changes, exc_class=exc.BadRequest)
changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}]
res = self.update_with_values(changes)
self.assertEqual('active', res['status'])
@ -297,6 +453,19 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
'value': None}]
self.update_with_values(changes, exc_class=exc.BadRequest)
# Enable delayed delete
self.config(delayed_delete=True)
# Delete artifact and check its status
self.controller.delete(self.req, 'sample_artifact',
self.sample_artifact['id'])
art = self.controller.show(self.req, 'sample_artifact',
self.sample_artifact['id'])
self.assertEqual('deleted', art['status'])
changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}]
self.assertRaises(exc.InvalidStatusTransition,
self.update_with_values, changes)
def test_update_artifact_mutable_fields(self):
changes = [{'op': 'replace', 'path': '/string_required',
'value': 'some_string'}]

View File

@ -180,6 +180,11 @@ class TestArtifactUpload(base.BaseTestArtifactAPI):
self.req, 'sample_artifact', self.sample_artifact['id'], 'INVALID',
BytesIO(b'aaa'), 'application/octet-stream')
self.assertRaises(
exc.BadRequest, self.controller.upload_blob,
self.req, 'sample_artifact', self.sample_artifact['id'],
'blob/key', BytesIO(b'aaa'), 'application/octet-stream')
def test_upload_non_blob_field(self):
self.assertRaises(
exc.BadRequest, self.controller.upload_blob,

View File

@ -43,6 +43,13 @@ class TestTrustedAuthMiddleware(base.BaseTestCase):
self.assertEqual('user1', req.context.user)
self.assertEqual('tenant1', req.context.tenant)
self.assertEqual(['role1', 'role2'], req.context.roles)
self.assertIn('service_catalog', req.context.to_dict())
def test_no_auth_token(self):
req = self._build_request(None)
del req.headers['x-auth-token']
self.assertRaises(exc.Unauthorized,
self._build_middleware().process_request, req)
def test_wrong_format(self):
req = self._build_request('WRONG_FORMAT')
@ -142,3 +149,25 @@ class TestTrustedAuthMiddleware(base.BaseTestCase):
resp_req_id = resp_req_id.decode('utf-8')
self.assertFalse(resp_req_id.startswith('req-req-'))
self.assertTrue(resp_req_id.startswith('req-'))
def test_response_no_request_id(self):
req = self._build_request('user1:tenant1:role1,role2')
req.context = context.RequestContext()
del req.context.request_id
resp = webob.Response()
resp.request = req
self._build_middleware().process_response(resp)
self.assertNotIn('x-openstack-request-id', resp.headers)
def test_response_no_request_id_prefix(self):
# prefix is 'req-'
req = self._build_request('user1:tenant1:role1,role2')
req.context = context.RequestContext()
req.context.request_id = "STRING_WITH_NO_PREFIX"
resp = webob.Response()
resp.request = req
self._build_middleware().process_response(resp)
self.assertEqual('req-STRING_WITH_NO_PREFIX',
resp.headers['x-openstack-request-id'])