Skip to content

Commit e88f0c4

Browse files
smithagosmithasa
and
smithasa
authored
Prioritize original transfer syntax when multiple accepts with no quality provided (#2753)
## Description SLIM viewer sends multiple accept headers to get frames. Original is the last, this causes us to pick another accept and fail because today our code needs entire file to do transcoding and will throw when file size is greater than 100M. This fix will make us prioritize picking original if available in the accept list. ## Related issues ImagingDataCommons/dicom-microscopy-viewer#95 ## Testing Added unit tests --------- Co-authored-by: smithasa <smithasa@SMITHASAMI1>
1 parent 64b4322 commit e88f0c4

File tree

3 files changed

+46
-6
lines changed

3 files changed

+46
-6
lines changed

src/Microsoft.Health.Dicom.Core.UnitTests/Features/Retrieve/AcceptHeaderHandlerTests.cs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55

66
using System.Collections.Generic;
77
using System.Linq;
8+
using FellowOakDicom;
89
using Microsoft.Extensions.Logging.Abstractions;
910
using Microsoft.Health.Dicom.Core.Exceptions;
1011
using Microsoft.Health.Dicom.Core.Features.Retrieve;
1112
using Microsoft.Health.Dicom.Core.Messages;
1213
using Microsoft.Health.Dicom.Core.Messages.Retrieve;
14+
using Microsoft.Health.Dicom.Core.Web;
1315
using Xunit;
1416

1517
namespace Microsoft.Health.Dicom.Core.UnitTests.Features.Retrieve;
@@ -139,7 +141,7 @@ public void
139141
AcceptHeader requestedAcceptHeader1 = new AcceptHeader(
140142
ValidStudyAcceptHeaderDescriptor.MediaType,
141143
ValidStudyAcceptHeaderDescriptor.PayloadType,
142-
ValidStudyAcceptHeaderDescriptor.AcceptableTransferSyntaxes.First(),
144+
DicomTransferSyntaxUids.Original,
143145
quality: 0.3
144146
);
145147

@@ -164,4 +166,26 @@ public void
164166

165167
Assert.Equivalent(requestedAcceptHeader2, matchedAcceptHeader, strict: true);
166168
}
169+
170+
[Fact]
171+
public void GivenMultipleMatchedAcceptHeaderNoQuality_WhenTransferSyntaxRequested_ThenShouldReturnOriginal()
172+
{
173+
AcceptHeader requestedAcceptHeader1 = new AcceptHeader(
174+
payloadType: PayloadTypes.MultipartRelated,
175+
mediaType: KnownContentTypes.ImageJpeg2000,
176+
transferSyntax: DicomTransferSyntax.JPEG2000Lossless.UID.UID);
177+
178+
AcceptHeader requestedAcceptHeader2 = new AcceptHeader(
179+
payloadType: PayloadTypes.SinglePart,
180+
mediaType: KnownContentTypes.ApplicationOctetStream,
181+
transferSyntax: DicomTransferSyntaxUids.Original);
182+
183+
184+
AcceptHeader matchedAcceptHeader = _handler.GetValidAcceptHeader(
185+
ResourceType.Frames,
186+
new[] { requestedAcceptHeader1, requestedAcceptHeader2 }
187+
);
188+
189+
Assert.Equivalent(requestedAcceptHeader2, matchedAcceptHeader, strict: true);
190+
}
167191
}

src/Microsoft.Health.Dicom.Core/Features/Retrieve/AcceptHeaderHandler.cs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ private AcceptHeaderHandler(
5959
public AcceptHeader GetValidAcceptHeader(ResourceType resourceType, IReadOnlyCollection<AcceptHeader> acceptHeaders)
6060
{
6161
EnsureArg.IsNotNull(acceptHeaders, nameof(acceptHeaders));
62-
List<AcceptHeader> orderedHeaders = acceptHeaders.OrderByDescending(x => x.Quality).ToList();
62+
List<AcceptHeader> orderedHeaders = acceptHeaders.OrderByDescending(x => x.Quality ?? AcceptHeader.DefaultQuality).ToList();
6363

6464
_logger.LogInformation(
6565
"Getting transfer syntax for retrieving {ResourceType} with accept headers {AcceptHeaders}.",
@@ -68,26 +68,42 @@ public AcceptHeader GetValidAcceptHeader(ResourceType resourceType, IReadOnlyCol
6868

6969
List<AcceptHeaderDescriptor> descriptors = _acceptableDescriptors[resourceType];
7070

71-
// since these are ordered by quality already, we can return the first acceptable header
71+
AcceptHeader selectedHeader = null;
72+
// we will return the highest priority media type we support
7273
foreach (AcceptHeader header in orderedHeaders)
7374
{
7475
foreach (AcceptHeaderDescriptor descriptor in descriptors)
7576
{
76-
if (descriptor.IsAcceptable(header))
77+
if (descriptor.IsAcceptable(header) && (selectedHeader == null || IsHigherPriorityTransferSyntax(header, selectedHeader)))
7778
{
78-
return new AcceptHeader(
79+
selectedHeader = new AcceptHeader(
7980
header.MediaType,
8081
header.PayloadType,
8182
descriptor.GetTransferSyntax(header),
8283
header.Quality);
84+
85+
continue;
8386
}
8487
}
8588
}
8689

90+
if (selectedHeader != null)
91+
{
92+
_logger.LogInformation("Selected accept header {AcceptHeader} for retrieving {ResourceType}.", selectedHeader, resourceType);
93+
return selectedHeader;
94+
}
95+
8796
// none were valid
8897
throw new NotAcceptableException(DicomCoreResource.NotAcceptableHeaders);
8998
}
9099

100+
// if no quality provided prioritize returning original transfer syntax
101+
private static bool IsHigherPriorityTransferSyntax(AcceptHeader header, AcceptHeader selectedHeader)
102+
{
103+
bool isQualityGreater = (header.Quality ?? AcceptHeader.DefaultQuality) >= (selectedHeader.Quality ?? AcceptHeader.DefaultQuality);
104+
return (header.TransferSyntax.Value == DicomTransferSyntaxUids.Original && isQualityGreater);
105+
}
106+
91107
private static List<AcceptHeaderDescriptor> DescriptorsForGetNonFrameResource(PayloadTypes payloadTypes)
92108
{
93109
return new List<AcceptHeaderDescriptor>

src/Microsoft.Health.Dicom.Core/Messages/Retrieve/AcceptHeader.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// -------------------------------------------------------------------------------------------------
1+
// -------------------------------------------------------------------------------------------------
22
// Copyright (c) Microsoft Corporation. All rights reserved.
33
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
44
// -------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)