Merge pull request #10 from microsoft/personal/richyl2/metadata-blob-…

…storage

Personal/richyl2/metadata blob storage

Microsoft.Health.Dicom.sln

@@ -36,6 +36,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Health.Dicom.Cosm
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Health.Dicom.CosmosDb.UnitTests", "src\Microsoft.Health.Dicom.CosmosDb.UnitTests\Microsoft.Health.Dicom.CosmosDb.UnitTests.csproj", "{FA0B32A4-E623-4B24-902D-BFDC6E42E612}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Health.Dicom.Metadata", "src\Microsoft.Health.Dicom.Metadata\Microsoft.Health.Dicom.Metadata.csproj", "{1E91CFB9-45D0-4742-A23D-0FF4E2D73A73}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Health.Dicom.Metadata.UnitTests", "src\Microsoft.Health.Dicom.Metadata.UnitTests\Microsoft.Health.Dicom.Metadata.UnitTests.csproj", "{C66FAF73-02E3-41E0-8794-594F2E38E36F}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -90,6 +94,14 @@ Global
 		{FA0B32A4-E623-4B24-902D-BFDC6E42E612}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{FA0B32A4-E623-4B24-902D-BFDC6E42E612}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{FA0B32A4-E623-4B24-902D-BFDC6E42E612}.Release|Any CPU.Build.0 = Release|Any CPU
