Correct multi-value and uid-list matching filters

The multi-value match should be a logical AND of all values.

>  Such a match is successful only if each and every value in the Key Attribute matches a value in the corresponding Attribute in the entity.
https://dicom.nema.org/medical/dicom/current/output/chtml/part04/sect_C.2.2.2.8.html

The UID list matching has separate semantics and should be a logical
OR. The two value types are now parsed unambiguously.

Author: georgegeddes

chris_backend/dicomweb/query.py

@@ -137,6 +137,11 @@ class MultipleValue[T]:
     values: list[T]
 
 
+@dataclass
+class UIDListValue:
+    values: list[str]
+
+
 @dataclass
 class Attribute:
     keyword: str
@@ -174,19 +179,28 @@ def cap_multi_value(self) -> bool:
             return False
         return self.vm != '1'
 
+    def cap_uid_list(self) -> bool:
+        return self.vr in UID_LIST_VRS
+
     def parse(self, value: str, multi_value: bool = False) -> Any:
         if value == '':
             return UniversalValue()
         if value == '""':
             return EmptyValue()
         if self.cap_range() and "-" in value:
             return self.parse_range(value)
-        if self.vr in UID_LIST_VRS or (self.cap_multi_value() and multi_value):
+        if self.cap_multi_value() and multi_value:
             parsed_value = self.parse_multiple(value)
             length = len(parsed_value.values)
             if length == 1:
                 return parsed_value.values[0]
             return parsed_value
+        if self.cap_uid_list():
+            uid_list_value = self.parse_uid_list(value)
+            length = len(uid_list_value.values)
+            if length == 1:
+                return uid_list_value.values[0]
+            return uid_list_value
         return self.parse_single(value)
 
     def parse_single(self, value: str) -> Any:
@@ -208,6 +222,9 @@ def parse_range(self, value: str) -> RangeValue:
     def parse_multiple(self, value: str) -> MultipleValue:
         return MultipleValue([self.parse_single(v) for v in re.split(r'[\\,]', value) if len(v) > 0])
 
+    def parse_uid_list(self, value: str) -> UIDListValue:
+        return UIDListValue([self.parse_single(v) for v in re.split(',', value) if len(v) > 0])
+
 
 # ---------------------------------------------------------------------------
 # Supported Query fields
@@ -304,7 +321,13 @@ def apply(self, qs: QuerySet, search_query: SearchQuery) -> QuerySet:
                         q |= range_q
                         continue
                     case MultipleValue(multi_values):
-                        q |= Q(**{f'{attribute.orm_field}__in': multi_values})
+                        multi_value_q = Q()
+                        for value in multi_values:
+                            multi_value_q &= Q(**{f'{attribute.orm_field}__contains': value})
+                        q |= multi_value_q
+                        continue
+                    case UIDListValue(uid_list):
+                        q |= Q(**{f'{attribute.orm_field}__in': uid_list})
                         continue
                     case UniversalValue():
                         # PS3.4 §C.2.2.2.3 Universal Matching

chris_backend/dicomweb/tests/test_query.py

@@ -277,14 +277,20 @@ def test_multiple_value_emits_in(self):
         # VM>1) has no such field in the resource maps, so drive it with a
         # synthetic ModalitiesInStudy (CS VM 1-n) mapped onto a real column. The
         # ungated UID-list IN path is covered separately by the real-map tests.
-        from pacsfiles.models import PACSSeries
-        attr = Attribute('ModalitiesInStudy', orm_field='Modality')
-        qf = QueryFilter('series')
+        from dicomweb.models import PACSStudy
+        qf = QueryFilter('study')
+
+        # TODO: remove monkeypatch after ModalitiesInStudy implemented
+        attr = Attribute('ModalitiesInStudy', orm_field='PatientName')
         qf.tag_map = {attr.tag: attr}
+
         sq = SearchQuery.from_query_dict(
             QueryDict('ModalitiesInStudy=CT,MR&multiplevaluematching=true'))
-        sql = qf.apply(PACSSeries.objects.all(), sq).query.sql_with_params()[0]
-        self.assertIn('IN (', sql)
+        sql, params = qf.apply(PACSStudy.objects.all(), sq).query.sql_with_params()
+        self.assertIn(' AND ', sql)
+        self.assertIn(' LIKE ', sql)
+        self.assertIn('%CT%', params)
+        self.assertIn('%MR%', params)
 
     def test_quoted_empty_skipped_without_flag(self):
         self.assertFalse(self._has_where('series', 'PatientName=""'))
@@ -419,17 +425,16 @@ def test_multiple_value_matching_monkeypatch(self):
         # multi-value matching path.
         # TODO: remove this once the above expected failure is passing
         from pacsfiles.models import PACSSeries
-        self._series(PatientName='A', Modality='CT')
-        self._series(PatientName='B', Modality='MR')
-        self._series(PatientName='C', Modality='US')
+        self._series(PatientName='A', StudyInstanceUID='A.1', Modality=r'CT\MR')
+        self._series(PatientName='B', StudyInstanceUID='B.1', Modality=r'US')
         attr = Attribute('ModalitiesInStudy', orm_field='Modality')
         qf = QueryFilter('series')
         qf.tag_map = {attr.tag: attr}
         sq = SearchQuery.from_query_dict(
             QueryDict('ModalitiesInStudy=CT,MR&multiplevaluematching=true'))
         names = set(qf.apply(PACSSeries.objects.all(), sq)
                     .values_list('PatientName', flat=True))
-        self.assertEqual(names, {'A', 'B'})
+        self.assertEqual(names, {'A'})
 
     def test_uid_list_matches_multiple_rows(self):
         # StudyInstanceUID=<uid>,<uid> (UI VR, ungated IN path via UID_LIST_VRS)