diff --git a/src/test/java/org/nrg/testing/xnat/tests/TestDicomWebQido.java b/src/test/java/org/nrg/testing/xnat/tests/TestDicomWebQido.java index d7b9182..f512e98 100644 --- a/src/test/java/org/nrg/testing/xnat/tests/TestDicomWebQido.java +++ b/src/test/java/org/nrg/testing/xnat/tests/TestDicomWebQido.java @@ -19,8 +19,13 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; +import java.time.LocalDate; +import java.time.LocalTime; +import java.time.format.DateTimeFormatter; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static org.nrg.testing.TestGroups.*; import static org.testng.Assert.*; @@ -47,7 +52,7 @@ */ @TestRequires( specificPluginRequirements = { - @PluginRequirement(pluginId = "dicomwebproxy") + @PluginRequirement(pluginId = "dicomwebplugin") }, data = { TestData.EXTRACTION_MR, // Primary: Small MR for smoke tests @@ -76,6 +81,12 @@ private static String tagKey(int tag) { private String mrStudyUID; private String ctStudyUID; + // Probed StudyDate / StudyTime for the MR session (yyyyMMdd / HHmmss). + // Populated in setupQidoTests; used by the date/time range tests below + // to build query values around known values. + private String mrStudyDate; + private String mrStudyTime; + @BeforeClass public void setupQidoTests() { // Set up session import extensions BEFORE creating project @@ -88,6 +99,10 @@ public void setupQidoTests() { // Extract UIDs from created sessions for validation extractSessionUIDs(); + + // Probe QIDO for the MR study's StudyDate/StudyTime so + // range tests below can build query values around them. + probeMrStudyDateAndTime(); } @AfterClass(alwaysRun = true) @@ -104,7 +119,6 @@ private void extractSessionUIDs() { JsonPath mrJson = restDriver.mainInterface().jsonQuery() .get(mainInterface().subjectAssessorUrl(mrSession)) .then().assertThat().statusCode(200).and().extract().jsonPath(); - System.out.println("retrieved identifiers: " + mrJson.prettyPrint()); Map mrDataFields = mrJson.get("items[0].data_fields"); mrStudyUID = (String) mrDataFields.get("UID"); @@ -1323,6 +1337,390 @@ public void testDicomJsonEncoding() { } } + // ======================================== + // QIDO-RS: Combined Date-Time Range Queries + // ======================================== + // + // These tests exercise combined DA + TM range matching per + // PS3.18 §8.3.4.1.1 → PS3.4 §C.2.2.2.5.4. When both StudyDate + // and StudyTime are Range Matching values, the server must + // treat them as a single DT range rather than as two + // independent conjuncts. The PS3.4 §C.2.2.2.5.4 Note gives + // the canonical example: + // + // "a Study Date of '20060705-20060707' and a Study Time of + // '1000-1800' will match the time period of July 5, 10am + // until July 7, 6pm, rather than the three time periods + // of 10am until 6pm on each of July 5, July 6 and July 7." + // + // The distinguishing test below constructs a StudyTime range + // whose upper bound is strictly before the MR session's + // StudyTime. Under independent conjunction the MR session + // would be excluded; under combined DT semantics the MR + // session's (date, time) falls within the diagonal band and + // must be included. + + private static final DateTimeFormatter DA_FORMAT = + DateTimeFormatter.ofPattern("yyyyMMdd"); + private static final DateTimeFormatter TM_FORMAT = + DateTimeFormatter.ofPattern("HHmmss"); + + /** + * Probe QIDO for the MR study's StudyDate/StudyTime so range + * tests below can build queries around known values. Called + * from {@link #setupQidoTests()}. + */ + private void probeMrStudyDateAndTime() { + String endpoint = String.format( + "/dicomweb/projects/%s/studies?StudyInstanceUID=%s", + testProject.getId(), mrStudyUID); + Response response = mainQueryBase() + .get(formatXapiUrl(endpoint)) + .then().assertThat().statusCode(200) + .extract().response(); + List> studies = response.jsonPath().getList("$"); + assertEquals(studies.size(), 1, + "MR study probe should return exactly 1 study"); + Map study = studies.get(0); + mrStudyDate = getDicomTagValue(study, tagKey(Tag.StudyDate)); + String time = getDicomTagValue(study, tagKey(Tag.StudyTime)); + // Strip fractional seconds; range tests only compare HHmmss. + mrStudyTime = (time == null || time.length() <= 6) + ? time : time.substring(0, 6); + assertNotNull(mrStudyDate, + "MR study probe should yield a non-null StudyDate"); + assertEquals(mrStudyDate.length(), 8, + "StudyDate should be 8-digit yyyyMMdd"); + } + + /** Return true iff any study in {@code studies} carries {@code uid}. */ + private boolean containsStudyUID(List> studies, + String uid) { + return studies.stream().anyMatch(s -> { + Map tag = (Map) + s.get(tagKey(Tag.StudyInstanceUID)); + if (tag == null) { + return false; + } + List values = (List) tag.get("Value"); + return values != null && values.contains(uid); + }); + } + + /** + * Distinguishing test: multi-day StudyDate range paired with a + * StudyTime range whose upper bound is strictly before the MR + * session's actual time. + * + *

Under independent conjunction the MR session would be + * excluded (its time falls outside the TM range). Under + * combined DT semantics the MR session's (date, time) falls + * within the diagonal band [dayBefore + 00:00:00, + * dayAfter + timeBeforeMr] and must be included. This test + * fails if the server ever regresses to independent + * conjunction. + */ + @Test(priority = 2) + public void testQidoSearchStudies_WithCombinedDateTimeRange_ExcludingTime_MatchesUnderCombined() { + // This test needs a non-null, non-midnight StudyTime in the + // MR test data so we can construct a "before mrStudyTime" + // upper bound and demonstrate the combined-vs-independent + // divergence. Failing here is a signal to improve the test + // data (EXTRACTION_MR), not a signal about server behavior. + assertNotNull(mrStudyTime, + "MR test data must have a non-null StudyTime to exercise " + + "combined date-time matching; probe returned null. " + + "Replace EXTRACTION_MR with a fixture that carries a " + + "populated (0008,0030)."); + assertNotEquals(mrStudyTime, "000000", + "MR test data StudyTime is exactly midnight (000000); this " + + "test cannot construct a strictly-before upper bound " + + "without underflowing. Replace EXTRACTION_MR with a " + + "fixture whose StudyTime is not 00:00:00."); + LocalDate mrDate = LocalDate.parse(mrStudyDate, DA_FORMAT); + LocalTime mrTime = LocalTime.parse(mrStudyTime, TM_FORMAT); + String dayBefore = mrDate.minusDays(1).format(DA_FORMAT); + String dayAfter = mrDate.plusDays(1).format(DA_FORMAT); + String timeBeforeMr = mrTime.minusSeconds(1).format(TM_FORMAT); + + String dateRange = dayBefore + "-" + dayAfter; + String timeRange = "000000-" + timeBeforeMr; + String endpoint = String.format( + "/dicomweb/projects/%s/studies?StudyDate=%s&StudyTime=%s", + testProject.getId(), dateRange, timeRange); + Response response = mainQueryBase() + .get(formatXapiUrl(endpoint)) + .then().assertThat().statusCode(200) + .contentType("application/dicom+json") + .extract().response(); + List> studies = response.jsonPath().getList("$"); + + assertTrue(containsStudyUID(studies, mrStudyUID), + "MR study should be INCLUDED under combined DT semantics " + + "(date " + dateRange + ", time " + timeRange + "); " + + "independent-conjunction would incorrectly exclude it " + + "because the study's time falls outside the TM range."); + } + + /** + * Regression: a single-day StudyDate range paired with a + * StudyTime range that includes the MR session's actual time + * must return the MR session. Combined and independent + * semantics agree on this case, so the test verifies that + * pre-existing behavior is preserved. + */ + @Test(priority = 2) + public void testQidoSearchStudies_WithCombinedDateTimeRange_SingleDayWithMatchingTime_IncludesStudy() { + // This regression check needs a non-null StudyTime in the MR + // test data. Failing here means the test data lacks a + // populated (0008,0030), not that the server misbehaves. + assertNotNull(mrStudyTime, + "MR test data must have a non-null StudyTime for this " + + "regression check; probe returned null. Replace " + + "EXTRACTION_MR with a fixture that carries a populated " + + "(0008,0030)."); + String dateRange = mrStudyDate + "-" + mrStudyDate; + String timeRange = "000000-235959"; + String endpoint = String.format( + "/dicomweb/projects/%s/studies?StudyDate=%s&StudyTime=%s", + testProject.getId(), dateRange, timeRange); + Response response = mainQueryBase() + .get(formatXapiUrl(endpoint)) + .then().assertThat().statusCode(200) + .extract().response(); + List> studies = response.jsonPath().getList("$"); + + assertTrue(containsStudyUID(studies, mrStudyUID), + "Single-day StudyDate range with a StudyTime range " + + "covering the whole day should include the MR study."); + } + + // ======================================== + // QIDO-RS: {attributeID} Forms (Keyword vs. Hex Tag) + // ======================================== + // + // Per PS3.18 §6.7.1.1, the {attributeID} in a QIDO-RS query + // parameter may be either a DICOM keyword (e.g. StudyDate) or an + // 8-digit hexadecimal Data Element Tag (e.g. 00080020). Weasis + // and OHIF historically send hex-tag form; XNAT's DQR and web + // front-end send keywords. Both must work identically, otherwise + // clients silently see "no filter applied" instead of "filter + // applied" — the failure mode that motivated PLUGINS-32x. + + /** + * Return the 8-character uppercase hex string used as the QIDO + * {@code {attributeID}} for a dcm4che {@code Tag} constant. + */ + private static String tagId(int tag) { + return tagKey(tag); + } + + /** + * Extract the set of StudyInstanceUIDs from a QIDO study + * response. Used to compare result sets across queries that + * differ only in {@code {attributeID}} form. + */ + private Set studyUidsIn(List> studies) { + Set uids = new HashSet<>(); + for (Map study : studies) { + Map tag = (Map) + study.get(tagKey(Tag.StudyInstanceUID)); + if (tag == null) continue; + List values = (List) tag.get("Value"); + if (values != null) uids.addAll(values); + } + return uids; + } + + /** + * Perform a QIDO study query with the given raw query string and + * return the parsed JSON array. Fails the test if the response + * is not HTTP 200. + */ + private List> qidoStudies(String rawQueryString) { + String endpoint = String.format( + "/dicomweb/projects/%s/studies%s", + testProject.getId(), + rawQueryString.isEmpty() ? "" : "?" + rawQueryString); + Response response = mainQueryBase() + .get(formatXapiUrl(endpoint)) + .then() + .assertThat() + .statusCode(200) + .contentType("application/dicom+json") + .extract() + .response(); + return response.jsonPath().getList("$"); + } + + /** + * Hex-tag form of the StudyInstanceUID filter must return the + * same single study as the keyword form. Mirrors + * {@link #testQidoSearchStudies_WithStudyUIDFilter()}. + */ + @Test(priority = 1) + public void testQidoSearchStudies_HexTagStudyInstanceUID() { + List> studies = qidoStudies( + tagId(Tag.StudyInstanceUID) + "=" + ctStudyUID); + assertEquals(studies.size(), 1, + "Hex-tag StudyInstanceUID filter should return exactly 1 study"); + Map uidTag = (Map) + studies.get(0).get(tagKey(Tag.StudyInstanceUID)); + List values = (List) uidTag.get("Value"); + assertEquals(values.get(0), ctStudyUID, + "Returned study should match queried StudyInstanceUID"); + } + + /** + * Hex-tag form of the Modality filter must exclude non-matching + * modalities exactly as the keyword form does. + */ + @Test(priority = 1) + public void testQidoSearchStudies_HexTagModality() { + List> studies = qidoStudies( + tagId(Tag.Modality) + "=CT"); + assertTrue(containsStudyUID(studies, ctStudyUID), + "Hex-tag Modality=CT filter should include the CT study"); + assertFalse(containsStudyUID(studies, mrStudyUID), + "Hex-tag Modality=CT filter should exclude the MR study"); + } + + /** + * ModalitiesInStudy (0008,0061) is treated as an alias for + * Modality (0008,0060) — verify both the alias keyword and its + * hex-tag form take effect. + */ + @Test(priority = 1) + public void testQidoSearchStudies_HexTagModalitiesInStudyAliasesModality() { + List> studies = qidoStudies( + tagId(Tag.ModalitiesInStudy) + "=CT"); + assertTrue(containsStudyUID(studies, ctStudyUID), + "Hex-tag ModalitiesInStudy=CT should include the CT study " + + "(alias for Modality)"); + assertFalse(containsStudyUID(studies, mrStudyUID), + "Hex-tag ModalitiesInStudy=CT should exclude the MR study"); + } + + /** + * The motivating case: hex-tag form of the StudyDate range must + * actually filter (not be silently ignored). A single-day range + * around the known MR study date must include the MR study and + * exclude studies on other dates. + */ + @Test(priority = 1) + public void testQidoSearchStudies_HexTagStudyDateRange() { + String dateRange = mrStudyDate + "-" + mrStudyDate; + List> studies = qidoStudies( + tagId(Tag.StudyDate) + "=" + dateRange); + assertTrue(containsStudyUID(studies, mrStudyUID), + "Hex-tag StudyDate range " + dateRange + + " should include the MR study"); + } + + /** + * Hex-tag {@code {attributeID}} must be accepted case-insensitively + * (PS3.18 doesn't require a specific case; clients emit both). + */ + @Test(priority = 1) + public void testQidoSearchStudies_HexTagLowerCase() { + List> studies = qidoStudies( + tagId(Tag.StudyInstanceUID).toLowerCase() + + "=" + ctStudyUID); + assertEquals(studies.size(), 1, + "Lowercase hex-tag filter should return exactly 1 study"); + } + + /** + * Some clients send the DIMSE-style {@code (gggg,eeee)} form. + * The plugin tolerates it as a convenience even though PS3.18 + * doesn't require this spelling. + */ + @Test(priority = 1) + public void testQidoSearchStudies_ParenthesizedTagForm() { + // (0008,0060) instead of 00080060 + String endpoint = String.format( + "/dicomweb/projects/%s/studies?(0008,0060)=CT", + testProject.getId()); + Response response = mainQueryBase() + .get(formatXapiUrl(endpoint)) + .then().assertThat().statusCode(200) + .contentType("application/dicom+json") + .extract().response(); + List> studies = response.jsonPath().getList("$"); + assertTrue(containsStudyUID(studies, ctStudyUID), + "Parenthesized (0008,0060)=CT filter should include the CT study"); + assertFalse(containsStudyUID(studies, mrStudyUID), + "Parenthesized (0008,0060)=CT filter should exclude the MR study"); + } + + /** + * The strongest statement of the fix: for the same underlying + * query, the keyword and hex-tag forms must return the exact + * same set of studies (by UID). If one form applies a filter + * and the other doesn't, this test catches it directly. + */ + @Test(priority = 1) + public void testQidoSearchStudies_KeywordAndHexTagFormsReturnSameStudies() { + Set byKeyword = studyUidsIn(qidoStudies("Modality=CT")); + Set byHexTag = studyUidsIn(qidoStudies( + tagId(Tag.Modality) + "=CT")); + assertEquals(byHexTag, byKeyword, + "Keyword and hex-tag Modality=CT must return the same " + + "set of studies. Keyword: " + byKeyword + + " Hex-tag: " + byHexTag); + + // Repeat with the date-range case, which was the specific + // regression that motivated adding hex-tag support. + String dateRange = mrStudyDate + "-" + mrStudyDate; + Set dateByKeyword = studyUidsIn(qidoStudies( + "StudyDate=" + dateRange)); + Set dateByHexTag = studyUidsIn(qidoStudies( + tagId(Tag.StudyDate) + "=" + dateRange)); + assertEquals(dateByHexTag, dateByKeyword, + "Keyword and hex-tag StudyDate range must return the same " + + "set of studies. Keyword: " + dateByKeyword + + " Hex-tag: " + dateByHexTag); + } + + /** + * Series-level QIDO must also accept hex-tag form (SeriesInstanceUID + * and Modality are the two study-and-series-level parameters). + */ + @Test(priority = 2) + public void testQidoSearchSeries_HexTagModalityMatchesKeyword() { + String base = String.format("/dicomweb/projects/%s/studies/%s/series", + testProject.getId(), ctStudyUID); + + Response kwResponse = mainQueryBase() + .get(formatXapiUrl(base + "?Modality=CT")) + .then().assertThat().statusCode(200) + .contentType("application/dicom+json") + .extract().response(); + List> byKeyword = kwResponse.jsonPath().getList("$"); + + Response hexResponse = mainQueryBase() + .get(formatXapiUrl(base + "?" + tagId(Tag.Modality) + "=CT")) + .then().assertThat().statusCode(200) + .contentType("application/dicom+json") + .extract().response(); + List> byHexTag = hexResponse.jsonPath().getList("$"); + + Set kwUids = new HashSet<>(); + for (Map s : byKeyword) { + kwUids.add((String) ((List) ((Map) + s.get(tagKey(Tag.SeriesInstanceUID))).get("Value")).get(0)); + } + Set hxUids = new HashSet<>(); + for (Map s : byHexTag) { + hxUids.add((String) ((List) ((Map) + s.get(tagKey(Tag.SeriesInstanceUID))).get("Value")).get(0)); + } + assertEquals(hxUids, kwUids, + "Series-level Modality=CT: keyword and hex-tag forms must " + + "return the same series UIDs. Keyword: " + kwUids + + " Hex-tag: " + hxUids); + } + // ======================================== // Helper Methods // ========================================