diff --git a/glare/api/v1/resource.py b/glare/api/v1/resource.py index ef9888d..aa8892e 100644 --- a/glare/api/v1/resource.py +++ b/glare/api/v1/resource.py @@ -141,7 +141,8 @@ class RequestDeserializer(api_versioning.VersionedResource, # Initially patch object doesn't validate input. It's only checked # when we call get operation on each method tuple(map(patch._get_operation, patch.patch)) - except (jsonpatch.InvalidJsonPatch, TypeError): + except (jsonpatch.InvalidJsonPatch, TypeError, AttributeError, + jsonpatch.JsonPointerException): msg = _("Json Patch body is malformed") raise exc.BadRequest(msg) return {'patch': patch} @@ -156,6 +157,10 @@ class RequestDeserializer(api_versioning.VersionedResource, msg = _("url is required when specifying external location. " "Cannot find 'url' in request body: %s") % str(data) raise exc.BadRequest(msg) + if 'md5' not in data: + msg = _("Incorrect blob metadata. MD5 must be specified " + "for external location in artifact blob.") + raise exc.BadRequest(msg) else: data = req.body_file @@ -226,6 +231,13 @@ class ArtifactsController(api_versioning.VersionedResource): if req.context.tenant is None or req.context.read_only: msg = _("It's forbidden to anonymous users to create artifacts.") raise exc.Forbidden(msg) + if not values.get('name'): + msg = _("Name must be specified at creation.") + raise exc.BadRequest(msg) + for field in ('visibility', 'status'): + if field in values: + msg = _("%s is not allowed in a request at creation.") % field + raise exc.BadRequest(msg) return self.engine.create(req.context, type_name, values) @supported_versions(min_ver='1.0') @@ -239,7 +251,7 @@ class ArtifactsController(api_versioning.VersionedResource): :param patch: json patch with artifact changes :return: definition of updated artifact """ - return self.engine.update(req.context, type_name, artifact_id, patch) + return self.engine.save(req.context, type_name, artifact_id, patch) @supported_versions(min_ver='1.0') @log_request_progress @@ -262,7 +274,7 @@ class ArtifactsController(api_versioning.VersionedResource): :param artifact_id: id of artifact to show :return: definition of requested artifact """ - return self.engine.get(req.context, type_name, artifact_id) + return self.engine.show(req.context, type_name, artifact_id) @supported_versions(min_ver='1.0') @log_request_progress diff --git a/glare/common/exception.py b/glare/common/exception.py index 1e47f73..aec6ae0 100644 --- a/glare/common/exception.py +++ b/glare/common/exception.py @@ -48,10 +48,6 @@ class BadRequest(GlareException): message = _("Bad request") -class InvalidStatusTransition(BadRequest): - message = _("Transition status from %(orig)s to %(new)s was not valid") - - class InvalidParameterValue(BadRequest): message = _("Invalid filter value ") diff --git a/glare/common/utils.py b/glare/common/utils.py index ba8dbd2..2d04308 100644 --- a/glare/common/utils.py +++ b/glare/common/utils.py @@ -502,44 +502,6 @@ def _get_element_type(element_type): return 'String' -class DictDiffer(object): - """Calculate the difference between two dictionaries as: - (1) items added - (2) items removed - (3) keys same in both but changed values - (4) keys same in both and unchanged values - """ - - def __init__(self, current_dict, past_dict): - self.current_dict, self.past_dict = current_dict, past_dict - self.current_keys, self.past_keys = [ - set(d.keys()) for d in (current_dict, past_dict)] - self.intersect = self.current_keys.intersection( - self.past_keys) - - def added(self): - return self.current_keys - self.intersect - - def removed(self): - return self.past_keys - self.intersect - - def changed(self): - return set(o for o in self.intersect - if self.past_dict[o] != self.current_dict[o]) - - def unchanged(self): - return set(o for o in self.intersect - if self.past_dict[o] == self.current_dict[o]) - - def __str__(self): - msg = "\nResult output:\n" - msg += "\tAdded keys: %s\n" % ', '.join(self.added()) - msg += "\tRemoved keys: %s\n" % ', '.join(self.removed()) - msg += "\tChanged keys: %s\n" % ', '.join(self.changed()) - msg += "\tUnchanged keys: %s\n" % ', '.join(self.unchanged()) - return msg - - class BlobIterator(object): """Reads data from a blob, one chunk at a time. """ @@ -556,3 +518,65 @@ class BlobIterator(object): bytes_left -= len(data) yield data raise StopIteration() + + +def validate_status_transition(af, from_status, to_status): + if from_status == 'deleted': + msg = _("Cannot change status if artifact is deleted.") + raise exception.Forbidden(msg) + if to_status == 'active': + if from_status == 'drafted': + for name, type_obj in af.fields.items(): + if type_obj.required_on_activate and getattr(af, name) is None: + msg = _("'%s' field value must be set before " + "activation.") % name + raise exception.Forbidden(msg) + elif to_status == 'drafted': + if from_status != 'drafted': + msg = _("Cannot change status to 'drafted'") % from_status + raise exception.Forbidden(msg) + elif to_status == 'deactivated': + if from_status not in ('active', 'deactivated'): + msg = _("Cannot deactivate artifact if it's not active.") + raise exception.Forbidden(msg) + elif to_status == 'deleted': + msg = _("Cannot delete artifact with PATCH requests. Use special " + "API to do this.") + raise exception.Forbidden(msg) + else: + msg = _("Unknown artifact status: %s.") % to_status + raise exception.BadRequest(msg) + + +def validate_visibility_transition(af, from_visibility, to_visibility): + if to_visibility == 'private': + if from_visibility != 'private': + msg = _("Cannot make artifact private again.") + raise exception.Forbidden() + elif to_visibility == 'public': + if af.status != 'active': + msg = _("Cannot change visibility to 'public' if artifact" + " is not active.") + raise exception.Forbidden(msg) + else: + msg = _("Unknown artifact visibility: %s.") % to_visibility + raise exception.BadRequest(msg) + + +def validate_change_allowed(af, field_name): + """Validate if fields can be set for the artifact.""" + if field_name not in af.fields: + msg = _("Cannot add new field '%s' to artifact.") % field_name + raise exception.BadRequest(msg) + if af.status not in ('active', 'drafted'): + msg = _("Forbidden to change fields " + "if artifact is not active or drafted.") + raise exception.Forbidden(message=msg) + if af.fields[field_name].system is True: + msg = _("Forbidden to specify system field %s. It is not " + "available for modifying by users.") % field_name + raise exception.Forbidden(msg) + if af.status == 'active' and not af.fields[field_name].mutable: + msg = (_("Forbidden to change field '%s' after activation.") + % field_name) + raise exception.Forbidden(message=msg) diff --git a/glare/db/artifact_api.py b/glare/db/artifact_api.py index 7430190..ec79434 100644 --- a/glare/db/artifact_api.py +++ b/glare/db/artifact_api.py @@ -48,23 +48,8 @@ class ArtifactAPI(object): @retry(retry_on_exception=_retry_on_connection_error, wait_fixed=1000, stop_max_attempt_number=20) - def create(self, context, values, type): - """Create new artifact in db and return dict of values to the user - - :param context: user context - :param values: dict of values that needs to be saved to db - :param type: string indicates artifact of what type to create - :return: dict of created values - """ - values = self._serialize_values(values) - values['type_name'] = type - session = api.get_session() - return api.create(context, values, session) - - @retry(retry_on_exception=_retry_on_connection_error, wait_fixed=1000, - stop_max_attempt_number=20) - def update(self, context, artifact_id, values): - """Update artifact values in database + def save(self, context, artifact_id, values): + """Save artifact values in database :param artifact_id: id of artifact that needs to be updated :param context: user context @@ -72,9 +57,11 @@ class ArtifactAPI(object): :return: dict of updated artifact values """ session = api.get_session() - return api.update(context, artifact_id, - self._serialize_values(values), session) + return api.create_or_update( + context, artifact_id, self._serialize_values(values), session) + @retry(retry_on_exception=_retry_on_connection_error, wait_fixed=1000, + stop_max_attempt_number=20) def update_blob(self, context, artifact_id, values): """Create and update blob records in db @@ -84,8 +71,8 @@ class ArtifactAPI(object): :return: dict of updated artifact values """ session = api.get_session() - return api.update(context, artifact_id, - {'blobs': values}, session) + return api.create_or_update( + context, artifact_id, {'blobs': values}, session) @retry(retry_on_exception=_retry_on_connection_error, wait_fixed=1000, stop_max_attempt_number=20) diff --git a/glare/db/sqlalchemy/api.py b/glare/db/sqlalchemy/api.py index 966e99d..0dca5ba 100644 --- a/glare/db/sqlalchemy/api.py +++ b/glare/db/sqlalchemy/api.py @@ -99,14 +99,6 @@ def drop_db(): models.unregister_models(engine) -def create(context, values, session): - return _create_or_update(context, None, values, session) - - -def update(context, artifact_id, values, session): - return _create_or_update(context, artifact_id, values, session) - - @retry(retry_on_exception=_retry_on_deadlock, wait_fixed=500, stop_max_attempt_number=50) def delete(context, artifact_id, session): @@ -126,14 +118,13 @@ def _drop_protected_attrs(model_class, values): @retry(retry_on_exception=_retry_on_deadlock, wait_fixed=500, stop_max_attempt_number=50) @utils.no_4byte_params -def _create_or_update(context, artifact_id, values, session): +def create_or_update(context, artifact_id, values, session): with session.begin(): _drop_protected_attrs(models.Artifact, values) if artifact_id is None: # create new artifact artifact = models.Artifact() artifact.id = values.pop('id') - artifact.created_at = timeutils.utcnow() else: # update the existing artifact artifact = _get(context, artifact_id, session) @@ -601,6 +592,7 @@ def _do_blobs(artifact, new_blobs): @retry(retry_on_exception=_retry_on_deadlock, wait_fixed=500, stop_max_attempt_number=50) +@utils.no_4byte_params def create_lock(context, lock_key, session): """Try to create lock record.""" with session.begin(): diff --git a/glare/engine.py b/glare/engine.py index 1afe37f..0e834e7 100644 --- a/glare/engine.py +++ b/glare/engine.py @@ -13,21 +13,24 @@ # License for the specific language governing permissions and limitations # under the License. +from copy import deepcopy import os import jsonpatch from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils +from oslo_utils import timeutils +from oslo_utils import uuidutils from glare.common import exception from glare.common import policy from glare.common import store_api from glare.common import utils +from glare.db import artifact_api from glare.i18n import _ +from glare import locking from glare.notification import Notifier -from glare.objects import base -from glare.objects.meta import fields as glare_fields from glare.objects.meta import registry CONF = cfg.CONF @@ -62,8 +65,37 @@ class Engine(object): self.schemas[type_name] = registry.ArtifactRegistry.\ get_artifact_type(type_name).gen_schemas() - @classmethod - def _get_artifact(cls, ctx, type_name, artifact_id, read_only=False): + lock_engine = locking.LockEngine(artifact_api.ArtifactLockApi()) + + def _create_scoped_lock(self, context, type_name, name, version, + owner, visibility='private'): + """Create scoped lock for artifact.""" + # validate that artifact doesn't exist for the scope + filters = [('name', 'eq:' + name), ('version', 'eq:' + version)] + if visibility == 'public': + filters.extend([('visibility', 'public')]) + elif visibility == 'private': + filters.extend([('owner', 'eq:' + owner), + ('visibility', 'private')]) + + scope_id = "%s:%s:%s" % (type_name, name, version) + if visibility != 'public': + scope_id += ':%s' % owner + lock = self.lock_engine.acquire(context, scope_id) + + try: + if len(self.list(context, type_name, filters)) > 0: + msg = _("Artifact with this name and version is already " + "exists for this scope.") + raise exception.Conflict(msg) + except Exception: + with excutils.save_and_reraise_exception(logger=LOG): + self.lock_engine.release(lock) + + return lock + + @staticmethod + def _show_artifact(ctx, type_name, artifact_id, read_only=False): """Return artifact requested by user. Check access permissions and policies. @@ -76,7 +108,7 @@ class Engine(object): """ artifact_type = registry.ArtifactRegistry.get_artifact_type(type_name) # only artifact is available for class users - af = artifact_type.get(ctx, artifact_id) + af = artifact_type.show(ctx, artifact_id) if not read_only: if not ctx.is_admin and ctx.tenant != af.owner or ctx.read_only: raise exception.Forbidden() @@ -96,8 +128,68 @@ class Engine(object): raise exception.NotFound(message=msg) return self.schemas[type_name] - @classmethod - def create(cls, context, type_name, values): + def _apply_patch(self, context, af, patch): + # This function is a collection of hacks and workarounds to make + # json patch apply changes to oslo_vo object. + action_names = {'artifact:update'} + af_dict = af.to_dict() + try: + for operation in patch._ops: + # apply the change to make sure that it's correct + af_dict = operation.apply(af_dict) + + # format of location is "/key/value" or just "/key" + # first case symbolizes that we have dict or list insertion, + # second, that we work with a field itself. + items = operation.location.split('/', 2) + field_name = items[1] + if af.is_blob(field_name) or af.is_blob_dict(field_name): + msg = _("Cannot add blob with this request. " + "Use special Blob API for that.") + raise exception.BadRequest(msg) + if len(items) == 2 and operation.operation['op'] == 'remove': + msg = _("Cannot remove field '%s' from " + "artifact.") % field_name + raise exception.BadRequest(msg) + + # work with hooks and define action names + if field_name == 'visibility': + utils.validate_visibility_transition( + af, + from_visibility=af.visibility, + to_visibility=af_dict['visibility'] + ) + if af_dict['visibility'] == 'public': + af.validate_publish(context, af) + action_names.add('artifact:publish') + elif field_name == 'status': + utils.validate_status_transition( + af, from_status=af.status, to_status=af_dict['status']) + if af_dict['status'] == 'deactivated': + action_names.add('artifact:deactivate') + elif af_dict['status'] == 'active': + if af.status == 'deactivated': + action_names.add('artifact:reactivate') + else: + af.validate_activate(context, af) + action_names.add('artifact:activate') + else: + utils.validate_change_allowed(af, field_name) + + old_val = getattr(af, field_name) + setattr(af, field_name, af_dict[field_name]) + new_val = getattr(af, field_name) + if new_val == old_val: + # No need to save value to db if it's not changed + af.obj_reset_changes([field_name]) + + except (jsonpatch.JsonPatchException, + jsonpatch.JsonPointerException, TypeError) as e: + raise exception.BadRequest(message=str(e)) + + return action_names + + def create(self, context, type_name, values): """Create artifact record in Glare. :param context: user context @@ -108,15 +200,33 @@ class Engine(object): action_name = "artifact:create" policy.authorize(action_name, values, context) artifact_type = registry.ArtifactRegistry.get_artifact_type(type_name) - # acquire version lock and execute artifact create - af = artifact_type.create(context, values) - # notify about new artifact - Notifier.notify(context, action_name, af) - # return artifact to the user - return af.to_dict() + version = values.get('version', artifact_type.DEFAULT_ARTIFACT_VERSION) + init_values = { + 'id': uuidutils.generate_uuid(), + 'name': values.pop('name'), + 'version': version, + 'owner': context.tenant, + 'created_at': timeutils.utcnow(), + 'updated_at': timeutils.utcnow() + } + af = artifact_type.init_artifact(context, init_values) + # acquire scoped lock and execute artifact create + with self._create_scoped_lock(context, type_name, af.name, + af.version, context.tenant): + for field_name, value in values.items(): + if af.is_blob(field_name) or af.is_blob_dict(field_name): + msg = _("Cannot add blob with this request. " + "Use special Blob API for that.") + raise exception.BadRequest(msg) + utils.validate_change_allowed(af, field_name) + setattr(af, field_name, value) + af = af.create(context) + # notify about new artifact + Notifier.notify(context, action_name, af) + # return artifact to the user + return af.to_dict() - @classmethod - def update(cls, context, type_name, artifact_id, patch): + def save(self, context, type_name, artifact_id, patch): """Update artifact with json patch. Apply patch to artifact and validate artifact before updating it @@ -129,55 +239,36 @@ class Engine(object): :param patch: json patch object :return: dict representation of updated artifact """ - - def _get_updates(af_dict, patch_with_upd): - """Get updated values for artifact and json patch. - - :param af_dict: current artifact definition as dict - :param patch_with_upd: json-patch object - :return: dict of updated attributes and their values - """ - try: - af_dict_patched = patch_with_upd.apply(af_dict) - diff = utils.DictDiffer(af_dict_patched, af_dict) - - # we mustn't add or remove attributes from artifact - if diff.added() or diff.removed(): - msg = _( - "Forbidden to add or remove attributes from artifact. " - "Added attributes %(added)s. " - "Removed attributes %(removed)s") % { - 'added': diff.added(), 'removed': diff.removed() - } - raise exception.BadRequest(message=msg) - - return {key: af_dict_patched[key] for key in diff.changed()} - - except (jsonpatch.JsonPatchException, - jsonpatch.JsonPointerException, - KeyError) as e: - raise exception.BadRequest(message=str(e)) - except TypeError as e: - msg = _("Incorrect type of the element. Reason: %s") % str(e) - raise exception.BadRequest(msg) lock_key = "%s:%s" % (type_name, artifact_id) - with base.BaseArtifact.lock_engine.acquire(context, lock_key): - af = cls._get_artifact(context, type_name, artifact_id) - af_dict = af.to_dict() - updates = _get_updates(af_dict, patch) + with self.lock_engine.acquire(context, lock_key): + af = self._show_artifact(context, type_name, artifact_id) + af.obj_reset_changes() + action_names = self._apply_patch(context, af, patch) + updates = af.obj_changes_to_primitive() + LOG.debug("Update diff successfully calculated for artifact " "%(af)s %(diff)s", {'af': artifact_id, 'diff': updates}) if not updates: - return af_dict - action = af.get_action_for_updates(context, af, updates) - action_name = "artifact:%s" % action.__name__ - policy.authorize(action_name, af_dict, context) - modified_af = action(context, af, updates) - Notifier.notify(context, action_name, modified_af) + return af.to_dict() + + for action_name in action_names: + policy.authorize(action_name, af.to_dict(), context) + + if any(i in updates for i in ('name', 'version', 'visibility')): + # to change an artifact scope it's required to set a lock first + with self._create_scoped_lock( + context, type_name, updates.get('name', af.name), + updates.get('version', af.version), af.owner, + updates.get('visibility', af.visibility)): + modified_af = af.save(context) + else: + modified_af = af.save(context) + + for action_name in action_names: + Notifier.notify(context, action_name, modified_af) return modified_af.to_dict() - @classmethod - def get(cls, context, type_name, artifact_id): + def show(self, context, type_name, artifact_id): """Show detailed artifact info. :param context: user context @@ -186,12 +277,12 @@ class Engine(object): :return: definition of requested artifact """ policy.authorize("artifact:get", {}, context) - af = cls._get_artifact(context, type_name, artifact_id, - read_only=True) + af = self._show_artifact(context, type_name, artifact_id, + read_only=True) return af.to_dict() - @classmethod - def list(cls, context, type_name, filters, marker=None, limit=None, + @staticmethod + def list(context, type_name, filters, marker=None, limit=None, sort=None, latest=False): """Return list of artifacts requested by user. @@ -215,21 +306,50 @@ class Engine(object): limit, sort, latest)] return af_list - @classmethod - def delete(cls, context, type_name, artifact_id): + @staticmethod + def _delete_blobs(context, af, blobs): + for name, blob in blobs.items(): + if af.is_blob(name): + if not blob['external']: + try: + store_api.delete_blob(blob['url'], context=context) + except exception.NotFound: + # data has already been removed + pass + af.db_api.update_blob(context, af.id, {name: None}) + elif af.is_blob_dict(name): + upd_blob = deepcopy(blob) + for key, val in blob.items(): + if not val['external']: + try: + store_api.delete_blob(val['url'], context=context) + except exception.NotFound: + pass + del upd_blob[key] + af.db_api.update_blob(context, af.id, {name: upd_blob}) + + def delete(self, context, type_name, artifact_id): """Delete artifact from Glare. :param context: User context :param type_name: Artifact type name :param artifact_id: id of artifact to delete """ - af = cls._get_artifact(context, type_name, artifact_id) - policy.authorize("artifact:delete", af.to_dict(), context) - af.delete(context, af) - Notifier.notify(context, "artifact.delete", af) + af = self._show_artifact(context, type_name, artifact_id) + action_name = 'artifact:delete' + policy.authorize(action_name, af.to_dict(), context) + af.validate_delete(context, af) + blobs = af.delete(context, af) + if not CONF.delayed_delete: + if blobs: + # delete blobs one by one + self._delete_blobs(context, af, blobs) + LOG.info("Blobs successfully deleted for artifact %s", af.id) + # delete artifact itself + af.db_api.delete(context, af.id) + Notifier.notify(context, action_name, af) - @classmethod - def add_blob_location(cls, context, type_name, artifact_id, field_name, + def add_blob_location(self, context, type_name, artifact_id, field_name, location, blob_meta, blob_key=None): """Add external location to blob. @@ -243,29 +363,21 @@ class Engine(object): in this dict :return: dict representation of updated artifact """ - af = cls._get_artifact(context, type_name, artifact_id) - action_name = 'artifact:set_location' - policy.authorize(action_name, af.to_dict(), context) - af.validate_upload_allowed(af, field_name, blob_key) - blob_name = "%s[%s]" % (field_name, blob_key)\ if blob_key else field_name - blob = {'url': location, 'size': None, 'md5': None, 'sha1': None, - 'sha256': None, 'status': glare_fields.BlobFieldType.ACTIVE, + blob = {'url': location, 'size': None, 'md5': blob_meta.get("md5"), + 'sha1': blob_meta.get("sha1"), 'id': uuidutils.generate_uuid(), + 'sha256': blob_meta.get("sha256"), 'status': 'active', 'external': True, 'content_type': None} - md5 = blob_meta.pop("md5", None) - if md5 is None: - msg = (_("Incorrect blob metadata %(meta)s. MD5 must be specified " - "for external location in artifact blob %(blob_name)."), - {"meta": str(blob_meta), "blob_name": blob_name}) - raise exception.BadRequest(msg) - else: - blob["md5"] = md5 - blob["sha1"] = blob_meta.pop("sha1", None) - blob["sha256"] = blob_meta.pop("sha256", None) - modified_af = cls.update_blob( - context, type_name, artifact_id, blob, field_name, blob_key) + + lock_key = "%s:%s" % (type_name, artifact_id) + with self.lock_engine.acquire(context, lock_key): + af = self._show_artifact(context, type_name, artifact_id) + action_name = 'artifact:set_location' + policy.authorize(action_name, af.to_dict(), context) + modified_af = self._init_blob( + context, af, blob, field_name, blob_key) LOG.info("External location %(location)s has been created " "successfully for artifact %(artifact)s blob %(blob)s", {'location': location, 'artifact': af.id, @@ -274,8 +386,7 @@ class Engine(object): Notifier.notify(context, action_name, modified_af) return modified_af.to_dict() - @classmethod - def upload_blob(cls, context, type_name, artifact_id, field_name, fd, + def upload_blob(self, context, type_name, artifact_id, field_name, fd, content_type, blob_key=None): """Upload Artifact blob. @@ -289,110 +400,125 @@ class Engine(object): in this dictionary :return: dict representation of updated artifact """ + + blob_name = "%s[%s]" % (field_name, blob_key) \ + if blob_key else field_name + blob_id = uuidutils.generate_uuid() + # create an an empty blob instance in db with 'saving' status + blob = {'url': None, 'size': None, 'md5': None, 'sha1': None, + 'sha256': None, 'id': blob_id, 'status': 'saving', + 'external': False, 'content_type': content_type} + + lock_key = "%s:%s" % (type_name, artifact_id) + with self.lock_engine.acquire(context, lock_key): + af = self._show_artifact(context, type_name, artifact_id) + action_name = "artifact:upload" + policy.authorize(action_name, af.to_dict(), context) + modified_af = self._init_blob( + context, af, blob, field_name, blob_key) + + LOG.debug("Parameters validation for artifact %(artifact)s blob " + "upload passed for blob %(blob_name)s. " + "Start blob uploading to backend.", + {'artifact': af.id, 'blob_name': blob_name}) + + # try to perform blob uploading to storage path = None - af = cls._get_artifact(context, type_name, artifact_id) - action_name = "artifact:upload" - policy.authorize(action_name, af.to_dict(), context) - af.validate_upload_allowed(af, field_name, blob_key) try: - # create an an empty blob instance in db with 'saving' status - blob = {'url': None, 'size': None, 'md5': None, 'sha1': None, - 'sha256': None, - 'status': glare_fields.BlobFieldType.SAVING, - 'external': False, 'content_type': content_type} - modified_af = cls.update_blob( - context, type_name, artifact_id, blob, field_name, blob_key) - - if blob_key is None: - blob_id = getattr(modified_af, field_name)['id'] - else: - blob_id = getattr(modified_af, field_name)[blob_key]['id'] - - # try to perform blob uploading to storage backend try: - try: - # call upload hook first - fd, path = af.validate_upload(context, af, field_name, fd) - except Exception as e: - raise exception.BadRequest(message=str(e)) + # call upload hook first + fd, path = af.validate_upload(context, af, field_name, fd) + except Exception as e: + raise exception.BadRequest(message=str(e)) - max_allowed_size = af.get_max_blob_size(field_name) - # Check if we wanna upload to a folder (and not just to a Blob) - if blob_key is not None: - blobs_dict = getattr(af, field_name) - overall_folder_size = sum( - blob["size"] for blob in blobs_dict.values() - if blob["size"] is not None) - max_folder_size_allowed_ = af.get_max_folder_size(field_name) \ - - overall_folder_size # always non-negative - max_allowed_size = min(max_allowed_size, - max_folder_size_allowed_) + max_allowed_size = af.get_max_blob_size(field_name) + # Check if we wanna upload to a folder (and not just to a Blob) + if blob_key is not None: + blobs_dict = getattr(af, field_name) + overall_folder_size = sum( + blob["size"] for blob in blobs_dict.values() + if blob["size"] is not None) + max_folder_size_allowed = af.get_max_folder_size(field_name) \ + - overall_folder_size # always non-negative + max_allowed_size = min(max_allowed_size, + max_folder_size_allowed) - default_store = af.get_default_store( - context, af, field_name, blob_key) - location_uri, size, checksums = store_api.save_blob_to_store( - blob_id, fd, context, max_allowed_size, - store_type=default_store) - except Exception: - # if upload failed remove blob from db and storage - with excutils.save_and_reraise_exception(logger=LOG): - if blob_key is None: - af.update_blob(context, af.id, field_name, None) - else: - blob_dict_attr = getattr(modified_af, field_name) - del blob_dict_attr[blob_key] - af.update_blob(context, af.id, - field_name, blob_dict_attr) - blob_name = "%s[%s]" % (field_name, blob_key) \ - if blob_key else field_name - LOG.info("Successfully finished blob upload for artifact " - "%(artifact)s blob field %(blob)s.", - {'artifact': af.id, 'blob': blob_name}) - - # update blob info and activate it - blob.update({'url': location_uri, - 'status': glare_fields.BlobFieldType.ACTIVE, - 'size': size}) - blob.update(checksums) - modified_af = cls.update_blob( - context, type_name, artifact_id, blob, field_name, blob_key) - - Notifier.notify(context, action_name, modified_af) - return modified_af.to_dict() + default_store = af.get_default_store( + context, af, field_name, blob_key) + location_uri, size, checksums = store_api.save_blob_to_store( + blob_id, fd, context, max_allowed_size, + store_type=default_store) + except Exception: + # if upload failed remove blob from db and storage + with excutils.save_and_reraise_exception(logger=LOG): + if blob_key is None: + af.update_blob(context, af.id, field_name, None) + else: + blob_dict_attr = getattr(modified_af, field_name) + del blob_dict_attr[blob_key] + af.update_blob(context, af.id, field_name, blob_dict_attr) finally: if path: os.remove(path) - @classmethod - def update_blob(cls, context, type_name, artifact_id, blob, - field_name, blob_key=None): - """Update blob info. + LOG.info("Successfully finished blob uploading for artifact " + "%(artifact)s blob field %(blob)s.", + {'artifact': af.id, 'blob': blob_name}) - :param context: user context - :param type_name: name of artifact type - :param artifact_id: id of the artifact to be updated - :param blob: blob representation in dict format - :param field_name: name of blob or blob dict field - :param blob_key: if field_name is blob dict it specifies key - in this dict + # update blob info and activate it + blob.update({'url': location_uri, + 'status': 'active', + 'size': size}) + blob.update(checksums) - :return: dict representation of updated artifact - """ - lock_key = "%s:%s" % (type_name, artifact_id) - with base.BaseArtifact.lock_engine.acquire(context, lock_key): - af = cls._get_artifact(context, type_name, artifact_id) - if blob_key is None: - setattr(af, field_name, blob) - return af.update_blob( - context, af.id, field_name, getattr(af, field_name)) + with self.lock_engine.acquire(context, lock_key): + af = af.show(context, artifact_id) + if blob_key: + field_value = getattr(af, field_name) + field_value[blob_key] = blob else: - blob_dict_attr = getattr(af, field_name) - blob_dict_attr[blob_key] = blob - return af.update_blob( - context, af.id, field_name, blob_dict_attr) + field_value = blob + modified_af = af.update_blob( + context, af.id, field_name, field_value) - @classmethod - def download_blob(cls, context, type_name, artifact_id, field_name, + Notifier.notify(context, action_name, modified_af) + return modified_af.to_dict() + + @staticmethod + def _init_blob(context, af, value, field_name, blob_key=None): + """Validate if given blob is ready for uploading. + + :param af: current artifact object + :param value: dict representation of the blob + :param field_name: blob or blob dict field name + :param blob_key: indicates key name if field_name is a blob dict + """ + if blob_key: + if not af.is_blob_dict(field_name): + msg = _("%s is not a blob dict") % field_name + raise exception.BadRequest(msg) + field_value = getattr(af, field_name) + if field_value.get(blob_key) is not None: + msg = (_("Cannot re-upload blob value to blob dict %(blob)s " + "with key %(key)s for artifact %(af)s") % + {'blob': field_name, 'key': blob_key, 'af': af.id}) + raise exception.Conflict(message=msg) + field_value[blob_key] = value + value = field_value + else: + if not af.is_blob(field_name): + msg = _("%s is not a blob") % field_name + raise exception.BadRequest(msg) + field_value = getattr(af, field_name, None) + if field_value is not None: + msg = _("Cannot re-upload blob %(blob)s for artifact " + "%(af)s") % {'blob': field_name, 'af': af.id} + raise exception.Conflict(message=msg) + utils.validate_change_allowed(af, field_name) + + return af.update_blob(context, af.id, field_name, value) + + def download_blob(self, context, type_name, artifact_id, field_name, blob_key=None): """Download binary data from Glare Artifact. @@ -404,8 +530,8 @@ class Engine(object): in this dict :return: file iterator for requested file """ - af = cls._get_artifact(context, type_name, artifact_id, - read_only=True) + af = self._show_artifact(context, type_name, artifact_id, + read_only=True) policy.authorize("artifact:download", af.to_dict(), context) blob_name = "%s[%s]" % (field_name, blob_key)\ @@ -419,12 +545,12 @@ class Engine(object): msg = _("%s is not a blob dict") % field_name raise exception.BadRequest(msg) - if af.status == af.STATUS.DEACTIVATED and not context.is_admin: + if af.status == 'deactivated' and not context.is_admin: msg = _("Only admin is allowed to download artifact data " "when it's deactivated") raise exception.Forbidden(message=msg) - if af.status == af.STATUS.DELETED: + if af.status == 'deleted': msg = _("Cannot download data when artifact is deleted") raise exception.Forbidden(message=msg) @@ -438,7 +564,7 @@ class Engine(object): msg = _("Blob with name %s is not found") % blob_name raise exception.NotFound(message=msg) - if blob is None or blob['status'] != glare_fields.BlobFieldType.ACTIVE: + if blob is None or blob['status'] != 'active': msg = _("%s is not ready for download") % blob_name raise exception.BadRequest(message=msg) @@ -455,14 +581,13 @@ class Engine(object): path = None try: - try: - # call download hook first - data, path = af.validate_download( - context, af, field_name, data) - except Exception as e: - raise exception.BadRequest(message=str(e)) - - return data, meta + # call download hook in the end + data, path = af.validate_download( + context, af, field_name, data) + except Exception as e: + raise exception.BadRequest(message=str(e)) finally: if path: os.remove(path) + + return data, meta diff --git a/glare/objects/all.py b/glare/objects/all.py index cf0fd61..0ad3465 100644 --- a/glare/objects/all.py +++ b/glare/objects/all.py @@ -31,37 +31,16 @@ class All(base.BaseArtifact): } @classmethod - def create(cls, context, values): + def create(cls, context): raise exception.Forbidden("This type is read only.") - @classmethod - def update(cls, context, af, values): - raise exception.Forbidden("This type is read only.") - - @classmethod - def get_action_for_updates(cls, context, artifact, updates): + def save(self, context): raise exception.Forbidden("This type is read only.") @classmethod def delete(cls, context, af): raise exception.Forbidden("This type is read only.") - @classmethod - def activate(cls, context, af, values): - raise exception.Forbidden("This type is read only.") - - @classmethod - def reactivate(cls, context, af, values): - raise exception.Forbidden("This type is read only.") - - @classmethod - def deactivate(cls, context, af, values): - raise exception.Forbidden("This type is read only.") - - @classmethod - def publish(cls, context, af, values): - raise exception.Forbidden("This type is read only.") - @classmethod def update_blob(cls, context, af_id, field_name, values): raise exception.Forbidden("This type is read only.") diff --git a/glare/objects/base.py b/glare/objects/base.py index 3998a11..f40b379 100644 --- a/glare/objects/base.py +++ b/glare/objects/base.py @@ -13,22 +13,15 @@ # License for the specific language governing permissions and limitations # under the License. -from copy import deepcopy - from oslo_config import cfg from oslo_log import log as logging -from oslo_utils import timeutils -from oslo_utils import uuidutils from oslo_versionedobjects import base from oslo_versionedobjects import fields -import six from glare.common import exception -from glare.common import store_api from glare.common import utils from glare.db import artifact_api from glare.i18n import _ -from glare import locking from glare.objects.meta import fields as glare_fields from glare.objects.meta import validators from glare.objects.meta import wrappers @@ -63,7 +56,7 @@ class BaseArtifact(base.VersionedObject): DEFAULT_ARTIFACT_VERSION = '0.0.0' - STATUS = glare_fields.ArtifactStatusField + STATUS = ('drafted', 'active', 'deactivated', 'deleted') Field = wrappers.Field.init DictField = wrappers.DictField.init @@ -82,9 +75,9 @@ class BaseArtifact(base.VersionedObject): required_on_activate=False, nullable=False, sortable=True, description="ID of user/tenant who " "uploaded artifact."), - 'status': Field(glare_fields.ArtifactStatusField, mutable=True, - default=glare_fields.ArtifactStatusField.DRAFTED, - nullable=False, sortable=True, + 'status': Field(fields.StringField, default='drafted', + nullable=False, sortable=True, mutable=True, + validators=[validators.AllowedValues(STATUS)], description="Artifact status."), 'created_at': Field(fields.DateTimeField, system=True, nullable=False, sortable=True, @@ -131,7 +124,6 @@ class BaseArtifact(base.VersionedObject): } db_api = artifact_api.ArtifactAPI() - lock_engine = locking.LockEngine(artifact_api.ArtifactLockApi()) @classmethod def is_blob(cls, field_name): @@ -154,7 +146,7 @@ class BaseArtifact(base.VersionedObject): glare_fields.BlobFieldType) @classmethod - def _init_artifact(cls, context, values): + def init_artifact(cls, context, values): """Initialize an empty versioned object with values. Initialize vo object with default values and values specified by user. @@ -189,185 +181,29 @@ class BaseArtifact(base.VersionedObject): """ raise NotImplementedError() - @classmethod - def _get_scoped_lock(cls, af, values): - """Create scope lock for artifact update. - - :param values: artifact values - :return: Lock object - """ - name = values.get('name', af.name) - version = values.get('version', af.version) - visibility = values.get('visibility', af.visibility) - scope_id = None - if (name, version, visibility) != (af.name, af.version, af.visibility): - # no version change == no lock for version - scope_id = "%s:%s:%s" % (cls.get_type_name(), name, str(version)) - if visibility != 'public': - scope_id += ':%s' % str(af.obj_context.tenant) - - return cls.lock_engine.acquire(af.obj_context, scope_id) - - @classmethod - def create(cls, context, values): + def create(self, context): """Create new artifact in Glare repo. :param context: user context - :param values: dictionary with specified artifact fields :return: created artifact object """ - name = values.get('name') - ver = str(values.setdefault('version', cls.DEFAULT_ARTIFACT_VERSION)) - scope_id = "%s:%s:%s" % (cls.get_type_name(), name, ver) - with cls.lock_engine.acquire(context, scope_id): - cls._validate_versioning(context, name, ver) - # validate other values - cls._validate_change_allowed(values) - # validate visibility - if 'visibility' in values: - msg = _("visibility is not allowed in a request " - "for artifact create.") - raise exception.BadRequest(msg) - values['id'] = uuidutils.generate_uuid() - values['owner'] = context.tenant - values['created_at'] = timeutils.utcnow() - values['updated_at'] = values['created_at'] - af = cls._init_artifact(context, values) - LOG.info("Parameters validation for artifact creation " - "passed for request %s.", context.request_id) - af_vals = cls.db_api.create( - context, af._obj_changes_to_primitive(), cls.get_type_name()) - return cls._init_artifact(context, af_vals) + values = self.obj_changes_to_primitive() + values['type_name'] = self.get_type_name() + af_vals = self.db_api.save(context, None, values) + return self.init_artifact(context, af_vals) - @classmethod - def _validate_versioning(cls, context, name, version, is_public=False): - """Validate if artifact with given name and version already exists. + def save(self, context): + """Save artifact in Glare repo. :param context: user context - :param name: name of artifact to be checked - :param version: version of artifact - :param is_public: flag that indicates to search artifact globally - """ - if version is not None and name not in (None, ""): - filters = [('name', 'eq:' + name), ('version', 'eq:' + version)] - if is_public is False: - filters.extend([('owner', 'eq:' + context.tenant), - ('visibility', 'private')]) - else: - filters.extend([('visibility', 'public')]) - if len(cls.list(context, filters)) > 0: - msg = _("Artifact with this name and version already " - "exists for this owner.") - raise exception.Conflict(msg) - else: - msg = _("Cannot set artifact version without name and version.") - raise exception.BadRequest(msg) - - @classmethod - def _validate_change_allowed(cls, field_names, af=None, - validate_blob_names=True): - """Validate if fields can be updated in artifact.""" - af_status = cls.STATUS.DRAFTED if af is None else af.status - if af_status not in (cls.STATUS.ACTIVE, cls.STATUS.DRAFTED): - msg = _("Forbidden to change fields " - "if artifact is not active or drafted.") - raise exception.Forbidden(message=msg) - - for field_name in field_names: - if field_name not in cls.fields: - msg = _("%s field does not exist") % field_name - raise exception.BadRequest(msg) - field = cls.fields[field_name] - if field.system is True: - msg = _("Cannot specify system field %s. It is not " - "available for modifying by users.") % field_name - raise exception.Forbidden(msg) - if af_status == cls.STATUS.ACTIVE and not field.mutable: - msg = (_("Forbidden to change field '%s' after activation.") - % field_name) - raise exception.Forbidden(message=msg) - if validate_blob_names and \ - (cls.is_blob(field_name) or cls.is_blob_dict(field_name)): - msg = _("Cannot add blob %s with this request. " - "Use special Blob API for that.") % field_name - raise exception.BadRequest(msg) - - @classmethod - def update(cls, context, af, values): - """Update artifact in Glare repo. - - :param context: user context - :param af: current definition of artifact - :param values: dictionary with changes for artifact :return: updated artifact object """ - # reset all changes of artifact to reuse them after update - af.obj_reset_changes() - with cls._get_scoped_lock(af, values): - # validate version - if 'name' in values or 'version' in values: - new_name = values.get('name') if 'name' in values else af.name - if not isinstance(new_name, six.string_types): - new_name = str(new_name) - new_version = values.get('version') \ - if 'version' in values else af.version - if not isinstance(new_version, six.string_types): - new_version = str(new_version) - cls._validate_versioning(context, new_name, new_version) - - # validate other values - cls._validate_change_allowed(values, af) - # apply values to the artifact. if all changes applied then update - # values in db or raise an exception in other case. - for key, value in values.items(): - setattr(af, key, value) - - LOG.info("Parameters validation for artifact %(artifact)s " - "update passed for request %(request)s.", - {'artifact': af.id, 'request': context.request_id}) - updated_af = cls.db_api.update( - context, af.id, af._obj_changes_to_primitive()) - return cls._init_artifact(context, updated_af) + updated_af = self.db_api.save(context, self.id, + self.obj_changes_to_primitive()) + return self.init_artifact(context, updated_af) @classmethod - def get_action_for_updates(cls, context, af, values): - """Define the appropriate method for artifact update. - - Based on update params this method defines what action engine should - call for artifact update: activate, deactivate, reactivate, publish or - just a regular update of artifact fields. - - :param context: user context - :param af: current definition of artifact - :param values: dictionary with changes for artifact - :return: method reference for updates dict - """ - action = cls.update - if 'visibility' in values: - # validate publish action format - action = cls.publish - elif 'status' in values: - status = values['status'] - if status == cls.STATUS.DEACTIVATED: - action = cls.deactivate - elif status == cls.STATUS.ACTIVE: - if af.status == af.STATUS.DEACTIVATED: - action = cls.reactivate - else: - action = cls.activate - else: - msg = (_("Incorrect status value. You may specify only %s " - "statuses.") % ' and '.join( - [af.STATUS.ACTIVE, af.STATUS.DEACTIVATED])) - raise exception.BadRequest(message=msg) - - LOG.debug("Action %(action)s defined to updates %(updates)s.", - {'action': action.__name__, 'updates': values}) - - return action - - @classmethod - def get(cls, context, artifact_id): + def show(cls, context, artifact_id): """Return Artifact from Glare repo :param context: user context @@ -375,7 +211,7 @@ class BaseArtifact(base.VersionedObject): :return: requested artifact object """ af = cls.db_api.get(context, artifact_id) - return cls._init_artifact(context, af) + return cls.init_artifact(context, af) @classmethod def _get_field_type(cls, obj): @@ -510,7 +346,7 @@ class BaseArtifact(base.VersionedObject): sort.append(default_sort) default_filter_parameters = [ - ('status', None, 'neq', None, cls.STATUS.DELETED)] + ('status', None, 'neq', None, 'deleted')] if cls.get_type_name() != 'all': default_filter_parameters.append( ('type_name', None, 'eq', None, cls.get_type_name())) @@ -520,32 +356,10 @@ class BaseArtifact(base.VersionedObject): if default_filter not in filters: filters.append(default_filter) - return [cls._init_artifact(context, af) + return [cls.init_artifact(context, af) for af in cls.db_api.list( context, filters, marker, limit, sort, latest)] - @classmethod - def _delete_blobs(cls, blobs, context, af): - for name, blob in blobs.items(): - if cls.is_blob(name): - if not blob['external']: - try: - store_api.delete_blob(blob['url'], context=context) - except exception.NotFound: - # data has already been removed - pass - cls.db_api.update_blob(context, af.id, {name: None}) - elif cls.is_blob_dict(name): - upd_blob = deepcopy(blob) - for key, val in blob.items(): - if not val['external']: - try: - store_api.delete_blob(val['url'], context=context) - except exception.NotFound: - pass - del upd_blob[key] - cls.db_api.update_blob(context, af.id, {name: upd_blob}) - @classmethod def delete(cls, context, af): """Delete artifact and all its blobs from Glare. @@ -553,9 +367,8 @@ class BaseArtifact(base.VersionedObject): :param context: user context :param af: artifact object targeted for deletion """ - cls.validate_delete(context, af) # marking artifact as deleted - cls.db_api.update(context, af.id, {'status': cls.STATUS.DELETED}) + cls.db_api.save(context, af.id, {'status': 'deleted'}) # collect all uploaded blobs blobs = {} @@ -568,124 +381,7 @@ class BaseArtifact(base.VersionedObject): LOG.debug("Marked artifact %(artifact)s as deleted.", {'artifact': af.id}) - if not CONF.delayed_delete: - if blobs: - # delete blobs one by one - cls._delete_blobs(blobs, context, af) - LOG.info("Blobs successfully deleted for artifact %s", af.id) - # delete artifact itself - cls.db_api.delete(context, af.id) - - @classmethod - def activate(cls, context, af, values): - """Activate artifact and make it available for usage. - - :param context: user context - :param af: current artifact object - :param values: dictionary with changes for artifact - :return: artifact object with changed status - """ - # validate that came to artifact as updates - if values != {'status': cls.STATUS.ACTIVE}: - msg = _("Only {'status': %s} is allowed in a request " - "for activation.") % cls.STATUS.ACTIVE - raise exception.BadRequest(msg) - - for name, type_obj in af.fields.items(): - if type_obj.required_on_activate and getattr(af, name) is None: - msg = _( - "'%s' field value must be set before activation") % name - raise exception.BadRequest(msg) - - cls.validate_activate(context, af) - if af.status != cls.STATUS.DRAFTED: - raise exception.InvalidStatusTransition( - orig=af.status, new=cls.STATUS.ACTIVE - ) - LOG.info("Parameters validation for artifact %(artifact)s " - "activate passed for request %(request)s.", - {'artifact': af.id, 'request': context.request_id}) - af = cls.db_api.update(context, af.id, {'status': cls.STATUS.ACTIVE}) - return cls._init_artifact(context, af) - - @classmethod - def reactivate(cls, context, af, values): - """Make Artifact active after deactivation - - :param context: user context - :param af: current artifact object - :param values: dictionary with changes for artifact - :return: artifact object with changed status - """ - # validate that came to artifact as updates - if values != {'status': cls.STATUS.ACTIVE}: - msg = _("Only {'status': %s} is allowed in a request " - "for reactivation.") % cls.STATUS.ACTIVE - raise exception.BadRequest(msg) - LOG.info("Parameters validation for artifact %(artifact)s " - "reactivate passed for request %(request)s.", - {'artifact': af.id, 'request': context.request_id}) - af = cls.db_api.update(context, af.id, {'status': cls.STATUS.ACTIVE}) - return cls._init_artifact(context, af) - - @classmethod - def deactivate(cls, context, af, values): - """Deny Artifact downloading due to security concerns. - - If user uploaded suspicious artifact then administrators(or other - users - it depends on policy configurations) can deny artifact data - to be downloaded by regular users by making artifact deactivated. - After additional investigation artifact can be reactivated or - deleted from Glare. - - :param context: user context - :param af: current artifact object - :param values: dictionary with changes for artifact - :return: artifact object with changed status - """ - if values != {'status': cls.STATUS.DEACTIVATED}: - msg = _("Only {'status': %s} is allowed in a request " - "for deactivation.") % cls.STATUS.DEACTIVATED - raise exception.BadRequest(msg) - - if af.status != cls.STATUS.ACTIVE: - raise exception.InvalidStatusTransition( - orig=af.status, new=cls.STATUS.ACTIVE - ) - LOG.info("Parameters validation for artifact %(artifact)s " - "deactivate passed for request %(request)s.", - {'artifact': af.id, 'request': context.request_id}) - af = cls.db_api.update(context, af.id, - {'status': cls.STATUS.DEACTIVATED}) - return cls._init_artifact(context, af) - - @classmethod - def publish(cls, context, af, values): - """Make artifact available for all tenants. - - :param context: user context - :param af: current artifact object - :param values: dictionary with changes for artifact - :return: artifact object with changed visibility - """ - if values != {'visibility': 'public'}: - msg = _("Only {'visibility': 'public'} is allowed in a request " - "for artifact publish.") - raise exception.BadRequest(msg) - - with cls._get_scoped_lock(af, values): - if af.status != cls.STATUS.ACTIVE: - msg = _("Cannot publish non-active artifact") - raise exception.BadRequest(msg) - - cls._validate_versioning(context, af.name, af.version, - is_public=True) - cls.validate_publish(context, af) - LOG.info("Parameters validation for artifact %(artifact)s " - "publish passed for request %(request)s.", - {'artifact': af.id, 'request': context.request_id}) - af = cls.db_api.update(context, af.id, {'visibility': 'public'}) - return cls._init_artifact(context, af) + return blobs @classmethod def get_max_blob_size(cls, field_name): @@ -705,42 +401,6 @@ class BaseArtifact(base.VersionedObject): """ return getattr(cls.fields[field_name], 'max_folder_size') - @classmethod - def validate_upload_allowed(cls, af, field_name, blob_key=None): - """Validate if given blob is ready for uploading. - - :param af: current artifact object - :param field_name: blob or blob dict field name - :param blob_key: indicates key name if field_name is a blob dict - """ - - blob_name = "%s[%s]" % (field_name, blob_key)\ - if blob_key else field_name - - cls._validate_change_allowed([field_name], af, - validate_blob_names=False) - if blob_key: - if not cls.is_blob_dict(field_name): - msg = _("%s is not a blob dict") % field_name - raise exception.BadRequest(msg) - if getattr(af, field_name).get(blob_key) is not None: - msg = (_("Cannot re-upload blob value to blob dict %(blob)s " - "with key %(key)s for artifact %(af)s") % - {'blob': field_name, 'key': blob_key, 'af': af.id}) - raise exception.Conflict(message=msg) - else: - if not cls.is_blob(field_name): - msg = _("%s is not a blob") % field_name - raise exception.BadRequest(msg) - if getattr(af, field_name) is not None: - msg = _("Cannot re-upload blob %(blob)s for artifact " - "%(af)s") % {'blob': field_name, 'af': af.id} - raise exception.Conflict(message=msg) - LOG.debug("Parameters validation for artifact %(artifact)s blob " - "upload passed for blob %(blob_name)s. " - "Start blob uploading to backend.", - {'artifact': af.id, 'blob_name': blob_name}) - @classmethod def update_blob(cls, context, af_id, field_name, values): """Update blob info in database. @@ -752,10 +412,10 @@ class BaseArtifact(base.VersionedObject): :return: updated artifact definition in Glare """ af_upd = cls.db_api.update_blob(context, af_id, {field_name: values}) - return cls._init_artifact(context, af_upd) + return cls.init_artifact(context, af_upd) @classmethod - def validate_activate(cls, context, af, values=None): + def validate_activate(cls, context, af): """Validation hook for activation.""" pass @@ -814,7 +474,7 @@ class BaseArtifact(base.VersionedObject): """ return self.obj_to_primitive()['versioned_object.data'] - def _obj_changes_to_primitive(self): + def obj_changes_to_primitive(self): changes = self.obj_get_changes() res = {} for key, val in changes.items(): @@ -894,8 +554,7 @@ class BaseArtifact(base.VersionedObject): schema['format'] = 'date-time' if field_name == 'status': - schema['enum'] = list( - glare_fields.ArtifactStatusField.ARTIFACT_STATUS) + schema['enum'] = cls.STATUS if field.description: schema['description'] = field.description diff --git a/glare/objects/meta/fields.py b/glare/objects/meta/fields.py index 79b3faf..bd333e0 100644 --- a/glare/objects/meta/fields.py +++ b/glare/objects/meta/fields.py @@ -12,10 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. - import jsonschema from jsonschema import exceptions as json_exceptions -from oslo_utils import uuidutils from oslo_versionedobjects import fields import semantic_version import six @@ -26,22 +24,6 @@ from glare.common import exception from glare.i18n import _ -class ArtifactStatusField(fields.StateMachine): - ARTIFACT_STATUS = (DRAFTED, ACTIVE, DEACTIVATED, DELETED) = ( - 'drafted', 'active', 'deactivated', 'deleted') - - ALLOWED_TRANSITIONS = { - DRAFTED: {DRAFTED, ACTIVE, DELETED}, - ACTIVE: {ACTIVE, DEACTIVATED, DELETED}, - DEACTIVATED: {DEACTIVATED, ACTIVE, DELETED}, - DELETED: {DELETED} - } - - def __init__(self, **kwargs): - super(ArtifactStatusField, self).__init__(self.ARTIFACT_STATUS, - **kwargs) - - class Version(fields.FieldType): @staticmethod @@ -83,7 +65,6 @@ class BlobFieldType(fields.FieldType): if not isinstance(value, dict): raise ValueError(_("Blob value must be dict. Got %s type instead") % type(value)) - value.setdefault('id', uuidutils.generate_uuid()) try: jsonschema.validate(value, BlobFieldType.BLOB_SCHEMA) except json_exceptions.ValidationError as e: diff --git a/glare/objects/meta/file_utils.py b/glare/objects/meta/file_utils.py index 9f570d6..2df8059 100644 --- a/glare/objects/meta/file_utils.py +++ b/glare/objects/meta/file_utils.py @@ -22,6 +22,7 @@ import zipfile from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils +from oslo_utils import uuidutils from glare.common import store_api from glare.objects.meta import fields as glare_fields @@ -69,16 +70,15 @@ def upload_content_file(context, af, data, blob_dict, key_name, :param key_name: name of key in the dictionary :param content_type: (optional) specifies mime type of uploading data """ + blob_id = uuidutils.generate_uuid() # create an an empty blob instance in db with 'saving' status blob = {'url': None, 'size': None, 'md5': None, 'sha1': None, 'sha256': None, 'status': glare_fields.BlobFieldType.SAVING, - 'external': False, 'content_type': content_type} + 'external': False, 'content_type': content_type, 'id': blob_id} getattr(af, blob_dict)[key_name] = blob af = af.update_blob(context, af.id, blob_dict, getattr(af, blob_dict)) - blob_id = getattr(af, blob_dict)[key_name]['id'] - # try to perform blob uploading to storage backend try: default_store = af.get_default_store(context, af, blob_dict, key_name) diff --git a/glare/objects/meta/wrappers.py b/glare/objects/meta/wrappers.py index 2fcd794..59463a2 100644 --- a/glare/objects/meta/wrappers.py +++ b/glare/objects/meta/wrappers.py @@ -103,8 +103,7 @@ class Field(object): @staticmethod def get_allowed_filter_ops(field): - if field in (fields.StringField, fields.String, - glare_fields.ArtifactStatusField): + if field in (fields.StringField, fields.String): return [FILTER_EQ, FILTER_NEQ, FILTER_IN] elif field in (fields.IntegerField, fields.Integer, fields.FloatField, fields.Float, glare_fields.VersionField): diff --git a/glare/tests/functional/test_sample_artifact.py b/glare/tests/functional/test_sample_artifact.py index c85cb9e..1038586 100644 --- a/glare/tests/functional/test_sample_artifact.py +++ b/glare/tests/functional/test_sample_artifact.py @@ -1052,25 +1052,23 @@ class TestArtifactOps(base.TestArtifact): "version": "0.0.1"}) # cannot activate artifact without required for activate attributes url = '/sample_artifact/%s' % private_art['id'] - self.patch(url=url, data=self.make_active, status=400) + self.patch(url=url, data=self.make_active, status=403) add_required = [{ "op": "replace", "path": "/string_required", "value": "string" }] self.patch(url=url, data=add_required) - # cannot activate if body contains non status changes - incorrect = self.make_active + [{"op": "replace", - "path": "/name", - "value": "test"}] - self.patch(url=url, data=incorrect, status=400) - # can activate if body contains only status changes - make_active_without_updates = self.make_active + add_required - active_art = self.patch(url=url, data=make_active_without_updates) + # can activate if body contains non status changes + make_active_with_updates = self.make_active + [{"op": "replace", + "path": "/description", + "value": "test"}] + active_art = self.patch(url=url, data=make_active_with_updates) private_art['status'] = 'active' private_art['activated_at'] = active_art['activated_at'] private_art['updated_at'] = active_art['updated_at'] private_art['string_required'] = 'string' + private_art['description'] = 'test' self.assertEqual(private_art, active_art) # check that active artifact is not available for other user self.set_user("user2") @@ -1091,19 +1089,31 @@ class TestArtifactOps(base.TestArtifact): "version": "0.0.1"}) url = '/sample_artifact/%s' % private_art['id'] + # test that we cannot publish drafted artifact + self.patch(url=url, data=self.make_public, status=403) + self.patch(url=url, data=self.make_active) - # test that only visibility must be specified in the request - incorrect = self.make_public + [{"op": "replace", - "path": "/string_mutable", - "value": "test"}] - self.patch(url=url, data=incorrect, status=400) + # test that cannot publish deactivated artifact + self.patch(url, data=self.make_deactivated) + self.patch(url, data=self.make_public, status=403) + + self.patch(url=url, data=self.make_active) + + # test that visibility can be specified in the request with + # other updates + make_public_with_updates = self.make_public + [ + {"op": "replace", + "path": "/string_mutable", + "value": "test"}] + self.patch(url=url, data=make_public_with_updates) # check public artifact public_art = self.patch(url=url, data=self.make_public) private_art['activated_at'] = public_art['activated_at'] private_art['visibility'] = 'public' private_art['status'] = 'active' private_art['updated_at'] = public_art['updated_at'] + private_art['string_mutable'] = 'test' self.assertEqual(private_art, public_art) # check that public artifact available for simple user self.set_user("user1") @@ -1114,14 +1124,9 @@ class TestArtifactOps(base.TestArtifact): data={"name": "test_af", "string_required": "test_str", "version": "0.0.1"}) dup_url = '/sample_artifact/%s' % duplicate_art['id'] - # test that we cannot publish drafted artifact - self.patch(url=dup_url, data=self.make_public, status=400) # proceed with duplicate testing self.patch(url=dup_url, data=self.make_active) self.patch(url=dup_url, data=self.make_public, status=409) - # test that cannot publish deactivated artifact - self.patch(dup_url, data=self.make_deactivated) - self.patch(dup_url, data=self.make_public, status=400) def test_delete(self): # try ro delete not existing artifact @@ -1189,20 +1194,19 @@ class TestArtifactOps(base.TestArtifact): data={"name": "test_af", "string_required": "test_str", "version": "0.0.1"}) url = '/sample_artifact/%s' % private_art['id'] - self.admin_action(private_art['id'], self.make_deactivated, - status=400) + self.admin_action(private_art['id'], self.make_deactivated, 403) self.patch(url, self.make_active) self.set_user('admin') - # test cannot deactivate if there is something else in request - incorrect = self.make_deactivated + [{"op": "replace", - "path": "/name", - "value": "test"}] - self.patch(url, incorrect, 400) - self.set_user('user1') + # test can deactivate if there is something else in request + make_deactived_with_updates = [ + {"op": "replace", + "path": "/description", + "value": "test"}] + self.make_deactivated # test artifact deactivate success - deactive_art = self.admin_action(private_art['id'], - self.make_deactivated) - self.assertEqual("deactivated", deactive_art["status"]) + deactivated_art = self.admin_action( + private_art['id'], make_deactived_with_updates) + self.assertEqual("deactivated", deactivated_art["status"]) + self.assertEqual("test", deactivated_art["description"]) # test deactivate is idempotent self.patch(url, self.make_deactivated) @@ -1214,14 +1218,16 @@ class TestArtifactOps(base.TestArtifact): url = '/sample_artifact/%s' % private_art['id'] self.patch(url, self.make_active) self.admin_action(private_art['id'], self.make_deactivated) - # test cannot reactivate if there is something else in request - incorrect = self.make_active + [{"op": "replace", - "path": "/name", - "value": "test"}] - self.patch(url, incorrect, 400) - # test artifact reactivate success - deactive_art = self.patch(url, self.make_active) - self.assertEqual("active", deactive_art["status"]) + # test can reactivate if there is something else in request + make_reactived_with_updates = self.make_active + [ + {"op": "replace", + "path": "/description", + "value": "test"}] + # test artifact deactivate success + reactivated_art = self.admin_action( + private_art['id'], make_reactived_with_updates) + self.assertEqual("active", reactivated_art["status"]) + self.assertEqual("test", reactivated_art["description"]) class TestUpdate(base.TestArtifact): @@ -1379,7 +1385,7 @@ class TestUpdate(base.TestArtifact): self.admin_action(private_art['id'], self.make_deactivated) # test that nobody(even admin) can publish deactivated artifact self.set_user("admin") - self.patch(url, self.make_public, 400) + self.patch(url, self.make_public, 403) self.set_user("user1") self.patch(url, upd_mutable, 403) self.admin_action(private_art['id'], self.make_active) @@ -2138,6 +2144,78 @@ class TestUpdate(base.TestArtifact): url = '/sample_artifact/%s' % art1['id'] self.patch(url=url, data=data, status=400) + def test_update_malformed_json_patch(self): + data = {'name': 'ttt'} + art1 = self.create_artifact(data=data) + + data = [{'op': 'replace', 'path': None, 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': '/', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': '//', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': 'name/', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': '*/*', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'add', 'path': None, 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'add', 'path': '/', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'add', 'path': '//', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'add', 'path': 'name/', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'add', 'path': '*/*', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'add', 'path': '/name'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': None}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': '/'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': '//'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': 'name/'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'replace', 'path': '*/*'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + + data = [{'op': 'no-op', 'path': '/name', 'value': 'aaa'}] + url = '/sample_artifact/%s' % art1['id'] + self.patch(url=url, data=data, status=400) + class TestLinks(base.TestArtifact): def test_manage_links(self): diff --git a/glare/tests/functional/test_schemas.py b/glare/tests/functional/test_schemas.py index 486052e..5076bd4 100644 --- a/glare/tests/functional/test_schemas.py +++ b/glare/tests/functional/test_schemas.py @@ -15,7 +15,6 @@ import jsonschema -from glare.common import utils from glare.tests.functional import base fixture_base_props = { @@ -939,15 +938,11 @@ class TestSchemas(base.TestArtifact): # Get schemas for specific artifact type for at in self.enabled_types: result = self.get(url='/schemas/%s' % at) - self.assertEqual(fixtures[at], result['schemas'][at], - utils.DictDiffer( - fixtures[at]['properties'], - result['schemas'][at]['properties'])) + self.assertEqual(fixtures[at], result['schemas'][at]) # Get list schemas of artifacts result = self.get(url='/schemas') - self.assertEqual(fixtures, result['schemas'], utils.DictDiffer( - fixtures, result['schemas'])) + self.assertEqual(fixtures, result['schemas']) # Validation of schemas result = self.get(url='/schemas')['schemas'] diff --git a/glare/tests/unit/api/test_delete.py b/glare/tests/unit/api/test_delete.py index cacd343..d23ad56 100644 --- a/glare/tests/unit/api/test_delete.py +++ b/glare/tests/unit/api/test_delete.py @@ -101,7 +101,7 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): def test_delete_deleted_artifact(self): # Change status of the artifact to 'deleted' - artifact_api.ArtifactAPI().update( + artifact_api.ArtifactAPI().save( self.req.context, self.artifact['id'], {'status': 'deleted'}) # Delete should work properly self.controller.delete(self.req, 'sample_artifact', diff --git a/glare/tests/unit/api/test_locations.py b/glare/tests/unit/api/test_locations.py index 4b0114d..bc2b1da 100644 --- a/glare/tests/unit/api/test_locations.py +++ b/glare/tests/unit/api/test_locations.py @@ -86,14 +86,6 @@ class TestLocations(base.BaseTestArtifactAPI): self.req, 'sample_artifact', self.sample_artifact['id'], 'dict_of_blobs/blob', body, self.ct) - def test_add_location_no_md5(self): - body = {'url': 'https://FAKE_LOCATION.com', - "sha1": "fake_sha", "sha256": "fake_sha256"} - self.assertRaises( - exc.BadRequest, self.controller.upload_blob, - self.req, 'sample_artifact', self.sample_artifact['id'], - 'dict_of_blobs/blob', body, self.ct) - def test_add_location_saving_blob(self): body = {'url': 'https://FAKE_LOCATION.com', 'md5': "fake", 'sha1': "fake_sha", "sha256": "fake_sha256"} diff --git a/glare/tests/unit/api/test_update.py b/glare/tests/unit/api/test_update.py index 0bb1ab3..e379eee 100644 --- a/glare/tests/unit/api/test_update.py +++ b/glare/tests/unit/api/test_update.py @@ -284,6 +284,12 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): changes = [{'op': 'replace', 'path': '/', 'value': 'a'}] self.update_with_values(changes, exc_class=exc.BadRequest) + changes = [{'op': 'add', 'path': '/wrong_field', 'value': 'a'}] + self.update_with_values(changes, exc_class=exc.BadRequest) + + changes = [{'op': 'add', '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) @@ -291,6 +297,21 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): changes = [{'op': 'remove', 'path': '/list_of_int/10'}] self.update_with_values(changes, exc_class=exc.BadRequest) + changes = [{'op': 'remove', 'path': '/status'}] + self.update_with_values(changes, exc_class=exc.BadRequest) + + changes = [ + {'op': 'add', 'path': '/list_of_int/-', 'value': 4}, + {'op': 'add', 'path': '/dict_of_str/k', 'value': 'new_val'}, + ] + self.update_with_values(changes) + changes = [{'op': 'remove', 'path': '/list_of_int/0'}] + res = self.update_with_values(changes) + self.assertEqual([], res['list_of_int']) + changes = [{'op': 'remove', 'path': '/dict_of_str/k'}] + res = self.update_with_values(changes) + self.assertEqual({}, res['dict_of_str']) + def test_update_artifact_blob(self): changes = [{'op': 'replace', 'path': '/blob', 'value': 'a'}] self.update_with_values(changes, exc_class=exc.BadRequest) @@ -328,20 +349,19 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): changes = [{'op': 'replace', 'path': '/visibility', 'value': 'public'}] - self.update_with_values(changes, exc_class=exc.BadRequest) + self.update_with_values(changes, exc_class=exc.Forbidden) changes = [{'op': 'replace', 'path': '/visibility', 'value': None}] 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']) - - changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}] + 'value': 'some_string'}, + {'op': 'replace', 'path': '/status', + 'value': 'active'}] res = self.update_with_values(changes) self.assertEqual('active', res['status']) + self.assertEqual('some_string', res['string_required']) changes = [{'op': 'replace', 'path': '/visibility', 'value': 'public'}] res = self.update_with_values(changes) @@ -353,7 +373,7 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): changes = [{'op': 'replace', 'path': '/visibility', 'value': 'private'}] - self.update_with_values(changes, exc_class=exc.BadRequest) + self.update_with_values(changes, exc_class=exc.Forbidden) def test_update_artifact_status(self): self.req = self.get_fake_request(user=self.users['admin']) @@ -366,7 +386,7 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): # 'string_required' is set changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}] - self.update_with_values(changes, exc_class=exc.BadRequest) + self.update_with_values(changes, exc_class=exc.Forbidden) changes = [{'op': 'replace', 'path': '/status', 'value': None}] @@ -375,21 +395,13 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): # It's forbidden to deactivate drafted artifact changes = [{'op': 'replace', 'path': '/status', 'value': 'deactivated'}] - self.update_with_values(changes, exc_class=exc.BadRequest) + self.update_with_values(changes, exc_class=exc.Forbidden) 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', @@ -419,39 +431,44 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): 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']) - - changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}] - 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) - self.assertEqual('deactivated', res['status']) - - changes = [{'op': 'replace', 'path': '/status', - 'value': 'deactivated'}] - res = self.update_with_values(changes) - self.assertEqual('deactivated', res['status']) - - # It's forbidden to change artifact status with other fields in + # It's possible 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) + self.sample_artifact = self.update_with_values(changes) + self.assertEqual('new_name', self.sample_artifact['name']) + self.assertEqual('active', self.sample_artifact['status']) + + changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}] + res = self.update_with_values(changes) + self.assertEqual('active', res['status']) + + # It's possible 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.sample_artifact = self.update_with_values(changes) + self.assertEqual('str', self.sample_artifact['string_mutable']) + self.assertEqual('deactivated', self.sample_artifact['status']) + + changes = [{'op': 'replace', 'path': '/status', + 'value': 'deactivated'}] + res = self.update_with_values(changes) + self.assertEqual('deactivated', res['status']) + + # It's possible to change artifact status with other fields in + # one request + changes = [ + {'op': 'replace', 'path': '/status', 'value': 'active'}, + {'op': 'replace', 'path': '/description', 'value': 'test'}, + ] + self.sample_artifact = self.update_with_values(changes) + self.assertEqual('test', self.sample_artifact['description']) + self.assertEqual('active', self.sample_artifact['status']) changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}] res = self.update_with_values(changes) @@ -471,7 +488,7 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI): self.assertEqual('deleted', art['status']) changes = [{'op': 'replace', 'path': '/status', 'value': 'active'}] - self.assertRaises(exc.InvalidStatusTransition, + self.assertRaises(exc.Forbidden, self.update_with_values, changes) def test_update_artifact_mutable_fields(self): diff --git a/glare/tests/unit/test_utils.py b/glare/tests/unit/test_utils.py index aa836fc..6c7bbbb 100644 --- a/glare/tests/unit/test_utils.py +++ b/glare/tests/unit/test_utils.py @@ -65,21 +65,6 @@ class TestUtils(base.BaseTestCase): self.assertRaises(exc.BadRequest, test_func, **{'param': bad_char}) - def test_str_repr(self): - past_dict = {"a": 1, "b": 2, "d": 4} - current_dic = {"b": 2, "d": "different value!", "e": "new!"} - dict_diff = utils.DictDiffer(current_dic, past_dict) - - self.assertEqual({'a'}, dict_diff.removed()) - self.assertEqual({'b'}, dict_diff.unchanged()) - self.assertEqual({'d'}, dict_diff.changed()) - self.assertEqual({'e'}, dict_diff.added()) - - expected_dict_str = "\nResult output:\n\tAdded keys: " \ - "e\n\tRemoved keys:" \ - " a\n\tChanged keys: d\n\tUnchanged keys: b\n" - self.assertEqual(str(dict_diff), expected_dict_str) - class TestReaders(base.BaseTestCase): """Test various readers in glare.common.utils"""