+		{1E91CFB9-45D0-4742-A23D-0FF4E2D73A73}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{1E91CFB9-45D0-4742-A23D-0FF4E2D73A73}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{1E91CFB9-45D0-4742-A23D-0FF4E2D73A73}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{1E91CFB9-45D0-4742-A23D-0FF4E2D73A73}.Release|Any CPU.Build.0 = Release|Any CPU
+		{C66FAF73-02E3-41E0-8794-594F2E38E36F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{C66FAF73-02E3-41E0-8794-594F2E38E36F}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{C66FAF73-02E3-41E0-8794-594F2E38E36F}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{C66FAF73-02E3-41E0-8794-594F2E38E36F}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -107,9 +119,11 @@ Global
 		{FA0484A7-AA0C-4CC6-A75F-1D6B23DD847D} = {176641B3-297C-4E04-A83D-8F80F80485E8}
 		{E78AB378-6CF4-442A-BC34-81A1B7431ECD} = {176641B3-297C-4E04-A83D-8F80F80485E8}
 		{FA0B32A4-E623-4B24-902D-BFDC6E42E612} = {176641B3-297C-4E04-A83D-8F80F80485E8}
+		{1E91CFB9-45D0-4742-A23D-0FF4E2D73A73} = {176641B3-297C-4E04-A83D-8F80F80485E8}
+		{C66FAF73-02E3-41E0-8794-594F2E38E36F} = {176641B3-297C-4E04-A83D-8F80F80485E8}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
-		RESX_SortFileContentOnSave = True
 		SolutionGuid = {E370FB31-CF95-47D1-B1E1-863A77973FF8}
+		RESX_SortFileContentOnSave = True
 	EndGlobalSection
 EndGlobal

src/Microsoft.Health.Dicom.Api/Registration/DicomServerServiceCollectionExtensions.cs

@@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Builder
     public static class DicomServerServiceCollectionExtensions
     {
         /// <summary>
-        /// Adds services for enabling a FHIR server.
+        /// Adds services for enabling a DICOM server.
         /// </summary>
         /// <param name="services">The services collection.</param>
         /// <returns>A <see cref="IDicomServerBuilder"/> object.</returns>

src/Microsoft.Health.Dicom.Blob/Registration/DicomServerBuilderBlobRegistrationExtensions.cs

@@ -19,7 +19,7 @@ namespace Microsoft.Extensions.DependencyInjection
 {
     public static class DicomServerBuilderBlobRegistrationExtensions
     {
-        private static readonly string DicomServerBlobConfigurationSectionName = $"DicomWeb:{BlobClientRegistrationExtensions.BlobStoreConfigurationSectionName}";
+        private static readonly string DicomServerBlobConfigurationSectionName = $"DicomWeb:DicomStore";
 
         /// <summary>
         /// Adds the blob data store for the DICOM server.

src/Microsoft.Health.Dicom.Core.UnitTests/Features/Persistence/DicomAttributeIdExtensionsTests.cs

@@ -12,6 +12,50 @@ namespace Microsoft.Health.Dicom.Core.UnitTests.Features.Persistence
 {
     public class DicomAttributeIdExtensionsTests
     {
+        [Fact]
+        public void GivenDatasetAndInvalidParameters_WhenAddingAttributeValues_ArgumentExceptionIsThrown()
+        {
+            var dicomDataset = new DicomDataset();
+            var testAttributeId = new DicomAttributeId(DicomTag.ReferencedStudySequence, DicomTag.StudyInstanceUID);
+            var testDicomItem = new DicomLongString(DicomTag.StudyInstanceUID, Guid.NewGuid().ToString());
+            Assert.Throws<ArgumentNullException>(() => dicomDataset.Add((DicomAttributeId)null, testDicomItem));
+            Assert.Throws<ArgumentNullException>(() => dicomDataset.Add(testAttributeId, null));
+            Assert.Throws<ArgumentNullException>(() => DicomAttributeIdExtensions.Add(null, testAttributeId, testDicomItem));
+            Assert.Throws<ArgumentException>(() => dicomDataset.Add(testAttributeId, new DicomLongString(DicomTag.SeriesInstanceUID, Guid.NewGuid().ToString())));
+        }
+
+        [Fact]
+        public void GivenDicomAttributeIdWithSequenceElements_WhenAddingDicomItemToDataset_IsAddedCorrectly()
+        {
+            var dicomDataset = new DicomDataset();
+            var testStudyId = Guid.NewGuid().ToString();
+            dicomDataset.Add(
+                new DicomAttributeId(DicomTag.ReferencedStudySequence, DicomTag.StudyInstanceUID),
+                new DicomLongString(DicomTag.StudyInstanceUID, testStudyId));
+
+            Assert.True(dicomDataset.TryGetSequence(DicomTag.ReferencedStudySequence, out DicomSequence referencedStudySequence));
+            Assert.Single(referencedStudySequence.Items);
+            Assert.Single(referencedStudySequence.Items[0]);
+            Assert.Equal(testStudyId, referencedStudySequence.Items[0].GetSingleValue<string>(DicomTag.StudyInstanceUID));
+        }
+
+        [Fact]
+        public void GivenDicomAttributeIdWithSequenceElements_WhenAddingSequenceDicomItemToDataset_IsAddedCorrectly()
+        {
+            var dicomDataset = new DicomDataset();
+            var resultSequence = new DicomSequence(
+                DicomTag.ReferencedSeriesSequence,
+                new DicomDataset() { { DicomTag.SeriesInstanceUID, Guid.NewGuid().ToString() } },
+                new DicomDataset() { { DicomTag.SeriesDescription, "TestDescription" } });
+            dicomDataset.Add(
+                new DicomAttributeId(DicomTag.ReferencedStudySequence, resultSequence.Tag), resultSequence);
+
+            Assert.True(dicomDataset.TryGetSequence(DicomTag.ReferencedStudySequence, out DicomSequence referencedStudySequence));
+            Assert.Single(referencedStudySequence.Items);
+            Assert.Single(referencedStudySequence.Items[0]);
+            Assert.Equal(resultSequence, referencedStudySequence.Items[0].GetSequence(resultSequence.Tag));
+        }
+
         [Fact]
         public void GivenDatasetAndInvalidParameters_WhenFetchedAttributeValues_ArgumentExceptionIsThrown()
         {

src/Microsoft.Health.Dicom.Core.UnitTests/Features/Persistence/DicomAttributeIdTests.cs

@@ -17,13 +17,11 @@ public void GivenDicomAttributeId_WhenConstructingWithInvalidParameters_Argument
         {
             Assert.Throws<ArgumentNullException>(() => new DicomAttributeId((DicomTag[])null));
             Assert.Throws<ArgumentException>(() => new DicomAttributeId(Array.Empty<DicomTag>()));
-            Assert.Throws<FormatException>(() => new DicomAttributeId(DicomTag.RightImageSequence));
-            Assert.Throws<FormatException>(() => new DicomAttributeId(DicomTag.RightImageSequence, DicomTag.ROIContourSequence));
+            Assert.Throws<FormatException>(() => new DicomAttributeId(DicomTag.RightImageSequence, DicomTag.StudyDate, DicomTag.ROIContourSequence));
             Assert.Throws<FormatException>(() => new DicomAttributeId(DicomTag.StudyDate, DicomTag.RightLensSequence));
 
             Assert.Throws<ArgumentNullException>(() => new DicomAttributeId((string)null));
             Assert.Throws<ArgumentException>(() => new DicomAttributeId(string.Empty));
-            Assert.Throws<FormatException>(() => new DicomAttributeId("0020000D.0020000D"));
             Assert.Throws<FormatException>(() => new DicomAttributeId("INVALID"));
             Assert.Throws<FormatException>(() => new DicomAttributeId("INVALID.INVALID"));
 

src/Microsoft.Health.Dicom.Core/Features/Persistence/DicomAttributeId.cs

@@ -62,10 +62,10 @@ public DicomAttributeId(params DicomTag[] dicomTags)
         public int Length => _dicomTags.Length;
 
         /// <summary>
-        /// Gets the non-sequence DICOM tag.
+        /// Gets the last DICOM tag.
         /// </summary>
         [JsonIgnore]
-        public DicomTag InstanceDicomTag => _dicomTags[Length - 1];
+        public DicomTag FinalDicomTag => _dicomTags[Length - 1];
 
         public DicomTag GetDicomTag(int index)
         {
@@ -141,19 +141,16 @@ private static string ConvertToString(DicomTag dicomTag, bool writeTagsAsKeyword
 
         private void Validate()
         {
-            // Validate all but the leaf tag are sequence elements
+            // Validate all the tags but the last has a value representation of sequence.
             for (var i = 0; i < Length - 1; i++)
             {
                 if (!_dicomTags[i].DictionaryEntry.ValueRepresentations.Contains(DicomVR.SQ))
                 {
-                    throw new FormatException($"All tags but the last must be sequence elements. The provided DICOM tag {_dicomTags[i].DictionaryEntry.Keyword} does not have a 'sequence' value type.");
+                    throw new FormatException($"All tags other than the last must have a value representation of 'sequence'. The provided DICOM tag {_dicomTags[i].DictionaryEntry.Keyword} does not have a 'sequence' value type.");
                 }
             }
 
-            if (InstanceDicomTag.DictionaryEntry.ValueRepresentations.Contains(DicomVR.SQ))
-            {
-                throw new FormatException($"The last DICOM tag must not have the value representation 'sequence'. The provided DICOM tag {InstanceDicomTag.DictionaryEntry.Keyword} is known as having a 'sequence' value type.");
-            }
+            // Note: The last tag can have any value representation including sequence.
         }
     }
 }

src/Microsoft.Health.Dicom.Core/Features/Persistence/DicomAttributeIdExtensions.cs

@@ -4,56 +4,89 @@
 // -------------------------------------------------------------------------------------------------
 
 using System.Collections.Generic;
+using System.Linq;
 using Dicom;
 using EnsureThat;
 
 namespace Microsoft.Health.Dicom.Core.Features.Persistence
 {
     public static class DicomAttributeIdExtensions
     {
+        public static void Add(this DicomDataset dicomDataset, DicomAttributeId attributeId, DicomItem dicomItem)
+        {
+            EnsureArg.IsNotNull(dicomDataset, nameof(dicomDataset));
+            EnsureArg.IsNotNull(attributeId, nameof(attributeId));
+            EnsureArg.IsNotNull(dicomItem, nameof(dicomItem));
+            EnsureArg.IsTrue(attributeId.FinalDicomTag == dicomItem.Tag);
+
+            DicomDataset currentDataset = dicomDataset;
+            for (var i = 0; i < attributeId.Length; i++)
+            {
+                DicomTag dicomTag = attributeId.GetDicomTag(i);
+                if (i == attributeId.Length - 1)
+                {
+                    currentDataset.Add(dicomItem);
+                }
+                else
+                {
+                    if (!currentDataset.TryGetSequence(dicomTag, out DicomSequence dicomSequence))
+                    {
+                        dicomSequence = new DicomSequence(dicomTag);
+                        currentDataset.Add(dicomSequence);
+                    }
+
+                    currentDataset = new DicomDataset();
+                    dicomSequence.Items.Add(currentDataset);
+                }
+            }
+        }
+
+        public static bool TryGetDicomItems(this DicomDataset dicomDataset, DicomAttributeId attributeId, out DicomItem[] dicomItems)
+        {
+            EnsureArg.IsNotNull(dicomDataset, nameof(dicomDataset));
+            EnsureArg.IsNotNull(attributeId, nameof(attributeId));
+
+            var result = new List<DicomItem>();
+            dicomDataset.TryAppendValuesToList(result, attributeId);
+
+            dicomItems = result.Count > 0 ? result.ToArray() : null;
+            return result.Count > 0;
+        }
+
         public static bool TryGetValues<TItem>(this DicomDataset dicomDataset, DicomAttributeId attributeId, out TItem[] values)
         {
             EnsureArg.IsNotNull(dicomDataset, nameof(dicomDataset));
             EnsureArg.IsNotNull(attributeId, nameof(attributeId));
 
             var result = new List<TItem>();
-            dicomDataset.TryAppendValuesToList(result, attributeId);
+            if (dicomDataset.TryGetDicomItems(attributeId, out DicomItem[] dicomItems))
+            {
+                IEnumerable<DicomElement> elements = dicomItems.OfType<DicomElement>();
+                result.AddRange(elements.SelectMany(x => Enumerable.Range(0, x.Count).Select(y => x.Get<TItem>(y))));
+            }
 
             values = result.Count > 0 ? result.ToArray() : null;
             return result.Count > 0;
         }
 
-        private static void TryAppendValuesToList<TItem>(
-            this DicomDataset dicomDataset,
-            List<TItem> list,
-            DicomAttributeId attributeId,
-            int startAttributeIndex = 0)
+        private static void TryAppendValuesToList(
+           this DicomDataset dicomDataset,
+           List<DicomItem> list,
+           DicomAttributeId attributeId,
+           int attributeIndex = 0)
         {
-            EnsureArg.IsGte(startAttributeIndex, 0, nameof(startAttributeIndex));
+            EnsureArg.IsGte(attributeIndex, 0, nameof(attributeIndex));
+            DicomTag currentDicomTag = attributeId.GetDicomTag(attributeIndex);
 
-            // Handle for now sequence elements (last item in attribute ID).
-            if (startAttributeIndex == attributeId.Length - 1)
+            if (attributeIndex + 1 == attributeId.Length)
             {
-                // Now first validate this DICOM tag exists in the dataset.
-                // Note: GetValueCount throws exception when the tag does not exist.
-                if (dicomDataset.TryGetValue(attributeId.InstanceDicomTag, 0, out TItem firstItem))
-                {
-                    list.Add(firstItem);
-
-                    for (int i = 1; i < dicomDataset.GetValueCount(attributeId.InstanceDicomTag); i++)
-                    {
-                        if (dicomDataset.TryGetValue(attributeId.InstanceDicomTag, i, out TItem value))
-                        {
-                            list.Add(value);
-                        }
-                    }
-                }
+                list.AddRange(dicomDataset.Where(x => x.Tag == currentDicomTag));
             }
-            else if (dicomDataset.TryGetSequence(attributeId.GetDicomTag(startAttributeIndex), out DicomSequence dicomSequence))
+            else if (dicomDataset.TryGetSequence(currentDicomTag, out DicomSequence dicomSequence))
             {
                 foreach (DicomDataset sequenceDataset in dicomSequence.Items)
                 {
-                    sequenceDataset.TryAppendValuesToList(list, attributeId, startAttributeIndex + 1);
+                    sequenceDataset.TryAppendValuesToList(list, attributeId, attributeIndex + 1);
                 }
             }
         }

src/Microsoft.Health.Dicom.Core/Features/Persistence/DicomInstance.cs

@@ -51,5 +51,8 @@ public override bool Equals(object obj)
 
         public override int GetHashCode()
             => (StudyInstanceUID + SeriesInstanceUID + SopInstanceUID).GetHashCode(EqualsStringComparison);
+
+        public override string ToString()
+            => $"Study Instance UID: {StudyInstanceUID}, Series Instance UID: {SeriesInstanceUID}, SOP Instance UID {SopInstanceUID}";
     }
 }

src/Microsoft.Health.Dicom.Core/Features/Persistence/IDicomMetadataStore.cs

@@ -0,0 +1,36 @@
+// -------------------------------------------------------------------------------------------------
+// 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.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+using Dicom;
+
+namespace Microsoft.Health.Dicom.Core.Features.Persistence
+{
+    public interface IDicomMetadataStore
+    {
+        Task AddStudySeriesDicomMetadataAsync(IEnumerable<DicomDataset> instances, CancellationToken cancellationToken = default);
+
+        Task<DicomDataset> GetStudyDicomMetadataWithAllOptionalAsync(string studyInstanceUID, CancellationToken cancellationToken = default);
+
+        Task<DicomDataset> GetStudyDicomMetadataAsync(string studyInstanceUID, HashSet<DicomAttributeId> optionalAttributes = null, CancellationToken cancellationToken = default);
+
+        Task<DicomDataset> GetSeriesDicomMetadataWithAllOptionalAsync(string studyInstanceUID, string seriesInstanceUID, CancellationToken cancellationToken = default);
+
+        Task<DicomDataset> GetSeriesDicomMetadataAsync(
+            string studyInstanceUID, string seriesInstanceUID, HashSet<DicomAttributeId> optionalAttributes = null, CancellationToken cancellationToken = default);
+
+        Task<IEnumerable<DicomInstance>> GetInstancesInStudyAsync(string studyInstanceUID, CancellationToken cancellationToken = default);
+
+        Task<IEnumerable<DicomInstance>> GetInstancesInSeriesAsync(string studyInstanceUID, string seriesInstanceUID, CancellationToken cancellationToken = default);
+
+        Task<IEnumerable<DicomInstance>> DeleteStudyAsync(string studyInstanceUID, CancellationToken cancellationToken = default);
+
+        Task<IEnumerable<DicomInstance>> DeleteSeriesAsync(string studyInstanceUID, string seriesInstanceUID, CancellationToken cancellationToken = default);
+
+        Task DeleteInstanceAsync(string studyInstanceUID, string seriesInstanceUID, string sopInstanceUID, CancellationToken cancellationToken = default);
+    }
+}

src/Microsoft.Health.Dicom.CosmosDb.UnitTests/Features/Storage/DicomCosmosDataStoreTests.cs

@@ -31,7 +31,7 @@ public DicomCosmosDataStoreTests()
                 Substitute.For<IScoped<IDocumentClient>>(),
                 new CosmosDataStoreConfiguration(),
                 namedCosmosCollectionConfigurationAccessor,
-                new DicomCosmosConfiguration());
+                new DicomIndexingConfiguration());
         }
 
         [Fact]

src/Microsoft.Health.Dicom.CosmosDb/Config/DicomCosmosConfiguration.cs → src/Microsoft.Health.Dicom.CosmosDb/Config/DicomIndexingConfiguration.cs

@@ -9,7 +9,7 @@
 
 namespace Microsoft.Health.Dicom.CosmosDb.Config
 {
-    public class DicomCosmosConfiguration
+    public class DicomIndexingConfiguration
     {
         /// <summary>
         /// Gets the DICOM tags that should be indexed and made queryable.

src/Microsoft.Health.Dicom.CosmosDb/Features/Storage/CosmosQueryBuilder.cs

@@ -23,10 +23,10 @@ internal class CosmosQueryBuilder
         private const string StudySqlQuerySearchFormat = "SELECT DISTINCT VALUE {{ \"" + nameof(DicomStudy.StudyInstanceUID) + "\": c." + DocumentProperties.StudyInstanceUID + " }} FROM c {0} OFFSET " + OffsetParameterName + " LIMIT " + LimitParameterName;
         private const string SeriesSqlQuerySearchFormat = "SELECT VALUE {{ \"" + nameof(DicomSeries.StudyInstanceUID) + "\": c." + DocumentProperties.StudyInstanceUID + ", \"" + nameof(DicomSeries.SeriesInstanceUID) + "\": c." + DocumentProperties.SeriesInstanceUID + " }} FROM c {0} OFFSET " + OffsetParameterName + " LIMIT " + LimitParameterName;
         private const string InstanceSqlQuerySearchFormat = "SELECT VALUE {{ \"" + nameof(DicomInstance.StudyInstanceUID) + "\": c." + DocumentProperties.StudyInstanceUID + ", \"" + nameof(DicomInstance.SeriesInstanceUID) + "\": c." + DocumentProperties.SeriesInstanceUID + ", \"" + nameof(DicomInstance.SopInstanceUID) + "\": f." + DocumentProperties.SopInstanceUID + " }} FROM c JOIN f in c." + DocumentProperties.Instances + " {0} OFFSET " + OffsetParameterName + " LIMIT " + LimitParameterName;
-        private readonly DicomCosmosConfiguration _dicomConfiguration;
+        private readonly DicomIndexingConfiguration _dicomConfiguration;
         private readonly IFormatProvider _stringFormatProvider;
 
-        public CosmosQueryBuilder(DicomCosmosConfiguration dicomConfiguration)
+        public CosmosQueryBuilder(DicomIndexingConfiguration dicomConfiguration)
         {
             EnsureArg.IsNotNull(dicomConfiguration, nameof(dicomConfiguration));