Skip to content

Commit

Permalink
Support DT TM types in Extended Query Tags (#1072)
Browse files Browse the repository at this point in the history
* SQL level changes for supporting DT and TM in Extended Query Tags.

* Updated code such that immutable user types are not updated. All the previous SqlIndexDataStore versions call the previous user type whereas the latest version uses the new user table type.

* Addressed comments.

* Removed TagValueUTC from IXC_ExtendedQueryTagDateTime index as we are not using this column yet for QIDO. Once offsets are supported in QIDO, we can add this back.

* Undid changes from EndAddInstancec stored procedure as Will is working on removing this stored procedure.

* Introduced a new schema version constant, SupportDTAndTMInExtendedQueryTagSchemaVersion, that indicates if DT and TM VR types are supported for extended query tag in the specified version. Following this, this constant has been used to check and use respective fields in the builder eliminating the need for multiple versions of ExtendedQueryTagDataRows and ExtendedQueryTagDataRowsBuilder.
Tests have also been updated, and all the tests are passing.

* Addressed comments.

* Fixed casing for Utc.

* Supporting DT and TM as Extended Query Tags.

- DT and TM are now accepted as Extended Query Tags.
- On STOW, newly created tags are indexed properly.
- Single Value Match working for DT and TM.
- Range Value Match working for DT and TM.
- Deletion: when an instance is deleted, it's respective entries are deleted from respective extended query tags tables.
- Deletion: when DT or TM extended query tag is deleted, respective entries are also deleted from extended query tags tables.
- Added unit tests for DT and TM, and range query matching.

* Merged with latest main.
Fixed reindexing for DT and TM in cases where studies already existed and then extended query tags were created.

* Addressed comments.
If offset is provided in the query string for QIDO, user gets a specific message that offset is not supported in QIDO.

* Addressed comments.
Fixed UTC and local date time calculations.
Using fo-dicom validation for DT and TM.
Updated unit tests for DT.

* Updated query parser to use DateTime instead of DateTimeOffset.

* Addressed comments.
Using fo-dicom parser to parse DT and TM.
Consolidated local and utc date time parsers into one.

* Utilizing GetSingleValueOrDefault for TM type.

* Addressed comments. Fixed the DT logic. Now there can be two cases:
1. Offset is provided either in the date time value, or in the TimezoneOffsetFromUTC field in the dicom dataset.
2. Offset is not provided.
In the first case, the base date time is considered as the literal date time and stored in TagValue. Offset field is applied to this literal date time to convert to UTC and is stored in TagValueUTC.
In the second case where offset information is not present, null is stored in TagValueUTC.

* Updated extended query tags documentation.

* Added tests to test the scenario where offset is not present in the DT value but is present in TimezoneOffsetFromUTC value. This test caught an issue with the code.
TimeSpan.TryParseExact method does not support negative offsets by default. This is tackled by checking the sign in the offset and then applying TimeSpanStyles.AssumeNegative.

* Updating documentation adding information about offsets not being supported when querying on DT type.
  • Loading branch information
alishahahmed authored Oct 9, 2021
1 parent 529c300 commit dada9b7
Show file tree
Hide file tree
Showing 30 changed files with 989 additions and 42 deletions.
6 changes: 4 additions & 2 deletions docs/how-to-guides/extended-query-tags.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ The matching types stated below are valid for extended query tags.

| Search Type | Supported VR | Example |
| :---------- | :-------------- | :----------------------------------------------------------- |
| Range Query | Date (DA) | {attributeID}={value1}-{value2}. For date/ time values, we supported an inclusive range on the tag. This will be mapped to `attributeID >= {value1} AND attributeID <= {value2}`. |
| Range Query | Date (DA), Date Time (DT), Time (TM) | {attributeID}={value1}-{value2}. For date/ time values, we supported an inclusive range on the tag. This will be mapped to `attributeID >= {value1} AND attributeID <= {value2}`. |
| Exact Match | All | {attributeID}={value1} |
| Fuzzy Match | PersonName (PN) | Matches any component of the patient name which starts with the value. |

Expand All @@ -221,6 +221,7 @@ Currently, only the following VR types are supported:
- Age String (AS)
- Code String (CS)
- Date (DA)
- Date Time (DT)
- Decimal String (DS)
- Floating Point Double (FD)
- Floating Point Single (FL)
Expand All @@ -230,11 +231,12 @@ Currently, only the following VR types are supported:
- Short String (SH)
- Signed Long (SL)
- Signed Short (SS)
- Time (TM)
- Unique Identifier [UID] (UI)
- Unsigned Long (UL)
- Unsigned Short (US)

Sequential tags i.e. tags under a tag of type Sequence of Items (SQ) are currently not supported.
Sequential tags i.e. tags under a tag of type Sequence of Items (SQ) are currently not supported. Passing in offsets when querying on DT type is also not supported.

All management APIs are currently synchronous. This means that a [delete](#remove-an-extended-query-tag) request may run long as it attempts to remove any infrastructure that was put in place to support querying against the extended query tag.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System.Collections;
using System.Collections.Generic;

namespace Microsoft.Health.Dicom.Core.UnitTests.Extensions
{
public class DateTimeValidTestData : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
{
yield return new object[] { "20200301010203.123+0500", 2020, 03, 01, 01, 02, 03, 123 };
yield return new object[] { "20200301010203.123-0500", 2020, 03, 01, 01, 02, 03, 123 };
yield return new object[] { "20200301010203-0500", 2020, 03, 01, 01, 02, 03, 0 };
yield return new object[] { "202003010102-0500", 2020, 03, 01, 01, 02, 0, 0 };
yield return new object[] { "2020030101-0500", 2020, 03, 01, 01, 0, 0, 0 };
yield return new object[] { "20200301-0500", 2020, 03, 01, 0, 0, 0, 0 };
yield return new object[] { "202003-0500", 2020, 03, 01, 00, 0, 0, 0 };
yield return new object[] { "2020-0500", 2020, 01, 01, 00, 0, 0, 0 };
yield return new object[] { "20200301010203.123", 2020, 03, 01, 01, 02, 03, 123 };
yield return new object[] { "20200301010203", 2020, 03, 01, 01, 02, 03, 0 };
yield return new object[] { "202003010102", 2020, 03, 01, 01, 02, 0, 0 };
yield return new object[] { "2020030101", 2020, 03, 01, 01, 0, 0, 0 };
yield return new object[] { "20200301", 2020, 03, 01, 0, 0, 0, 0 };
yield return new object[] { "202003", 2020, 03, 01, 0, 0, 0, 0 };
yield return new object[] { "2020", 2020, 01, 01, 0, 0, 0, 0 };
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System.Collections;
using System.Collections.Generic;

namespace Microsoft.Health.Dicom.Core.UnitTests.Extensions
{
public class DateTimeValidUtcTestData : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
{
yield return new object[] { "20200301010203.123+0500", 2020, 02, 29, 20, 02, 03, 123 };
yield return new object[] { "20200301010203.123-0500", 2020, 03, 01, 06, 02, 03, 123 };
yield return new object[] { "20200301010203-0500", 2020, 03, 01, 06, 02, 03, 0 };
yield return new object[] { "202003010102-0500", 2020, 03, 01, 06, 02, 0, 0 };
yield return new object[] { "2020030101-0500", 2020, 03, 01, 06, 0, 0, 0 };
yield return new object[] { "20200301-0500", 2020, 03, 01, 05, 0, 0, 0 };
yield return new object[] { "202003-0500", 2020, 03, 01, 05, 0, 0, 0 };
yield return new object[] { "2020-0500", 2020, 01, 01, 05, 0, 0, 0 };
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System.Collections;
using System.Collections.Generic;

namespace Microsoft.Health.Dicom.Core.UnitTests.Extensions
{
public class DateTimeWithTimezoneOffsetFromUtcValidTestData : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
{
yield return new object[] { "20200301010203.123", "+0500", 2020, 03, 01, 06, 02, 03, 123 };
yield return new object[] { "20200301010203.123", "-0500", 2020, 02, 29, 20, 02, 03, 123 };
yield return new object[] { "20200301010203", "-0500", 2020, 02, 29, 20, 02, 03, 0 };
yield return new object[] { "202003010102", "-0500", 2020, 02, 29, 20, 02, 0, 0 };
yield return new object[] { "2020030101", "-0500", 2020, 02, 29, 20, 0, 0, 0 };
yield return new object[] { "20200301", "-0500", 2020, 02, 29, 19, 0, 0, 0 };
yield return new object[] { "202003", "-0500", 2020, 02, 29, 19, 0, 0, 0 };
yield return new object[] { "2020", "-0500", 2019, 12, 31, 19, 0, 0, 0 };
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ public void GivenAValidDicomDateValue_WhenGetStringDateAsDateTimeIsCalled_ThenCo
_dicomDataset.Add(DicomTag.StudyDate, "20200301");

Assert.Equal(
new DateTime(2020, 3, 1, 0, 0, 0, 0, DateTimeKind.Utc),
_dicomDataset.GetStringDateAsDate(DicomTag.StudyDate));
new DateTime(2020, 3, 1, 0, 0, 0, 0, DateTimeKind.Local),
_dicomDataset.GetStringDateAsDate(DicomTag.StudyDate).Value);
}

[Fact]
Expand All @@ -72,6 +72,170 @@ public void GivenAnInvalidDicomDateValue_WhenGetStringDateAsDateTimeIsCalled_The
Assert.Null(_dicomDataset.GetStringDateAsDate(DicomTag.StudyDate));
}

[Fact]
public void GivenNoDicomDateTimeValue_WhenGetStringDateTimeAsLiteralAndUtcDateTimesIsCalled_ThenNullShouldBeReturned()
{
Tuple<DateTime?, DateTime?> result = _dicomDataset.GetStringDateTimeAsLiteralAndUtcDateTimes(DicomTag.AcquisitionDateTime);
Assert.Null(result.Item1);
Assert.Null(result.Item2);
}

[Theory]
[ClassData(typeof(DateTimeValidTestData))]
public void GivenAValidDicomDateTimeValue_WhenGetStringDateTimeAsLiteralAndUtcDateTimesIsCalled_ThenCorrectLiteralDateTimesShouldBeReturned(
string acquisitionDateTime,
int year,
int month,
int day,
int hour,
int minute,
int second,
int millisecond
)
{
_dicomDataset.Add(DicomTag.AcquisitionDateTime, acquisitionDateTime);
Assert.Equal(
new DateTime(
year,
month,
day,
hour,
minute,
second,
millisecond),
_dicomDataset.GetStringDateTimeAsLiteralAndUtcDateTimes(DicomTag.AcquisitionDateTime).Item1.Value);
}

[Theory]
[ClassData(typeof(DateTimeValidUtcTestData))]
public void GivenAValidDicomDateTimeValueWithOffset_WhenGetStringDateTimeAsLiteralAndUtcDateTimesIsCalled_ThenCorrectUtcDateTimesShouldBeReturned(
string acquisitionDateTime,
int year,
int month,
int day,
int hour,
int minute,
int second,
int millisecond
)
{
_dicomDataset.Add(DicomTag.AcquisitionDateTime, acquisitionDateTime);
Assert.Equal(
new DateTime(
year,
month,
day,
hour,
minute,
second,
millisecond),
_dicomDataset.GetStringDateTimeAsLiteralAndUtcDateTimes(DicomTag.AcquisitionDateTime).Item2.Value);
}

[Theory]
[ClassData(typeof(DateTimeWithTimezoneOffsetFromUtcValidTestData))]
public void GivenAValidDicomDateTimeWithoutOffsetWithTimezoneOffsetFromUtc_WhenGetStringDateTimeAsLiteralAndUtcDateTimesIsCalled_ThenCorrectUtcDateTimesShouldBeReturned(
string acquisitionDateTime,
string timezoneOffsetFromUTC,
int year,
int month,
int day,
int hour,
int minute,
int second,
int millisecond)
{
_dicomDataset.Add(DicomTag.AcquisitionDateTime, acquisitionDateTime);
_dicomDataset.Add(DicomTag.TimezoneOffsetFromUTC, timezoneOffsetFromUTC);
Assert.Equal(
new DateTime(
year,
month,
day,
hour,
minute,
second,
millisecond),
_dicomDataset.GetStringDateTimeAsLiteralAndUtcDateTimes(DicomTag.AcquisitionDateTime).Item2.Value);
}

[Fact]
public void GivenAValidDicomDateTimeValueWithoutOffset_WhenGetStringDateTimeAsLiteralAndUtcDateTimesIsCalled_ThenNullIsReturnedForUtcDateTime()
{
_dicomDataset.Add(DicomTag.AcquisitionDateTime, "20200102030405.678");

Assert.Null(_dicomDataset.GetStringDateTimeAsLiteralAndUtcDateTimes(DicomTag.AcquisitionDateTime).Item2);
}

[Theory]
[InlineData("20200301010203.123+9900")]
[InlineData("20200301010203.123-9900")]
[InlineData("20200301010203123+0500")]
[InlineData("20209901010203+0500")]
[InlineData("20200399010203+0500")]
[InlineData("20200301990203+0500")]
[InlineData("20200301019903+0500")]
[InlineData("20200301010299+0500")]
[InlineData("20200301010299.")]
[InlineData("20200301010299123")]
[InlineData("20209901010203")]
[InlineData("20200399010203")]
[InlineData("20200301990203")]
[InlineData("20200301019903")]
[InlineData("20200301010299")]
[InlineData("31")]
public void GivenAnInvalidDicomDateTimeValue_WhenGetStringDateTimeAsLiteralAndUtcDateTimesIsCalled_ThenNullShouldBeReturned(string acquisitionDateTime)
{
_dicomDataset.Add(DicomTag.AcquisitionDateTime, acquisitionDateTime);

Assert.Null(_dicomDataset.GetStringDateTimeAsLiteralAndUtcDateTimes(DicomTag.AcquisitionDateTime).Item1);
}

[Fact]
public void GivenNoDicomTimeValue_WhenGetStringTimeAsLongIsCalled_ThenNullShouldBeReturned()
{
Assert.Null(_dicomDataset.GetStringTimeAsLong(DicomTag.StudyTime));
}

[Theory]
[InlineData("010203.123", 01, 02, 03, 123)]
[InlineData("010203", 01, 02, 03, 0)]
[InlineData("0102", 01, 02, 0, 0)]
[InlineData("01", 01, 0, 0, 0)]
public void GivenAValidDicomTimeValue_WhenGetStringTimeAsLongIsCalled_ThenCorrectTimeTicksShouldBeReturned(
string studyTime,
int hour,
int minute,
int second,
int millisecond
)
{
_dicomDataset.Add(DicomTag.StudyTime, studyTime);
Assert.Equal(
new DateTime(
01,
01,
01,
hour,
minute,
second,
millisecond).Ticks,
_dicomDataset.GetStringTimeAsLong(DicomTag.StudyTime).Value);
}

[Theory]
[InlineData("010299123")]
[InlineData("010299")]
[InlineData("019903")]
[InlineData("990203")]
[InlineData("2")]
public void GivenAnInvalidDicomTimeValue_WhenGetStringTimeAsLongIsCalled_ThenNullShouldBeReturned(string studyTime)
{
_dicomDataset.Add(DicomTag.StudyTime, studyTime);

Assert.Null(_dicomDataset.GetStringTimeAsLong(DicomTag.StudyTime));
}

[Fact]
public void GivenANullValue_WhenAddValueIfNotNullIsCalled_ThenValueShouldNotBeAdded()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,58 @@ public void GivenExtendedQueryDateTag_WithUrl_ParseSucceeds()
Assert.Equal(queryTag, queryExpression.FilterConditions.First().QueryTag);
}

[Fact]
public void GivenExtendedQueryDateTimeTag_WithUrl_ParseSucceeds()
{
QueryTag queryTag = new QueryTag(DicomTag.DateTime.BuildExtendedQueryTagStoreEntry(level: QueryTagLevel.Study));

QueryExpression queryExpression = _queryParser.Parse(CreateParameters(GetSingleton("DateTime", "20200301195109.10-20200501195110.20"), QueryResource.AllStudies), new[] { queryTag });
Assert.Equal(queryTag, queryExpression.FilterConditions.First().QueryTag);
}

[Theory]
[InlineData("19510910010203", "20200220020304")]
public void GivenDateTime_WithValidRangeMatch_CheckCondition(string minValue, string maxValue)
{
EnsureArg.IsNotNull(minValue, nameof(maxValue));
QueryTag queryTag = new QueryTag(DicomTag.DateTime.BuildExtendedQueryTagStoreEntry(level: QueryTagLevel.Study));

QueryExpression queryExpression = _queryParser.Parse(CreateParameters(GetSingleton("DateTime", string.Concat(minValue, "-", maxValue)), QueryResource.AllStudies), new[] { queryTag });
var cond = queryExpression.FilterConditions.First() as DateRangeValueMatchCondition;
Assert.NotNull(cond);
Assert.True(cond.QueryTag.Tag == DicomTag.DateTime);
Assert.True(cond.Minimum == DateTime.ParseExact(minValue, QueryParser.DateTimeTagValueFormats, null));
Assert.True(cond.Maximum == DateTime.ParseExact(maxValue, QueryParser.DateTimeTagValueFormats, null));
}

[Fact]
public void GivenExtendedQueryTimeTag_WithUrl_ParseSucceeds()
{
QueryTag queryTag = new QueryTag(DicomTag.Time.BuildExtendedQueryTagStoreEntry(level: QueryTagLevel.Study));

QueryExpression queryExpression = _queryParser.Parse(CreateParameters(GetSingleton("Time", "195109.10-195110.20"), QueryResource.AllStudies), new[] { queryTag });
Assert.Equal(queryTag, queryExpression.FilterConditions.First().QueryTag);
}

[Theory]
[InlineData("010203", "020304")]
public void GivenStudyTime_WithValidRangeMatch_CheckCondition(string minValue, string maxValue)
{
EnsureArg.IsNotNull(minValue, nameof(maxValue));
QueryTag queryTag = new QueryTag(DicomTag.Time.BuildExtendedQueryTagStoreEntry(level: QueryTagLevel.Study));

QueryExpression queryExpression = _queryParser.Parse(CreateParameters(GetSingleton("Time", string.Concat(minValue, "-", maxValue)), QueryResource.AllStudies), new[] { queryTag });
var cond = queryExpression.FilterConditions.First() as LongRangeValueMatchCondition;
Assert.NotNull(cond);
Assert.True(cond.QueryTag.Tag == DicomTag.Time);

long minTicks = new DicomTime(cond.QueryTag.Tag, new string[] { minValue }).Get<DateTime>().Ticks;
long maxTicks = new DicomTime(cond.QueryTag.Tag, new string[] { maxValue }).Get<DateTime>().Ticks;

Assert.True(cond.Minimum == minTicks);
Assert.True(cond.Maximum == maxTicks);
}

[Fact]
public void GivenExtendedQueryPersonNameTag_WithUrl_ParseSucceeds()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,24 @@ public void GivenDateIsInvalidException_WhenGetMessage_ShouldReturnExpected()
Assert.Equal($"Dicom element '{name}' with value '{value}' failed validation for VR 'DA': Value cannot be parsed as a valid date.", exception.Message);
}

[Fact]
public void GivenDateTimeIsInvalidException_WhenGetMessage_ShouldReturnExpected()
{
var name = "tagname";
var value = "tagvalue";
var exception = ElementValidationExceptionFactory.CreateDateTimeIsInvalidException(name, value);
Assert.Equal($"Dicom element '{name}' with value '{value}' failed validation for VR 'DT': Value cannot be parsed as a valid DateTime.", exception.Message);
}

[Fact]
public void GivenTimeIsInvalidException_WhenGetMessage_ShouldReturnExpected()
{
var name = "tagname";
var value = "tagvalue";
var exception = ElementValidationExceptionFactory.CreateTimeIsInvalidException(name, value);
Assert.Equal($"Dicom element '{name}' with value '{value}' failed validation for VR 'TM': Value cannot be parsed as a valid Time.", exception.Message);
}

[Fact]
public void GivenExceedMaxLengthException_WhenGetMessage_ShouldReturnExpected()
{
Expand Down
Loading

0 comments on commit dada9b7

Please sign in to comment.