Massive refactoring

All the tricks are done by professionals.
Please, don't try this at home!

Change-Id: I9000065fde57043a4e0de3a409505cf66ac3360d

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

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 ")
 

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)

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):
+    def save(self, context, artifact_id, values):
-        """Create new artifact in db and return dict of values to the user
+        """Save artifact values in database
-
-        :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
 
         :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,
+        return api.create_or_update(
-                          self._serialize_values(values), session)
+            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,
+        return api.create_or_update(
-                          {'blobs': values}, session)
+            context, artifact_id, {'blobs': values}, session)
 
     @retry(retry_on_exception=_retry_on_connection_error, wait_fixed=1000,
            stop_max_attempt_number=20)

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():

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
+    lock_engine = locking.LockEngine(artifact_api.ArtifactLockApi())
-    def _get_artifact(cls, ctx, type_name, artifact_id, read_only=False):
+
+    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 _apply_patch(self, context, af, patch):
-    def create(cls, context, type_name, values):
+        # 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
+        version = values.get('version', artifact_type.DEFAULT_ARTIFACT_VERSION)
-        af = artifact_type.create(context, values)
+        init_values = {
-        # notify about new artifact
+            'id': uuidutils.generate_uuid(),
-        Notifier.notify(context, action_name, af)
+            'name': values.pop('name'),
-        # return artifact to the user
+            'version': version,
-        return af.to_dict()
+            '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 save(self, context, type_name, artifact_id, patch):
-    def update(cls, 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):
+        with self.lock_engine.acquire(context, lock_key):
-            af = cls._get_artifact(context, type_name, artifact_id)
+            af = self._show_artifact(context, type_name, artifact_id)
-            af_dict = af.to_dict()
+            af.obj_reset_changes()
-            updates = _get_updates(af_dict, patch)
+            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
+                return af.to_dict()
-            action = af.get_action_for_updates(context, af, updates)
+
-            action_name = "artifact:%s" % action.__name__
+            for action_name in action_names:
-            policy.authorize(action_name, af_dict, context)
+                policy.authorize(action_name, af.to_dict(), context)
-            modified_af = action(context, af, updates)
+
-            Notifier.notify(context, action_name, modified_af)
+            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 show(self, context, type_name, artifact_id):
-    def get(cls, 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,
+        af = self._show_artifact(context, type_name, artifact_id,
-                               read_only=True)
+                                 read_only=True)
         return af.to_dict()
 
-    @classmethod
+    @staticmethod
-    def list(cls, context, type_name, filters, marker=None, limit=None,
+    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
+    @staticmethod
-    def delete(cls, context, type_name, artifact_id):
+    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)
+        af = self._show_artifact(context, type_name, artifact_id)
-        policy.authorize("artifact:delete", af.to_dict(), context)
+        action_name = 'artifact:delete'
-        af.delete(context, af)
+        policy.authorize(action_name, af.to_dict(), context)
-        Notifier.notify(context, "artifact.delete", af)
+        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(self, context, type_name, artifact_id, field_name,
-    def add_blob_location(cls, 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,
+        blob = {'url': location, 'size': None, 'md5': blob_meta.get("md5"),
-                'sha256': None, 'status': glare_fields.BlobFieldType.ACTIVE,
+                '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:
+        lock_key = "%s:%s" % (type_name, artifact_id)
-            msg = (_("Incorrect blob metadata %(meta)s. MD5 must be specified "
+        with self.lock_engine.acquire(context, lock_key):
-                     "for external location in artifact blob %(blob_name)."),
+            af = self._show_artifact(context, type_name, artifact_id)
-                   {"meta": str(blob_meta), "blob_name": blob_name})
+            action_name = 'artifact:set_location'
-            raise exception.BadRequest(msg)
+            policy.authorize(action_name, af.to_dict(), context)
-        else:
+            modified_af = self._init_blob(
-            blob["md5"] = md5
+                context, af, blob, field_name, blob_key)
-            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)
         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(self, context, type_name, artifact_id, field_name, fd,
-    def upload_blob(cls, 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
-                    # call upload hook first
+                fd, path = af.validate_upload(context, af, field_name, fd)
-                    fd, path = af.validate_upload(context, af, field_name, fd)
+            except Exception as e:
-                except Exception as e:
+                raise exception.BadRequest(message=str(e))
-                    raise exception.BadRequest(message=str(e))
 
-                max_allowed_size = af.get_max_blob_size(field_name)
+            max_allowed_size = af.get_max_blob_size(field_name)
-                # Check if we wanna upload to a folder (and not just to a Blob)
+            # Check if we wanna upload to a folder (and not just to a Blob)
-                if blob_key is not None:
+            if blob_key is not None:
-                    blobs_dict = getattr(af, field_name)
+                blobs_dict = getattr(af, field_name)
-                    overall_folder_size = sum(
+                overall_folder_size = sum(
-                        blob["size"] for blob in blobs_dict.values()
+                    blob["size"] for blob in blobs_dict.values()
-                        if blob["size"] is not None)
+                    if blob["size"] is not None)
-                    max_folder_size_allowed_ = af.get_max_folder_size(field_name) \
+                max_folder_size_allowed = af.get_max_folder_size(field_name) \
-                        - overall_folder_size  # always non-negative
+                    - overall_folder_size  # always non-negative
-                    max_allowed_size = min(max_allowed_size,
+                max_allowed_size = min(max_allowed_size,
-                                           max_folder_size_allowed_)
+                                       max_folder_size_allowed)
 
-                default_store = af.get_default_store(
+            default_store = af.get_default_store(
-                    context, af, field_name, blob_key)
+                context, af, field_name, blob_key)
-                location_uri, size, checksums = store_api.save_blob_to_store(
+            location_uri, size, checksums = store_api.save_blob_to_store(
-                    blob_id, fd, context, max_allowed_size,
+                blob_id, fd, context, max_allowed_size,
-                    store_type=default_store)
+                store_type=default_store)
-            except Exception:
+        except Exception:
-                # if upload failed remove blob from db and storage
+            # if upload failed remove blob from db and storage
-                with excutils.save_and_reraise_exception(logger=LOG):
+            with excutils.save_and_reraise_exception(logger=LOG):
-                    if blob_key is None:
+                if blob_key is None:
-                        af.update_blob(context, af.id, field_name, None)
+                    af.update_blob(context, af.id, field_name, None)
-                    else:
+                else:
-                        blob_dict_attr = getattr(modified_af, field_name)
+                    blob_dict_attr = getattr(modified_af, field_name)
-                        del blob_dict_attr[blob_key]
+                    del blob_dict_attr[blob_key]
-                        af.update_blob(context, af.id,
+                    af.update_blob(context, af.id, field_name, blob_dict_attr)
-                                       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()
         finally:
             if path:
                 os.remove(path)
 
-    @classmethod
+        LOG.info("Successfully finished blob uploading for artifact "
-    def update_blob(cls, context, type_name, artifact_id, blob,
+                 "%(artifact)s blob field %(blob)s.",
-                    field_name, blob_key=None):
+                 {'artifact': af.id, 'blob': blob_name})
-        """Update blob info.
 
-        :param context: user context
+        # update blob info and activate it
-        :param type_name: name of artifact type
+        blob.update({'url': location_uri,
-        :param artifact_id: id of the artifact to be updated
+                     'status': 'active',
-        :param blob: blob representation in dict format
+                     'size': size})
-        :param field_name: name of blob or blob dict field
+        blob.update(checksums)
-        :param blob_key: if field_name is blob dict it specifies key
-         in this dict
 
-        :return: dict representation of updated artifact
+        with self.lock_engine.acquire(context, lock_key):
-        """
+            af = af.show(context, artifact_id)
-        lock_key = "%s:%s" % (type_name, artifact_id)
+            if blob_key:
-        with base.BaseArtifact.lock_engine.acquire(context, lock_key):
+                field_value = getattr(af, field_name)
-            af = cls._get_artifact(context, type_name, artifact_id)
+                field_value[blob_key] = blob
-            if blob_key is None:
-                setattr(af, field_name, blob)
-                return af.update_blob(
-                    context, af.id, field_name, getattr(af, field_name))
             else:
-                blob_dict_attr = getattr(af, field_name)
+                field_value = blob
-                blob_dict_attr[blob_key] = blob
+            modified_af = af.update_blob(
-                return af.update_blob(
+                context, af.id, field_name, field_value)
-                    context, af.id, field_name, blob_dict_attr)
 
-    @classmethod
+        Notifier.notify(context, action_name, modified_af)
-    def download_blob(cls, context, type_name, artifact_id, field_name,
+        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,
+        af = self._show_artifact(context, type_name, artifact_id,
-                               read_only=True)
+                                 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 in the end
-                # call download hook first
+            data, path = af.validate_download(
-                data, path = af.validate_download(
+                context, af, field_name, data)
-                    context, af, field_name, data)
+        except Exception as e:
-            except Exception as e:
+            raise exception.BadRequest(message=str(e))
-                raise exception.BadRequest(message=str(e))
-
-            return data, meta
         finally:
             if path:
                 os.remove(path)
+
+        return data, meta

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 save(self, context):
-    def update(cls, context, af, values):
-        raise exception.Forbidden("This type is read only.")
-
-    @classmethod
-    def get_action_for_updates(cls, context, artifact, updates):
         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.")

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,
+        'status': Field(fields.StringField, default='drafted',
-                        default=glare_fields.ArtifactStatusField.DRAFTED,
+                        nullable=False, sortable=True, mutable=True,
-                        nullable=False, sortable=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 create(self, context):
-    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):
         """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')
+        values = self.obj_changes_to_primitive()
-        ver = str(values.setdefault('version', cls.DEFAULT_ARTIFACT_VERSION))
+        values['type_name'] = self.get_type_name()
-        scope_id = "%s:%s:%s" % (cls.get_type_name(), name, ver)
+        af_vals = self.db_api.save(context, None, values)
-        with cls.lock_engine.acquire(context, scope_id):
+        return self.init_artifact(context, af_vals)
-            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)
 
-    @classmethod
+    def save(self, context):
-    def _validate_versioning(cls, context, name, version, is_public=False):
+        """Save artifact in Glare repo.
-        """Validate if artifact with given name and version already exists.
 
         :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
+        updated_af = self.db_api.save(context, self.id,
-        af.obj_reset_changes()
+                                      self.obj_changes_to_primitive())
-        with cls._get_scoped_lock(af, values):
+        return self.init_artifact(context, updated_af)
-            # 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)
 
     @classmethod
-    def get_action_for_updates(cls, context, af, values):
+    def show(cls, context, artifact_id):
-        """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):
         """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:
+        return blobs
-            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)
 
     @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(
+            schema['enum'] = cls.STATUS
-                glare_fields.ArtifactStatusField.ARTIFACT_STATUS)
 
         if field.description:
             schema['description'] = field.description

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:

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)

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,
+        if field in (fields.StringField, fields.String):
-                     glare_fields.ArtifactStatusField):
             return [FILTER_EQ, FILTER_NEQ, FILTER_IN]
         elif field in (fields.IntegerField, fields.Integer, fields.FloatField,
                        fields.Float, glare_fields.VersionField):

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
+        # can activate if body contains non status changes
-        incorrect = self.make_active + [{"op": "replace",
+        make_active_with_updates = self.make_active + [{"op": "replace",
-                                         "path": "/name",
+                                                        "path": "/description",
-                                         "value": "test"}]
+                                                        "value": "test"}]
-        self.patch(url=url, data=incorrect, status=400)
+        active_art = self.patch(url=url, data=make_active_with_updates)
-        # 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)
         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
+        # test that cannot publish deactivated artifact
-        incorrect = self.make_public + [{"op": "replace",
+        self.patch(url, data=self.make_deactivated)
-                                         "path": "/string_mutable",
+        self.patch(url, data=self.make_public, status=403)
-                                         "value": "test"}]
+
-        self.patch(url=url, data=incorrect, status=400)
+        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,
+        self.admin_action(private_art['id'], self.make_deactivated, 403)
-                          status=400)
         self.patch(url, self.make_active)
         self.set_user('admin')
-        # test cannot deactivate if there is something else in request
+        # test can deactivate if there is something else in request
-        incorrect = self.make_deactivated + [{"op": "replace",
+        make_deactived_with_updates = [
-                                              "path": "/name",
+            {"op": "replace",
-                                              "value": "test"}]
+             "path": "/description",
-        self.patch(url, incorrect, 400)
+             "value": "test"}] + self.make_deactivated
-        self.set_user('user1')
         # test artifact deactivate success
-        deactive_art = self.admin_action(private_art['id'],
+        deactivated_art = self.admin_action(
-                                         self.make_deactivated)
+            private_art['id'], make_deactived_with_updates)
-        self.assertEqual("deactivated", deactive_art["status"])
+        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
+        # test can reactivate if there is something else in request
-        incorrect = self.make_active + [{"op": "replace",
+        make_reactived_with_updates = self.make_active + [
-                                         "path": "/name",
+            {"op": "replace",
-                                         "value": "test"}]
+             "path": "/description",
-        self.patch(url, incorrect, 400)
+             "value": "test"}]
-        # test artifact reactivate success
+        # test artifact deactivate success
-        deactive_art = self.patch(url, self.make_active)
+        reactivated_art = self.admin_action(
-        self.assertEqual("active", deactive_art["status"])
+            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):

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],
+            self.assertEqual(fixtures[at], result['schemas'][at])
-                             utils.DictDiffer(
-                                 fixtures[at]['properties'],
-                                 result['schemas'][at]['properties']))
 
         # Get list schemas of artifacts
         result = self.get(url='/schemas')
-        self.assertEqual(fixtures, result['schemas'], utils.DictDiffer(
+        self.assertEqual(fixtures, result['schemas'])
-            fixtures, result['schemas']))
 
         # Validation of schemas
         result = self.get(url='/schemas')['schemas']

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',

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"}

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'}]
+                    'value': 'some_string'},
-        res = self.update_with_values(changes)
+                   {'op': 'replace', 'path': '/status',
-        self.assertEqual('some_string', res['string_required'])
+                    'value': 'active'}]
-
-        changes = [{'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'}]
+        # It's possible to change artifact status with other fields in
-        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
         # 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):

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"""