Rework link validation

Currently links are validating in two places: in 'update'
method of Base artifact type and in the Link type 'coerce' method.
This looks ugly and it's better to combine all the validations
in one place, where they supposed to be - in the 'coerce' method.

Change-Id: I7cf74c111f7a41fdda2925f232718693110c26de
This commit is contained in:
Mike Fedosin 2017-07-04 19:37:17 +03:00
parent b0c2a93aa5
commit c69a3eba6d
2 changed files with 23 additions and 59 deletions

View File

@ -22,7 +22,6 @@ from oslo_utils import timeutils
from oslo_versionedobjects import base
from oslo_versionedobjects import fields
import six
import six.moves.urllib.request as urlrequest
from glare.common import exception
from glare.common import store_api
@ -154,37 +153,6 @@ class BaseArtifact(base.VersionedObject):
cls.fields[field_name].element_type ==
glare_fields.BlobFieldType)
@classmethod
def is_link(cls, field_name):
"""Helper to check that a field is a link.
:param field_name: name of the field
:return: True if field is a link, False otherwise
"""
return isinstance(cls.fields.get(field_name), glare_fields.Link)
@classmethod
def is_link_dict(cls, field_name):
"""Helper to check that a field is a link dict.
:param field_name: name of the field
:return: True if field is a link dict, False otherwise
"""
return (isinstance(cls.fields.get(field_name), glare_fields.Dict) and
cls.fields[field_name].element_type ==
glare_fields.LinkFieldType)
@classmethod
def is_link_list(cls, field_name):
"""Helper to check that a field is a link list.
:param field_name: name of the field
:return: True if the field is a link list, False otherwise
"""
return (isinstance(cls.fields.get(field_name), glare_fields.List) and
cls.fields[field_name].element_type ==
glare_fields.LinkFieldType)
@classmethod
def _init_artifact(cls, context, values):
"""Initialize an empty versioned object with values.
@ -352,20 +320,6 @@ class BaseArtifact(base.VersionedObject):
# 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():
try:
# check updates for links and validate them
if cls.is_link(key) and value is not None:
cls._validate_link(key, value, context)
elif cls.is_link_dict(key) and value:
for l in value:
cls._validate_link(key, value[l], context)
elif cls.is_link_list(key) and value:
for l in value:
cls._validate_link(key, l, context)
except Exception as e:
msg = (_("Bad link in artifact %(af)s: %(msg)s")
% {"af": af.id, "msg": str(e)})
raise exception.BadRequest(msg)
setattr(af, key, value)
LOG.info("Parameters validation for artifact %(artifact)s "
@ -412,19 +366,6 @@ class BaseArtifact(base.VersionedObject):
return action
@classmethod
def _validate_link(cls, key, value, ctx):
# check format
glare_fields.LinkFieldType.coerce(None, key, value)
# check containment
if glare_fields.LinkFieldType.is_external(value):
with urlrequest.urlopen(value) as data:
data.read(1)
else:
filters = [('id', None, 'eq', None, value.split('/')[3])]
if len(cls.db_api.list(ctx, filters, None, 1, [], False)) == 0:
raise exception.NotFound
@classmethod
def get(cls, context, artifact_id):
"""Return Artifact from Glare repo

View File

@ -20,7 +20,9 @@ from oslo_versionedobjects import fields
import semantic_version
import six
import six.moves.urllib.parse as urlparse
import six.moves.urllib.request as urlrequest
from glare.common import exception
from glare.i18n import _
@ -150,6 +152,17 @@ class LinkFieldType(fields.FieldType):
if link.scheme not in ('http', 'https'):
raise ValueError(_('Only http and https requests '
'are allowed in url %s') % value)
try:
with urlrequest.urlopen(value) as data:
data.read(1)
except Exception:
raise ValueError(
_('Link %(link)s is not valid in field '
'%(field)s. The link must be either valid url or '
'reference to artifact. Example: '
'http://glarehost:9494/artifacts/<artifact_type>/'
'<artifact_id>'
) % {'link': value, 'field': field})
else:
result = value.split('/')
if len(result) != 4 or result[1] != 'artifacts':
@ -159,6 +172,16 @@ class LinkFieldType(fields.FieldType):
'reference to artifact. Example: '
'/artifacts/<artifact_type>/<artifact_id>'
) % {'link': value, 'field': field})
# try to find the referenced artifact
try:
obj.db_api.get(obj.obj_context, result[3])
except exception.NotFound:
raise ValueError(
_("Link %(link)s is not valid in field %(field)s, because "
"artifact with id %(art_id)s doesn't exist"
) % {'link': value, 'field': field, 'art_id': result[3]}
)
return value