You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2017/04/25 22:12:53 UTC
[4/5] qpid-dispatch git commit: DISPATCH-205: Schema.validate()
called many times
DISPATCH-205: Schema.validate() called many times
Replace wildly inefficient validation scheme with one that checks each new
entity exactly once for unique attribute and singleton clashes.
Better error message for forbidden python imports
Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/3b1fe114
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/3b1fe114
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/3b1fe114
Branch: refs/heads/master
Commit: 3b1fe11453b2ac7a776a56c94f135cedafd9bd0b
Parents: af6c53e
Author: Alan Conway <ac...@redhat.com>
Authored: Tue Mar 28 13:33:50 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Apr 25 18:08:53 2017 -0400
----------------------------------------------------------------------
python/qpid_dispatch/management/entity.py | 3 +
python/qpid_dispatch_internal/dispatch.py | 10 +-
.../qpid_dispatch_internal/management/agent.py | 8 +-
.../management/qdrouter.py | 24 ++--
.../qpid_dispatch_internal/management/schema.py | 142 +++++++------------
.../management/schema_doc.py | 4 +-
tests/management/schema.py | 2 +-
7 files changed, 77 insertions(+), 116 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/python/qpid_dispatch/management/entity.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch/management/entity.py b/python/qpid_dispatch/management/entity.py
index 17ff00a..4e5466a 100644
--- a/python/qpid_dispatch/management/entity.py
+++ b/python/qpid_dispatch/management/entity.py
@@ -62,6 +62,9 @@ class EntityBase(object):
def __getattr__(self, name):
return self.attributes[name]
+ def __contains__(self, name):
+ return name in self.attributes
+
@staticmethod
def _pyname(name): return name.replace('-', '_')
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/python/qpid_dispatch_internal/dispatch.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/dispatch.py b/python/qpid_dispatch_internal/dispatch.py
index e397f86..c35fb5d 100644
--- a/python/qpid_dispatch_internal/dispatch.py
+++ b/python/qpid_dispatch_internal/dispatch.py
@@ -111,8 +111,12 @@ class QdDll(ctypes.PyDLL):
return self._prototype(getattr(self, fname), restype, argtypes, check)
-# Prevent accidental loading of the proton module.
-
+# Prevent accidental loading of the proton python module inside dispatch.
+# The proton-C library is linked with the dispatch C library, loading the proton
+# python module loads a second copy of the library and mayhem ensues.
+#
+# Note the FORBIDDEN list is over-written to disable this tests in mock python
+# testing code.
FORBIDDEN = ["proton"]
def check_forbidden():
@@ -122,7 +126,7 @@ def check_forbidden():
def import_check(name, *args, **kw):
if name in FORBIDDEN:
- raise ImportError("Attempted to load forbidden module '%s'." % name)
+ raise ImportError("Python code running inside a dispatch router cannot import '%s', use the 'dispatch' module for internal messaging" % name)
return builtin_import(name, *args, **kw)
check_forbidden()
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/python/qpid_dispatch_internal/management/agent.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/management/agent.py b/python/qpid_dispatch_internal/management/agent.py
index 7f50cb8..45b7d6f 100644
--- a/python/qpid_dispatch_internal/management/agent.py
+++ b/python/qpid_dispatch_internal/management/agent.py
@@ -52,7 +52,6 @@ getting current information from the implementation object.
## Threading:
-
The agent is locked to be thread safe, called in the following threads:
- Reading configuration file in initialization thread (no contention).
- Management requests arriving in multiple, concurrent connection threads.
@@ -61,9 +60,6 @@ The agent is locked to be thread safe, called in the following threads:
When refreshing attributes, the agent must also read C implementation object
data that may be updated in other threads.
-# FIXME aconway 2015-02-09:
-Temporary solution is to lock the entire dispatch router lock during full refresh.
-Better solution coming soon...
"""
import traceback, json, pstats
@@ -208,7 +204,7 @@ class EntityAdapter(SchemaEntity):
"""Handle update request with new attributes from management client"""
self.entity_type.update_check(request.body, self.attributes)
newattrs = dict(self.attributes, **request.body)
- self.entity_type.validate(newattrs, update=True)
+ self.entity_type.validate(newattrs)
self.attributes = newattrs
self._update()
return (OK, self.attributes)
@@ -561,7 +557,7 @@ class EntityCache(object):
self.log(LOG_DEBUG, "Add entity: %s" % entity)
entity.validate() # Fill in defaults etc.
# Validate in the context of the existing entities for uniqueness
- self.schema.validate_full(chain(iter([entity]), iter(self.entities)))
+ self.schema.validate_add(entity, self.entities)
self.entities.append(entity)
def _add_implementation(self, implementation, adapter=None):
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/python/qpid_dispatch_internal/management/qdrouter.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/management/qdrouter.py b/python/qpid_dispatch_internal/management/qdrouter.py
index 5f21913..2590b64 100644
--- a/python/qpid_dispatch_internal/management/qdrouter.py
+++ b/python/qpid_dispatch_internal/management/qdrouter.py
@@ -44,24 +44,20 @@ class QdSchema(schema.Schema):
self.configuration_entity = self.entity_type(self.CONFIGURATION_ENTITY)
self.operational_entity = self.entity_type(self.OPERATIONAL_ENTITY)
- def validate_full(self, entities, **kwargs):
+ def validate_add(self, attributes, entities):
"""
- In addition to L{schema.Schema.validate}, check the following:
-
- listeners and connectors can only have role=inter-router if the
- router has mode=interior.
-
-
- @param entities: List of attribute name:value maps.
- @param kwargs: See L{schema.Schema.validate}
+ Check that listeners and connectors can only have role=inter-router if the router has
+ mode=interior.
"""
- entities = list(entities) # Need to traverse twice
- super(QdSchema, self).validate_all(entities, **kwargs)
+ entities = list(entities) # Iterate twice
+ super(QdSchema, self).validate_add(attributes, entities)
+ entities.append(attributes)
inter_router = not_interior = None
for e in entities:
- if self.short_name(e.type) == "router" and e.mode != "interior":
- not_interior = e.mode
- if self.short_name(e.type) in ["listener", "connector"] and e.role == "inter-router":
+ short_type = self.short_name(e['type'])
+ if short_type == "router" and e['mode'] != "interior":
+ not_interior = e['mode']
+ if short_type in ["listener", "connector"] and e['role'] == "inter-router":
inter_router = e
if not_interior and inter_router:
raise schema.ValidationError(
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/python/qpid_dispatch_internal/management/schema.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/management/schema.py b/python/qpid_dispatch_internal/management/schema.py
index 7559c73..590b33e 100644
--- a/python/qpid_dispatch_internal/management/schema.py
+++ b/python/qpid_dispatch_internal/management/schema.py
@@ -35,12 +35,6 @@ class ValidationError(Exception):
"""Error raised if schema validation fails"""
pass
-def quotestr(value, quote="'"):
- """Quote value if it is a string type, str() it if not """
- if isinstance(value, basestring): return "'%s'" % value
- else: return str(value)
-
-
class Type(object):
"""Base class for schema types.
@@ -54,11 +48,9 @@ class Type(object):
"""
self.name, self.pytype = name, pytype
- def validate(self, value, **kwargs): # pylint: disable=unused-argument
+ def validate(self, value):
"""
Convert value to the correct python type.
-
- @param kwargs: See L{Schema.validate_all}
"""
return self.pytype(value)
@@ -82,11 +74,10 @@ class BooleanType(Type):
VALUES = {"yes":1, "true":1, "on":1, "no":0, "false":0, "off":0}
- def validate(self, value, **kwargs):
+ def validate(self, value):
"""
@param value: A string such as "yes", "false" etc. is converted appropriately.
Any other type is converted using python's bool()
- @param kwargs: See L{Schema.validate_all}
@return A python bool.
"""
try:
@@ -122,11 +113,10 @@ class EnumType(Type):
super(EnumType, self).__init__("enum%s"%([str(t) for t in tags]), int)
self.tags = tags
- def validate(self, value, **kwargs):
+ def validate(self, value):
"""
@param value: May be a string from the set of enum tag strings or anything
that can convert to an int - in which case it must be in the enum range.
- @param kwargs: See L{Schema.validate_all}
@return: An EnumValue.
"""
if value in self.tags:
@@ -147,7 +137,7 @@ class EnumType(Type):
def __str__(self):
"""String description of enum type."""
- return "One of [%s]" % ', '.join([quotestr(tag) for tag in self.tags])
+ return "One of [%s]" % ', '.join([("'%s'" %tag) for tag in self.tags])
BUILTIN_TYPES = OrderedDict(
(t.name, t) for t in [Type("string", str),
@@ -177,18 +167,6 @@ def _dump_dict(items):
"""
return OrderedDict((k, v) for k, v in items if v)
-def _is_unique(found, item):
- """
- Return true if found is None or item is not in found (adds item to found.)
- Return false if item is in found.
- """
- if found is None or found is False:
- return True
- if item not in found:
- found.add(item)
- return True
- return False
-
class AttributeType(object):
"""
Definition of an attribute.
@@ -234,38 +212,28 @@ class AttributeType(object):
ex, msg, trace = sys.exc_info()
raise ValidationError, "Attribute '%s': %s" % (name, msg), trace
- def missing_value(self, check_required=True, add_default=True, **kwargs):
+ def missing_value(self):
"""
Fill in missing default and fixed values.
- @keyword check_required: Raise an exception if required attributes are misssing.
- @keyword add_default: Add a default value for missing attributes.
- @param kwargs: See L{Schema.validate_all}
"""
if self.value is not None: # Fixed value attribute
return self.value
- if add_default and self.default is not None:
+ if self.default is not None:
return self.default
- if check_required and self.required:
+ if self.required:
raise ValidationError("Missing required attribute '%s'" % (self.name))
- def validate(self, value, check_unique=None, **kwargs):
+ def validate(self, value):
"""
Validate value for this attribute definition.
@param value: The value to validate.
- @keyword check_unique: set of (name, value) to check for attribute uniqueness.
- None means don't check for uniqueness.
- @param create: if true, check that the attribute allows create
- @param update: if true, check that the attribute allows update
- @param kwargs: See L{Schema.validate_all}
@return: value converted to the correct python type. Rais exception if any check fails.
"""
- if self.unique and not _is_unique(check_unique, (self.name, value)):
- raise ValidationError("Duplicate value '%s' for unique attribute '%s'"%(value, self.name))
if self.value and value != self.value:
raise ValidationError("Attribute '%s' has fixed value '%s' but given '%s'"%(
self.name, self.value, value))
try:
- return self.atype.validate(value, **kwargs)
+ return self.atype.validate(value)
except (TypeError, ValueError), e:
raise ValidationError, str(e), sys.exc_info()[2]
@@ -320,8 +288,9 @@ class EntityType(object):
@ivar singleton: If true only one entity of this type is allowed.
@ivar referential: True if an entity can be referred to by name from another entity.
"""
- def __init__(self, name, schema, attributes=None, operations=None, operationDefs=None, description="",
- fullName=True, singleton=False, deprecated=False, extends=None, referential=False, **kwargs):
+ def __init__(self, name, schema, attributes=None, operations=None, operationDefs=None,
+ description="", fullName=True, singleton=False, deprecated=False,
+ extends=None, referential=False):
"""
@param name: name of the entity type.
@param schema: schema for this type.
@@ -400,25 +369,19 @@ class EntityType(object):
"""Return only attribute types defined in this entity type"""
return [a for a in self.attributes.itervalues() if a.defined_in == self]
- def validate(self, attributes, check_singleton=None, **kwargs):
+ def validate(self, attributes):
"""
Validate attributes for entity type.
@param attributes: Map attributes name:value or Entity with attributes property.
Modifies attributes: adds defaults, converts values.
- @param check_singleton: set of entity-type name to enable singleton checking.
- None to disable.
- @param kwargs: See L{Schema.validate_all}
"""
-
if isinstance(attributes, SchemaEntity): attributes = attributes.attributes
- if self.singleton and not _is_unique(check_singleton, self.name):
- raise ValidationError("Multiple instances of singleton '%s'"%self.name)
try:
# Add missing values
for attr in self.attributes.itervalues():
if attributes.get(attr.name) is None:
- value = attr.missing_value(**kwargs)
+ value = attr.missing_value()
if value is not None: attributes[attr.name] = value
if value is None and attr.name in attributes:
del attributes[attr.name]
@@ -427,7 +390,7 @@ class EntityType(object):
for name, value in attributes.iteritems():
if name == 'type':
value = self.schema.long_name(value)
- attributes[name] = self.attribute(name).validate(value, **kwargs)
+ attributes[name] = self.attribute(name).validate(value)
except ValidationError, e:
raise ValidationError, "%s: %s"%(self, e), sys.exc_info()[2]
@@ -540,48 +503,49 @@ class Schema(object):
def entity_type(self, name, error=True):
return self._lookup(self.entity_types, name, "No such entity type '%s'", error)
- def validate_entity(self, attributes, check_required=True, add_default=True,
- check_unique=None, check_singleton=None):
+ def validate_entity(self, attributes):
"""
Validate a single entity.
@param attributes: Map of attribute name: value
- @keyword check_required: Raise exception if required attributes are missing.
- @keyword add_default: Add defaults for missing attributes.
- @keyword check_unique: Used by L{validate_all}
- @keyword check_singleton: Used by L{validate_all}
"""
attributes['type'] = self.long_name(attributes['type'])
entity_type = self.entity_type(attributes['type'])
- entity_type.validate(
- attributes,
- check_required=check_required,
- add_default=add_default,
- check_unique=check_unique,
- check_singleton=check_singleton)
-
- def validate_all(self, attribute_maps, check_required=True, add_default=True,
- check_unique=True, check_singleton=True):
- """
- Validate a list of attribute maps representing entity attributes.
- Verify singleton entities and unique attributes are unique.
- Modifies attribute_maps, adds default values, converts values.
-
- @param attribute_maps: List of attribute name:value maps.
- @keyword check_required: Raise exception if required attributes are missing.
- @keyword add_default: Add defaults for missing attributes.
- @keyword check_unique: Raise exception if unique attributes are duplicated.
- @keyword check_singleton: Raise exception if singleton entities are duplicated
- """
- if check_singleton: check_singleton = set()
- if check_unique: check_unique = set()
-
- for e in attribute_maps:
- self.validate_entity(e,
- check_required=check_required,
- add_default=add_default,
- check_unique=check_unique,
- check_singleton=check_singleton)
+ entity_type.validate(attributes)
+
+ def validate_all(self, attribute_maps):
+ """
+ Validate all the entities from entity_iter, return a list of valid entities.
+ """
+ entities = []
+ for a in attribute_maps:
+ self.validate_add(a, entities)
+ entities.append(a);
+
+ def validate_add(self, attributes, entities):
+ """
+ Validate that attributes would be valid when added to entities.
+ Assumes entities are already valid
+ @raise ValidationError if adding e violates a global constraint like uniqueness.
+ """
+ self.validate_entity(attributes)
+ entity_type = self.entity_type(attributes['type'])
+ # Find all the unique attribute types present in attributes
+ unique = [a for a in entity_type.attributes.values() if a.unique and a.name in attributes]
+ if not unique and not entity_type.singleton:
+ return # Nothing to do
+ for e in entities:
+ if entity_type.singleton and attributes['type'] == e['type']:
+ raise ValidationError("Adding %s singleton %s when %s already exists" %
+ (attributes['type'], attributes, e))
+ for a in unique:
+ try:
+ if entity_type.attributes[a.name] == a and attributes[a.name] == e[a.name]:
+ raise ValidationError(
+ "adding %s duplicates unique attribute '%s' from existing %s"%
+ (attributes, a.name, e))
+ except KeyError:
+ continue # Missing attribute or definition means no clash
def entity(self, attributes):
"""Convert an attribute map into an L{SchemaEntity}"""
@@ -617,5 +581,5 @@ class SchemaEntity(EntityBase):
super(SchemaEntity, self)._set(name, value)
self.validate()
- def validate(self, **kwargs):
- self.entity_type.validate(self.attributes, **kwargs)
+ def validate(self):
+ self.entity_type.validate(self.attributes)
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/python/qpid_dispatch_internal/management/schema_doc.py
----------------------------------------------------------------------
diff --git a/python/qpid_dispatch_internal/management/schema_doc.py b/python/qpid_dispatch_internal/management/schema_doc.py
index ecf2825..a9a0881 100644
--- a/python/qpid_dispatch_internal/management/schema_doc.py
+++ b/python/qpid_dispatch_internal/management/schema_doc.py
@@ -21,8 +21,6 @@
from collections import namedtuple
import sys
-from .schema import quotestr
-
class SchemaWriter(object):
"""Write the schema as an asciidoc document"""
@@ -57,7 +55,7 @@ class SchemaWriter(object):
default = None # Don't show defaults that are references, confusing.
return ' (%s)' % (', '.join(
filter(None, [str(attr.atype),
- default and "default=%s" % quotestr(default),
+ default and "default='%s'" % default,
attr.required and "required",
attr.unique and "unique",
show_create and attr.create and "`CREATE`",
http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/3b1fe114/tests/management/schema.py
----------------------------------------------------------------------
diff --git a/tests/management/schema.py b/tests/management/schema.py
index 9ab3fcf..dd22276 100644
--- a/tests/management/schema.py
+++ b/tests/management/schema.py
@@ -165,7 +165,7 @@ class SchemaTest(unittest.TestCase):
# This will make sure that deprecated flag defaults to false for entities
self.assertFalse(s.entity_types['org.example.connector'].deprecated)
-
+
# This will make sure that deprecated flag defaults to false for attributes of entities
self.assertFalse(s.entity_types['org.example.listener'].attributes['host'].deprecated)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org