# HG changeset patch # User Sylvain Thénault # Date 1374495993 -7200 # Node ID 0fb4b67bde58696b9dc4ee6a0b7d635bcf4e133d # Parent c05652b108cecc541b66fd979e90640eca56ae2e [schema/security] add __hash__ to rql expression. Closes #3013535 This is required so that when some rql expression participate to a dictionary key, only the expression is considered (consistent with comparison). This behaviour is expected in security at least, see the related bug for instance. This scramble msplanner unit tests a bit. diff -r c05652b108ce -r 0fb4b67bde58 schema.py --- a/schema.py Mon Jul 15 10:59:34 2013 +0200 +++ b/schema.py Mon Jul 22 14:26:33 2013 +0200 @@ -711,6 +711,9 @@ return self.expression == other.expression return False + def __hash__(self): + return hash(self.expression) + def __deepcopy__(self, memo): return self.__class__(self.expression, self.mainvars) def __getstate__(self): diff -r c05652b108ce -r 0fb4b67bde58 server/test/unittest_msplanner.py --- a/server/test/unittest_msplanner.py Mon Jul 15 10:59:34 2013 +0200 +++ b/server/test/unittest_msplanner.py Mon Jul 22 14:26:33 2013 +0200 @@ -801,10 +801,8 @@ [{'C': 'Division', 'E': 'Note', 'D': 'Affaire', 'G': 'SubDivision', 'F': 'Societe', 'I': 'Affaire', 'H': 'Affaire', 'J': 'Affaire', 'X': 'Affaire'}])], None, None, [self.system], {'E': 'table0.C0'}, []), ('OneFetchStep', - [('Any X WHERE X has_text "bla", EXISTS(X owned_by %s), X is Basket' % ueid, - [{'X': 'Basket'}]), - ('Any X WHERE X has_text "bla", EXISTS(X owned_by %s), X is CWUser' % ueid, - [{'X': 'CWUser'}]), + [('Any X WHERE X has_text "bla", EXISTS(X owned_by %s), X is IN(Basket, CWUser)' % ueid, + [{'X': 'Basket'}, {'X': 'CWUser'}]), ('Any X WHERE X has_text "bla", X is IN(Card, Comment, Division, Email, EmailThread, File, Folder, Note, Personne, Societe, SubDivision, Tag)', [{'X': 'Card'}, {'X': 'Comment'}, {'X': 'Division'}, {'X': 'Email'}, {'X': 'EmailThread'}, @@ -829,10 +827,8 @@ [{'C': 'Division', 'E': 'Note', 'D': 'Affaire', 'G': 'SubDivision', 'F': 'Societe', 'I': 'Affaire', 'H': 'Affaire', 'J': 'Affaire', 'X': 'Affaire'}])], [self.system], {'E': 'table1.C0'}, {'X': 'table0.C0'}, []), ('FetchStep', - [('Any X WHERE X has_text "bla", EXISTS(X owned_by %s), X is Basket' % ueid, - [{'X': 'Basket'}]), - ('Any X WHERE X has_text "bla", EXISTS(X owned_by %s), X is CWUser' % ueid, - [{'X': 'CWUser'}]), + [('Any X WHERE X has_text "bla", EXISTS(X owned_by %s), X is IN(Basket, CWUser)' % ueid, + [{'X': 'Basket'}, {'X': 'CWUser'}]), ('Any X WHERE X has_text "bla", X is IN(Card, Comment, Division, Email, EmailThread, File, Folder, Note, Personne, Societe, SubDivision, Tag)', [{'X': 'Card'}, {'X': 'Comment'}, {'X': 'Division'}, {'X': 'Email'}, {'X': 'EmailThread'}, @@ -909,12 +905,11 @@ self._test('Any MAX(X)', [('FetchStep', [('Any E WHERE E type "X", E is Note', [{'E': 'Note'}])], [self.cards, self.system], None, {'E': 'table1.C0'}, []), - ('FetchStep', [('Any X WHERE X is CWUser', [{'X': 'CWUser'}])], + ('FetchStep', [('Any X WHERE X is IN(CWUser)', [{'X': 'CWUser'}])], [self.ldap, self.system], None, {'X': 'table2.C0'}, []), ('UnionFetchStep', [ ('FetchStep', [('Any X WHERE EXISTS(%s use_email X), X is EmailAddress' % ueid, - [{'X': 'EmailAddress'}]), - ('Any X WHERE EXISTS(X owned_by %s), X is Basket' % ueid, [{'X': 'Basket'}])], + [{'X': 'EmailAddress'}])], [self.system], {}, {'X': 'table0.C0'}, []), ('UnionFetchStep', [('FetchStep', [('Any X WHERE X is IN(Card, Note, State)', @@ -942,11 +937,17 @@ {'X': 'Workflow'}, {'X': 'WorkflowTransition'}])], [self.system], {}, {'X': 'table0.C0'}, []), ]), - ('FetchStep', [('Any X WHERE EXISTS(X owned_by %s), X is CWUser' % ueid, [{'X': 'CWUser'}])], - [self.system], {'X': 'table2.C0'}, {'X': 'table0.C0'}, []), ('FetchStep', [('Any X WHERE (EXISTS(X owned_by %(ueid)s)) OR ((((EXISTS(D concerne C?, C owned_by %(ueid)s, C type "X", X identity D, C is Division, D is Affaire)) OR (EXISTS(H concerne G?, G owned_by %(ueid)s, G type "X", X identity H, G is SubDivision, H is Affaire))) OR (EXISTS(I concerne F?, F owned_by %(ueid)s, F type "X", X identity I, F is Societe, I is Affaire))) OR (EXISTS(J concerne E?, E owned_by %(ueid)s, X identity J, E is Note, J is Affaire))), X is Affaire' % {'ueid': ueid}, [{'C': 'Division', 'E': 'Note', 'D': 'Affaire', 'G': 'SubDivision', 'F': 'Societe', 'I': 'Affaire', 'H': 'Affaire', 'J': 'Affaire', 'X': 'Affaire'}])], [self.system], {'E': 'table1.C0'}, {'X': 'table0.C0'}, []), + ('UnionFetchStep', [ + ('FetchStep', [('Any X WHERE EXISTS(X owned_by %s), X is Basket' % ueid, + [{'X': 'Basket'}])], + [self.system], {}, {'X': 'table0.C0'}, []), + ('FetchStep', [('Any X WHERE EXISTS(X owned_by %s), X is CWUser' % ueid, + [{'X': 'CWUser'}])], + [self.system], {'X': 'table2.C0'}, {'X': 'table0.C0'}, []), + ]), ]), ('OneFetchStep', [('Any MAX(X)', ALL_SOLS)], None, None, [self.system], {'X': 'table0.C0'}, []) @@ -969,23 +970,13 @@ [self.cards, self.system], None, {'X': 'table1.C0'}, []), ('FetchStep', [('Any E WHERE E type "X", E is Note', [{'E': 'Note'}])], [self.cards, self.system], None, {'E': 'table2.C0'}, []), - ('FetchStep', [('Any X WHERE X is CWUser', [{'X': 'CWUser'}])], + ('FetchStep', [('Any X WHERE X is IN(CWUser)', [{'X': 'CWUser'}])], [self.ldap, self.system], None, {'X': 'table3.C0'}, []), ('UnionFetchStep', [('FetchStep', [('Any ET,X WHERE X is ET, EXISTS(%s use_email X), ET is CWEType, X is EmailAddress' % ueid, - [{'ET': 'CWEType', 'X': 'EmailAddress'}]), ('Any ET,X WHERE X is ET, EXISTS(X owned_by %s), ET is CWEType, X is Basket' % ueid, - [{'ET': 'CWEType', 'X': 'Basket'}])], + [{'ET': 'CWEType', 'X': 'EmailAddress'}]), + ], [self.system], {}, {'ET': 'table0.C0', 'X': 'table0.C1'}, []), - ('FetchStep', [('Any ET,X WHERE X is ET, (EXISTS(X owned_by %(ueid)s)) OR ((((EXISTS(D concerne C?, C owned_by %(ueid)s, C type "X", X identity D, C is Division, D is Affaire)) OR (EXISTS(H concerne G?, G owned_by %(ueid)s, G type "X", X identity H, G is SubDivision, H is Affaire))) OR (EXISTS(I concerne F?, F owned_by %(ueid)s, F type "X", X identity I, F is Societe, I is Affaire))) OR (EXISTS(J concerne E?, E owned_by %(ueid)s, X identity J, E is Note, J is Affaire))), ET is CWEType, X is Affaire' % {'ueid': ueid}, - [{'C': 'Division', 'E': 'Note', 'D': 'Affaire', - 'G': 'SubDivision', 'F': 'Societe', 'I': 'Affaire', - 'H': 'Affaire', 'J': 'Affaire', 'X': 'Affaire', - 'ET': 'CWEType'}])], - [self.system], {'E': 'table2.C0'}, {'ET': 'table0.C0', 'X': 'table0.C1'}, - []), - ('FetchStep', [('Any ET,X WHERE X is ET, EXISTS(X owned_by %s), ET is CWEType, X is CWUser' % ueid, - [{'ET': 'CWEType', 'X': 'CWUser'}])], - [self.system], {'X': 'table3.C0'}, {'ET': 'table0.C0', 'X': 'table0.C1'}, []), # extra UnionFetchStep could be avoided but has no cost, so don't care ('UnionFetchStep', [('FetchStep', [('Any ET,X WHERE X is ET, ET is CWEType, X is IN(BaseTransition, Bookmark, CWAttribute, CWCache, CWConstraint, CWConstraintType, CWEType, CWGroup, CWPermission, CWProperty, CWRType, CWRelation, CWSource, CWUniqueTogetherConstraint, Comment, Division, Email, EmailPart, EmailThread, ExternalUri, File, Folder, Old, Personne, RQLExpression, Societe, SubDivision, SubWorkflowExitPoint, Tag, TrInfo, Transition, Workflow, WorkflowTransition)', @@ -1018,6 +1009,22 @@ {'ET': 'CWEType', 'X': 'State'}])], [self.system], {'X': 'table1.C0'}, {'ET': 'table0.C0', 'X': 'table0.C1'}, []), ]), + + ('FetchStep', [('Any ET,X WHERE X is ET, (EXISTS(X owned_by %(ueid)s)) OR ((((EXISTS(D concerne C?, C owned_by %(ueid)s, C type "X", X identity D, C is Division, D is Affaire)) OR (EXISTS(H concerne G?, G owned_by %(ueid)s, G type "X", X identity H, G is SubDivision, H is Affaire))) OR (EXISTS(I concerne F?, F owned_by %(ueid)s, F type "X", X identity I, F is Societe, I is Affaire))) OR (EXISTS(J concerne E?, E owned_by %(ueid)s, X identity J, E is Note, J is Affaire))), ET is CWEType, X is Affaire' % {'ueid': ueid}, + [{'C': 'Division', 'E': 'Note', 'D': 'Affaire', + 'G': 'SubDivision', 'F': 'Societe', 'I': 'Affaire', + 'H': 'Affaire', 'J': 'Affaire', 'X': 'Affaire', + 'ET': 'CWEType'}])], + [self.system], {'E': 'table2.C0'}, {'ET': 'table0.C0', 'X': 'table0.C1'}, + []), + ('UnionFetchStep', [ + ('FetchStep', [('Any ET,X WHERE X is ET, EXISTS(X owned_by %s), ET is CWEType, X is Basket' % ueid, + [{'ET': 'CWEType', 'X': 'Basket'}])], + [self.system], {}, {'ET': 'table0.C0', 'X': 'table0.C1'}, []), + ('FetchStep', [('Any ET,X WHERE X is ET, EXISTS(X owned_by %s), ET is CWEType, X is CWUser' % ueid, + [{'ET': 'CWEType', 'X': 'CWUser'}])], + [self.system], {'X': 'table3.C0'}, {'ET': 'table0.C0', 'X': 'table0.C1'}, []), + ]), ]), ('OneFetchStep', [('Any ET,COUNT(X) GROUPBY ET ORDERBY ET', X_ET_ALL_SOLS)], diff -r c05652b108ce -r 0fb4b67bde58 test/unittest_rqlrewrite.py --- a/test/unittest_rqlrewrite.py Mon Jul 15 10:59:34 2013 +0200 +++ b/test/unittest_rqlrewrite.py Mon Jul 22 14:26:33 2013 +0200 @@ -1,4 +1,4 @@ -# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved. +# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved. # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr # # This file is part of CubicWeb. @@ -23,7 +23,7 @@ from cubicweb import Unauthorized, rqlrewrite from cubicweb.schema import RRQLExpression, ERQLExpression -from cubicweb.devtools import repotest, TestServerConfiguration +from cubicweb.devtools import repotest, TestServerConfiguration, BaseApptestConfiguration def setUpModule(*args): @@ -46,7 +46,8 @@ def eid_func_map(eid): return {1: 'CWUser', - 2: 'Card'}[eid] + 2: 'Card', + 3: 'Affaire'}[eid] def rewrite(rqlst, snippets_map, kwargs, existingvars=None): class FakeVReg: @@ -202,6 +203,17 @@ 'WITH LA BEING (Any LA WHERE (EXISTS(A created_by B, LA documented_by A)) OR (EXISTS(E created_by B, LA concerne E)), ' 'B eid %(D)s, LA is Affaire)') + + def test_ambiguous_optional_same_exprs(self): + """See #3013535""" + # see test of the same name in RewriteFullTC: original problem is + # unreproducible here because it actually lies in + # RQLRewriter.insert_local_checks + rqlst = parse('Any A,AR,X,CD WHERE A concerne X?, A ref AR, A eid %(a)s, X creation_date CD') + rewrite(rqlst, {('X', 'X'): ('X created_by U',),}, {'a': 3}) + self.assertEqual(rqlst.as_string(), + u'Any A,AR,X,CD WHERE A concerne X?, A ref AR, A eid %(a)s WITH X,CD BEING (Any X,CD WHERE X creation_date CD, EXISTS(X created_by B), B eid %(A)s, X is IN(Division, Note, Societe))') + def test_optional_var_inlined(self): c1 = ('X require_permission P') c2 = ('X inlined_card O, O require_permission P') @@ -292,6 +304,7 @@ self.assertEqual(rqlst.as_string(), "Any C WHERE C in_state STATE, C is Card, " "EXISTS(STATE name 'hop'), STATE is State") + def test_relation_optimization_3_rhs(self): snippet = ('TW? subworkflow_exit X, TW name "hop"') rqlst = parse('WorkflowTransition C WHERE C subworkflow_exit EXIT') @@ -308,6 +321,7 @@ self.assertEqual(rqlst.as_string(), "Any C WHERE C in_state STATE?, C is Card, " "EXISTS(C in_state A, A name 'hop', A is State), STATE is State") + def test_relation_non_optimization_1_rhs(self): snippet = ('TW subworkflow_exit X, TW name "hop"') rqlst = parse('SubWorkflowExitPoint EXIT WHERE C? subworkflow_exit EXIT') @@ -459,5 +473,37 @@ rqlst = parse('Any A, R WHERE A ref R, S is Affaire') rewrite(rqlst, {('A', 'X'): (c_ok, c_bad)}, {}) + +from cubicweb.devtools.testlib import CubicWebTC +from logilab.common.decorators import classproperty + +class RewriteFullTC(CubicWebTC): + @classproperty + def config(cls): + return BaseApptestConfiguration(apphome=cls.datapath('rewrite')) + + def process(self, rql, args=None): + if args is None: + args = {} + querier = self.repo.querier + union = querier.parse(rql) + querier.solutions(self.session, union, args) + querier._annotate(union) + plan = querier.plan_factory(union, args, self.session) + plan.preprocess(union) + return union + + def test_ambiguous_optional_same_exprs(self): + """See #3013535""" + edef1 = self.schema['Societe'] + edef2 = self.schema['Division'] + edef3 = self.schema['Note'] + with self.temporary_permissions((edef1, {'read': (ERQLExpression('X owned_by U'),)}), + (edef2, {'read': (ERQLExpression('X owned_by U'),)}), + (edef3, {'read': (ERQLExpression('X owned_by U'),)})): + union = self.process('Any A,AR,X,CD WHERE A concerne X?, A ref AR, X creation_date CD') + self.assertEqual(union.as_string(), 'Any A,AR,X,CD WHERE A concerne X?, A ref AR, A is Affaire WITH X,CD BEING (Any X,CD WHERE X creation_date CD, EXISTS(X owned_by %(A)s), X is IN(Division, Note, Societe))') + + if __name__ == '__main__': unittest_main()