From ca98b69301d14a7237c5db00f48c146278ffdb59 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 23 Aug 2024 14:53:49 +0200 Subject: [PATCH 01/26] Don't call _get_subst() when the result is not needed --- src/icat/query.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 975cc2e8..f5d50493 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -528,8 +528,8 @@ def where_clause(self): .. versionadded:: 0.21.0 """ - subst = self._get_subst() if self.conditions: + subst = self._get_subst() conds = [] for a in sorted(self.conditions.keys()): attr = self._dosubst(a, subst, False) @@ -545,8 +545,8 @@ def order_clause(self): .. versionadded:: 0.21.0 """ - subst = self._get_subst() if self.order: + subst = self._get_subst() orders = [] for a in self.order.keys(): orders.append(self.order[a] % self._dosubst(a, subst, False)) From c67d44a81e393d5f339d42eaa721d3b0eb12bd81 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 23 Aug 2024 16:06:43 +0200 Subject: [PATCH 02/26] Add helper classes OrderItem and ConditionItem to represent the items in the ORDER BY and the WHERE clause respectively --- src/icat/query.py | 150 +++++++++++++++++++++++++++++----------------- 1 file changed, 95 insertions(+), 55 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index f5d50493..238efb70 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -59,6 +59,78 @@ :meth:`icat.query.Query.setJoinSpecs` method. """ +# ========================= helper classes =========================== +# Note: these helpers are needed in the internal data structures in +# class Query. There are intentionally not included in the +# python-icat documentation and are not considered part of the API. +# There is no commitement on compatibility between versions. + +class ItemBase: + """Abstract base class for OrderItem and ConditionItem. + """ + + db_func_re = re.compile(r"(?:([A-Za-z_]+)\()?([A-Za-z.]+)(?(1)\))") + + def split_db_functs(self, attr): + m = self.db_func_re.fullmatch(attr) + if not m: + raise ValueError("Invalid attribute '%s'" % attr) + return m.group(2,1) + + +class OrderItem(ItemBase): + """Represent an item in the ORDER BY clause. + """ + + def __init__(self, obj, cloneFrom=None): + if cloneFrom: + self.attr = obj or cloneFrom.attr + self.jpql_func = cloneFrom.jpql_func + self.direction = cloneFrom.direction + else: + if isinstance(obj, tuple): + obj, direction = obj + if direction not in ("ASC", "DESC"): + raise ValueError("Invalid ordering direction '%s'" + % direction) + else: + direction = None + attr, jpql_func = self.split_db_functs(obj) + self.attr = attr + self.jpql_func = jpql_func + self.direction = direction + + @property + def formatstr(self): + if self.jpql_func: + if self.direction: + return "%s(%%s) %s" % (self.jpql_func, self.direction) + else: + return "%s(%%s)" % self.jpql_func + else: + if self.direction: + return "%%s %s" % self.direction + else: + return "%s" + + +class ConditionItem(ItemBase): + """Represent an item in the WHERE clause. + """ + def __init__(self, obj, rhs): + attr, jpql_func = self.split_db_functs(obj) + self.attr = attr + self.jpql_func = jpql_func + self.rhs = rhs + + @property + def formatstr(self): + rhs = self.rhs.replace('%', '%%') + if self.jpql_func: + return "%s(%%s) %s" % (self.jpql_func, rhs) + else: + return "%%s %s" % (rhs) + # ========================== class Query ============================= class Query(): @@ -106,8 +178,6 @@ class Query(): add the `join_specs` argument. """ - _db_func_re = re.compile(r"(?:([A-Za-z_]+)\()?([A-Za-z.]+)(?(1)\))") - def __init__(self, client, entity, attributes=None, aggregate=None, order=None, conditions=None, includes=None, limit=None, @@ -205,12 +275,6 @@ def _dosubst(self, obj, subst, addas=True): n += " AS %s" % (subst[obj]) return n - def _split_db_functs(self, attr): - m = self._db_func_re.fullmatch(attr) - if not m: - raise ValueError("Invalid attribute '%s'" % attr) - return m.group(2,1) - def _get_subst(self): if self._subst is None: joinattrs = ( set(self.order.keys()) | @@ -346,22 +410,16 @@ def setOrder(self, order): if order is True: for a in self.entity.getNaturalOrder(self.client): - self.order[a] = "%s" + item = OrderItem(a) + self.order[item.attr] = item elif order: for obj in order: - if isinstance(obj, tuple): - obj, direction = obj - if direction not in ("ASC", "DESC"): - raise ValueError("Invalid ordering direction '%s'" - % direction) - else: - direction = None - attr, jpql_func = self._split_db_functs(obj) + item = OrderItem(obj) - for (pattr, attrInfo, rclass) in self._attrpath(attr): + for (pattr, attrInfo, rclass) in self._attrpath(item.attr): if attrInfo.relType == "ONE": if (not attrInfo.notNullable and pattr not in self.conditions and @@ -375,33 +433,24 @@ def setOrder(self, order): warn(QueryOneToManyOrderWarning(pattr), stacklevel=sl) - if jpql_func: - if rclass is not None: - raise ValueError("Cannot apply a JPQL function " - "to a related object: %s" % obj) - if direction: - vstr = "%s(%%s) %s" % (jpql_func, direction) - else: - vstr = "%s(%%s)" % jpql_func - else: - if direction: - vstr = "%%s %s" % direction - else: - vstr = "%s" if rclass is None: - # attr is an attribute, use it right away. - if attr in self.order: - raise ValueError("Cannot add %s more than once" % attr) - self.order[attr] = vstr + # the item is an attribute, use it right away. + if item.attr in self.order: + raise ValueError("Cannot add %s more than once" % item.attr) + self.order[item.attr] = item else: # attr is a related object, use the natural order # of its class. + if item.jpql_func: + raise ValueError("Cannot apply a JPQL function " + "to a related object: %s" % obj) for ra in rclass.getNaturalOrder(self.client): - rattr = "%s.%s" % (attr, ra) + rattr = "%s.%s" % (item.attr, ra) if rattr in self.order: raise ValueError("Cannot add %s more than once" % rattr) - self.order[rattr] = vstr + ritem = OrderItem(rattr, cloneFrom=item) + self.order[ritem.attr] = ritem def addConditions(self, conditions): """Add conditions to the constraints to build the WHERE clause from. @@ -422,12 +471,6 @@ def addConditions(self, conditions): .. versionchanged:: 0.20.0 allow a JPQL function in the attribute. """ - def _cond_value(rhs, func): - rhs = rhs.replace('%', '%%') - if func: - return "%s(%%s) %s" % (func, rhs) - else: - return "%%s %s" % (rhs) if conditions: self._subst = None for k in conditions.keys(): @@ -435,14 +478,12 @@ def _cond_value(rhs, func): conds = [conditions[k]] else: conds = conditions[k] - a, jpql_func = self._split_db_functs(k) - for (pattr, attrInfo, rclass) in self._attrpath(a): - pass - v = [ _cond_value(rhs, jpql_func) for rhs in conds ] - if a in self.conditions: - self.conditions[a].extend(v) - else: - self.conditions[a] = v + for rhs in conds: + item = ConditionItem(k, rhs) + for (pattr, attrInfo, rclass) in self._attrpath(item.attr): + pass + l = self.conditions.setdefault(item.attr, []) + l.append(item) def addIncludes(self, includes): """Add related objects to build the INCLUDE clause from. @@ -534,7 +575,7 @@ def where_clause(self): for a in sorted(self.conditions.keys()): attr = self._dosubst(a, subst, False) for c in self.conditions[a]: - conds.append(c % attr) + conds.append(c.formatstr % attr) return "WHERE " + " AND ".join(conds) else: return None @@ -547,9 +588,8 @@ def order_clause(self): """ if self.order: subst = self._get_subst() - orders = [] - for a in self.order.keys(): - orders.append(self.order[a] % self._dosubst(a, subst, False)) + orders = [ self.order[a].formatstr % self._dosubst(a, subst, False) + for a in self.order.keys() ] return "ORDER BY " + ", ".join(orders) else: return None From 811de80cb59bf5ec849aff1a83aaba699c48888c Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 23 Aug 2024 18:27:28 +0200 Subject: [PATCH 03/26] Internally represent order items as a list rather than as a OrderedDict --- src/icat/query.py | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 238efb70..58cfee57 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -1,7 +1,6 @@ """Provide the Query class. """ -from collections import OrderedDict import re from warnings import warn from collections.abc import Mapping @@ -277,7 +276,7 @@ def _dosubst(self, obj, subst, addas=True): def _get_subst(self): if self._subst is None: - joinattrs = ( set(self.order.keys()) | + joinattrs = ( self._order_attrs | set(self.conditions.keys()) | set(self.attributes) ) self._subst = self._makesubst(joinattrs) @@ -403,15 +402,12 @@ def setOrder(self, order): allow a JPQL function in the attribute. """ self._subst = None - # Note: with Python 3.7 and newer we could simplify this using - # a standard dict() rather than an OrderedDict(). - self.order = OrderedDict() + self.order = [] if order is True: - for a in self.entity.getNaturalOrder(self.client): - item = OrderItem(a) - self.order[item.attr] = item + self.order = [ OrderItem(a) + for a in self.entity.getNaturalOrder(self.client) ] elif order: @@ -435,9 +431,7 @@ def setOrder(self, order): if rclass is None: # the item is an attribute, use it right away. - if item.attr in self.order: - raise ValueError("Cannot add %s more than once" % item.attr) - self.order[item.attr] = item + self.order.append(item) else: # attr is a related object, use the natural order # of its class. @@ -446,11 +440,7 @@ def setOrder(self, order): "to a related object: %s" % obj) for ra in rclass.getNaturalOrder(self.client): rattr = "%s.%s" % (item.attr, ra) - if rattr in self.order: - raise ValueError("Cannot add %s more than once" - % rattr) - ritem = OrderItem(rattr, cloneFrom=item) - self.order[ritem.attr] = ritem + self.order.append(OrderItem(rattr, cloneFrom=item)) def addConditions(self, conditions): """Add conditions to the constraints to build the WHERE clause from. @@ -519,6 +509,10 @@ def setLimit(self, limit): else: self.limit = None + @property + def _order_attrs(self): + return { item.attr for item in self.order } + @property def select_clause(self): """The SELECT clause of the query. @@ -588,8 +582,8 @@ def order_clause(self): """ if self.order: subst = self._get_subst() - orders = [ self.order[a].formatstr % self._dosubst(a, subst, False) - for a in self.order.keys() ] + orders = [ item.formatstr % self._dosubst(item.attr, subst, False) + for item in self.order ] return "ORDER BY " + ", ".join(orders) else: return None From 4dec2ed76c9206d14df2fb09c3068834a86ac269 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 23 Aug 2024 19:52:13 +0200 Subject: [PATCH 04/26] Move some code from the init methods of OrderItem and ConditionItem to ItemBase --- src/icat/query.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 58cfee57..2bc63b10 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -67,7 +67,6 @@ class ItemBase: """Abstract base class for OrderItem and ConditionItem. """ - db_func_re = re.compile(r"(?:([A-Za-z_]+)\()?([A-Za-z.]+)(?(1)\))") def split_db_functs(self, attr): @@ -76,6 +75,10 @@ def split_db_functs(self, attr): raise ValueError("Invalid attribute '%s'" % attr) return m.group(2,1) + def __init__(self, obj): + attr, jpql_func = self.split_db_functs(obj) + self.attr = attr + self.jpql_func = jpql_func class OrderItem(ItemBase): """Represent an item in the ORDER BY clause. @@ -94,9 +97,7 @@ def __init__(self, obj, cloneFrom=None): % direction) else: direction = None - attr, jpql_func = self.split_db_functs(obj) - self.attr = attr - self.jpql_func = jpql_func + super().__init__(obj) self.direction = direction @property @@ -117,9 +118,7 @@ class ConditionItem(ItemBase): """Represent an item in the WHERE clause. """ def __init__(self, obj, rhs): - attr, jpql_func = self.split_db_functs(obj) - self.attr = attr - self.jpql_func = jpql_func + super().__init__(obj) self.rhs = rhs @property From 9e27157b45004826afa71d317d1be6868bc9a3ef Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 23 Aug 2024 20:58:29 +0200 Subject: [PATCH 05/26] Internally represent conditions items as a list rather than as a dict --- src/icat/query.py | 77 +++++++++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 2bc63b10..418f2421 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -200,7 +200,7 @@ def __init__(self, client, entity, self.setAttributes(attributes) self.setAggregate(aggregate) - self.conditions = dict() + self.conditions = [] self.addConditions(conditions) self.includes = set() self.addIncludes(includes) @@ -276,7 +276,7 @@ def _dosubst(self, obj, subst, addas=True): def _get_subst(self): if self._subst is None: joinattrs = ( self._order_attrs | - set(self.conditions.keys()) | + self._conditions_attrs | set(self.attributes) ) self._subst = self._makesubst(joinattrs) return self._subst @@ -417,7 +417,7 @@ def setOrder(self, order): for (pattr, attrInfo, rclass) in self._attrpath(item.attr): if attrInfo.relType == "ONE": if (not attrInfo.notNullable and - pattr not in self.conditions and + pattr not in self._conditions_attrs and pattr not in self.join_specs): sl = 3 if self._init else 2 warn(QueryNullableOrderWarning(pattr), @@ -445,34 +445,45 @@ def addConditions(self, conditions): """Add conditions to the constraints to build the WHERE clause from. :param conditions: the conditions to restrict the search - result. This must be a mapping of attribute names to - conditions on that attribute. The latter may either be a - string with a single condition or a list of strings to add - more then one condition on a single attribute. The - attribute name (the key of the condition) can be wrapped - with a JPQL function (such as "UPPER(title)"). If the - query already has a condition on a given attribute, the - previous condition(s) will be retained and the new - condition(s) added to that. - :type conditions: :class:`dict` - :raise ValueError: if any key in `conditions` is not valid. + result. This must be a list of tuples with a pair of an + attribute name and a condition on that attribute + respectively. The attribute name may be wrapped with a + JPQL function (such as "UPPER(title)"). + + For backward compatibility with previous versions, this + may alternatively be a mapping of attribute names to a + (lists of) conditions. + :type conditions: :class:`list` of :class:`tuple` or :class:`dict` + :raise ValueError: if any attribute in `conditions` is not valid. .. versionchanged:: 0.20.0 allow a JPQL function in the attribute. + + .. versionchanged:: 2.0.0 + expect a :class:`list` of :class:`tuple` in the + `conditions` argument. + .. deprecated:: 2.0.0 + accept a :class:`dict` in the `conditions` argument. """ if conditions: self._subst = None - for k in conditions.keys(): - if isinstance(conditions[k], str): - conds = [conditions[k]] - else: - conds = conditions[k] - for rhs in conds: - item = ConditionItem(k, rhs) - for (pattr, attrInfo, rclass) in self._attrpath(item.attr): - pass - l = self.conditions.setdefault(item.attr, []) - l.append(item) + + if isinstance(conditions, Mapping): + # Convert the conditions argument to a list of tuples. + conds = [] + for obj,v in conditions.items(): + if isinstance(v, str): + conds.append( (obj,v) ) + else: + for rhs in v: + conds.append( (obj,rhs) ) + conditions = conds + + for obj,rhs in conditions: + item = ConditionItem(obj, rhs) + for (pattr, attrInfo, rclass) in self._attrpath(item.attr): + pass + self.conditions.append(item) def addIncludes(self, includes): """Add related objects to build the INCLUDE clause from. @@ -512,6 +523,10 @@ def setLimit(self, limit): def _order_attrs(self): return { item.attr for item in self.order } + @property + def _conditions_attrs(self): + return { item.attr for item in self.conditions } + @property def select_clause(self): """The SELECT clause of the query. @@ -564,11 +579,9 @@ def where_clause(self): """ if self.conditions: subst = self._get_subst() - conds = [] - for a in sorted(self.conditions.keys()): - attr = self._dosubst(a, subst, False) - for c in self.conditions[a]: - conds.append(c.formatstr % attr) + sortkey = lambda item: item.attr + conds = [ item.formatstr % self._dosubst(item.attr, subst, False) + for item in sorted(self.conditions, key=sortkey) ] return "WHERE " + " AND ".join(conds) else: return None @@ -645,9 +658,7 @@ def copy(self): q.attributes = list(self.attributes) q.aggregate = self.aggregate q.order = self.order.copy() - q.conditions = dict() - for k, v in self.conditions.items(): - q.conditions[k] = self.conditions[k].copy() + q.conditions = self.conditions.copy() q.includes = self.includes.copy() q.limit = self.limit q.join_specs = self.join_specs.copy() From 4d58e3dcc6b3e71b4daf881b24c0ca41a10a546e Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 23 Aug 2024 21:02:29 +0200 Subject: [PATCH 06/26] Typo in comment --- src/icat/query.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 418f2421..106eedf3 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -60,9 +60,9 @@ # ========================= helper classes =========================== # Note: these helpers are needed in the internal data structures in -# class Query. There are intentionally not included in the -# python-icat documentation and are not considered part of the API. -# There is no commitement on compatibility between versions. +# class Query. They are intentionally not included in the python-icat +# documentation and are not considered part of the API. There is no +# commitment on compatibility between versions. class ItemBase: """Abstract base class for OrderItem and ConditionItem. From 397c453a723c3b89ca7ac7e5a6a9ab95770f01ed Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Sat, 24 Aug 2024 15:24:47 +0200 Subject: [PATCH 07/26] Fix formal string representation operator Query.__repr__(), Ref. #94 --- src/icat/query.py | 53 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 106eedf3..cd3d9efc 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -80,6 +80,16 @@ def __init__(self, obj): self.attr = attr self.jpql_func = jpql_func + @property + def obj(self): + if self.jpql_func: + return "%s(%s)" % (self.jpql_func, self.attr) + else: + return self.attr + + def __repr__(self): + return repr(self.obj) + class OrderItem(ItemBase): """Represent an item in the ORDER BY clause. """ @@ -113,6 +123,13 @@ def formatstr(self): else: return "%s" + @property + def obj(self): + obj = super().obj + if self.direction: + obj = (obj, self.direction) + return obj + class ConditionItem(ItemBase): """Represent an item in the WHERE clause. @@ -129,6 +146,10 @@ def formatstr(self): else: return "%%s %s" % (rhs) + @property + def obj(self): + return (super().obj, self.rhs) + # ========================== class Query ============================= class Query(): @@ -629,14 +650,30 @@ def limit_clause(self): def __repr__(self): """Return a formal representation of the query. """ - return ("%s(%s, %s, attributes=%s, aggregate=%s, order=%s, " - "conditions=%s, includes=%s, limit=%s, join_specs=%s)" - % (self.__class__.__name__, - repr(self.client), repr(self.entity.BeanName), - repr(self.attributes), repr(self.aggregate), - repr(self.order), repr(self.conditions), - repr(self.includes), repr(self.limit), - repr(self.join_specs))) + kwargs = [] + if self.attributes: + kwargs.append("attributes=%s" % repr(self.attributes)) + if self.aggregate: + kwargs.append("aggregate=%s" % repr(self.aggregate)) + if self.order: + kwargs.append("order=%s" % repr(self.order)) + if self.conditions: + kwargs.append("conditions=%s" % repr(self.conditions)) + if self.includes: + kwargs.append("includes=%s" % repr(self.includes)) + if self.limit: + kwargs.append("limit=%s" % repr(self.limit)) + if self.join_specs: + kwargs.append("join_specs=%s" % repr(self.join_specs)) + if kwargs: + return ("%s(%s, %s, %s)" + % (self.__class__.__name__, + repr(self.client), repr(self.entity.BeanName), + ", ".join(kwargs))) + else: + return ("%s(%s, %s)" + % (self.__class__.__name__, + repr(self.client), repr(self.entity.BeanName))) def __str__(self): """Return a string representation of the query. From c6c0c52ebce5ee4cb3704f144f600e62391ba46b Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Sat, 24 Aug 2024 22:34:41 +0200 Subject: [PATCH 08/26] Minor documentation tweaks --- src/icat/query.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index cd3d9efc..2158e9e1 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -410,9 +410,7 @@ def setOrder(self, order): the latter being either "ASC" or "DESC" for ascending or descending order respectively. :type order: iterable or :class:`bool` - :raise ValueError: if any attribute in `order` is not valid or - if any attribute appears more than once in the resulting - ORDER BY clause. + :raise ValueError: if any attribute in `order` is not valid. .. versionchanged:: 0.19.0 allow one to many relationships in `order`. Emit a @@ -466,14 +464,15 @@ def addConditions(self, conditions): """Add conditions to the constraints to build the WHERE clause from. :param conditions: the conditions to restrict the search - result. This must be a list of tuples with a pair of an - attribute name and a condition on that attribute + result. This must be a list of tuples, each being a pair + of an attribute name and a condition on that attribute respectively. The attribute name may be wrapped with a JPQL function (such as "UPPER(title)"). For backward compatibility with previous versions, this may alternatively be a mapping of attribute names to a - (lists of) conditions. + (lists of) conditions. This legacy use is deprecated, + though. :type conditions: :class:`list` of :class:`tuple` or :class:`dict` :raise ValueError: if any attribute in `conditions` is not valid. From f5014cbdf19474b4c257a9521e176ef3915275e2 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Mon, 26 Aug 2024 17:33:33 +0200 Subject: [PATCH 09/26] Update the source code creating queries to use the new format of the conditions argument. Not yet covered: tests, documentation examples. --- src/icat/client.py | 14 +++++++------- src/icat/dump_queries.py | 12 ++++++------ src/icat/dumpfile_xml.py | 6 +++--- src/icat/entities.py | 19 +++++++++---------- src/scripts/wipeicat.py | 4 ++-- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/icat/client.py b/src/icat/client.py index 8c0cf2cf..48f84d19 100644 --- a/src/icat/client.py +++ b/src/icat/client.py @@ -549,9 +549,9 @@ def searchChunked(self, query, skip=0, count=None, chunksize=100): # Mark all datasets as complete # This will *not* work as expected! - query = Query(client, "Dataset", conditions={ - "complete": "= False" - }, includes="1", order=["id"]) + query = Query(client, "Dataset", conditions=[ + ("complete", "= False") + ], includes="1", order=["id"]) for ds in client.searchChunked(query): ds.complete = True ds.update() @@ -643,11 +643,11 @@ def searchUniqueKey(self, key, objindex=None): attr = f.name if f.relType == "ATTRIBUTE": cond = "= '%s'" % simpleqp_unquote(av[attr]) - query.addConditions({attr:cond}) + query.addConditions([(attr, cond)]) elif f.relType == "ONE": rk = str("%s_%s" % (f.type, av[attr])) ro = self.searchUniqueKey(rk, objindex) - query.addConditions({"%s.id" % attr:"= %d" % ro.id}) + query.addConditions([("%s.id" % attr, "= %d" % ro.id)]) else: raise ValueError("malformed '%s': invalid attribute '%s'" % (key, attr)) @@ -689,11 +689,11 @@ def searchMatching(self, obj, includes=None): if v is None: raise ValueError("%s is not set" % a) if a in obj.InstAttr: - query.addConditions({a: "= '%s'" % v}) + query.addConditions([(a, "= '%s'" % v)]) elif a in obj.InstRel: if v.id is None: raise ValueError("%s.id is not set" % a) - query.addConditions({"%s.id" % a: "= %d" % v.id}) + query.addConditions([("%s.id" % a, "= %d" % v.id)]) else: raise InternalError("Invalid constraint '%s' in %s." % (a, obj.BeanName)) diff --git a/src/icat/dump_queries.py b/src/icat/dump_queries.py index 7c8aa53b..1b6a4fa1 100644 --- a/src/icat/dump_queries.py +++ b/src/icat/dump_queries.py @@ -143,16 +143,16 @@ def getInvestigationQueries(client, invid): return [ Query(client, "Investigation", - conditions={"id": "= %d" % invid}, includes=inv_includes), + conditions=[("id", "= %d" % invid)], includes=inv_includes), Query(client, "Sample", order=["name"], - conditions={"investigation.id": "= %d" % invid}, + conditions=[("investigation.id", "= %d" % invid)], includes={"investigation", "type.facility", "parameters", "parameters.type.facility"}), Query(client, "Dataset", order=["name"], - conditions={"investigation.id": "= %d" % invid}, + conditions=[("investigation.id", "= %d" % invid)], includes=ds_includes), Query(client, "Datafile", order=["dataset.name", "name"], - conditions={"dataset.investigation.id": "= %d" % invid}, + conditions=[("dataset.investigation.id", "= %d" % invid)], includes={"dataset", "datafileFormat.facility", "parameters.type.facility"}) ] @@ -199,11 +199,11 @@ def getDataPublicationQueries(client, pubid): # ICAT >= 5.0.0 return [ Query(client, "DataPublication", order=True, - conditions={"id": "= %d" % pubid}, + conditions=[("id", "= %d" % pubid)], includes={"facility", "content", "type.facility", "dates", "fundingReferences.funding", "relatedItems"}), Query(client, "DataPublicationUser", order=True, - conditions={"publication.id": "= %d" % pubid}, + conditions=[("publication.id", "= %d" % pubid)], includes={"publication", "user", "affiliations"}), ] else: diff --git a/src/icat/dumpfile_xml.py b/src/icat/dumpfile_xml.py index 2373c4ea..7148d800 100644 --- a/src/icat/dumpfile_xml.py +++ b/src/icat/dumpfile_xml.py @@ -68,15 +68,15 @@ def _searchByReference(self, element, objtype, objindex): else: # object is referenced by attributes. attrs = set(element.keys()) - {'id'} - conditions = dict() + conditions = [] for attr in attrs: if attr.endswith(".ref"): ref = element.get(attr) robj = self.client.searchUniqueKey(ref, objindex) attr = "%s.id" % attr[:-4] - conditions[attr] = "= %d" % robj.id + conditions.append( (attr, "= %d" % robj.id) ) else: - conditions[attr] = "= '%s'" % element.get(attr) + conditions.append( (attr, "= '%s'" % element.get(attr)) ) query = Query(self.client, objtype, conditions=conditions) return self.client.assertedSearch(query)[0] diff --git a/src/icat/entities.py b/src/icat/entities.py index 7761aeef..84f7cb9e 100644 --- a/src/icat/entities.py +++ b/src/icat/entities.py @@ -20,6 +20,7 @@ from .entity import Entity from .exception import InternalError +from .query import Query class GroupingMixin: @@ -44,12 +45,11 @@ def getUsers(self, attribute=None): corresponding attribute for all users in the group, otherwise return the users. """ + query = Query(self.client, "User", conditions=[ + ("userGroups.grouping.id", "= %d" % self.id) + ]) if attribute is not None: - query = ("User.%s <-> UserGroup <-> %s [id=%d]" - % (attribute, self.BeanName, self.id)) - else: - query = ("User <-> UserGroup <-> %s [id=%d]" - % (self.BeanName, self.id)) + query.setAttributes(attribute) return self.client.search(query) @@ -72,12 +72,11 @@ def getInstrumentScientists(self, attribute=None): given, return the corresponding attribute for all users related to the instrument, otherwise return the users. """ + query = Query(self.client, "User", conditions=[ + ("instrumentScientists.instrument.id", "= %d" % self.id) + ]) if attribute is not None: - query = ("User.%s <-> InstrumentScientist <-> Instrument [id=%d]" - % (attribute, self.id)) - else: - query = ("User <-> InstrumentScientist <-> Instrument [id=%d]" - % (self.id)) + query.setAttributes(attribute) return self.client.search(query) diff --git a/src/scripts/wipeicat.py b/src/scripts/wipeicat.py index 4f6679ee..ffe8f1dc 100644 --- a/src/scripts/wipeicat.py +++ b/src/scripts/wipeicat.py @@ -62,7 +62,7 @@ def deleteobjs(query): # Delete all datafiles having location not set directly from ICAT # first, because they would cause trouble when we try to delete the # remaining datafiles from IDS, see Issue icatproject/ids.server#63. -deleteobjs(Query(client, "Datafile", conditions={"location": "IS NULL"})) +deleteobjs(Query(client, "Datafile", conditions=[("location", "IS NULL")])) # To delete datafiles from IDS, we must restore the datasets first, # because IDS can only delete datafiles that are online. But @@ -79,7 +79,7 @@ def deleteobjs(query): # without considering IDS. if client.ids: dfquery = Query(client, "Datafile", - conditions={"location": "IS NOT NULL"}, limit=(0, 1)) + conditions=[("location", "IS NOT NULL")], limit=(0, 1)) retriedDatasets = set() while True: deleteDatasets = [] From 2fc5f826de1322114e9ce0d93cf0d76425f9d01f Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Mon, 26 Aug 2024 18:27:13 +0200 Subject: [PATCH 10/26] Emit a deprecation warning when passing a mapping in the conditions argument to Query.addConditions() --- src/icat/query.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/icat/query.py b/src/icat/query.py index 2158e9e1..e1a3fb82 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -490,6 +490,10 @@ def addConditions(self, conditions): if isinstance(conditions, Mapping): # Convert the conditions argument to a list of tuples. + sl = 3 if self._init else 2 + warn("Passing a mapping in the conditions argument is " + "deprecated and will be removed in python-icat 3.0.", + DeprecationWarning, stacklevel=sl) conds = [] for obj,v in conditions.items(): if isinstance(v, str): From e47ae77380e9409b5980167cf393d662841782d0 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Mon, 26 Aug 2024 19:19:34 +0200 Subject: [PATCH 11/26] Update the test_06_query.py to use the new format of the conditions argument when creating queries. Not yet done: verify that evaluating the formal string representation of Query works as expected, add tests for the legacy use of a mapping and verify that the deprecation warning is indeed being raised. --- tests/test_06_query.py | 147 +++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 71 deletions(-) diff --git a/tests/test_06_query.py b/tests/test_06_query.py index cc76c200..53c7ecac 100644 --- a/tests/test_06_query.py +++ b/tests/test_06_query.py @@ -45,7 +45,9 @@ def test_query_simple(client): # The investigation is reused in other tests. global investigation name = "10100601-ST" - query = Query(client, "Investigation", conditions={"name":"= '%s'" % name}) + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % name) + ]) print(str(query)) assert "Investigation" in query.select_clause assert query.join_clause is None @@ -67,11 +69,11 @@ def test_query_datafile(client): 'dataset': "e208945", 'investigation': "12100409-ST" } - conditions = { - "name": "= '%s'" % dfdata['name'], - "dataset.name": "= '%s'" % dfdata['dataset'], - "dataset.investigation.name": "= '%s'" % dfdata['investigation'], - } + conditions = [ + ("name", "= '%s'" % dfdata['name']), + ("dataset.name", "= '%s'" % dfdata['dataset']), + ("dataset.investigation.name", "= '%s'" % dfdata['investigation']), + ] query = Query(client, "Datafile", conditions=conditions) print(str(query)) assert "Datafile" in query.select_clause @@ -88,11 +90,11 @@ def test_query_datafile(client): assert df.name == dfdata['name'] # Same example, but use placeholders in the query string now. - conditions = { - "name": "= '%(name)s'", - "dataset.name": "= '%(dataset)s'", - "dataset.investigation.name": "= '%(investigation)s'", - } + conditions = [ + ("name", "= '%(name)s'"), + ("dataset.name", "= '%(dataset)s'"), + ("dataset.investigation.name", "= '%(investigation)s'"), + ] query = Query(client, "Datafile", conditions=conditions) print(str(query)) print(str(query) % dfdata) @@ -112,7 +114,7 @@ def test_query_investigation_includes(client): "investigationGroups.grouping", "parameters", "parameters.type.facility" } query = Query(client, "Investigation", - conditions={"id": "= %d" % investigation.id}, + conditions=[("id", "= %d" % investigation.id)], includes=includes) print(str(query)) assert "Investigation" in query.select_clause @@ -137,8 +139,8 @@ def test_query_instruments(client): """ query = Query(client, "Instrument", order=["name"], - conditions={ "investigationInstruments.investigation.id": - "= %d" % investigation.id }, + conditions=[ ("investigationInstruments.investigation.id", + "= %d" % investigation.id) ], includes={"facility", "instrumentScientists.user"}) print(str(query)) assert "Instrument" in query.select_clause @@ -157,8 +159,8 @@ def test_query_datafile_by_investigation(client): """The datafiles related to a given investigation in natural order. """ query = Query(client, "Datafile", order=True, - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }, + conditions=[( "dataset.investigation.id", + "= %d" % investigation.id )], includes={"dataset", "datafileFormat.facility", "parameters.type.facility"}) print(str(query)) @@ -226,8 +228,8 @@ def test_query_order_direction(client): # Try without specifying the ordering direction first: query = Query(client, "Datafile", order=["name"], - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[( "dataset.investigation.id", + "= %d" % investigation.id )]) print(str(query)) assert "Datafile" in query.select_clause assert "investigation" in query.join_clause @@ -238,23 +240,23 @@ def test_query_order_direction(client): # Ascending order is the default, so we should get the same result: query = Query(client, "Datafile", order=[("name", "ASC")], - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[( "dataset.investigation.id", + "= %d" % investigation.id )]) print(str(query)) assert client.search(query) == res # Descending order should give the reverse result: query = Query(client, "Datafile", order=[("name", "DESC")], - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[( "dataset.investigation.id", + "= %d" % investigation.id )]) print(str(query)) assert list(reversed(client.search(query))) == res # We may even combine different ordering directions on multiple # attributes of the same query: query = Query(client, "Datafile", order=[("dataset.name", "DESC"), ("name", "ASC")], - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[( "dataset.investigation.id", + "= %d" % investigation.id )]) print(str(query)) assert sorted(client.search(query), key=lambda df: df.name) == res @@ -288,7 +290,7 @@ def test_query_order_direction_relation(client): def test_query_condition_greaterthen(client): """Other relations then equal may be used in the conditions too. """ - condition = {"datafileCreateTime": ">= '2012-01-01'"} + condition = [("datafileCreateTime", ">= '2012-01-01'")] query = Query(client, "Datafile", conditions=condition) print(str(query)) assert "Datafile" in query.select_clause @@ -296,16 +298,19 @@ def test_query_condition_greaterthen(client): assert "datafileCreateTime" in query.where_clause res = client.search(query) assert len(res) == 4 + have_icat_5 - condition = {"datafileCreateTime": "< '2012-01-01'"} + condition = [("datafileCreateTime", "< '2012-01-01'")] query = Query(client, "Datafile", conditions=condition) print(str(query)) res = client.search(query) assert len(res) == 6 -def test_query_condition_list(client): - """We may also add a list of conditions on a single attribute. +def test_query_multiple_conditions(client): + """We may also add multiple conditions on a single attribute. """ - condition = {"datafileCreateTime": [">= '2012-01-01'", "< '2013-01-01'"]} + condition = [ + ("datafileCreateTime", ">= '2012-01-01'"), + ("datafileCreateTime", "< '2013-01-01'"), + ] query = Query(client, "Datafile", conditions=condition) print(str(query)) assert "Datafile" in query.select_clause @@ -317,8 +322,8 @@ def test_query_condition_list(client): # The last example also works by adding the conditions separately. query = Query(client, "Datafile") - query.addConditions({"datafileCreateTime": ">= '2012-01-01'"}) - query.addConditions({"datafileCreateTime": "< '2013-01-01'"}) + query.addConditions([("datafileCreateTime", ">= '2012-01-01'")]) + query.addConditions([("datafileCreateTime", "< '2013-01-01'")]) print(str(query)) assert str(query) == qstr res = client.search(query) @@ -330,7 +335,7 @@ def test_query_in_operator(client): (This may be needed to work around ICAT Issue 128.) """ query = Query(client, "Investigation", - conditions={"id": "in (%d)" % investigation.id}) + conditions=[("id", "in (%d)" % investigation.id)]) print(str(query)) assert "Investigation" in query.select_clause assert query.join_clause is None @@ -346,7 +351,7 @@ def test_query_condition_obj(client): """We may place conditions on related objects. This is in particular useful to test whether a relation is null. """ - query = Query(client, "Rule", conditions={"grouping": "IS NULL"}) + query = Query(client, "Rule", conditions=[("grouping", "IS NULL")]) print(str(query)) assert "Rule" in query.select_clause res = client.search(query) @@ -357,10 +362,10 @@ def test_query_condition_jpql_function(client): This test also applies `UPPER()` on the data to mitigate instances of Oracle databases which are case sensitive. """ - conditions = { - "UPPER(title)": "like UPPER('%Ni-Mn-Ga flat cone%')", - "UPPER(datasets.name)": "like UPPER('%e208341%')", - } + conditions = [ + ("UPPER(title)", "like UPPER('%Ni-Mn-Ga flat cone%')"), + ("UPPER(datasets.name)", "like UPPER('%e208341%')"), + ] query = Query(client, "Investigation", conditions=conditions) print(str(query)) assert "Investigation" in query.select_clause @@ -375,8 +380,8 @@ def test_query_condition_jpql_function_namelen(client): of the JPQL function in the condition is easier to verify in the result. """ - conditions = { "name": "LIKE 'db/%'", - "LENGTH(fullName)": "> 11" } + conditions = [ ("name", "LIKE 'db/%'"), + ("LENGTH(fullName)", "> 11") ] query = Query(client, "User", conditions=conditions) print(str(query)) assert "User" in query.select_clause @@ -390,8 +395,8 @@ def test_query_condition_jpql_function_mixed(client): This test case failed for an early implementation of JPQL functions, see discussion in #89. """ - conditions = { "name": "LIKE 'db/%'", - "LENGTH(fullName)": "> 11", "fullName": "> 'C'" } + conditions = [ ("name", "LIKE 'db/%'"), + ("LENGTH(fullName)", "> 11"), ("fullName", "> 'C'") ] query = Query(client, "User", conditions=conditions) print(str(query)) assert "User" in query.select_clause @@ -407,7 +412,7 @@ def test_query_order_jpql_function(client): fullName. (In the example data, the longest and second longest fullName is somewhat ambiguous due to character encoding issues.) """ - query = Query(client, "User", conditions={ "name": "LIKE 'db/%'" }, + query = Query(client, "User", conditions=[ ("name", "LIKE 'db/%'") ], order=[("LENGTH(fullName)", "DESC")], limit=(2,1)) print(str(query)) assert "User" in query.select_clause @@ -454,7 +459,7 @@ def test_query_rule_order_group_suppress_warn_cond(client, recwarn): """ recwarn.clear() query = Query(client, "Rule", order=['grouping', 'what', 'id'], - conditions={"grouping": "IS NOT NULL"}) + conditions=[("grouping", "IS NOT NULL")]) assert len(recwarn.list) == 0 print(str(query)) assert "Rule" in query.select_clause @@ -610,7 +615,7 @@ def test_query_limit(client): """Add a LIMIT clause to an earlier example. """ query = Query(client, "Rule", order=['grouping', 'what', 'id'], - conditions={"grouping":"IS NOT NULL"}) + conditions=[("grouping", "IS NOT NULL")]) query.setLimit( (0,10) ) print(str(query)) assert "Rule" in query.select_clause @@ -625,7 +630,7 @@ def test_query_limit_placeholder(client): """LIMIT clauses are particular useful with placeholders. """ query = Query(client, "Rule", order=['grouping', 'what', 'id'], - conditions={"grouping":"IS NOT NULL"}) + conditions=[("grouping", "IS NOT NULL")]) query.setLimit( ("%d","%d") ) chunksize = 45 print(str(query)) @@ -651,7 +656,7 @@ def test_query_non_ascii(client): # by both Python 2 and Python 3. fullName = b'Rudolph Beck-D\xc3\xbclmen'.decode('utf8') query = Query(client, "User", - conditions={ "fullName": "= '%s'" % fullName }) + conditions=[ ("fullName", "= '%s'" % fullName) ]) print(str(query)) assert "User" in query.select_clause assert query.join_clause is None @@ -671,8 +676,8 @@ def test_query_str(client): effects at all. It was fixed in changes 4688517 and 905dd8c. """ query = Query(client, "Datafile", order=True, - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }, + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ], includes={"dataset", "datafileFormat.facility", "parameters.type.facility"}) r = repr(query) @@ -686,7 +691,7 @@ def test_query_str(client): def test_query_metaattr(client): """Test adding a condition on a meta attribute. Issue #6 """ - query = Query(client, "Datafile", conditions={ "modId": "= 'jdoe'" }) + query = Query(client, "Datafile", conditions=[ ("modId", "= 'jdoe'") ]) print(str(query)) assert "Datafile" in query.select_clause assert query.join_clause is None @@ -719,8 +724,8 @@ def test_query_attribute_datafile_name(client): added in Issue #28. """ query = Query(client, "Datafile", attributes="name", order=True, - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ]) print(str(query)) assert "name" in query.select_clause assert "investigation" in query.join_clause @@ -739,8 +744,8 @@ def test_query_attribute_datafile_name_list(client): single element. """ query = Query(client, "Datafile", attributes=["name"], order=True, - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ]) print(str(query)) assert "name" in query.select_clause assert "investigation" in query.join_clause @@ -759,8 +764,8 @@ def test_query_related_obj_attribute(client): """ require_icat_version("4.5.0", "SELECT related object's attribute") query = Query(client, "Datafile", attributes="datafileFormat.name", - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ]) print(str(query)) assert "name" in query.select_clause assert "datafileFormat" in query.join_clause @@ -807,7 +812,7 @@ def test_query_mulitple_attributes_related_obj(client): ("10100601-ST", "e208342")] query = Query(client, "Dataset", attributes=("investigation.name", "name"), order=True, - conditions={"investigation.startDate": "< '2011-01-01'"}) + conditions=[("investigation.startDate", "< '2011-01-01'")]) print(str(query)) assert "name" in query.select_clause assert "investigation" in query.join_clause @@ -841,12 +846,12 @@ def test_query_mulitple_attributes_distinct(client): # Try the query without DISTINCT first so that we can verify the effect. query = Query(client, "InvestigationUser", attributes=("investigation.name", "role"), - conditions={"investigation.name": "= '08100122-EF'"}) + conditions=[("investigation.name", "= '08100122-EF'")]) print(str(query)) res = client.search(query) query = Query(client, "InvestigationUser", attributes=("investigation.name", "role"), - conditions={"investigation.name": "= '08100122-EF'"}, + conditions=[("investigation.name", "= '08100122-EF'")], aggregate="DISTINCT") print(str(query)) assert "DISTINCT" in query.select_clause @@ -869,8 +874,8 @@ def test_query_aggregate_distinct_attribute(client): require_icat_version("4.7.0", "SELECT DISTINCT in queries") query = Query(client, "Datafile", attributes="datafileFormat.name", - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ]) print(str(query)) res = client.search(query) assert sorted(res) == ["NeXus", "NeXus", "other", "other"] @@ -893,8 +898,8 @@ def test_query_aggregate_distinct_related_obj(client): require_icat_version("4.7.0", "SELECT DISTINCT in queries") query = Query(client, "Datafile", attributes="datafileFormat", - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ]) print(str(query)) res = client.search(query) assert len(res) == 4 @@ -948,8 +953,8 @@ def test_query_aggregate_misc(client, attribute, aggregate, expected): require_icat_version("4.7.0", "SELECT DISTINCT in queries") query = Query(client, "Datafile", attributes=attribute, aggregate=aggregate, - conditions={ "dataset.investigation.id": - "= %d" % investigation.id }) + conditions=[ ("dataset.investigation.id", + "= %d" % investigation.id) ]) print(str(query)) if ':' not in aggregate: assert aggregate in query.select_clause @@ -965,18 +970,18 @@ def test_query_aggregate_misc(client, attribute, aggregate, expected): ("Datafile", dict(attributes="name", order=True)), ("InvestigationUser", dict(attributes=("investigation.name", "role"), - conditions={"investigation.name": "= '08100122-EF'"}, + conditions=[("investigation.name", "= '08100122-EF'")], aggregate="DISTINCT")), ("Datafile", dict(order=[("name", "ASC")])), - ("Datafile", dict(conditions={ - "name": "= 'e208945.nxs'", - "dataset.name": "= 'e208945'", - "dataset.investigation.name": "= '12100409-ST'", - })), + ("Datafile", dict(conditions=[ + ("name", "= 'e208945.nxs'"), + ("dataset.name", "= 'e208945'"), + ("dataset.investigation.name", "= '12100409-ST'"), + ])), ("Instrument", dict(order=["name"], includes={"facility", "instrumentScientists.user"})), ("Rule", dict(order=['grouping', 'what', 'id'], - conditions={"grouping":"IS NOT NULL"}, + conditions=[("grouping", "IS NOT NULL")], limit=(0,10))), ("Rule", dict(order=['grouping', 'what', 'id'], join_specs={"grouping": "LEFT OUTER JOIN"})), From ada4100828df00d3c04362c32af126ca9c52848c Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Tue, 27 Aug 2024 16:50:57 +0200 Subject: [PATCH 12/26] - Update all tests to use the new format of the conditions argument when creating queries - While we are at it: eliminate a few more occurences of the old legacy ICAT query syntax in the tests --- tests/conftest.py | 4 +- tests/test_05_setup.py | 106 +++++++++++++++++++----------------- tests/test_06_client.py | 22 ++++---- tests/test_06_icatingest.py | 28 +++++----- tests/test_06_ids.py | 36 +++++++----- tests/test_06_ingest.py | 44 +++++++-------- 6 files changed, 127 insertions(+), 113 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 104901da..db20222c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -282,8 +282,8 @@ def cleanup_objs(rootclient): # uploaded (i.e. the location is not NULL), need to # delete it from IDS first. query = Query(rootclient, "Datafile", - conditions={"dataset.id": "= %d" % obj.id, - "location": "IS NOT NULL"}) + conditions=[("dataset.id", "= %d" % obj.id), + ("location", "IS NOT NULL")]) rootclient.deleteData(rootclient.search(query)) rootclient.delete(obj) diff --git a/tests/test_05_setup.py b/tests/test_05_setup.py index 230884c0..57f62ab7 100644 --- a/tests/test_05_setup.py +++ b/tests/test_05_setup.py @@ -67,24 +67,24 @@ def initobj(obj, attrs): setattr(obj, a, attrs[a]) def get_datafile(client, df): - query = Query(client, "Datafile", conditions={ - "name":"= '%s'" % df['name'], - "dataset.name":"= '%s'" % df['dataset'], - "dataset.investigation.name":"= '%s'" % df['investigation'] - }) + query = Query(client, "Datafile", conditions=[ + ("name", "= '%s'" % df['name']), + ("dataset.name", "= '%s'" % df['dataset']), + ("dataset.investigation.name", "= '%s'" % df['investigation']) + ]) return client.assertedSearch(query)[0] def create_datafile(client, data, df): - query = Query(client, "Dataset", conditions={ - "name":"= '%s'" % df['dataset'], - "investigation.name":"= '%s'" % df['investigation'] - }) + query = Query(client, "Dataset", conditions=[ + ("name", "= '%s'" % df['dataset']), + ("investigation.name", "= '%s'" % df['investigation']), + ]) dataset = client.assertedSearch(query)[0] dff = data['datafile_formats'][df['format']] - query = Query(client, "DatafileFormat", conditions={ - "name":"= '%s'" % dff['name'], - "version":"= '%s'" % dff['version'], - }) + query = Query(client, "DatafileFormat", conditions=[ + ("name", "= '%s'" % dff['name']), + ("version", "= '%s'" % dff['version']), + ]) datafile_format = client.assertedSearch(query)[0] datafile = client.new("Datafile") initobj(datafile, df) @@ -95,8 +95,10 @@ def create_datafile(client, data, df): param = client.new("DatafileParameter") initobj(param, p) ptdata = data['parameter_types'][p['type']] - query = ("ParameterType [name='%s' AND units='%s']" - % (ptdata['name'], ptdata['units'])) + query = Query(client, "ParameterType", conditions=[ + ("name", "= '%s'" % ptdata['name']), + ("units", "= '%s'" % ptdata['units']), + ]) param.type = client.assertedSearch(query)[0] datafile.parameters.append(param) datafile.create() @@ -109,26 +111,26 @@ def fix_file_size(inv_name): # to fix in Investigation and Dataset. Nothing to do. return client.login(conf.auth, conf.credentials) - inv_query = Query(client, "Investigation", conditions={ - "name":"= '%s'" % inv_name - }, includes="1") + inv_query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % inv_name), + ], includes="1") inv = client.assertedSearch(inv_query)[0] inv.fileCount = 0 inv.fileSize = 0 - ds_query = Query(client, "Dataset", conditions={ - "investigation.id": "= %d" % inv.id - }, includes="1") + ds_query = Query(client, "Dataset", conditions=[ + ("investigation.id", "= %d" % inv.id), + ], includes="1") for ds in client.search(ds_query): - fileCount_query = Query(client, "Datafile", conditions={ - "dataset.id": "= %d" % ds.id - }, aggregate="COUNT") + fileCount_query = Query(client, "Datafile", conditions=[ + ("dataset.id", "= %d" % ds.id), + ], aggregate="COUNT") ds.fileCount = int(client.assertedSearch(fileCount_query)[0]) if not ds.fileCount: ds.fileSize = 0 else: - fileSize_query = Query(client, "Datafile", conditions={ - "dataset.id": "= %d" % ds.id - }, attributes="fileSize", aggregate="SUM") + fileSize_query = Query(client, "Datafile", conditions=[ + ("dataset.id", "= %d" % ds.id), + ], attributes="fileSize", aggregate="SUM") ds.fileSize = int(client.assertedSearch(fileSize_query)[0]) ds.update() inv.fileCount += ds.fileCount @@ -231,10 +233,14 @@ def test_add_study(data, user, studyname): studydata = data['studies'][studyname] study = client.new("Study") initobj(study, studydata) - query = "User [name='%s']" % data['users'][studydata['user']]['name'] + query = Query(client, "User", conditions=[ + ("name", "= '%s'" % data['users'][studydata['user']]['name']), + ]) study.user = client.assertedSearch(query)[0] for invname in studydata['investigations']: - query = "Investigation [name='%s']" % invname + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % invname), + ]) si = client.new("StudyInvestigation") si.investigation = client.assertedSearch(query)[0] study.studyInvestigations.append(si) @@ -251,7 +257,9 @@ def test_add_publication(data, user, pubname): pubdata = data['publications'][pubname] publication = client.new("Publication") initobj(publication, pubdata) - query = "Investigation [name='%s']" % pubdata['investigation'] + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % pubdata['investigation']), + ]) publication.investigation = client.assertedSearch(query)[0] publication.create() @@ -269,13 +277,13 @@ def test_add_data_publication(data, pubname): client.login(conf.auth, conf.credentials) content = client.new("DataCollection") ds = pubdata['dataset'] - query = Query(client, "Investigation", conditions={ - "name":"= '%s'" % ds['investigation'] - }) + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % ds['investigation']), + ]) investigation = client.assertedSearch(query)[0] - query = Query(client, "DatasetType", conditions={ - "name":"= '%s'" % data['dataset_types'][ds['type']]['name'] - }) + query = Query(client, "DatasetType", conditions=[ + ("name", "= '%s'" % data['dataset_types'][ds['type']]['name']), + ]) dataset_type = client.assertedSearch(query)[0] dataset = client.new("Dataset") initobj(dataset, ds) @@ -283,10 +291,10 @@ def test_add_data_publication(data, pubname): dataset.type = dataset_type for df in ds['datafiles']: dff = data['datafile_formats'][df['format']] - query = Query(client, "DatafileFormat", conditions={ - "name":"= '%s'" % dff['name'], - "version":"= '%s'" % dff['version'], - }) + query = Query(client, "DatafileFormat", conditions=[ + ("name", "= '%s'" % dff['name']), + ("version", "= '%s'" % dff['version']), + ]) datafile_format = client.assertedSearch(query)[0] datafile = client.new("Datafile") initobj(datafile, df) @@ -307,16 +315,16 @@ def test_add_data_publication(data, pubname): client.login(conf.auth, conf.credentials) data_publication = client.new("DataPublication") initobj(data_publication, pubdata) - query = Query(client, "Facility", conditions={ - "name": "= '%s'" % data['facilities'][pubdata['facility']]['name'] - }) + query = Query(client, "Facility", conditions=[ + ("name", "= '%s'" % data['facilities'][pubdata['facility']]['name']), + ]) data_publication.facility = client.assertedSearch(query)[0] data_publication.content = content if pubdata['type']: t = data['data_publication_types'][pubdata['type']] - query = Query(client, "DataPublicationType", conditions={ - "name": "= '%s'" % t['name'] - }) + query = Query(client, "DataPublicationType", conditions=[ + ("name", "= '%s'" % t['name']), + ]) data_publication.type = client.assertedSearch(query)[0] for d in pubdata['dates']: data_publication.dates.append(client.new("DataPublicationDate", **d)) @@ -325,9 +333,9 @@ def test_add_data_publication(data, pubname): for u in pubdata['users']: pub_user = client.new("DataPublicationUser") initobj(pub_user, u) - query = Query(client, "User", conditions={ - "name": "= '%s'" % data['users'][u['user']]['name'] - }) + query = Query(client, "User", conditions=[ + ("name", "= '%s'" % data['users'][u['user']]['name']) + ]) pub_user.user = client.assertedSearch(query)[0] for a in u['affiliations']: pub_user.affiliations.append(client.new("Affiliation", **a)) diff --git a/tests/test_06_client.py b/tests/test_06_client.py index 8702316a..34295afc 100644 --- a/tests/test_06_client.py +++ b/tests/test_06_client.py @@ -203,11 +203,11 @@ def test_assertedSearch_unique_mulitple_fields(client): ), pytest.param(lambda client: Query(client, "User"), id="query_user"), pytest.param( - lambda client: Query(client, "User", order=True, conditions={ - "userGroups.grouping.investigationGroups.role": "= 'writer'", - "userGroups.grouping.investigationGroups.investigation.name": - "= '08100122-EF'" - }), + lambda client: Query(client, "User", order=True, conditions=[ + ("userGroups.grouping.investigationGroups.role", "= 'writer'"), + ("userGroups.grouping.investigationGroups.investigation.name", + "= '08100122-EF'") + ]), id="query_writer" ), ]) @@ -272,9 +272,9 @@ def test_searchChunked_limit(client, skip, count, chunksize): id="jpql" ), pytest.param( - lambda client: Query(client, "User", order=True, conditions={ - "name": "LIKE 'j%'", - }), + lambda client: Query(client, "User", order=True, conditions=[ + ("name", "LIKE 'j%'"), + ]), id="query" ), ]) @@ -313,7 +313,7 @@ def test_searchChunked_id(client, query): (9901ec6). """ refq = Query(client, "Investigation", attributes="id", limit=(0,1), - conditions={"name": "= '08100122-EF'"}) + conditions=[("name", "= '08100122-EF'")]) id = client.assertedSearch(refq)[0] # The search by id must return exactly one result. The broken # version returns the same object over and over again in an @@ -330,7 +330,7 @@ def test_searchChunked_limit_bug(client): repeatedly yield the same object in an endless loop. """ facility = client.assertedSearch("Facility")[0] - query = Query(client, "Facility", conditions={"id": "= %d" % facility.id}) + query = Query(client, "Facility", conditions=[("id", "= %d" % facility.id)]) count = 0 for f in client.searchChunked(query): count += 1 @@ -346,7 +346,7 @@ def test_searchChunked_limit_bug_chunksize(client): chunksize. Ref. #57. """ facility = client.assertedSearch("Facility")[0] - query = Query(client, "Facility", conditions={"id": "= %d" % facility.id}) + query = Query(client, "Facility", conditions=[("id", "= %d" % facility.id)]) count = 0 for f in client.searchChunked(query, chunksize=1): count += 1 diff --git a/tests/test_06_icatingest.py b/tests/test_06_icatingest.py index be7e138a..d23d6139 100644 --- a/tests/test_06_icatingest.py +++ b/tests/test_06_icatingest.py @@ -61,7 +61,7 @@ def dataset(client, cleanup_objs): def verify_dataset_params(client, dataset, params): query = Query(client, "DatasetParameter", - conditions={"dataset.id": "= %d" % dataset.id}, + conditions=[("dataset.id", "= %d" % dataset.id)], includes={"type"}) ps = client.search(query) assert len(ps) == len(params) @@ -297,10 +297,10 @@ def test_ingest_datafiles(tmpdirsec, client, dataset, cmdargs): # Verify that the datafiles have been created but not uploaded. dataset = client.searchMatching(dataset) for fname in dummyfiles: - query = Query(client, "Datafile", conditions={ - "name": "= '%s'" % fname, - "dataset.id": "= %d" % dataset.id, - }) + query = Query(client, "Datafile", conditions=[ + ("name", "= '%s'" % fname), + ("dataset.id", "= %d" % dataset.id), + ]) df = client.assertedSearch(query)[0] assert df.location is None @@ -322,10 +322,10 @@ def test_ingest_datafiles_upload(tmpdirsec, client, dataset, cmdargs): # Verify that the datafiles have been uploaded. dataset = client.searchMatching(dataset) for f in dummyfiles: - query = Query(client, "Datafile", conditions={ - "name": "= '%s'" % f.name, - "dataset.id": "= %d" % dataset.id, - }) + query = Query(client, "Datafile", conditions=[ + ("name", "= '%s'" % f.name), + ("dataset.id", "= %d" % dataset.id), + ]) df = client.assertedSearch(query)[0] assert df.location is not None assert df.fileSize == f.size @@ -344,7 +344,7 @@ def test_ingest_dataset_samples(client, cleanup_objs, cmdargs): "e209004": None, } inv = client.assertedSearch("Investigation [name='10100601-ST']")[0] - query = Query(client, "SampleType", conditions={"name": "= 'NiMnGa'"}) + query = Query(client, "SampleType", conditions=[("name", "= 'NiMnGa'")]) sample_type = client.assertedSearch(query)[0] ds_type = client.assertedSearch("DatasetType [name='raw']")[0] for name in ds_sample_map.keys(): @@ -363,10 +363,10 @@ def test_ingest_dataset_samples(client, cleanup_objs, cmdargs): args = cmdargs + ["-i", sample_ds] callscript("icatingest.py", args) for ds_name, sample_name in ds_sample_map.items(): - query = Query(client, "Dataset", conditions={ - "investigation.id": "= %d" % inv.id, - "name": "= '%s'" % ds_name, - }, includes=["sample"]) + query = Query(client, "Dataset", conditions=[ + ("investigation.id", "= %d" % inv.id), + ("name", "= '%s'" % ds_name), + ], includes=["sample"]) ds = client.assertedSearch(query)[0] if sample_name: assert ds.sample diff --git a/tests/test_06_ids.py b/tests/test_06_ids.py index d9c42ed1..79ba844d 100644 --- a/tests/test_06_ids.py +++ b/tests/test_06_ids.py @@ -119,17 +119,17 @@ def client(setupicat): # ============================= helper ============================= def getDataset(client, case): - query = Query(client, "Dataset", conditions={ - "investigation.name": "= '%s'" % case['invname'], - "name": "= '%s'" % case['dsname'], - }) + query = Query(client, "Dataset", conditions=[ + ("investigation.name", "= '%s'" % case['invname']), + ("name", "= '%s'" % case['dsname']), + ]) return (client.assertedSearch(query)[0]) def getDatafileFormat(client, case): l = case['dfformat'].split(':') - query = Query(client, "DatafileFormat", conditions={ - "name": "= '%s'" % l[0], - }) + query = Query(client, "DatafileFormat", conditions=[ + ("name", "= '%s'" % l[0]), + ]) if len(l) > 1: query.addConditions({"version": "= '%s'" % l[1]}) return (client.assertedSearch(query)[0]) @@ -137,11 +137,11 @@ def getDatafileFormat(client, case): def getDatafile(client, case, dfname=None): if dfname is None: dfname = case['dfname'] - query = Query(client, "Datafile", conditions={ - "name": "= '%s'" % dfname, - "dataset.name": "= '%s'" % case['dsname'], - "dataset.investigation.name": "= '%s'" % case['invname'], - }) + query = Query(client, "Datafile", conditions=[ + ("name", "= '%s'" % dfname), + ("dataset.name", "= '%s'" % case['dsname']), + ("dataset.investigation.name", "= '%s'" % case['invname']), + ]) return (client.assertedSearch(query)[0]) def copyfile(infile, outfile, chunksize=8192): @@ -187,7 +187,9 @@ def test_download(tmpdirsec, client, case, method): zfname = tmpdirsec / ("%s.zip" % case['dsname']) print("\nDownload %s to file %s" % (case['dsname'], zfname)) dataset = getDataset(tclient, case) - query = "Datafile <-> Dataset [id=%d]" % dataset.id + query = Query(client, "Datafile", conditions=[ + ("dataset.id", "= %d" % dataset.id) + ]) datafiles = tclient.search(query) if method == 'getData': response = tclient.getData(datafiles) @@ -215,7 +217,9 @@ def test_download(tmpdirsec, client, case, method): dfname = tmpdirsec / ("dl_%s" % df['dfname']) print("\nDownload %s to file %s" % (case['dsname'], dfname)) dataset = getDataset(tclient, case) - query = "Datafile <-> Dataset [id=%d]" % dataset.id + query = Query(client, "Datafile", conditions=[ + ("dataset.id", "= %d" % dataset.id) + ]) datafiles = tclient.search(query) if method == 'getData': response = tclient.getData(datafiles) @@ -292,7 +296,9 @@ def test_getDatafileIds(client, case): selection = DataSelection([ds]) dfids = client.ids.getDatafileIds(selection) print("Datafile ids of dataset %s: %s" % (case['dsname'], str(dfids))) - query = "Datafile.id <-> Dataset [id=%d]" % ds.id + query = Query(client, "Datafile", attributes=["id"], conditions=[ + ("dataset.id", "= %d" % ds.id) + ]) assert set(dfids) == set(client.search(query)) def test_putData_datafileCreateTime(tmpdirsec, client): diff --git a/tests/test_06_ingest.py b/tests/test_06_ingest.py index baa0c73c..bed59389 100644 --- a/tests/test_06_ingest.py +++ b/tests/test_06_ingest.py @@ -18,9 +18,9 @@ logger = logging.getLogger(__name__) def get_test_investigation(client): - query = Query(client, "Investigation", conditions={ - "name": "= '12100409-ST'", - }) + query = Query(client, "Investigation", conditions=[ + ("name", "= '12100409-ST'"), + ]) return client.assertedSearch(query)[0] class NamedBytesIO(io.BytesIO): @@ -38,9 +38,9 @@ def client(setupicat): def samples(rootclient): """Create some samples that are referenced in some of the ingest files. """ - query = Query(rootclient, "SampleType", conditions={ - "name": "= 'Nickel(II) oxide SC'" - }) + query = Query(rootclient, "SampleType", conditions=[ + ("name", "= 'Nickel(II) oxide SC'") + ]) st = rootclient.assertedSearch(query)[0] inv = get_test_investigation(rootclient) samples = [] @@ -55,10 +55,10 @@ def samples(rootclient): def investigation(client, cleanup_objs): inv = get_test_investigation(client) yield inv - query = Query(client, "Dataset", conditions={ - "investigation.id": "= %d" % inv.id, - "name": "LIKE 'testingest_%'", - }) + query = Query(client, "Dataset", conditions=[ + ("investigation.id", "= %d" % inv.id), + ("name", "LIKE 'testingest_%'"), + ]) cleanup_objs.extend(client.search(query)) @pytest.fixture(scope="function") @@ -372,10 +372,10 @@ def test_ingest(client, investigation, samples, schemadir, case): ds.create() reader.ingest(datasets) for name in case.checks.keys(): - query = Query(client, "Dataset", conditions={ - "name": "= '%s'" % name, - "investigation.id": "= %d" % investigation.id, - }) + query = Query(client, "Dataset", conditions=[ + ("name", "= '%s'" % name), + ("investigation.id", "= %d" % investigation.id), + ]) ds = client.assertedSearch(query)[0] for query, res in case.checks[name]: assert client.assertedSearch(query % ds.id)[0] == res @@ -433,10 +433,10 @@ def test_ingest_fileobj(client, investigation, samples, schemadir, case): ds.create() reader.ingest(datasets) for name in case.checks.keys(): - query = Query(client, "Dataset", conditions={ - "name": "= '%s'" % name, - "investigation.id": "= %d" % investigation.id, - }) + query = Query(client, "Dataset", conditions=[ + ("name", "= '%s'" % name), + ("investigation.id", "= %d" % investigation.id), + ]) ds = client.assertedSearch(query)[0] for query, res in case.checks[name]: assert client.assertedSearch(query % ds.id)[0] == res @@ -697,10 +697,10 @@ def test_custom_ingest(client, investigation, samples, schemadir, case): ds.create() reader.ingest(datasets) for name in case.checks.keys(): - query = Query(client, "Dataset", conditions={ - "name": "= '%s'" % name, - "investigation.id": "= %d" % investigation.id, - }) + query = Query(client, "Dataset", conditions=[ + ("name", "= '%s'" % name), + ("investigation.id", "= %d" % investigation.id), + ]) ds = client.assertedSearch(query)[0] for query, res in case.checks[name]: assert client.assertedSearch(query % ds.id)[0] == res From 50a10630106a2c6f41b663cf7eca5ff49ae075ba Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Tue, 27 Aug 2024 18:40:04 +0200 Subject: [PATCH 13/26] Add checks to verify that evaluating the formal string representation operator of Query may indeed be used to create an equivalent query to the tests in test_06_query.py where it makes sense --- tests/test_06_query.py | 72 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/tests/test_06_query.py b/tests/test_06_query.py index 53c7ecac..4ad7e64f 100644 --- a/tests/test_06_query.py +++ b/tests/test_06_query.py @@ -18,6 +18,20 @@ def client(setupicat): return client +def verify_rebuild_query(client, query, res): + """Verify that we can rebuild an equivalent query (e.g. one that + yields the same search result) out of the formal string + representation of a given query + """ + qstr = repr(query) + client_str = repr(client) + assert qstr.find(client_str) >= 0 + qstr = qstr.replace(client_str, 'client') + rep_query = eval(qstr) + rep_res = client.search(rep_query) + assert rep_res == res + + # Note: the number of objects returned in the queries and their # attributes obviously depend on the content of the ICAT and need to # be kept in sync with the reference input used in the setupicat @@ -60,6 +74,7 @@ def test_query_simple(client): investigation = res[0] assert investigation.BeanName == "Investigation" assert investigation.name == name + verify_rebuild_query(client, query, res) def test_query_datafile(client): """Query a datafile by its name, dataset name, and investigation name. @@ -88,6 +103,7 @@ def test_query_datafile(client): df = res[0] assert df.BeanName == "Datafile" assert df.name == dfdata['name'] + verify_rebuild_query(client, query, res) # Same example, but use placeholders in the query string now. conditions = [ @@ -132,6 +148,7 @@ def test_query_investigation_includes(client): assert len(inv.investigationInstruments) > 0 assert len(inv.investigationUsers) > 0 assert len(inv.investigationGroups) > 0 + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_instruments(client): @@ -153,6 +170,7 @@ def test_query_instruments(client): instr = res[0] assert instr.BeanName == "Instrument" assert instr.facility.BeanName == "Facility" + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_datafile_by_investigation(client): @@ -170,6 +188,7 @@ def test_query_datafile_by_investigation(client): assert "datafileFormat" in query.include_clause res = client.search(query) assert len(res) == 4 + verify_rebuild_query(client, query, res) def test_query_relateddatafile(client): """RelatedDatafile is the entity type with the most complicated @@ -184,6 +203,7 @@ def test_query_relateddatafile(client): assert query.limit_clause is None res = client.search(query) assert len(res) == 1 + verify_rebuild_query(client, query, res) def test_query_datacollection(client): """There is no sensible order for DataCollection, fall back to id. @@ -198,6 +218,7 @@ def test_query_datacollection(client): assert query.limit_clause is None res = client.search(query) assert len(res) == 3 + 2*have_icat_5 + verify_rebuild_query(client, query, res) def test_query_datafiles_datafileformat(client, recwarn): """Datafiles ordered by format. @@ -218,6 +239,7 @@ def test_query_datafiles_datafileformat(client, recwarn): assert query.limit_clause is None res = client.search(query) assert len(res) == 10 + have_icat_5 + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_order_direction(client): @@ -237,6 +259,7 @@ def test_query_order_direction(client): assert "name" in query.order_clause res = client.search(query) assert len(res) == 4 + verify_rebuild_query(client, query, res) # Ascending order is the default, so we should get the same result: query = Query(client, "Datafile", order=[("name", "ASC")], @@ -244,13 +267,16 @@ def test_query_order_direction(client): "= %d" % investigation.id )]) print(str(query)) assert client.search(query) == res + verify_rebuild_query(client, query, res) # Descending order should give the reverse result: query = Query(client, "Datafile", order=[("name", "DESC")], conditions=[( "dataset.investigation.id", "= %d" % investigation.id )]) print(str(query)) - assert list(reversed(client.search(query))) == res + rev_res = client.search(query) + assert list(reversed(rev_res)) == res + verify_rebuild_query(client, query, rev_res) # We may even combine different ordering directions on multiple # attributes of the same query: query = Query(client, "Datafile", @@ -258,7 +284,9 @@ def test_query_order_direction(client): conditions=[( "dataset.investigation.id", "= %d" % investigation.id )]) print(str(query)) - assert sorted(client.search(query), key=lambda df: df.name) == res + mix_res = client.search(query) + assert sorted(mix_res, key=lambda df: df.name) == res + verify_rebuild_query(client, query, mix_res) def test_query_order_direction_relation(client): """An ordering direction qualifier on a many to one relation. @@ -269,6 +297,7 @@ def test_query_order_direction_relation(client): # datafiles in their natural order: query = Query(client, "Dataset", order=True, includes=["datafiles"]) dss = client.search(query) + verify_rebuild_query(client, query, dss) # Now, get all datafiles sorted by dataset in descending and name # in ascending order: query = Query(client, "Datafile", order=[("dataset", "DESC"), "name"]) @@ -278,6 +307,7 @@ def test_query_order_direction_relation(client): assert query.where_clause is None assert "name" in query.order_clause dff = client.search(query) + verify_rebuild_query(client, query, dff) # verify: offs = 0 for ds in reversed(dss): @@ -298,11 +328,13 @@ def test_query_condition_greaterthen(client): assert "datafileCreateTime" in query.where_clause res = client.search(query) assert len(res) == 4 + have_icat_5 + verify_rebuild_query(client, query, res) condition = [("datafileCreateTime", "< '2012-01-01'")] query = Query(client, "Datafile", conditions=condition) print(str(query)) res = client.search(query) assert len(res) == 6 + verify_rebuild_query(client, query, res) def test_query_multiple_conditions(client): """We may also add multiple conditions on a single attribute. @@ -319,6 +351,7 @@ def test_query_multiple_conditions(client): qstr = str(query) res = client.search(query) assert len(res) == 3 + have_icat_5 + verify_rebuild_query(client, query, res) # The last example also works by adding the conditions separately. query = Query(client, "Datafile") @@ -328,6 +361,7 @@ def test_query_multiple_conditions(client): assert str(query) == qstr res = client.search(query) assert len(res) == 3 + have_icat_5 + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_in_operator(client): @@ -346,6 +380,7 @@ def test_query_in_operator(client): assert inv.BeanName == "Investigation" assert inv.id == investigation.id assert inv.name == investigation.name + verify_rebuild_query(client, query, res) def test_query_condition_obj(client): """We may place conditions on related objects. @@ -356,6 +391,7 @@ def test_query_condition_obj(client): assert "Rule" in query.select_clause res = client.search(query) assert len(res) == all_rules - grp_rules + verify_rebuild_query(client, query, res) def test_query_condition_jpql_function(client): """Functions may be applied to field names of conditions. @@ -373,6 +409,7 @@ def test_query_condition_jpql_function(client): assert "UPPER" in query.where_clause res = client.search(query) assert len(res) == 1 + verify_rebuild_query(client, query, res) def test_query_condition_jpql_function_namelen(client): """Functions may be applied to field names of conditions. @@ -389,6 +426,7 @@ def test_query_condition_jpql_function_namelen(client): assert "LENGTH" in query.where_clause res = client.search(query) assert len(res) == 4 + verify_rebuild_query(client, query, res) def test_query_condition_jpql_function_mixed(client): """Mix conditions with and without JPQL function on the same attribute. @@ -404,6 +442,7 @@ def test_query_condition_jpql_function_mixed(client): assert "LENGTH" in query.where_clause res = client.search(query) assert len(res) == 3 + verify_rebuild_query(client, query, res) def test_query_order_jpql_function(client): """Functions may be applied to attribute names in order. @@ -422,6 +461,7 @@ def test_query_order_jpql_function(client): res = client.search(query) assert len(res) == 1 assert res[0].fullName == "Nicolas Bourbaki" + verify_rebuild_query(client, query, res) def test_query_rule_order(client): """Rule does not have a constraint, id is included in the natural order. @@ -434,6 +474,7 @@ def test_query_rule_order(client): assert "id" in query.order_clause res = client.search(query) assert len(res) == all_rules + verify_rebuild_query(client, query, res) def test_query_rule_order_group(client, recwarn): """Ordering rule on grouping implicitely adds a "grouping IS NOT NULL" @@ -453,6 +494,7 @@ def test_query_rule_order_group(client, recwarn): assert "what" in query.order_clause res = client.search(query) assert len(res) == grp_rules + verify_rebuild_query(client, query, res) def test_query_rule_order_group_suppress_warn_cond(client, recwarn): """The warning can be suppressed by making the condition explicit. @@ -468,6 +510,7 @@ def test_query_rule_order_group_suppress_warn_cond(client, recwarn): assert "what" in query.order_clause res = client.search(query) assert len(res) == grp_rules + verify_rebuild_query(client, query, res) def test_query_rule_order_group_suppress_warn_join(client, recwarn): """Another option to suppress the warning is to override the JOIN spec. @@ -485,6 +528,7 @@ def test_query_rule_order_group_suppress_warn_join(client, recwarn): assert "what" in query.order_clause res = client.search(query) assert len(res) == grp_rules + verify_rebuild_query(client, query, res) def test_query_rule_order_group_left_join(client, recwarn): """Another option to suppress the warning is to override the JOIN spec. @@ -501,6 +545,7 @@ def test_query_rule_order_group_left_join(client, recwarn): assert "what" in query.order_clause res = client.search(query) assert len(res) == all_rules + verify_rebuild_query(client, query, res) def test_query_order_one_to_many(client, recwarn): """Sort on a related object in a one to many relation. @@ -519,6 +564,7 @@ def test_query_order_one_to_many(client, recwarn): assert "fullName" in query.order_clause res = client.search(query) assert len(res) == 3 + verify_rebuild_query(client, query, res) def test_query_order_one_to_many_warning_suppressed(client, recwarn): """Again, the warning can be suppressed by overriding the JOIN spec. @@ -536,6 +582,7 @@ def test_query_order_one_to_many_warning_suppressed(client, recwarn): assert "fullName" in query.order_clause res = client.search(query) assert len(res) == 3 + verify_rebuild_query(client, query, res) def test_query_order_one_to_many_duplicate(client, recwarn): """Note that sorting on a one to many relation may have surprising @@ -550,6 +597,7 @@ def test_query_order_one_to_many_duplicate(client, recwarn): print(str(query)) res = client.search(query) assert len(res) == 3 + verify_rebuild_query(client, query, res) reference = res # The same query adding a ORDER BY clause, we get two duplicates in # the result. @@ -562,6 +610,7 @@ def test_query_order_one_to_many_duplicate(client, recwarn): res = client.search(query) assert len(res) > 3 assert set(res) == set(reference) + verify_rebuild_query(client, query, res) def test_query_order_one_to_many_missing(client, recwarn): """Note that sorting on a one to many relation may have surprising @@ -576,6 +625,7 @@ def test_query_order_one_to_many_missing(client, recwarn): print(str(query)) res = client.search(query) assert len(res) == 3 + verify_rebuild_query(client, query, res) reference = res # The same query adding a ORDER BY clause, one item, a sample # having no parameter with a string value, is missing from the result. @@ -587,6 +637,7 @@ def test_query_order_one_to_many_missing(client, recwarn): print(str(query)) res = client.search(query) assert len(res) == 2 + verify_rebuild_query(client, query, res) # We can fix it in this case using a LEFT JOIN. recwarn.clear() query = Query(client, "Sample", @@ -597,6 +648,7 @@ def test_query_order_one_to_many_missing(client, recwarn): res = client.search(query) assert len(res) == 3 assert set(res) == set(reference) + verify_rebuild_query(client, query, res) def test_query_order_suppress_warnings(client, recwarn): """Suppress all QueryWarnings. @@ -610,6 +662,7 @@ def test_query_order_suppress_warnings(client, recwarn): print(str(query)) res = client.search(query) assert len(res) == 3 + verify_rebuild_query(client, query, res) def test_query_limit(client): """Add a LIMIT clause to an earlier example. @@ -625,6 +678,7 @@ def test_query_limit(client): assert query.limit_clause is not None res = client.search(query) assert len(res) == 10 + verify_rebuild_query(client, query, res) def test_query_limit_placeholder(client): """LIMIT clauses are particular useful with placeholders. @@ -663,6 +717,7 @@ def test_query_non_ascii(client): assert fullName in query.where_clause res = client.search(query) assert len(res) == 1 + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_str(client): @@ -698,6 +753,7 @@ def test_query_metaattr(client): assert "modId" in query.where_clause res = client.search(query) assert len(res) == 0 + verify_rebuild_query(client, query, res) def test_query_include_1(client): """Test adding an "INCLUDE 1" clause. @@ -715,6 +771,7 @@ def test_query_include_1(client): assert inv.BeanName == "Investigation" assert inv.facility.BeanName == "Facility" assert inv.type.BeanName == "InvestigationType" + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_attribute_datafile_name(client): @@ -735,6 +792,7 @@ def test_query_attribute_datafile_name(client): assert len(res) == 4 for n in res: assert not isinstance(n, icat.entity.Entity) + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_attribute_datafile_name_list(client): @@ -755,6 +813,7 @@ def test_query_attribute_datafile_name_list(client): assert len(res) == 4 for n in res: assert not isinstance(n, icat.entity.Entity) + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_related_obj_attribute(client): @@ -775,6 +834,7 @@ def test_query_related_obj_attribute(client): assert len(res) == 4 for n in res: assert n in ['other', 'NeXus'] + verify_rebuild_query(client, query, res) def test_query_mulitple_attributes(client): """Query multiple attributes in the SELECT clause. @@ -798,6 +858,7 @@ def test_query_mulitple_attributes(client): assert query.order_clause is not None res = client.search(query) assert res == results + verify_rebuild_query(client, query, res) def test_query_mulitple_attributes_related_obj(client): """Query multiple attributes including attributes of related objects. @@ -820,6 +881,7 @@ def test_query_mulitple_attributes_related_obj(client): assert query.order_clause is not None res = client.search(query) assert res == results + verify_rebuild_query(client, query, res) def test_query_mulitple_attributes_oldicat_valueerror(client): """Query class should raise ValueError if multiple attributes are @@ -849,6 +911,7 @@ def test_query_mulitple_attributes_distinct(client): conditions=[("investigation.name", "= '08100122-EF'")]) print(str(query)) res = client.search(query) + verify_rebuild_query(client, query, res) query = Query(client, "InvestigationUser", attributes=("investigation.name", "role"), conditions=[("investigation.name", "= '08100122-EF'")], @@ -863,6 +926,7 @@ def test_query_mulitple_attributes_distinct(client): # duplicates, the result set is the same: assert len(res) > len(res_dist) assert set(res) == set(res_dist) + verify_rebuild_query(client, query, res_dist) @pytest.mark.dependency(depends=['get_investigation']) def test_query_aggregate_distinct_attribute(client): @@ -887,6 +951,7 @@ def test_query_aggregate_distinct_attribute(client): assert "id" in query.where_clause res = client.search(query) assert sorted(res) == ["NeXus", "other"] + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) def test_query_aggregate_distinct_related_obj(client): @@ -905,6 +970,7 @@ def test_query_aggregate_distinct_related_obj(client): assert len(res) == 4 for n in res: assert isinstance(n, icat.entity.Entity) + verify_rebuild_query(client, query, res) query.setAggregate("DISTINCT") print(str(query)) assert "DISTINCT" in query.select_clause @@ -915,6 +981,7 @@ def test_query_aggregate_distinct_related_obj(client): assert len(res) == 2 for n in res: assert isinstance(n, icat.entity.Entity) + verify_rebuild_query(client, query, res) @pytest.mark.dependency(depends=['get_investigation']) @pytest.mark.parametrize(("attribute", "aggregate", "expected"), [ @@ -965,6 +1032,7 @@ def test_query_aggregate_misc(client, attribute, aggregate, expected): res = client.search(query) assert len(res) == 1 assert res[0] == expected + verify_rebuild_query(client, query, res) @pytest.mark.parametrize(("entity", "kwargs"), [ ("Datafile", dict(attributes="name", order=True)), From 383c97578ebd8f31f4590aba8fbaafc14093f58b Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Wed, 28 Aug 2024 10:40:15 +0200 Subject: [PATCH 14/26] Add tests for the legacy use of passing a mapping in the conditions argument in class Query. Also verify that a DeprecationWarning is raised. --- tests/test_06_query.py | 144 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/test_06_query.py b/tests/test_06_query.py index 4ad7e64f..a46f1676 100644 --- a/tests/test_06_query.py +++ b/tests/test_06_query.py @@ -1067,3 +1067,147 @@ def test_query_copy(client, entity, kwargs): clone = query.copy() assert str(clone) == str(query) assert entity in clone.select_clause + +def test_query_legacy_conditions_simple(client): + """A simple query for an investigation by name. + Same as test_query_simple(), but pass the conditions as a mapping. + Deprecated, transitionally supported for backward compatibility. + """ + # The investigation is reused in other tests. + global investigation + name = "10100601-ST" + with pytest.deprecated_call(): + query = Query(client, "Investigation", conditions={ + "name": "= '%s'" % name + }) + print(str(query)) + assert "Investigation" in query.select_clause + assert query.join_clause is None + assert "name" in query.where_clause + assert query.order_clause is None + assert query.include_clause is None + assert query.limit_clause is None + res = client.search(query) + assert len(res) == 1 + investigation = res[0] + assert investigation.BeanName == "Investigation" + assert investigation.name == name + +def test_query_legacy_conditions_datafile(client): + """Query a datafile by its name, dataset name, and investigation name. + Same as test_query_datafile(), but pass the conditions as a + mapping. Deprecated, transitionally supported for backward + compatibility. + """ + dfdata = { + 'name': "e208945.nxs", + 'dataset': "e208945", + 'investigation': "12100409-ST", + } + conditions = { + "name": "= '%s'" % dfdata['name'], + "dataset.name": "= '%s'" % dfdata['dataset'], + "dataset.investigation.name": "= '%s'" % dfdata['investigation'], + } + with pytest.deprecated_call(): + query = Query(client, "Datafile", conditions=conditions) + print(str(query)) + assert "Datafile" in query.select_clause + assert "investigation" in query.join_clause + assert dfdata['investigation'] in query.where_clause + assert query.order_clause is None + assert query.include_clause is None + assert query.limit_clause is None + qstr = str(query) + res = client.search(query) + assert len(res) == 1 + df = res[0] + assert df.BeanName == "Datafile" + assert df.name == dfdata['name'] + + # Same example, but use placeholders in the query string now. + conditions = { + "name": "= '%(name)s'", + "dataset.name": "= '%(dataset)s'", + "dataset.investigation.name": "= '%(investigation)s'", + } + with pytest.deprecated_call(): + query = Query(client, "Datafile", conditions=conditions) + print(str(query)) + print(str(query) % dfdata) + assert str(query) % dfdata == qstr + res = client.search(str(query) % dfdata) + assert len(res) == 1 + assert res[0] == df + +@pytest.mark.dependency(depends=['get_investigation']) +def test_query_legacy_conditions_instruments(client): + """Query the instruments related to a given investigation. + Same as test_query_instruments(), but pass the conditions as a + mapping. Deprecated, transitionally supported for backward + compatibility. + """ + with pytest.deprecated_call(): + query = Query(client, "Instrument", + order=["name"], + conditions={ "investigationInstruments.investigation.id": + "= %d" % investigation.id }, + includes={"facility", "instrumentScientists.user"}) + print(str(query)) + assert "Instrument" in query.select_clause + assert "investigation" in query.join_clause + assert "id" in query.where_clause + assert "name" in query.order_clause + assert "instrumentScientists" in query.include_clause + res = client.search(query) + assert len(res) == 1 + instr = res[0] + assert instr.BeanName == "Instrument" + assert instr.facility.BeanName == "Facility" + +def test_query_legacy_conditions_list(client): + """We may also add a list of conditions on a single attribute. + Same as test_query_multiple_conditions(), but pass the conditions + as a mapping. Deprecated, transitionally supported for backward + compatibility. + """ + condition = { + "datafileCreateTime": [">= '2012-01-01'", "< '2013-01-01'" ] + } + with pytest.deprecated_call(): + query = Query(client, "Datafile", conditions=condition) + print(str(query)) + assert "Datafile" in query.select_clause + assert query.join_clause is None + assert "datafileCreateTime" in query.where_clause + qstr = str(query) + res = client.search(query) + assert len(res) == 3 + have_icat_5 + + # The last example also works by adding the conditions separately. + query = Query(client, "Datafile") + with pytest.deprecated_call(): + query.addConditions({"datafileCreateTime": ">= '2012-01-01'"}) + with pytest.deprecated_call(): + query.addConditions({"datafileCreateTime": "< '2013-01-01'"}) + print(str(query)) + assert str(query) == qstr + res = client.search(query) + assert len(res) == 3 + have_icat_5 + +def test_query_legacy_conditions_jpql_function_mixed(client): + """Mix conditions with and without JPQL function on the same attribute. + Same as test_query_condition_jpql_function_mixed(), but pass the + conditions as a mapping. Deprecated, transitionally supported for + backward compatibility. + """ + conditions = { "name": "LIKE 'db/%'", + "LENGTH(fullName)": "> 11", "fullName": "> 'C'" } + with pytest.deprecated_call(): + query = Query(client, "User", conditions=conditions) + print(str(query)) + assert "User" in query.select_clause + assert query.join_clause is None + assert "LENGTH" in query.where_clause + res = client.search(query) + assert len(res) == 3 From cfc9099a8fe5b9a2b3e8ecdab752f91aa7a24c02 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Wed, 28 Aug 2024 16:06:26 +0200 Subject: [PATCH 15/26] Update example scripts to use the new format of the conditions argument when creating queries. Eliminate a few more occurrences of the old legacy ICAT query syntax on the way. --- doc/examples/add-job.py | 60 +++++++------- doc/examples/create-datafile.py | 6 +- doc/examples/create-investigation.py | 14 ++-- doc/examples/dumpinvestigation.py | 119 ++++++++++++++++----------- doc/examples/dumprules.py | 6 +- doc/examples/ingest.py | 6 +- doc/examples/init-icat.py | 74 +++++++++-------- doc/examples/paramtype.py | 12 +-- doc/examples/querytest.py | 47 ++++++----- 9 files changed, 186 insertions(+), 158 deletions(-) diff --git a/doc/examples/add-job.py b/doc/examples/add-job.py index 682c7c69..308e0ed7 100644 --- a/doc/examples/add-job.py +++ b/doc/examples/add-job.py @@ -42,8 +42,10 @@ def makeparam(t, pdata): param = client.new(t) initobj(param, pdata) ptdata = data['parameter_types'][pdata['type']] - query = ("ParameterType [name='%s' AND units='%s']" - % (ptdata['name'], ptdata['units'])) + query = Query(client, "ParameterType", conditions=[ + ("name", "= '%s'" % ptdata['name']), + ("units", "= '%s'" % ptdata['units']), + ]) param.type = client.assertedSearch(query)[0] return param @@ -78,20 +80,20 @@ def makeparam(t, pdata): initobj(inputcollection, jobdata['input']) for ds in jobdata['input']['datasets']: - query = Query(client, "Dataset", conditions={ - "name":"= '%s'" % ds['name'], - "investigation.name":"= '%s'" % ds['investigation'] - }) + query = Query(client, "Dataset", conditions=[ + ("name", "= '%s'" % ds['name']), + ("investigation.name", "= '%s'" % ds['investigation']), + ]) dataset = client.assertedSearch(query)[0] dcs = client.new("DataCollectionDataset", dataset=dataset) inputcollection.dataCollectionDatasets.append(dcs) for df in jobdata['input']['datafiles']: - query = Query(client, "Datafile", conditions={ - "name":"= '%s'" % df['name'], - "dataset.name":"= '%s'" % df['dataset'], - "dataset.investigation.name":"= '%s'" % df['investigation'] - }) + query = Query(client, "Datafile", conditions=[ + ("name", "= '%s'" % df['name']), + ("dataset.name", "= '%s'" % df['dataset']), + ("dataset.investigation.name", "= '%s'" % df['investigation']), + ]) datafile = client.assertedSearch(query)[0] dcf = client.new("DataCollectionDatafile", datafile=datafile) inputcollection.dataCollectionDatafiles.append(dcf) @@ -112,13 +114,13 @@ def makeparam(t, pdata): initobj(outputcollection, jobdata['output']) for ds in jobdata['output']['datasets']: - query = Query(client, "Investigation", conditions={ - "name":"= '%s'" % ds['investigation'] - }) + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % ds['investigation']), + ]) investigation = client.assertedSearch(query)[0] - query = Query(client, "DatasetType", conditions={ - "name":"= '%s'" % data['dataset_types'][ds['type']]['name'] - }) + query = Query(client, "DatasetType", conditions=[ + ("name", "= '%s'" % data['dataset_types'][ds['type']]['name']) + ]) dataset_type = client.assertedSearch(query)[0] print("Dataset: creating '%s' ..." % ds['name']) dataset = client.new("Dataset") @@ -131,10 +133,10 @@ def makeparam(t, pdata): for df in ds['datafiles']: dff = data['datafile_formats'][df['format']] - query = Query(client, "DatafileFormat", conditions={ - "name":"= '%s'" % dff['name'], - "version":"= '%s'" % dff['version'], - }) + query = Query(client, "DatafileFormat", conditions=[ + ("name", "= '%s'" % dff['name']), + ("version", "= '%s'" % dff['version']), + ]) datafile_format = client.assertedSearch(query)[0] print("Datafile: creating '%s' ..." % df['name']) datafile = client.new("Datafile") @@ -157,16 +159,16 @@ def makeparam(t, pdata): outputcollection.dataCollectionDatasets.append(dcs) for df in jobdata['output']['datafiles']: - query = Query(client, "Dataset", conditions={ - "name":"= '%s'" % df['dataset'], - "investigation.name":"= '%s'" % df['investigation'] - }) + query = Query(client, "Dataset", conditions=[ + ("name", "= '%s'" % df['dataset']), + ("investigation.name", "= '%s'" % df['investigation']), + ]) dataset = client.assertedSearch(query)[0] dff = data['datafile_formats'][df['format']] - query = Query(client, "DatafileFormat", conditions={ - "name":"= '%s'" % dff['name'], - "version":"= '%s'" % dff['version'], - }) + query = Query(client, "DatafileFormat", conditions=[ + ("name", "= '%s'" % dff['name']), + ("version", "= '%s'" % dff['version']), + ]) datafile_format = client.assertedSearch(query)[0] print("Datafile: creating '%s' ..." % df['name']) datafile = client.new("Datafile") diff --git a/doc/examples/create-datafile.py b/doc/examples/create-datafile.py index 1fa9a2b2..0888efbc 100644 --- a/doc/examples/create-datafile.py +++ b/doc/examples/create-datafile.py @@ -56,13 +56,13 @@ raise RuntimeError("datafile %s not found" % df_path) query = Query(client, "DatasetType", - conditions={ "name": "= '%s'" % conf.dst_name }) + conditions=[ ("name", "= '%s'" % conf.dst_name) ]) dst = client.assertedSearch(query)[0] query = Query(client, "DatafileFormat", - conditions={ "name": "= '%s'" % conf.dff_name }) + conditions=[ ("name", "= '%s'" % conf.dff_name) ]) dff = client.assertedSearch(query)[0] query = Query(client, "Investigation", - conditions={ "name": "= '%s'" % conf.investigation }) + conditions=[ ("name", "= '%s'" % conf.investigation) ]) investigation = client.assertedSearch(query)[0] fstats = df_path.stat() diff --git a/doc/examples/create-investigation.py b/doc/examples/create-investigation.py index 6b578b42..345e3a0e 100644 --- a/doc/examples/create-investigation.py +++ b/doc/examples/create-investigation.py @@ -107,8 +107,10 @@ def getUser(client, attrs): ip = client.new("InvestigationParameter") initobj(ip, pdata) ptdata = data['parameter_types'][pdata['type']] - query = ("ParameterType [name='%s' AND units='%s']" - % (ptdata['name'], ptdata['units'])) + query = Query(client, "ParameterType", conditions=[ + ("name", "= '%s'" % ptdata['name']), + ("units", "= '%s'" % ptdata['units']), + ]) ip.type = client.assertedSearch(query)[0] investigation.parameters.append(ip) if 'shifts' in investigationdata: @@ -123,10 +125,10 @@ def getUser(client, attrs): sd = investigation.startDate or investigation.endDate ed = investigation.endDate or investigation.startDate if sd and ed: - query = Query(client, "FacilityCycle", conditions={ - "startDate": "<= '%s'" % parse_attr_string(ed, "Date"), - "endDate": "> '%s'" % parse_attr_string(sd, "Date"), - }) + query = Query(client, "FacilityCycle", conditions=[ + ("startDate", "<= '%s'" % parse_attr_string(ed, "Date")), + ("endDate", "> '%s'" % parse_attr_string(sd, "Date")), + ]) for fc in client.search(query): ifc = client.new("InvestigationFacilityCycle", facilityCycle=fc) investigation.investigationFacilityCycles.append(ifc) diff --git a/doc/examples/dumpinvestigation.py b/doc/examples/dumpinvestigation.py index d415d0c9..469a6e54 100644 --- a/doc/examples/dumpinvestigation.py +++ b/doc/examples/dumpinvestigation.py @@ -75,20 +75,32 @@ def mergesearch(sexps): # instruments related to the investigations. These are independent # searches, but the results are likely to overlap. So we need to # search and merge results first. Similar situation for ParameterType. -usersearch = [("User <-> InvestigationUser <-> Investigation [id=%d]"), - ("User <-> UserGroup <-> Grouping <-> InvestigationGroup " - "<-> Investigation [id=%d]"), - ("User <-> InstrumentScientist <-> Instrument " - "<-> InvestigationInstrument <-> Investigation [id=%d]")] -ptsearch = [("ParameterType INCLUDE Facility, PermissibleStringValue " - "<-> InvestigationParameter <-> Investigation [id=%d]"), - ("ParameterType INCLUDE Facility, PermissibleStringValue " - "<-> SampleParameter <-> Sample <-> Investigation [id=%d]"), - ("ParameterType INCLUDE Facility, PermissibleStringValue " - "<-> DatasetParameter <-> Dataset <-> Investigation [id=%d]"), - ("ParameterType INCLUDE Facility, PermissibleStringValue " - "<-> DatafileParameter <-> Datafile <-> Dataset " - "<-> Investigation [id=%d]"), ] +usersearch = [ + Query(client, "User", conditions=[ + ("investigationUsers.investigation.id", "= %d") + ]), + Query(client, "User", conditions=[ + ("userGroups.grouping.investigationGroups.investigation.id", "= %d") + ]), + Query(client, "User", conditions=[ + ("instrumentScientists.instrument.investigationInstruments." + "investigation.id", "= %d") + ]), +] +ptsearch = [ + Query(client, "ParameterType", conditions=[ + ("investigationParameters.investigation.id", "= %d") + ], includes=["facility", "permissibleStringValues"]), + Query(client, "ParameterType", conditions=[ + ("sampleParameters.sample.investigation.id", "= %d") + ], includes=["facility", "permissibleStringValues"]), + Query(client, "ParameterType", conditions=[ + ("datasetParameters.dataset.investigation.id", "= %d") + ], includes=["facility", "permissibleStringValues"]), + Query(client, "ParameterType", conditions=[ + ("datafileParameters.datafile.dataset.investigation.id", "= %d") + ], includes=["facility", "permissibleStringValues"]), +] # The set of objects to be included in the Investigation. inv_includes = { "facility", "type.facility", "investigationInstruments", @@ -103,42 +115,49 @@ def mergesearch(sexps): # list: either queries expressed as Query objects, or queries # expressed as string expressions, or lists of objects. In the first # two cases, the seacrh results will be written, in the last case, the -# objects are written as provided. We assume that there is only one -# relevant facility, e.g. that all objects related to the -# investigation are related to the same facility. We may thus ommit -# the facility from the ORDER BY clauses. -authtypes = [mergesearch([s % invid for s in usersearch]), - ("Grouping ORDER BY name INCLUDE UserGroup, User " - "<-> InvestigationGroup <-> Investigation [id=%d]" % invid)] -statictypes = [("Facility ORDER BY name"), - ("Instrument ORDER BY name " - "INCLUDE Facility, InstrumentScientist, User " - "<-> InvestigationInstrument <-> Investigation [id=%d]" - % invid), - (mergesearch([s % invid for s in ptsearch])), - ("InvestigationType ORDER BY name INCLUDE Facility " - "<-> Investigation [id=%d]" % invid), - ("SampleType ORDER BY name, molecularFormula INCLUDE Facility " - "<-> Sample <-> Investigation [id=%d]" % invid), - ("DatasetType ORDER BY name INCLUDE Facility " - "<-> Dataset <-> Investigation [id=%d]" % invid), - ("DatafileFormat ORDER BY name, version INCLUDE Facility " - "<-> Datafile <-> Dataset <-> Investigation [id=%d]" % invid)] -investtypes = [Query(client, "Investigation", - conditions={"id":"in (%d)" % invid}, - includes=inv_includes), - Query(client, "Sample", order=["name"], - conditions={"investigation.id":"= %d" % invid}, - includes={"investigation", "type.facility", - "parameters", "parameters.type.facility"}), - Query(client, "Dataset", order=["name"], - conditions={"investigation.id":"= %d" % invid}, - includes={"investigation", "type.facility", "sample", - "parameters", "parameters.type.facility"}), - Query(client, "Datafile", order=["dataset.name", "name"], - conditions={"dataset.investigation.id":"= %d" % invid}, - includes={"dataset", "datafileFormat.facility", - "parameters", "parameters.type.facility"})] +# objects are written as provided. +authtypes = [ + mergesearch([str(s) % invid for s in usersearch]), + Query(client, "Grouping", conditions=[ + ("investigationGroups.investigation.id", "= %d" % invid) + ], order=["name"], includes=["userGroups.user"]) +] +statictypes = [ + Query(client, "Facility", order=True), + Query(client, "Instrument", conditions=[ + ("investigationInstruments.investigation.id", "= %d" % invid) + ], order=True, includes=["facility", "instrumentScientists.user"]), + mergesearch([str(s) % invid for s in ptsearch]), + Query(client, "InvestigationType", conditions=[ + ("investigations.id", "= %d" % invid) + ], order=True, includes=["facility"]), + Query(client, "SampleType", conditions=[ + ("samples.investigation.id", "= %d" % invid) + ], order=True, includes=["facility"]), + Query(client, "DatasetType", conditions=[ + ("datasets.investigation.id", "= %d" % invid) + ], order=True, includes=["facility"]), + Query(client, "DatafileFormat", conditions=[ + ("datafiles.dataset.investigation.id", "= %d" % invid) + ], order=True, includes=["facility"]), +] +investtypes = [ + Query(client, "Investigation", + conditions=[("id", "in (%d)" % invid)], + includes=inv_includes), + Query(client, "Sample", order=["name"], + conditions=[("investigation.id", "= %d" % invid)], + includes={"investigation", "type.facility", + "parameters", "parameters.type.facility"}), + Query(client, "Dataset", order=["name"], + conditions=[("investigation.id", "= %d" % invid)], + includes={"investigation", "type.facility", "sample", + "parameters", "parameters.type.facility"}), + Query(client, "Datafile", order=["dataset.name", "name"], + conditions=[("dataset.investigation.id", "= %d" % invid)], + includes={"dataset", "datafileFormat.facility", + "parameters", "parameters.type.facility"}) +] with open_dumpfile(client, conf.file, conf.format, 'w') as dumpfile: dumpfile.writedata(authtypes) diff --git a/doc/examples/dumprules.py b/doc/examples/dumprules.py index 3bdd57fa..d839f835 100644 --- a/doc/examples/dumprules.py +++ b/doc/examples/dumprules.py @@ -36,7 +36,7 @@ groups = set() query = Query(client, "Rule", - conditions={"grouping": "IS NOT NULL"}, + conditions=[("grouping", "IS NOT NULL")], includes={"grouping.userGroups.user"}) for r in client.search(query): groups.add(r.grouping) @@ -45,9 +45,9 @@ sorted(groups, key=icat.entity.Entity.__sortkey__), Query(client, "PublicStep"), Query(client, "Rule", order=["what", "id"], - conditions={"grouping": "IS NULL"}), + conditions=[("grouping", "IS NULL")]), Query(client, "Rule", order=["grouping.name", "what", "id"], - conditions={"grouping": "IS NOT NULL"}, + conditions=[("grouping", "IS NOT NULL")], includes={"grouping"}), ] diff --git a/doc/examples/ingest.py b/doc/examples/ingest.py index 83a2333d..d8eabc0f 100644 --- a/doc/examples/ingest.py +++ b/doc/examples/ingest.py @@ -83,9 +83,9 @@ client, conf = config.getconfig() client.login(conf.auth, conf.credentials) -query = Query(client, "Investigation", conditions={ - "name": "= '%s'" % conf.investigation -}) +query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % conf.investigation), +]) investigation = client.assertedSearch(query)[0] diff --git a/doc/examples/init-icat.py b/doc/examples/init-icat.py index a632f1a7..6de088da 100644 --- a/doc/examples/init-icat.py +++ b/doc/examples/init-icat.py @@ -186,10 +186,12 @@ def getUser(client, attrs): for name, a in dpitems: query = Query(client, name) if a: - query.addConditions({("%s.id" % a): "IS NOT NULL"}) + query.addConditions([(("%s.id" % a), "IS NOT NULL")]) pr_items.append(query) pd_attr = "%s.publicationDate" % a if a else "publicationDate" - query = Query(client, name, conditions={pd_attr: "< CURRENT_TIMESTAMP"}) + query = Query(client, name, conditions=[ + (pd_attr, "< CURRENT_TIMESTAMP") + ]) all_items.append(query) client.createRules("R", all_items) client.createRules("R", pr_items, pubreader_group) @@ -233,7 +235,7 @@ def getUser(client, attrs): ] if "dataCollectionInvestigation" in client.typemap: dcitems.insert(3, ( "DataCollectionInvestigation", "dataCollection." )) -items = [ Query(client, name, conditions={ (a + "createId"): "= :user" }) +items = [ Query(client, name, conditions=[ ((a + "createId"), "= :user") ]) for name, a in dcitems ] client.createRules("CRUD", items) @@ -276,21 +278,21 @@ def getUser(client, attrs): items = [] for name, a, complete in invitems: # ... for writer group. - conditions={ - a + "investigationGroups.role":"= 'writer'", - a + "investigationGroups.grouping.userGroups.user.name":"= :user", - } + query = Query(client, name, conditions=[ + (a + "investigationGroups.role", "= 'writer'"), + (a + "investigationGroups.grouping.userGroups.user.name", "= :user"), + ]) if complete: - conditions[complete] = "= False" - items.append(Query(client, name, conditions=conditions)) + query.addConditions([(complete, "= False")]) + items.append(query) # ... for instrument scientists. - conditions={ - a + "investigationInstruments.instrument.instrumentScientists" - ".user.name":"= :user", - } + query = Query(client, name, conditions=[ + (a + "investigationInstruments.instrument.instrumentScientists" + ".user.name", "= :user"), + ]) if complete: - conditions[complete] = "= False" - items.append(Query(client, name, conditions=conditions)) + query.addConditions([(complete, "= False")]) + items.append(query) client.createRules("CUD", items) # Set permissions for the reader group. Actually, we give read @@ -303,31 +305,31 @@ def getUser(client, attrs): ( "Publication", "investigation.", "" ) ]) items = [] for name, a, c in invitems: - conditions={ - a + "investigationGroups.grouping.userGroups.user.name":"= :user", - } + conditions=[ + (a + "investigationGroups.grouping.userGroups.user.name", "= :user"), + ] items.append(Query(client, name, conditions=conditions)) - conditions={ - a + "investigationInstruments.instrument.instrumentScientists" - ".user.name":"= :user", - } + conditions=[ + (a + "investigationInstruments.instrument.instrumentScientists" + ".user.name", "= :user"), + ] items.append(Query(client, name, conditions=conditions)) client.createRules("R", items) # set permission to grant and to revoke permissions for the owner. tig = "grouping.investigationGroups" uig = "grouping.investigationGroups.investigation.investigationGroups" -item = Query(client, "UserGroup", conditions={ - uig + ".grouping.userGroups.user.name":"= :user", - uig + ".role":"= 'owner'", - tig + ".role":"in ('reader', 'writer')" -}) +item = Query(client, "UserGroup", conditions=[ + (uig + ".grouping.userGroups.user.name", "= :user"), + (uig + ".role", "= 'owner'"), + (tig + ".role", "in ('reader', 'writer')"), +]) client.createRules("CRUD", [ item ]) uig = "investigationGroups.investigation.investigationGroups" -item = Query(client, "Grouping", conditions={ - uig + ".grouping.userGroups.user.name":"= :user", - uig + ".role":"= 'owner'" -}) +item = Query(client, "Grouping", conditions=[ + (uig + ".grouping.userGroups.user.name", "= :user"), + (uig + ".role", "= 'owner'"), +]) client.createRules("R", [ item ]) @@ -344,12 +346,12 @@ def getUser(client, attrs): items = [] for name, a, dstype_name in invitems: - conditions={ - a + "releaseDate":"< CURRENT_TIMESTAMP", - } + query = Query(client, name, conditions=[ + (a + "releaseDate", "< CURRENT_TIMESTAMP"), + ]) if dstype_name: - conditions[dstype_name] = "= 'raw'" - items.append(Query(client, name, conditions=conditions)) + query.addConditions([(dstype_name, "= 'raw'")]) + items.append(query) client.createRules("R", items) diff --git a/doc/examples/paramtype.py b/doc/examples/paramtype.py index 865c5be4..b7999f16 100644 --- a/doc/examples/paramtype.py +++ b/doc/examples/paramtype.py @@ -104,17 +104,17 @@ def add_paramtype(client, conf): def ls_paramtypes(client, conf): query = Query(client, "ParameterType", includes=["permissibleStringValues"]) if conf.name: - query.addConditions({"name": "LIKE '%s%%'" % conf.name}) + query.addConditions([("name", "LIKE '%s%%'" % conf.name)]) if conf.applicable == "Investigation": - query.addConditions({"applicableToInvestigation": "= 'True'"}) + query.addConditions([("applicableToInvestigation", "= 'True'")]) elif conf.applicable == "DataCollection": - query.addConditions({"applicableToDataCollection": "= 'True'"}) + query.addConditions([("applicableToDataCollection", "= 'True'")]) elif conf.applicable == "Sample": - query.addConditions({"applicableToSample": "= 'True'"}) + query.addConditions([("applicableToSample", "= 'True'")]) elif conf.applicable == "Dataset": - query.addConditions({"applicableToDataset": "= 'True'"}) + query.addConditions([("applicableToDataset", "= 'True'")]) elif conf.applicable == "Datafile": - query.addConditions({"applicableToDatafile": "= 'True'"}) + query.addConditions([("applicableToDatafile", "= 'True'")]) for pt in client.search(query): if pt.unitsFullName: units = "%s (%s)" % (pt.units, pt.unitsFullName) diff --git a/doc/examples/querytest.py b/doc/examples/querytest.py index e8b2e9c2..49713a7b 100644 --- a/doc/examples/querytest.py +++ b/doc/examples/querytest.py @@ -40,7 +40,7 @@ print("\nA simple query for an investigation by name.") name = inp['datasets'][0]['investigation'] -q = Query(client, "Investigation", conditions={"name":"= '%s'" % name}) +q = Query(client, "Investigation", conditions=[("name", "= '%s'" % name)]) print(str(q)) res = client.search(q) print("%d result(s)" % len(res)) @@ -55,22 +55,22 @@ print("\nQuery a datafile by its name, dataset name, and investigation name:") df = inp['datafiles'][0] -conditions = { - "name":"= '%s'" % df['name'], - "dataset.name":"= '%s'" % df['dataset'], - "dataset.investigation.name":"= '%s'" % df['investigation'], -} +conditions = [ + ("name", "= '%s'" % df['name']), + ("dataset.name", "= '%s'" % df['dataset']), + ("dataset.investigation.name", "= '%s'" % df['investigation']), +] q = Query(client, "Datafile", conditions=conditions) print(str(q)) print("%d result(s)" % len(client.search(q))) print("\nSame example, but use placeholders in the query string now:") df = inp['datafiles'][0] -conditions = { - "name":"= '%(name)s'", - "dataset.name":"= '%(dataset)s'", - "dataset.investigation.name":"= '%(investigation)s'", -} +conditions = [ + ("name", "= '%(name)s'"), + ("dataset.name", "= '%(dataset)s'"), + ("dataset.investigation.name", "= '%(investigation)s'"), +] q = Query(client, "Datafile", conditions=conditions) print(str(q)) print(str(q) % df) @@ -84,22 +84,22 @@ "investigationGroups.grouping", "parameters", "parameters.type.facility" } q = Query(client, "Investigation", - conditions={"id":"= %d" % invid}, includes=includes) + conditions=[("id", "= %d" % invid)], includes=includes) print(str(q)) print("%d result(s)" % len(client.search(q))) print("\nQuery the instruments related to a given investigation.") q = Query(client, "Instrument", order=["name"], - conditions={ "investigationInstruments.investigation.id": - "= %d" % invid }, + conditions=[ ("investigationInstruments.investigation.id", + "= %d" % invid) ], includes={"facility", "instrumentScientists.user"}) print(str(q)) print("%d result(s)" % len(client.search(q))) print("\nThe datafiles related to a given investigation in natural order.") q = Query(client, "Datafile", order=True, - conditions={ "dataset.investigation.id":"= %d" % invid }, + conditions=[ ("dataset.investigation.id", "= %d" % invid) ], includes={"dataset", "datafileFormat.facility", "parameters.type.facility"}) print(str(q)) @@ -107,7 +107,7 @@ print("\nSame example, but skip the investigation in the order.") q = Query(client, "Datafile", order=['dataset.name', 'name'], - conditions={ "dataset.investigation.id":"= %d" % invid }, + conditions=[ ("dataset.investigation.id", "= %d" % invid) ], includes={"dataset", "datafileFormat.facility", "parameters.type.facility"}) print(str(q)) @@ -131,27 +131,30 @@ print("%d result(s)" % len(client.search(q))) print("\nOther relations then equal may be used in the conditions too.") -condition = {"datafileCreateTime":">= '2012-01-01'"} +condition = [ ("datafileCreateTime", ">= '2012-01-01'") ] q = Query(client, "Datafile", conditions=condition) print(str(q)) print("%d result(s)" % len(client.search(q))) -print("\nWe may also add a list of conditions on a single attribute.") -condition = {"datafileCreateTime":[">= '2012-01-01'", "< '2013-01-01'"]} +print("\nWe may also add multiple conditions on a single attribute.") +condition = [ + ("datafileCreateTime", ">= '2012-01-01'"), + ("datafileCreateTime", "< '2013-01-01'"), +] q = Query(client, "Datafile", conditions=condition) print(str(q)) print("%d result(s)" % len(client.search(q))) print("\nThe last example also works by adding the conditions separately.") q = Query(client, "Datafile") -q.addConditions({"datafileCreateTime":">= '2012-01-01'"}) -q.addConditions({"datafileCreateTime":"< '2013-01-01'"}) +q.addConditions([("datafileCreateTime", ">= '2012-01-01'")]) +q.addConditions([("datafileCreateTime", "< '2013-01-01'")]) print(str(q)) print("%d result(s)" % len(client.search(q))) print("\nUsing \"id in (i)\" rather than \"id = i\" also works.") print("(This may be needed to work around ICAT Issue 149.)") -q = Query(client, "Investigation", conditions={"id":"in (%d)" % invid}) +q = Query(client, "Investigation", conditions=[("id", "in (%d)" % invid)]) print(str(q)) print("%d result(s)" % len(client.search(q))) From 60024f9831d72177458afca2d968dde5ea4909f8 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Wed, 28 Aug 2024 17:50:47 +0200 Subject: [PATCH 16/26] Update the tutotial section "Searching for objects in the ICAT server" to use the new format of the conditions argument when creating queries. --- doc/src/tutorial-search.rst | 85 ++++++++++++++++++------------------- doc/tutorial/search.py | 77 ++++++++++++++++----------------- 2 files changed, 81 insertions(+), 81 deletions(-) diff --git a/doc/src/tutorial-search.rst b/doc/src/tutorial-search.rst index 9d1c5fec..aef89c1e 100644 --- a/doc/src/tutorial-search.rst +++ b/doc/src/tutorial-search.rst @@ -119,11 +119,11 @@ that lists all investigations:: In order to search for a particular investigation, we may add an appropriate condition. The `conditions` argument to -:class:`~icat.query.Query` should be a mapping of attribute names to -conditions on that attribute:: +:class:`~icat.query.Query` should be a list of tuples, each of them +being a pair of an attribute name and a conditions on that attribute:: >>> query = Query(client, "Investigation", - ... conditions={"name": "= '10100601-ST'"}) + ... conditions=[("name", "= '10100601-ST'")]) >>> print(query) SELECT o FROM Investigation o WHERE o.name = '10100601-ST' >>> client.search(query) @@ -146,7 +146,7 @@ conditions on that attribute:: We may also include related objects in the search results:: >>> query = Query(client, "Investigation", - ... conditions={"name": "= '10100601-ST'"}, + ... conditions=[("name", "= '10100601-ST'")], ... includes=["datasets"]) >>> print(query) SELECT o FROM Investigation o WHERE o.name = '10100601-ST' INCLUDE o.datasets @@ -212,7 +212,7 @@ which attribute a condition should be applied to. Consider the following query:: >>> query = Query(client, "Investigation", - ... conditions={"LENGTH(title)": "= 18"}) + ... conditions=[("LENGTH(title)", "= 18")]) >>> print(query) SELECT o FROM Investigation o WHERE LENGTH(o.title) = 18 >>> client.search(query) @@ -251,12 +251,12 @@ objects. This allows rather complex queries. Let us search for the datasets in this investigation that have been measured in a magnetic field larger then 5 Tesla and include its parameters in the result:: - >>> conditions = { - ... "investigation.name": "= '10100601-ST'", - ... "parameters.type.name": "= 'Magnetic field'", - ... "parameters.type.units": "= 'T'", - ... "parameters.numericValue": "> 5.0", - ... } + >>> conditions = [ + ... ("investigation.name", "= '10100601-ST'"), + ... ("parameters.type.name", "= 'Magnetic field'"), + ... ("parameters.type.units", "= 'T'"), + ... ("parameters.numericValue", "> 5.0"), + ... ] >>> query = Query(client, "Dataset", ... conditions=conditions, includes=["parameters.type"]) >>> print(query) @@ -337,9 +337,9 @@ of your Python program. Consider:: >>> def get_investigation(client, name, visitId=None): ... query = Query(client, "Investigation") - ... query.addConditions({"name": "= '%s'" % name}) + ... query.addConditions([("name", "= '%s'" % name)]) ... if visitId is not None: - ... query.addConditions({"visitId": "= '%s'" % visitId}) + ... query.addConditions([("visitId", "= '%s'" % visitId)]) ... print(query) ... return client.assertedSearch(query)[0] ... @@ -382,13 +382,12 @@ either by `name` alone or by `name` and `visitId`, depending on the arguments. It is also possible to put more then one conditions on a single -attribute: setting the corresponding value in the `conditions` -argument to a list of strings will result in combining the conditions -on that attribute. Search for all datafiles created in 2012:: +attribute. Search for all datafiles created in 2012:: - >>> conditions = { - ... "datafileCreateTime": [">= '2012-01-01'", "< '2013-01-01'"] - ... } + >>> conditions = [ + ... ("datafileCreateTime", ">= '2012-01-01'"), + ... ("datafileCreateTime", "< '2013-01-01'"), + ... ] >>> query = Query(client, "Datafile", conditions=conditions) >>> print(query) SELECT o FROM Datafile o WHERE o.datafileCreateTime >= '2012-01-01' AND o.datafileCreateTime < '2013-01-01' @@ -440,8 +439,8 @@ Of course, that last example also works when adding the conditions incrementally:: >>> query = Query(client, "Datafile") - >>> query.addConditions({"datafileCreateTime": ">= '2012-01-01'"}) - >>> query.addConditions({"datafileCreateTime": "< '2013-01-01'"}) + >>> query.addConditions([("datafileCreateTime", ">= '2012-01-01'")]) + >>> query.addConditions([("datafileCreateTime", "< '2013-01-01'")]) >>> print(query) SELECT o FROM Datafile o WHERE o.datafileCreateTime >= '2012-01-01' AND o.datafileCreateTime < '2013-01-01' @@ -487,11 +486,11 @@ transferring them to the client, and counting them at client side. Let's check for a given investigation, the minimum, maximum, and average magnetic field applied in the measurements:: - >>> conditions = { - ... "dataset.investigation.name": "= '10100601-ST'", - ... "type.name": "= 'Magnetic field'", - ... "type.units": "= 'T'", - ... } + >>> conditions = [ + ... ("dataset.investigation.name", "= '10100601-ST'"), + ... ("type.name", "= 'Magnetic field'"), + ... ("type.units", "= 'T'"), + ... ] >>> query = Query(client, "DatasetParameter", ... conditions=conditions, attributes="numericValue") >>> print(query) @@ -517,10 +516,10 @@ average magnetic field applied in the measurements:: For another example, let's search for all investigations, having any dataset with a magnetic field parameter set:: - >>> conditions = { - ... "datasets.parameters.type.name": "= 'Magnetic field'", - ... "datasets.parameters.type.units": "= 'T'", - ... } + >>> conditions = [ + ... ("datasets.parameters.type.name", "= 'Magnetic field'"), + ... ("datasets.parameters.type.units", "= 'T'"), + ... ] >>> query = Query(client, "Investigation", conditions=conditions) >>> print(query) SELECT o FROM Investigation o JOIN o.datasets AS s1 JOIN s1.parameters AS s2 JOIN s2.type AS s3 WHERE s3.name = 'Magnetic field' AND s3.units = 'T' @@ -582,10 +581,10 @@ respectively. We may fix that by applying `DISTINCT`:: `DISTINCT` may be combined with `COUNT`, `AVG`, and `SUM` in order to make sure not to count the same object more then once:: - >>> conditions = { - ... "datasets.parameters.type.name": "= 'Magnetic field'", - ... "datasets.parameters.type.units": "= 'T'", - ... } + >>> conditions = [ + ... ("datasets.parameters.type.name", "= 'Magnetic field'"), + ... ("datasets.parameters.type.units", "= 'T'"), + ... ] >>> query = Query(client, "Investigation", ... conditions=conditions, aggregate="COUNT") >>> print(query) @@ -770,9 +769,9 @@ in the `order` argument to :class:`~icat.query.Query`. Let's search for user sorted by the length of their name, from longest to shortest:: - >>> query = Query(client, "User", conditions={ - ... "fullName": "IS NOT NULL" - ... }, order=[("LENGTH(fullName)", "DESC")]) + >>> query = Query(client, "User", conditions=[ + ... ("fullName", "IS NOT NULL"), + ... ], order=[("LENGTH(fullName)", "DESC")]) >>> print(query) SELECT o FROM User o WHERE o.fullName IS NOT NULL ORDER BY LENGTH(o.fullName) DESC >>> for user in client.search(query): @@ -880,13 +879,13 @@ first glance, it has a particular use case:: """ try: dataset = client.new("Dataset") - query = Query(client, "Investigation", conditions={ - "name": "= '%s'" % inv_name - }) + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % inv_name), + ]) dataset.investigation = client.assertedSearch(query)[0] - query = Query(client, "DatasetType", conditions={ - "name": "= '%s'" % ds_type - }) + query = Query(client, "DatasetType", conditions=[ + ("name", "= '%s'" % ds_type), + ]) dataset.type = client.assertedSearch(query)[0] dataset.complete = False dataset.name = ds_name diff --git a/doc/tutorial/search.py b/doc/tutorial/search.py index a697581e..b782a19d 100644 --- a/doc/tutorial/search.py +++ b/doc/tutorial/search.py @@ -16,14 +16,14 @@ # -------------------- query = Query(client, "Investigation", - conditions={"name": "= '10100601-ST'"}) + conditions=[("name", "= '10100601-ST'")]) print(query) client.search(query) # -------------------- query = Query(client, "Investigation", - conditions={"name": "= '10100601-ST'"}, + conditions=[("name", "= '10100601-ST'")], includes=["datasets"]) print(query) client.search(query) @@ -31,18 +31,18 @@ # -------------------- query = Query(client, "Investigation", - conditions={"LENGTH(title)": "= 18"}) + conditions=[("LENGTH(title)", "= 18")]) print(query) client.search(query) # -------------------- -conditions = { - "investigation.name": "= '10100601-ST'", - "parameters.type.name": "= 'Magnetic field'", - "parameters.type.units": "= 'T'", - "parameters.numericValue": "> 5.0", -} +conditions = [ + ("investigation.name", "= '10100601-ST'"), + ("parameters.type.name", "= 'Magnetic field'"), + ("parameters.type.units", "= 'T'"), + ("parameters.numericValue", "> 5.0"), +] query = Query(client, "Dataset", conditions=conditions, includes=["parameters.type"]) print(query) @@ -52,9 +52,9 @@ def get_investigation(client, name, visitId=None): query = Query(client, "Investigation") - query.addConditions({"name": "= '%s'" % name}) + query.addConditions([("name", "= '%s'" % name)]) if visitId is not None: - query.addConditions({"visitId": "= '%s'" % visitId}) + query.addConditions([("visitId", "= '%s'" % visitId)]) print(query) return client.assertedSearch(query)[0] @@ -63,9 +63,10 @@ def get_investigation(client, name, visitId=None): # -------------------- -conditions = { - "datafileCreateTime": [">= '2012-01-01'", "< '2013-01-01'"] -} +conditions = [ + ("datafileCreateTime", ">= '2012-01-01'"), + ("datafileCreateTime", "< '2013-01-01'"), +] query = Query(client, "Datafile", conditions=conditions) print(query) client.search(query) @@ -73,8 +74,8 @@ def get_investigation(client, name, visitId=None): # -------------------- query = Query(client, "Datafile") -query.addConditions({"datafileCreateTime": ">= '2012-01-01'"}) -query.addConditions({"datafileCreateTime": "< '2013-01-01'"}) +query.addConditions([("datafileCreateTime", ">= '2012-01-01'")]) +query.addConditions([("datafileCreateTime", "< '2013-01-01'")]) print(query) # -------------------- @@ -99,11 +100,11 @@ def get_investigation(client, name, visitId=None): # -------------------- -conditions = { - "dataset.investigation.name": "= '10100601-ST'", - "type.name": "= 'Magnetic field'", - "type.units": "= 'T'", -} +conditions = [ + ("dataset.investigation.name", "= '10100601-ST'"), + ("type.name", "= 'Magnetic field'"), + ("type.units", "= 'T'"), +] query = Query(client, "DatasetParameter", conditions=conditions, attributes="numericValue") print(query) @@ -120,10 +121,10 @@ def get_investigation(client, name, visitId=None): # -------------------- -conditions = { - "datasets.parameters.type.name": "= 'Magnetic field'", - "datasets.parameters.type.units": "= 'T'", -} +conditions = [ + ("datasets.parameters.type.name", "= 'Magnetic field'"), + ("datasets.parameters.type.units", "= 'T'"), +] query = Query(client, "Investigation", conditions=conditions) print(query) client.search(query) @@ -136,10 +137,10 @@ def get_investigation(client, name, visitId=None): # -------------------- -conditions = { - "datasets.parameters.type.name": "= 'Magnetic field'", - "datasets.parameters.type.units": "= 'T'", -} +conditions = [ + ("datasets.parameters.type.name", "= 'Magnetic field'"), + ("datasets.parameters.type.units", "= 'T'"), +] query = Query(client, "Investigation", conditions=conditions, aggregate="COUNT") print(query) @@ -157,9 +158,9 @@ def get_investigation(client, name, visitId=None): # -------------------- -query = Query(client, "User", conditions={ - "fullName": "IS NOT NULL" -}, order=[("LENGTH(fullName)", "DESC")]) +query = Query(client, "User", conditions=[ + ("fullName", "IS NOT NULL"), +], order=[("LENGTH(fullName)", "DESC")]) print(query) for user in client.search(query): print("%d: %s" % (len(user.fullName), user.fullName)) @@ -196,13 +197,13 @@ def get_dataset(client, inv_name, ds_name, ds_type="raw"): """ try: dataset = client.new("Dataset") - query = Query(client, "Investigation", conditions={ - "name": "= '%s'" % inv_name - }) + query = Query(client, "Investigation", conditions=[ + ("name", "= '%s'" % inv_name), + ]) dataset.investigation = client.assertedSearch(query)[0] - query = Query(client, "DatasetType", conditions={ - "name": "= '%s'" % ds_type - }) + query = Query(client, "DatasetType", conditions=[ + ("name", "= '%s'" % ds_type), + ]) dataset.type = client.assertedSearch(query)[0] dataset.complete = False dataset.name = ds_name From 38decc0a03d3c944aa71343bf7a87ea0df27ff7e Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Thu, 29 Aug 2024 15:48:52 +0200 Subject: [PATCH 17/26] Add a change note to the documentation of Query.setOrder() --- src/icat/query.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/icat/query.py b/src/icat/query.py index e1a3fb82..65dae666 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -418,6 +418,10 @@ def setOrder(self, order): than raising a :exc:`ValueError` in this case. .. versionchanged:: 0.20.0 allow a JPQL function in the attribute. + .. versionchanged:: 2.0.0 + allow the same attribute to appear more than once in the + resulting ORDER BY clause, which actually may make sense + in combination with a JPQL function. """ self._subst = None self.order = [] From 93e1a1241c2e82bfb5c6222233f87efbbff3b5be Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Thu, 29 Aug 2024 20:44:37 +0200 Subject: [PATCH 18/26] Update the tutotial section "Upload and download files to and from IDS" to use the new format of the conditions argument when creating queries. --- doc/src/tutorial-ids.rst | 16 ++++++++-------- doc/tutorial/ids.py | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/doc/src/tutorial-ids.rst b/doc/src/tutorial-ids.rst index c71d221e..4a76fdf5 100644 --- a/doc/src/tutorial-ids.rst +++ b/doc/src/tutorial-ids.rst @@ -54,11 +54,11 @@ We need a dataset in ICAT that the uploaded files should be put into, so let's create one:: >>> from icat.query import Query - >>> query = Query(client, "Investigation", conditions={"name": "= '12100409-ST'"}) + >>> query = Query(client, "Investigation", conditions=[("name", "= '12100409-ST'")]) >>> investigation = client.assertedSearch(query)[0] >>> dataset = client.new("Dataset") >>> dataset.investigation = investigation - >>> query = Query(client, "DatasetType", conditions={"name": "= 'other'"}) + >>> query = Query(client, "DatasetType", conditions=[("name", "= 'other'")]) >>> dataset.type = client.assertedSearch(query)[0] >>> dataset.name = "greetings" >>> dataset.complete = False @@ -67,7 +67,7 @@ so let's create one:: For each of the files, we create a new datafile object and call the :meth:`~icat.client.Client.putData` method to upload it:: - >>> query = Query(client, "DatafileFormat", conditions={"name": "= 'Text'"}) + >>> query = Query(client, "DatafileFormat", conditions=[("name", "= 'Text'")]) >>> df_format = client.assertedSearch(query)[0] >>> for fname in ("greet-jdoe.txt", "greet-nbour.txt", "greet-rbeck.txt"): ... datafile = client.new("Datafile", @@ -131,10 +131,10 @@ Download files We can request a download of a set of data using the :meth:`~icat.client.Client.getData` method:: - >>> query = Query(client, "Datafile", conditions={ - ... "name": "= 'greet-jdoe.txt'", - ... "dataset.name": "= 'greetings'" - ... }) + >>> query = Query(client, "Datafile", conditions=[ + ... ("name", "= 'greet-jdoe.txt'"), + ... ("dataset.name", "= 'greetings'"), + ... ]) >>> df = client.assertedSearch(query)[0] >>> data = client.getData([df]) >>> type(data) @@ -153,7 +153,7 @@ with the requested files:: >>> from io import BytesIO >>> from zipfile import ZipFile - >>> query = Query(client, "Dataset", conditions={"name": "= 'greetings'"}) + >>> query = Query(client, "Dataset", conditions=[("name", "= 'greetings'")]) >>> ds = client.assertedSearch(query)[0] >>> data = client.getData([ds]) >>> buffer = BytesIO(data.read()) diff --git a/doc/tutorial/ids.py b/doc/tutorial/ids.py index f3156039..c525f85b 100644 --- a/doc/tutorial/ids.py +++ b/doc/tutorial/ids.py @@ -13,11 +13,11 @@ # -------------------- from icat.query import Query -query = Query(client, "Investigation", conditions={"name": "= '12100409-ST'"}) +query = Query(client, "Investigation", conditions=[("name", "= '12100409-ST'")]) investigation = client.assertedSearch(query)[0] dataset = client.new("Dataset") dataset.investigation = investigation -query = Query(client, "DatasetType", conditions={"name": "= 'other'"}) +query = Query(client, "DatasetType", conditions=[("name", "= 'other'")]) dataset.type = client.assertedSearch(query)[0] dataset.name = "greetings" dataset.complete = False @@ -25,7 +25,7 @@ # -------------------- -query = Query(client, "DatafileFormat", conditions={"name": "= 'Text'"}) +query = Query(client, "DatafileFormat", conditions=[("name", "= 'Text'")]) df_format = client.assertedSearch(query)[0] for fname in ("greet-jdoe.txt", "greet-nbour.txt", "greet-rbeck.txt"): datafile = client.new("Datafile", @@ -36,10 +36,10 @@ # Download files -query = Query(client, "Datafile", conditions={ - "name": "= 'greet-jdoe.txt'", - "dataset.name": "= 'greetings'" -}) +query = Query(client, "Datafile", conditions=[ + ("name", "= 'greet-jdoe.txt'"), + ("dataset.name", "= 'greetings'"), +]) df = client.assertedSearch(query)[0] data = client.getData([df]) type(data) @@ -49,7 +49,7 @@ from io import BytesIO from zipfile import ZipFile -query = Query(client, "Dataset", conditions={"name": "= 'greetings'"}) +query = Query(client, "Dataset", conditions=[("name", "= 'greetings'")]) ds = client.assertedSearch(query)[0] data = client.getData([ds]) buffer = BytesIO(data.read()) From f32588074a7cc374229ec7fb2909adb9fd570220 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Thu, 29 Aug 2024 20:53:05 +0200 Subject: [PATCH 19/26] Fix overly long lines in tutorial examples --- doc/src/tutorial-ids.rst | 8 ++++++-- doc/tutorial/ids.py | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/doc/src/tutorial-ids.rst b/doc/src/tutorial-ids.rst index 4a76fdf5..79671070 100644 --- a/doc/src/tutorial-ids.rst +++ b/doc/src/tutorial-ids.rst @@ -54,7 +54,9 @@ We need a dataset in ICAT that the uploaded files should be put into, so let's create one:: >>> from icat.query import Query - >>> query = Query(client, "Investigation", conditions=[("name", "= '12100409-ST'")]) + >>> query = Query(client, "Investigation", conditions=[ + ... ("name", "= '12100409-ST'") + ... ]) >>> investigation = client.assertedSearch(query)[0] >>> dataset = client.new("Dataset") >>> dataset.investigation = investigation @@ -67,7 +69,9 @@ so let's create one:: For each of the files, we create a new datafile object and call the :meth:`~icat.client.Client.putData` method to upload it:: - >>> query = Query(client, "DatafileFormat", conditions=[("name", "= 'Text'")]) + >>> query = Query(client, "DatafileFormat", conditions=[ + ... ("name", "= 'Text'") + ... ]) >>> df_format = client.assertedSearch(query)[0] >>> for fname in ("greet-jdoe.txt", "greet-nbour.txt", "greet-rbeck.txt"): ... datafile = client.new("Datafile", diff --git a/doc/tutorial/ids.py b/doc/tutorial/ids.py index c525f85b..aaa12146 100644 --- a/doc/tutorial/ids.py +++ b/doc/tutorial/ids.py @@ -13,7 +13,9 @@ # -------------------- from icat.query import Query -query = Query(client, "Investigation", conditions=[("name", "= '12100409-ST'")]) +query = Query(client, "Investigation", conditions=[ + ("name", "= '12100409-ST'") +]) investigation = client.assertedSearch(query)[0] dataset = client.new("Dataset") dataset.investigation = investigation @@ -25,7 +27,9 @@ # -------------------- -query = Query(client, "DatafileFormat", conditions=[("name", "= 'Text'")]) +query = Query(client, "DatafileFormat", conditions=[ + ("name", "= 'Text'") +]) df_format = client.assertedSearch(query)[0] for fname in ("greet-jdoe.txt", "greet-nbour.txt", "greet-rbeck.txt"): datafile = client.new("Datafile", From 587718a7749835fc066ac8405128ea4cd13bedc4 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Thu, 29 Aug 2024 21:32:32 +0200 Subject: [PATCH 20/26] Add a test using a query where one attributes appears more than once in the ORDER BY clause --- tests/test_06_query.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_06_query.py b/tests/test_06_query.py index a46f1676..f41be18d 100644 --- a/tests/test_06_query.py +++ b/tests/test_06_query.py @@ -463,6 +463,27 @@ def test_query_order_jpql_function(client): assert res[0].fullName == "Nicolas Bourbaki" verify_rebuild_query(client, query, res) +def test_query_order_attribute_multiple(client): + """Add the same attribute more than once to the order. + + The restriction that any attribute may only appear once to the + order has been removed in #158. + """ + query = Query(client, 'User', order=[ + ('LENGTH(fullName)', 'DESC'), + ('fullName', 'DESC'), + ]) + print(str(query)) + assert "User" in query.select_clause + assert query.join_clause is None + assert query.where_clause is None + assert query.order_clause.count("fullName") == 2 + assert "LENGTH" in query.order_clause + res = client.search(query) + assert len(res) > 4 + assert res[4].fullName == "Aelius Cordus" + verify_rebuild_query(client, query, res) + def test_query_rule_order(client): """Rule does not have a constraint, id is included in the natural order. """ From 5ed66922bee6996359ab52f67afdaa9bbffd6ae6 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Thu, 29 Aug 2024 22:17:15 +0200 Subject: [PATCH 21/26] Fixup cfc9099: still missed one occurrence of a mapping in the conditions argument --- doc/examples/querytest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/examples/querytest.py b/doc/examples/querytest.py index 49713a7b..2bc599f1 100644 --- a/doc/examples/querytest.py +++ b/doc/examples/querytest.py @@ -170,7 +170,7 @@ print("\nThe warning can be suppressed by making the condition explicit.") q = Query(client, "Rule", order=['grouping', 'what', 'id'], - conditions={"grouping":"IS NOT NULL"}) + conditions=[("grouping", "IS NOT NULL")]) print(str(q)) print("%d result(s)" % len(client.search(q))) From e9a545f9750397df91cd760d9ead89d07c5f27d3 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Thu, 29 Aug 2024 22:18:57 +0200 Subject: [PATCH 22/26] Remove an outdated entry in the known issues documentation page --- doc/src/known-issues.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/doc/src/known-issues.rst b/doc/src/known-issues.rst index 896d354d..0410d057 100644 --- a/doc/src/known-issues.rst +++ b/doc/src/known-issues.rst @@ -18,11 +18,6 @@ Known issues, bugs and limitations configuration variables and thus command line arguments are effective then those shown by the generic help message. -+ The return value of the `formal string representation operator`__ of - :class:`icat.query.Query` can not be used to recreate another query - object with the same value as required by Python standards, see - `Issue #94`_ for details. - + The entries in the no_proxy configuration variable are matched against the host part of the URL by simple string comparison. The host is excluded from proxy use if its name ends with any item in @@ -34,6 +29,4 @@ Known issues, bugs and limitations the implementation of the underlying Python library. -.. __: https://docs.python.org/3/reference/datamodel.html#object.__repr__ .. _Issue #72: https://github.com/icatproject/python-icat/issues/72 -.. _Issue #94: https://github.com/icatproject/python-icat/issues/94 From b358006b20b0549f168a3fc29af57d32163e3fdd Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 30 Aug 2024 10:02:50 +0200 Subject: [PATCH 23/26] Start the development for the next major release --- CHANGES.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index d422eba9..ac456c98 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,12 @@ Changelog ========= +.. _changes-2_0_0: + +2.0.0 (not yet released) +~~~~~~~~~~~~~~~~~~~~~~~~ + + .. _changes-1_4_0: 1.4.0 (2024-08-30) From 8e1e8725a049c0be72d6059416e87530f7f3be7e Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 30 Aug 2024 14:59:47 +0200 Subject: [PATCH 24/26] Review the order of methods in class Query to be consistent with the order of the keyword arguments and the order of calling the methods during initialization. This should essentially only have an impact on the presentation of the module reference page in the documentation. --- src/icat/query.py | 213 +++++++++++++++++++++++----------------------- 1 file changed, 106 insertions(+), 107 deletions(-) diff --git a/src/icat/query.py b/src/icat/query.py index 65dae666..71f4db67 100644 --- a/src/icat/query.py +++ b/src/icat/query.py @@ -168,15 +168,15 @@ class Query(): :param aggregate: the aggregate function to be applied in the SELECT clause, if any. See the :meth:`~icat.query.Query.setAggregate` method for details. - :param order: the sorting attributes to build the ORDER BY clause - from. See the :meth:`~icat.query.Query.setOrder` method for - details. :param conditions: the conditions to build the WHERE clause from. See the :meth:`~icat.query.Query.addConditions` method for details. :param includes: list of related objects to add to the INCLUDE clause. See the :meth:`~icat.query.Query.addIncludes` method for details. + :param order: the sorting attributes to build the ORDER BY clause + from. See the :meth:`~icat.query.Query.setOrder` method for + details. :param limit: a tuple (skip, count) to be used in the LIMIT clause. See the :meth:`~icat.query.Query.setLimit` method for details. @@ -198,9 +198,8 @@ class Query(): """ def __init__(self, client, entity, - attributes=None, aggregate=None, order=None, - conditions=None, includes=None, limit=None, - join_specs=None): + attributes=None, aggregate=None, conditions=None, + includes=None, order=None, limit=None, join_specs=None): """Initialize the query. """ @@ -219,13 +218,13 @@ def __init__(self, client, entity, else: raise EntityTypeError("Invalid entity type '%s'." % type(entity)) + self.conditions = [] + self.includes = set() self.setAttributes(attributes) self.setAggregate(aggregate) - self.conditions = [] + self.setJoinSpecs(join_specs) self.addConditions(conditions) - self.includes = set() self.addIncludes(includes) - self.setJoinSpecs(join_specs) self.setOrder(order) self.setLimit(limit) self._init = None @@ -364,38 +363,74 @@ def setAggregate(self, function): else: self.aggregate = None - def setJoinSpecs(self, join_specs): - """Override the join specifications. + def addConditions(self, conditions): + """Add conditions to the constraints to build the WHERE clause from. - :param join_specs: a mapping of related object names to join - specifications. Allowed values are "JOIN", "INNER JOIN", - "LEFT JOIN", and "LEFT OUTER JOIN". Any entry in this - mapping overrides how this particular related object is to - be joined. The default for any relation not included in - the mapping is "JOIN". A special value of :const:`None` - for `join_specs` is equivalent to the empty mapping. - :type join_specs: :class:`dict` - :raise TypeError: if `join_specs` is not a mapping. - :raise ValueError: if any key in `join_specs` is not a name of - a related object or if any value is not in the allowed - set. + :param conditions: the conditions to restrict the search + result. This must be a list of tuples, each being a pair + of an attribute name and a condition on that attribute + respectively. The attribute name may be wrapped with a + JPQL function (such as "UPPER(title)"). - .. versionadded:: 0.19.0 + For backward compatibility with previous versions, this + may alternatively be a mapping of attribute names to a + (lists of) conditions. This legacy use is deprecated, + though. + :type conditions: :class:`list` of :class:`tuple` or :class:`dict` + :raise ValueError: if any attribute in `conditions` is not valid. + + .. versionchanged:: 0.20.0 + allow a JPQL function in the attribute. + + .. versionchanged:: 2.0.0 + expect a :class:`list` of :class:`tuple` in the + `conditions` argument. + .. deprecated:: 2.0.0 + accept a :class:`dict` in the `conditions` argument. """ - if join_specs: - if not isinstance(join_specs, Mapping): - raise TypeError("join_specs must be a mapping") - for obj, js in join_specs.items(): - for (pattr, attrInfo, rclass) in self._attrpath(obj): + if conditions: + self._subst = None + + if isinstance(conditions, Mapping): + # Convert the conditions argument to a list of tuples. + sl = 3 if self._init else 2 + warn("Passing a mapping in the conditions argument is " + "deprecated and will be removed in python-icat 3.0.", + DeprecationWarning, stacklevel=sl) + conds = [] + for obj,v in conditions.items(): + if isinstance(v, str): + conds.append( (obj,v) ) + else: + for rhs in v: + conds.append( (obj,rhs) ) + conditions = conds + + for obj,rhs in conditions: + item = ConditionItem(obj, rhs) + for (pattr, attrInfo, rclass) in self._attrpath(item.attr): + pass + self.conditions.append(item) + + def addIncludes(self, includes): + """Add related objects to build the INCLUDE clause from. + + :param includes: list of related objects to add to the INCLUDE + clause. A special value of "1" may be used to set (the + equivalent of) an "INCLUDE 1" clause. + :type includes: iterable of :class:`str` + :raise ValueError: if any item in `includes` is not a related object. + """ + if includes == "1": + includes = list(self.entity.InstRel) + if includes: + for iobj in includes: + for (pattr, attrInfo, rclass) in self._attrpath(iobj): pass if rclass is None: - raise ValueError("%s.%s is not a related object" - % (self.entity.BeanName, obj)) - if js not in jpql_join_specs: - raise ValueError("invalid join specification %s" % js) - self.join_specs = join_specs - else: - self.join_specs = dict() + raise ValueError("%s.%s is not a related object." + % (self.entity.BeanName, iobj)) + self.includes.update(includes) def setOrder(self, order): """Set the order to build the ORDER BY clause from. @@ -464,75 +499,6 @@ def setOrder(self, order): rattr = "%s.%s" % (item.attr, ra) self.order.append(OrderItem(rattr, cloneFrom=item)) - def addConditions(self, conditions): - """Add conditions to the constraints to build the WHERE clause from. - - :param conditions: the conditions to restrict the search - result. This must be a list of tuples, each being a pair - of an attribute name and a condition on that attribute - respectively. The attribute name may be wrapped with a - JPQL function (such as "UPPER(title)"). - - For backward compatibility with previous versions, this - may alternatively be a mapping of attribute names to a - (lists of) conditions. This legacy use is deprecated, - though. - :type conditions: :class:`list` of :class:`tuple` or :class:`dict` - :raise ValueError: if any attribute in `conditions` is not valid. - - .. versionchanged:: 0.20.0 - allow a JPQL function in the attribute. - - .. versionchanged:: 2.0.0 - expect a :class:`list` of :class:`tuple` in the - `conditions` argument. - .. deprecated:: 2.0.0 - accept a :class:`dict` in the `conditions` argument. - """ - if conditions: - self._subst = None - - if isinstance(conditions, Mapping): - # Convert the conditions argument to a list of tuples. - sl = 3 if self._init else 2 - warn("Passing a mapping in the conditions argument is " - "deprecated and will be removed in python-icat 3.0.", - DeprecationWarning, stacklevel=sl) - conds = [] - for obj,v in conditions.items(): - if isinstance(v, str): - conds.append( (obj,v) ) - else: - for rhs in v: - conds.append( (obj,rhs) ) - conditions = conds - - for obj,rhs in conditions: - item = ConditionItem(obj, rhs) - for (pattr, attrInfo, rclass) in self._attrpath(item.attr): - pass - self.conditions.append(item) - - def addIncludes(self, includes): - """Add related objects to build the INCLUDE clause from. - - :param includes: list of related objects to add to the INCLUDE - clause. A special value of "1" may be used to set (the - equivalent of) an "INCLUDE 1" clause. - :type includes: iterable of :class:`str` - :raise ValueError: if any item in `includes` is not a related object. - """ - if includes == "1": - includes = list(self.entity.InstRel) - if includes: - for iobj in includes: - for (pattr, attrInfo, rclass) in self._attrpath(iobj): - pass - if rclass is None: - raise ValueError("%s.%s is not a related object." - % (self.entity.BeanName, iobj)) - self.includes.update(includes) - def setLimit(self, limit): """Set the limits to build the LIMIT clause from. @@ -547,6 +513,39 @@ def setLimit(self, limit): else: self.limit = None + def setJoinSpecs(self, join_specs): + """Override the join specifications. + + :param join_specs: a mapping of related object names to join + specifications. Allowed values are "JOIN", "INNER JOIN", + "LEFT JOIN", and "LEFT OUTER JOIN". Any entry in this + mapping overrides how this particular related object is to + be joined. The default for any relation not included in + the mapping is "JOIN". A special value of :const:`None` + for `join_specs` is equivalent to the empty mapping. + :type join_specs: :class:`dict` + :raise TypeError: if `join_specs` is not a mapping. + :raise ValueError: if any key in `join_specs` is not a name of + a related object or if any value is not in the allowed + set. + + .. versionadded:: 0.19.0 + """ + if join_specs: + if not isinstance(join_specs, Mapping): + raise TypeError("join_specs must be a mapping") + for obj, js in join_specs.items(): + for (pattr, attrInfo, rclass) in self._attrpath(obj): + pass + if rclass is None: + raise ValueError("%s.%s is not a related object" + % (self.entity.BeanName, obj)) + if js not in jpql_join_specs: + raise ValueError("invalid join specification %s" % js) + self.join_specs = join_specs + else: + self.join_specs = dict() + @property def _order_attrs(self): return { item.attr for item in self.order } @@ -662,12 +661,12 @@ def __repr__(self): kwargs.append("attributes=%s" % repr(self.attributes)) if self.aggregate: kwargs.append("aggregate=%s" % repr(self.aggregate)) - if self.order: - kwargs.append("order=%s" % repr(self.order)) if self.conditions: kwargs.append("conditions=%s" % repr(self.conditions)) if self.includes: kwargs.append("includes=%s" % repr(self.includes)) + if self.order: + kwargs.append("order=%s" % repr(self.order)) if self.limit: kwargs.append("limit=%s" % repr(self.limit)) if self.join_specs: From 6db11958c88757172585030a1a3e30ae9ce693ae Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 30 Aug 2024 15:36:41 +0200 Subject: [PATCH 25/26] Update changelog --- CHANGES.rst | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index ac456c98..affa1794 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,40 @@ Changelog 2.0.0 (not yet released) ~~~~~~~~~~~~~~~~~~~~~~~~ +Modified features +----------------- + ++ `#158`_: Review the internal data structures in class + :class:`icat.query.Query`. As a consequence, change the format of + the `conditions` argument to :class:`icat.query.Query` and + :meth:`icat.query.Query.addConditions` from a mapping to a list of + tuples. The legacy format is still supported, but deprecated. + Furthermore drop the restriction that any attribute may only appear + once in the `order` argument to :class:`icat.query.Query` and + :meth:`icat.query.Query.setOrder`. + +Incompatible changes and deprecations +------------------------------------- + ++ `#158`_: Deprecate passing a mapping in the `conditions` argument to + :class:`icat.query.Query` and :meth:`icat.query.Query.addConditions`. + In calling code, change:: + + query = Query(client, "Datafile", conditions={ + "dataset.name": "= 'e208945'", + "datafileCreateTime": [">= '2012-01-01'", "< '2013-01-01'" ], + }) + + to:: + + query = Query(client, "Datafile", conditions=[ + ("dataset.name", "= 'e208945'"), + ("datafileCreateTime", ">= '2012-01-01'"), + ("datafileCreateTime", "< '2013-01-01'" ), + ]) + +.. _#158: https://github.com/icatproject/python-icat/pull/158 + .. _changes-1_4_0: From 402581862845d1c85c0e87cc764fbab11ac54061 Mon Sep 17 00:00:00 2001 From: Rolf Krahl Date: Fri, 30 Aug 2024 16:44:00 +0200 Subject: [PATCH 26/26] Add a comment in test_09_deprecations.py that testing the legacy use of the conditions argument is done in test_06_query.py --- tests/test_09_deprecations.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_09_deprecations.py b/tests/test_09_deprecations.py index a7a8e84b..f6e1be47 100644 --- a/tests/test_09_deprecations.py +++ b/tests/test_09_deprecations.py @@ -8,5 +8,7 @@ # - Module variable icat.config.defaultsection. # It's easier to test this in the setting of the test_01_config.py # module. +# - Passing a mapping in the conditions argument to icat.query.Query +# and icat.query.Query.addConditions(): tested in test_06_query.py # # No other deprecations for the moment.