You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bloodhound.apache.org by as...@apache.org on 2013/05/17 09:35:48 UTC
svn commit: r1483676 - in /bloodhound/trunk: bloodhound_relations/
bloodhound_relations/bhrelations/ bloodhound_relations/bhrelations/tests/
installer/
Author: astaric
Date: Fri May 17 07:35:48 2013
New Revision: 1483676
URL: http://svn.apache.org/r1483676
Log:
Refactored relation validation. Towards #528.
Added:
bloodhound/trunk/bloodhound_relations/bhrelations/validation.py
Modified:
bloodhound/trunk/bloodhound_relations/bhrelations/api.py
bloodhound/trunk/bloodhound_relations/bhrelations/model.py
bloodhound/trunk/bloodhound_relations/bhrelations/tests/api.py
bloodhound/trunk/bloodhound_relations/setup.py
bloodhound/trunk/installer/bloodhound_setup.py
Modified: bloodhound/trunk/bloodhound_relations/bhrelations/api.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_relations/bhrelations/api.py?rev=1483676&r1=1483675&r2=1483676&view=diff
==============================================================================
--- bloodhound/trunk/bloodhound_relations/bhrelations/api.py (original)
+++ bloodhound/trunk/bloodhound_relations/bhrelations/api.py Fri May 17 07:35:48 2013
@@ -22,6 +22,7 @@ from pkg_resources import resource_filen
from bhrelations import db_default
from bhrelations.model import Relation
from multiproduct.api import ISupportMultiProductEnvironment
+from trac.config import OrderedExtensionsOption
from trac.core import (Component, implements, TracError, Interface,
ExtensionPoint)
from trac.env import IEnvironmentSetupParticipant
@@ -35,21 +36,6 @@ from trac.web.chrome import ITemplatePro
PLUGIN_NAME = 'Bloodhound Relations Plugin'
-class BaseValidationError(TracError):
- def __init__(self, message, title=None, show_traceback=False):
- super(BaseValidationError, self).__init__(
- message, title, show_traceback)
- self.failed_ids = []
-
-
-class CycleValidationError(BaseValidationError):
- pass
-
-
-class ParentValidationError(BaseValidationError):
- pass
-
-
#TODO: consider making the interface part of future
# I[*|Resource]ChangingListener approach based on output from the
# correlated discussion in Trac community
@@ -84,6 +70,18 @@ class IRelationChangingListener(Interfac
"""
+class IRelationValidator(Interface):
+ """
+ Extension point interface for relation validators.
+ """
+
+ def validate(relation):
+ """
+ Validate the relation. If relation is not valid, raise appropriate
+ exception.
+ """
+
+
class EnvironmentSetup(Component):
implements(IEnvironmentSetupParticipant, ISupportMultiProductEnvironment,
ITemplateProvider)
@@ -160,6 +158,15 @@ class RelationsSystem(Component):
RELATIONS_CONFIG_NAME = 'bhrelations_links'
changing_listeners = ExtensionPoint(IRelationChangingListener)
+ all_validators = ExtensionPoint(IRelationValidator)
+ global_validators = OrderedExtensionsOption(
+ 'bhrelations', 'global_validators',
+ interface=IRelationValidator,
+ default=['NoSelfReferenceValidator'],
+ include_missing=False,
+ doc="""Validators used to validate all relations,
+ regardless of their type."""
+ )
def __init__(self):
self._links, self._labels, self._validators, self._blockers, \
@@ -204,7 +211,6 @@ class RelationsSystem(Component):
return relation.clone_reverted(other_end)
def add_relation(self, relation):
- #TBD: add changes in source and destination ticket history
self.validate(relation)
with self.env.db_transaction:
relation.insert()
@@ -320,11 +326,11 @@ class RelationsSystem(Component):
label2 = config.get(end2 + '.label') or end2.capitalize()
labels[end2] = label2
- validator = config.get(name + '.validator')
- if validator:
- validators[end1] = validator
+ custom_validators = self._parse_validators(config, name)
+ if custom_validators:
+ validators[end1] = custom_validators
if end2:
- validators[end2] = validator
+ validators[end2] = custom_validators
blockers[end1] = config.getbool(end1 + '.blocks', default=False)
if end2:
@@ -342,102 +348,35 @@ class RelationsSystem(Component):
return links, labels, validators, blockers, copy_fields
+ def _parse_validators(self, section, name):
+ custom_validators = set(
+ '%sValidator' % validator for validator in
+ set(section.getlist(name + '.validators', [], ',', True)))
+ validators = []
+ if custom_validators:
+ for impl in self.all_validators:
+ if impl.__class__.__name__ in custom_validators:
+ validators.append(impl)
+ return validators
+
def validate(self, relation):
- self._validate_self_reference(relation)
+ """
+ Validate the relation using the configured validators. Validation is
+ always run on the relation with master type.
+ """
+ backrel = self.get_reverted_relation(relation)
+ if backrel and (backrel.type, relation.type) in self._links:
+ relation = backrel
+
+ for validator in self.global_validators:
+ validator.validate(relation)
- validator = self._get_validator(relation.type)
- if validator:
- validator(relation)
+ for validator in self._validators.get(relation.type, ()):
+ validator.validate(relation)
def is_blocker(self, relation_type):
return self._blockers[relation_type]
- def _get_validator(self, relation_type):
- #todo: implement generic validator factory based on interfaces
- validator_name = self._validators.get(relation_type)
- if validator_name == 'no_cycle':
- validator = self._validate_no_cycle
- elif validator_name == 'parent_child':
- validator = self._validate_parent
- else:
- validator = None
- return validator
-
- def _validate_self_reference(self, relation):
- if relation.source == relation.destination:
- error = CycleValidationError(
- 'Ticket cannot be self-referenced in a relation.')
- error.failed_ids = [relation.source]
- raise error
-
- def _validate_no_cycle(self, relation):
- """If a path exists from relation's destination to its source,
- adding the relation will create a cycle.
- """
- path = self._find_path(relation.destination,
- relation.source,
- relation.type)
- if path:
- cycle_str = [self.get_resource_name_from_id(resource_id)
- for resource_id in path]
- error = 'Cycle in ''%s'': %s' % (
- self.render_relation_type(relation.type),
- ' -> '.join(cycle_str))
- error = CycleValidationError(error)
- error.failed_ids = path
- raise error
-
- def _validate_parent(self, relation):
- self._validate_no_cycle(relation)
-
- if relation.type == self.PARENT_RELATION_TYPE:
- source = relation.source
- elif relation.type == self.CHILDREN_RELATION_TYPE:
- source = relation.destination
- else:
- return None
-
- parent_relations = self._select_relations(
- source, self.PARENT_RELATION_TYPE)
- if len(parent_relations) > 0:
- source_resource_name = self.get_resource_name_from_id(
- relation.source)
- parent_ids_ins_string = ", ".join(
- [self.get_resource_name_from_id(relation.destination)
- for relation in parent_relations]
- )
- error = "Multiple links in '%s': %s -> [%s]" % (
- self.render_relation_type(relation.type),
- source_resource_name,
- parent_ids_ins_string)
- ex = ParentValidationError(error)
- ex.failed_ids = [relation.destination
- for relation in parent_relations]
- raise ex
-
- def _find_path(self, source, destination, relation_type):
- known_nodes = set()
- new_nodes = {source}
- paths = {(source, source): [source]}
-
- while new_nodes:
- known_nodes = set.union(known_nodes, new_nodes)
- with self.env.db_query as db:
- relations = dict(db("""
- SELECT source, destination
- FROM bloodhound_relations
- WHERE type = '%(relation_type)s'
- AND source IN (%(new_nodes)s)
- """ % dict(
- relation_type=relation_type,
- new_nodes=', '.join("'%s'" % n for n in new_nodes))
- ))
- new_nodes = set(relations.values()) - known_nodes
- for s, d in relations.items():
- paths[(source, d)] = paths[(source, s)] + [d]
- if destination in new_nodes:
- return paths[(source, destination)]
-
def render_relation_type(self, end):
return self._labels[end]
@@ -461,7 +400,7 @@ class RelationsSystem(Component):
# all_blockers.extend(blockers)
return all_blockers
- def get_resource_name_from_id(self, resource_id):
+ def get_resource_name(self, resource_id):
resource = ResourceIdSerializer.get_resource_by_id(resource_id)
return get_resource_shortname(self.env, resource)
@@ -608,7 +547,7 @@ class TicketChangeRecordUpdater(Componen
if ticket_id is None:
return
- related_resource_name = relation_system.get_resource_name_from_id(
+ related_resource_name = relation_system.get_resource_name(
relation.destination)
if is_delete:
old_value = related_resource_name
Modified: bloodhound/trunk/bloodhound_relations/bhrelations/model.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_relations/bhrelations/model.py?rev=1483676&r1=1483675&r2=1483676&view=diff
==============================================================================
--- bloodhound/trunk/bloodhound_relations/bhrelations/model.py (original)
+++ bloodhound/trunk/bloodhound_relations/bhrelations/model.py Fri May 17 07:35:48 2013
@@ -93,3 +93,6 @@ class Relation(ModelBase):
destination=destination,
type=relation_type
))
+
+ def __str__(self):
+ return '%s %s %s' % (self.source, self.type, self.destination)
Modified: bloodhound/trunk/bloodhound_relations/bhrelations/tests/api.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_relations/bhrelations/tests/api.py?rev=1483676&r1=1483675&r2=1483676&view=diff
==============================================================================
--- bloodhound/trunk/bloodhound_relations/bhrelations/tests/api.py (original)
+++ bloodhound/trunk/bloodhound_relations/bhrelations/tests/api.py Fri May 17 07:35:48 2013
@@ -21,9 +21,9 @@ from datetime import datetime
from _sqlite3 import OperationalError, IntegrityError
import unittest
from bhrelations.api import (EnvironmentSetup, RelationsSystem,
- CycleValidationError, ParentValidationError,
TicketRelationsSpecifics)
from bhrelations.tests.mocks import TestRelationChangingListener
+from bhrelations.validation import ValidationError
from multiproduct.env import ProductEnvironment
from tests.env import MultiproductTestCase
from trac.ticket.model import Ticket
@@ -47,14 +47,16 @@ class BaseApiApiTestCase(MultiproductTes
)
config_name = RelationsSystem.RELATIONS_CONFIG_NAME
env.config.set(config_name, 'dependency', 'dependson,dependent')
- env.config.set(config_name, 'dependency.validator', 'no_cycle')
+ env.config.set(config_name, 'dependency.validators',
+ 'NoCycles,SingleProduct')
env.config.set(config_name, 'dependent.blocks', 'true')
env.config.set(config_name, 'parent_children', 'parent,children')
- env.config.set(config_name, 'parent_children.validator',
- 'parent_child')
+ env.config.set(config_name, 'parent_children.validators',
+ 'OneToMany,SingleProduct,NoCycles,Exclusive')
env.config.set(config_name, 'children.label', 'Overridden')
env.config.set(config_name, 'parent.copy_fields',
'summary, foo')
+ env.config.set(config_name, 'multiproduct_relation', 'mprel,mpbackrel')
env.config.set(config_name, 'oneway', 'refersto')
self.global_env = env
@@ -103,7 +105,6 @@ class ApiTestCase(BaseApiApiTestCase):
dependent = self._insert_and_load_ticket("A2")
#act
relations_system = self.relations_system
- self._debug_select()
relations_system.add(
ticket, dependent, "dependent")
#assert
@@ -239,7 +240,7 @@ class ApiTestCase(BaseApiApiTestCase):
try:
relations_system.add(ticket2, ticket1, "dependson")
self.fail("Should throw an exception")
- except CycleValidationError, ex:
+ except ValidationError as ex:
self.assertSequenceEqual(
["tp1:ticket:1", "tp1:ticket:2"], ex.failed_ids)
@@ -261,7 +262,7 @@ class ApiTestCase(BaseApiApiTestCase):
relations_system = self.relations_system
relations_system.add(ticket1, ticket2, "dependson")
self.assertRaises(
- CycleValidationError,
+ ValidationError,
relations_system.add,
ticket1,
ticket2,
@@ -277,7 +278,7 @@ class ApiTestCase(BaseApiApiTestCase):
relations_system.add(ticket1, ticket2, "dependson")
relations_system.add(ticket2, ticket3, "dependson")
self.assertRaises(
- CycleValidationError,
+ ValidationError,
relations_system.add,
ticket3,
ticket1,
@@ -292,7 +293,7 @@ class ApiTestCase(BaseApiApiTestCase):
relations_system = self.relations_system
relations_system.add(child, parent1, "parent")
self.assertRaises(
- ParentValidationError,
+ ValidationError,
relations_system.add,
child,
parent2,
@@ -307,7 +308,7 @@ class ApiTestCase(BaseApiApiTestCase):
relations_system = self.relations_system
relations_system.add(parent1, child, "children")
self.assertRaises(
- ParentValidationError,
+ ValidationError,
relations_system.add,
parent2,
child,
@@ -322,7 +323,7 @@ class ApiTestCase(BaseApiApiTestCase):
relations_system = self.relations_system
relations_system.add(parent1, child, "children")
self.assertRaises(
- ParentValidationError,
+ ValidationError,
relations_system.add,
parent2,
child,
@@ -391,7 +392,7 @@ class ApiTestCase(BaseApiApiTestCase):
ticket2 = self._insert_and_load_ticket_with_env(p2_env, "A2")
relations_system = self.relations_system
#act
- relations_system.add(ticket1, ticket2, "dependent")
+ relations_system.add(ticket1, ticket2, "mprel")
#assert
self.assertEqual(1, len(relations_system.get_relations(ticket1)))
self.assertEqual(1, len(relations_system.get_relations(ticket2)))
@@ -407,6 +408,60 @@ class ApiTestCase(BaseApiApiTestCase):
for row in db(sql):
print row
+ def test_parent_relation_is_incompatible_with_two_way_relations(self):
+ ticket1 = self._insert_and_load_ticket("A1")
+ ticket2 = self._insert_and_load_ticket("A2")
+ self.relations_system.add(ticket1, ticket2, "dependent")
+
+ self.assertRaises(
+ ValidationError,
+ self.relations_system.add,
+ ticket1,
+ ticket2,
+ "parent")
+ self.assertRaises(
+ ValidationError,
+ self.relations_system.add,
+ ticket1,
+ ticket2,
+ "children")
+
+ def test_parent_relation_is_incompatible_with_one_way_relations(self):
+ ticket1 = self._insert_and_load_ticket("A1")
+ ticket2 = self._insert_and_load_ticket("A2")
+ self.relations_system.add(ticket1, ticket2, "refersto")
+
+ self.assertRaises(
+ ValidationError,
+ self.relations_system.add,
+ ticket1,
+ ticket2,
+ "parent")
+ self.assertRaises(
+ ValidationError,
+ self.relations_system.add,
+ ticket1,
+ ticket2,
+ "children")
+
+ def test_parent_must_be_in_same_product(self):
+ ticket1 = self._insert_and_load_ticket("A1")
+ product2 = "tp2"
+ self._load_product_from_data(self.global_env, product2)
+ p2_env = ProductEnvironment(self.global_env, product2)
+ ticket2 = self._insert_and_load_ticket_with_env(p2_env, "A2")
+
+ self.assertRaises(
+ ValidationError,
+ self.relations_system.add,
+ ticket1, ticket2, "parent"
+ )
+ self.assertRaises(
+ ValidationError,
+ self.relations_system.add,
+ ticket1, ticket2, "children"
+ )
+
class RelationChangingListenerTestCase(BaseApiApiTestCase):
def test_can_sent_adding_event(self):
Added: bloodhound/trunk/bloodhound_relations/bhrelations/validation.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_relations/bhrelations/validation.py?rev=1483676&view=auto
==============================================================================
--- bloodhound/trunk/bloodhound_relations/bhrelations/validation.py (added)
+++ bloodhound/trunk/bloodhound_relations/bhrelations/validation.py Fri May 17 07:35:48 2013
@@ -0,0 +1,143 @@
+#!/usr/bin/env python
+# -*- coding: UTF-8 -*-
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from trac.core import Component, implements, TracError
+from trac.resource import get_resource_shortname
+
+from bhrelations.api import IRelationValidator, RelationsSystem, \
+ ResourceIdSerializer
+
+
+class Validator(Component):
+ implements(IRelationValidator)
+
+ def validate(self, relation):
+ raise NotImplementedError
+
+ def render_relation_type(self, end):
+ return RelationsSystem(self.env)._labels[end]
+
+ def get_resource_name(self, resource_id):
+ resource = ResourceIdSerializer.get_resource_by_id(resource_id)
+ return get_resource_shortname(self.env, resource)
+
+
+class NoCyclesValidator(Validator):
+ def validate(self, relation):
+ """If a path exists from relation's destination to its source,
+ adding the relation will create a cycle.
+ """
+ path = self._find_path(relation.destination,
+ relation.source,
+ relation.type)
+ if path:
+ cycle_str = map(self.get_resource_name, path)
+ error = 'Cycle in ''%s'': %s' % (
+ self.render_relation_type(relation.type),
+ ' -> '.join(cycle_str))
+ error = ValidationError(error)
+ error.failed_ids = path
+ raise error
+
+ def _find_path(self, source, destination, relation_type):
+ known_nodes = set()
+ new_nodes = set([source])
+ paths = {(source, source): [source]}
+
+ while new_nodes:
+ known_nodes = set.union(known_nodes, new_nodes)
+ with self.env.db_query as db:
+ relations = dict(db("""
+ SELECT source, destination
+ FROM bloodhound_relations
+ WHERE type = '%(relation_type)s'
+ AND source IN (%(new_nodes)s)
+ """ % dict(
+ relation_type=relation_type,
+ new_nodes=', '.join("'%s'" % n for n in new_nodes))
+ ))
+ new_nodes = set(relations.values()) - known_nodes
+ for s, d in relations.items():
+ paths[(source, d)] = paths[(source, s)] + [d]
+ if destination in new_nodes:
+ return paths[(source, destination)]
+
+
+class ExclusiveValidator(Validator):
+ def validate(self, relation):
+ rls = RelationsSystem(self.env)
+ incompatible_relations = [
+ rel for rel in rls._select_relations(relation.source)
+ if rel.destination == relation.destination
+ ] + [
+ rel for rel in rls._select_relations(relation.destination)
+ if rel.destination == relation.source
+ ]
+ if incompatible_relations:
+ raise ValidationError(
+ "Relation %s is incompatible with the "
+ "following existing relations: %s" % (
+ self.render_relation_type(relation.type),
+ ','.join(map(str, incompatible_relations))
+ )
+ )
+
+
+class SingleProductValidator(Validator):
+ def validate(self, relation):
+ product1, product2 = map(self.get_product,
+ (relation.source, relation.destination))
+ if product1 != product2:
+ raise ValidationError(
+ "Resources for %s relation must belong to the same product." %
+ self.render_relation_type(relation.type)
+ )
+
+ def get_product(self, resource_id):
+ return ResourceIdSerializer.split_full_id(resource_id)[0]
+
+
+class NoSelfReferenceValidator(Validator):
+ def validate(self, relation):
+ if relation.source == relation.destination:
+ error = ValidationError(
+ 'Ticket cannot be self-referenced in a relation.')
+ error.failed_ids = [relation.source]
+ raise error
+
+
+class OneToManyValidator(Validator):
+ def validate(self, relation):
+ rls = RelationsSystem(self.env)
+ existing_relations = rls._select_relations(relation.source,
+ relation.type)
+ if existing_relations:
+ raise ValidationError(
+ "%s can only have one %s" % (
+ relation.destination,
+ self.render_relation_type(relation.type)
+ ))
+
+
+class ValidationError(TracError):
+ def __init__(self, message, title=None, show_traceback=False):
+ super(ValidationError, self).__init__(
+ message, title, show_traceback)
+ self.failed_ids = []
Modified: bloodhound/trunk/bloodhound_relations/setup.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_relations/setup.py?rev=1483676&r1=1483675&r2=1483676&view=diff
==============================================================================
--- bloodhound/trunk/bloodhound_relations/setup.py (original)
+++ bloodhound/trunk/bloodhound_relations/setup.py Fri May 17 07:35:48 2013
@@ -104,6 +104,7 @@ PKG_INFO = {'bhrelations': ('bhrelations
ENTRY_POINTS = {
'trac.plugins': [
'bhrelations.api = bhrelations.api',
+ 'bhrelations.validation = bhrelations.validation',
'bhrelations.web_ui = bhrelations.web_ui',
'bhrelations.widgets.ticketrelations = bhrelations.widgets.relations',
],
Modified: bloodhound/trunk/installer/bloodhound_setup.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/installer/bloodhound_setup.py?rev=1483676&r1=1483675&r2=1483676&view=diff
==============================================================================
--- bloodhound/trunk/installer/bloodhound_setup.py (original)
+++ bloodhound/trunk/installer/bloodhound_setup.py Fri May 17 07:35:48 2013
@@ -92,13 +92,14 @@ BASE_CONFIG = {'components': {'bhtheme.*
'bhrelations_links': {
'children.label': 'Child',
'dependency': 'dependson,dependent',
- 'dependency.validator': 'no_cycle',
+ 'dependency.validators': 'NoCycles,SingleProduct',
'dependent.blocks': 'true',
'dependson.label': 'Depends on',
'dependent.label': 'Dependent',
'oneway': 'refersto',
'parent_children': 'parent,children',
- 'parent_children.validator': 'parent_child',
+ 'parent_children.validators':
+ 'OneToMany,SingleProduct,NoCycles,Exclusive',
'refersto.label': 'Refers to',
},