Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,7 @@ public ListedCapabilityStatement()
public IDictionary<string, JToken> AdditionalData { get; }

public ICollection<ReferenceComponent> Profile { get; }

public ICollection<string> Instantiates { get; internal set; }
}
}
6 changes: 6 additions & 0 deletions src/Microsoft.Health.Fhir.Shared.Api/Modules/FhirModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ ResourceElement SetMetadata(Resource resource, string versionId, DateTimeOffset
services.AddTransient(typeof(IScopeProvider<>), typeof(ScopeProvider<>));

services.AddScoped<IDocRefRequestConverter, DocRefRequestConverter>();

services.TypesInSameAssembly(KnownAssemblies.All)
.AssignableTo<IInstantiateCapability>()
.Transient()
.AsService<IInstantiateCapability>();
services.AddFactory<IScoped<IEnumerable<IInstantiateCapability>>>();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Health.Extensions.DependencyInjection;
using Microsoft.Health.Fhir.Core.Features.Conformance;
using Microsoft.Health.Fhir.Core.Features.Conformance.Models;
using Microsoft.Health.Fhir.Shared.Core.Features.Conformance;
using Microsoft.Health.Fhir.Tests.Common;
using Microsoft.Health.Test.Utilities;
using NSubstitute;
using Xunit;

namespace Microsoft.Health.Fhir.Shared.Core.UnitTests.Features.Conformance
{
[Trait(Traits.OwningTeam, OwningTeam.Fhir)]
[Trait(Traits.Category, Categories.Conformance)]
public class InstantiatesCapabilityProviderTests
{
private readonly InstantiatesCapabilityProvider _provider;
private readonly List<IInstantiateCapability> _capabilities;

public InstantiatesCapabilityProviderTests()
{
_capabilities = new List<IInstantiateCapability>();
var s = Substitute.For<IScoped<IEnumerable<IInstantiateCapability>>>();
s.Value.Returns(_capabilities);

_provider = new InstantiatesCapabilityProvider(() => s);
}

[Theory]
[InlineData(1)]
[InlineData(0)]
[InlineData(1, 0)]
[InlineData(0, 0, 0)]
[InlineData(1, 2, 3, 4)]
[InlineData(1, 0, 3, 0)]
public async Task GivenCapabilities_WhenBuilding_ThenInstantiatesFieldIsPopulated(
params int[] counts)
{
var urls = AddCapabilities(counts);
var instantiates = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var builder = Substitute.For<ICapabilityStatementBuilder>();
builder.Apply(Arg.Any<Action<ListedCapabilityStatement>>()).Returns(
x =>
{
var action = (Action<ListedCapabilityStatement>)x[0];
var statement = new ListedCapabilityStatement();
action(statement);
if (statement.Instantiates != null)
{
foreach (var i in statement.Instantiates)
{
instantiates.Add(i);
}
}

return builder;
});

await _provider.BuildAsync(builder, CancellationToken.None);

Assert.Equal(urls.Count, instantiates.Count);
Assert.All(
urls,
x =>
{
Assert.Contains(x, instantiates);
});
_capabilities.ForEach(x => x.Received(1).TryGetUrls(out Arg.Any<IEnumerable<string>>()));
builder.Received(urls.Any() ? 1 : 0).Apply(Arg.Any<Action<ListedCapabilityStatement>>());
}

private List<string> AddCapabilities(int[] counts)
{
_capabilities.Clear();
var capabilityUrls = new List<string>();
var i = 0;
foreach (var count in counts)
{
var app = $"app{i}";
var urls = Enumerable.Range(0, count).Select(x => $"http://hl7.org/fhir/{app}/CapabilityStatement/{x}");
var cap = Substitute.For<IInstantiateCapability>();
cap.TryGetUrls(out Arg.Any<IEnumerable<string>>()).Returns(
x =>
{
var c = count;
var copy = new List<string>(urls);
if (c > 0)
{
x[0] = copy;
return true;
}

x[0] = null;
return false;
});

_capabilities.Add(cap);
capabilityUrls.AddRange(urls);
i++;
}

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

using Microsoft.Extensions.Options;
using Microsoft.Health.Fhir.Core.Configs;
using Microsoft.Health.Fhir.Shared.Core.Features.Conformance;
using Microsoft.Health.Fhir.Tests.Common;
using Microsoft.Health.Test.Utilities;
using Xunit;

namespace Microsoft.Health.Fhir.Shared.Core.UnitTests.Features.Conformance
{
[Trait(Traits.OwningTeam, OwningTeam.Fhir)]
[Trait(Traits.Category, Categories.Conformance)]
public class SmartV2InstantiateCapabilityTests
{
[Theory]
[InlineData(true, false)]
[InlineData(false, true)]
[InlineData(false, false)]
public void GivenSecurityConfiguration_WhenSmartV2IsEnabled_ThenInstantiateCapabilityUrlsAreReturned(
bool enabled,
bool enabledWithout)
{
var securityConfiguration = new SecurityConfiguration
{
Authorization = new AuthorizationConfiguration
{
Enabled = enabled,
EnableSmartWithoutAuth = enabledWithout,
},
};

var instantiateCapability = new SmartV2InstantiateCapability(
Options.Create(securityConfiguration));
if (instantiateCapability.TryGetUrls(out var urls))
{
Assert.True(enabled || enabledWithout);
Assert.NotNull(urls);
Assert.NotEmpty(urls);
}
else
{
Assert.False(enabled || enabledWithout);
Assert.Null(urls);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\DocRefRequestConverterTests.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\FirelyTerminologyServiceProxyTests.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\GetSmartConfigurationHandlerTests.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\InstantiatesCapabilityProviderTests.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\SmartV2InstantiateCapabilityTests.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\TerminologyRequestHandlerTests.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Operations\ConvertData\MockLogger.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Operations\Everything\EverythingOperationContinuationTokenTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// -------------------------------------------------------------------------------------------------
// 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;

namespace Microsoft.Health.Fhir.Shared.Core.Features.Conformance
{
public interface IInstantiateCapability
{
bool TryGetUrls(out IEnumerable<string> urls);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using EnsureThat;
using Microsoft.Health.Extensions.DependencyInjection;
using Microsoft.Health.Fhir.Core.Features.Conformance;

namespace Microsoft.Health.Fhir.Shared.Core.Features.Conformance
{
public class InstantiatesCapabilityProvider : IProvideCapability
{
private readonly Func<IScoped<IEnumerable<IInstantiateCapability>>> _instantiateCapabilityDelegate;

public InstantiatesCapabilityProvider(
Func<IScoped<IEnumerable<IInstantiateCapability>>> instantiateCapabilityDelegate)
{
EnsureArg.IsNotNull(instantiateCapabilityDelegate, nameof(instantiateCapabilityDelegate));

_instantiateCapabilityDelegate = instantiateCapabilityDelegate;
}

public Task BuildAsync(
ICapabilityStatementBuilder builder,
CancellationToken cancellationToken)
{
EnsureArg.IsNotNull(builder, nameof(builder));

var instantiateUrls = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
using var instantiates = _instantiateCapabilityDelegate();
foreach (var instantiate in instantiates.Value)
{
if (instantiate.TryGetUrls(out var urls) && urls != null && urls.Any())
{
foreach (var url in urls)
{
instantiateUrls.Add(url);
}
}
}
Comment on lines +37 to +46

Check notice

Code scanning / CodeQL

Missed opportunity to use Where Note

This foreach loop
implicitly filters its target sequence
- consider filtering the sequence explicitly using '.Where(...)'.

Copilot Autofix

AI 4 days ago

In general, to fix this kind of issue you move the predicate from inside the loop into a LINQ Where clause on the sequence being iterated. This makes the loop iterate only over elements that satisfy the predicate, reducing nesting and clearly expressing the intent.

For this specific code, we need to keep access to both the IInstantiateCapability instance and the corresponding urls obtained via TryGetUrls. Because TryGetUrls uses an out parameter, we can’t directly put it inside the Where predicate without losing the urls value. A clean approach is to project the original sequence into a sequence of (instantiate, urls) pairs for which TryGetUrls succeeded and urls is non-null and non-empty. We can achieve this with Select, calling TryGetUrls once per element, then Where to filter only successful cases, and finally iterate over the filtered projection. Inside the loop, we then iterate over pair.urls to populate instantiateUrls, preserving the original behavior.

Concretely, in BuildAsync in src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/InstantiatesCapabilityProvider.cs, replace the outer foreach (var instantiate in instantiates.Value) loop and its internal if (instantiate.TryGetUrls(...)) with a LINQ pipeline:

  • Declare a local urls variable in a small lambda so TryGetUrls can assign it.
  • Use Select to turn each instantiate into an anonymous object { Instantiate = instantiate, Urls = urls }.
  • Use Where to filter to those entries where Urls is not null and has any items.
  • Iterate the result with foreach and add each URL to instantiateUrls.

No new using directives or external dependencies are needed; System.Linq is already imported.

Suggested changeset 1
src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/InstantiatesCapabilityProvider.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/InstantiatesCapabilityProvider.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/InstantiatesCapabilityProvider.cs
--- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/InstantiatesCapabilityProvider.cs
+++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/InstantiatesCapabilityProvider.cs
@@ -34,14 +34,17 @@
 
             var instantiateUrls = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
             using var instantiates = _instantiateCapabilityDelegate();
-            foreach (var instantiate in instantiates.Value)
+            foreach (var item in instantiates.Value
+                .Select(instantiate =>
+                {
+                    instantiate.TryGetUrls(out var urls);
+                    return new { Instantiate = instantiate, Urls = urls };
+                })
+                .Where(x => x.Urls != null && x.Urls.Any()))
             {
-                if (instantiate.TryGetUrls(out var urls) && urls != null && urls.Any())
+                foreach (var url in item.Urls)
                 {
-                    foreach (var url in urls)
-                    {
-                        instantiateUrls.Add(url);
-                    }
+                    instantiateUrls.Add(url);
                 }
             }
 
EOF
@@ -34,14 +34,17 @@

var instantiateUrls = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
using var instantiates = _instantiateCapabilityDelegate();
foreach (var instantiate in instantiates.Value)
foreach (var item in instantiates.Value
.Select(instantiate =>
{
instantiate.TryGetUrls(out var urls);
return new { Instantiate = instantiate, Urls = urls };
})
.Where(x => x.Urls != null && x.Urls.Any()))
{
if (instantiate.TryGetUrls(out var urls) && urls != null && urls.Any())
foreach (var url in item.Urls)
{
foreach (var url in urls)
{
instantiateUrls.Add(url);
}
instantiateUrls.Add(url);
}
}

Copilot is powered by AI and may make mistakes. Always verify output.

if (instantiateUrls.Any())
{
builder.Apply(
x =>
{
x.Instantiates = instantiateUrls;
});
}

return Task.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// -------------------------------------------------------------------------------------------------
// 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 EnsureThat;
using Microsoft.Extensions.Options;
using Microsoft.Health.Fhir.Core.Configs;

namespace Microsoft.Health.Fhir.Shared.Core.Features.Conformance
{
public class SmartV2InstantiateCapability : IInstantiateCapability
{
private static readonly string[] Urls =
{
"http://hl7.org/fhir/smart-app-launch/CapabilityStatement/smart-app-state-server",
};

private readonly SecurityConfiguration _securityConfiguration;

public SmartV2InstantiateCapability(IOptions<SecurityConfiguration> securityConfiguration)
{
EnsureArg.IsNotNull(securityConfiguration?.Value, nameof(securityConfiguration));

_securityConfiguration = securityConfiguration.Value;

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
securityConfiguration
may be null at this access as suggested by
this
null check.

Copilot Autofix

AI 4 days ago

General approach: ensure the securityConfiguration parameter is explicitly checked for null before dereferencing it, and avoid using a pattern (?.Value) that suggests to the analyzer that securityConfiguration might remain null when we later dereference it. We still want to validate the contained .Value as non-null, preserving existing semantics.

Best concrete fix in this snippet: change the constructor’s guard so that we first check securityConfiguration is non-null, and then check securityConfiguration.Value is non-null, both via EnsureArg. This makes the non-nullness of securityConfiguration clear and eliminates the path where CodeQL thinks it might be null at line 26. The rest of the class remains the same.

Changes (all in src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/SmartV2InstantiateCapability.cs):

  • Replace the single line EnsureArg.IsNotNull(securityConfiguration?.Value, nameof(securityConfiguration)); with two lines:
    • EnsureArg.IsNotNull(securityConfiguration, nameof(securityConfiguration));
    • EnsureArg.IsNotNull(securityConfiguration.Value, nameof(securityConfiguration));
  • Keep _securityConfiguration = securityConfiguration.Value; as is.

No new methods or imports are needed; we already use EnsureThat and have access to EnsureArg.

Suggested changeset 1
src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/SmartV2InstantiateCapability.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/SmartV2InstantiateCapability.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/SmartV2InstantiateCapability.cs
--- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/SmartV2InstantiateCapability.cs
+++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Conformance/SmartV2InstantiateCapability.cs
@@ -21,7 +21,8 @@
 
         public SmartV2InstantiateCapability(IOptions<SecurityConfiguration> securityConfiguration)
         {
-            EnsureArg.IsNotNull(securityConfiguration?.Value, nameof(securityConfiguration));
+            EnsureArg.IsNotNull(securityConfiguration, nameof(securityConfiguration));
+            EnsureArg.IsNotNull(securityConfiguration.Value, nameof(securityConfiguration));
 
             _securityConfiguration = securityConfiguration.Value;
         }
EOF
@@ -21,7 +21,8 @@

public SmartV2InstantiateCapability(IOptions<SecurityConfiguration> securityConfiguration)
{
EnsureArg.IsNotNull(securityConfiguration?.Value, nameof(securityConfiguration));
EnsureArg.IsNotNull(securityConfiguration, nameof(securityConfiguration));
EnsureArg.IsNotNull(securityConfiguration.Value, nameof(securityConfiguration));

_securityConfiguration = securityConfiguration.Value;
}
Copilot is powered by AI and may make mistakes. Always verify output.
}

public bool TryGetUrls(out IEnumerable<string> urls)
{
urls = null;
if (_securityConfiguration.Authorization.Enabled
|| _securityConfiguration.Authorization.EnableSmartWithoutAuth)
{
urls = Urls;
return true;
}

return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// -------------------------------------------------------------------------------------------------
// 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 Microsoft.Health.Fhir.Shared.Core.Features.Conformance;

namespace Microsoft.Health.Fhir.Core.Features.Conformance
{
public class USCore6InstantiateCapability : IInstantiateCapability
{
private static readonly string[] Urls =
{
"http://hl7.org/fhir/us/core/CapabilityStatement/us-core-server",
};

public bool TryGetUrls(out IEnumerable<string> urls)
{
urls = Urls;
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\DocRefRequestConverter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\IDocRefRequestConverter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\FirelyTerminologyServiceProxy.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\IInstantiateCapability.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\InstantiatesCapabilityProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\SmartV2InstantiateCapability.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\TerminologyRequestHandler.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Conformance\USCore6InstantiateCapability.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\DefaultParserSettings.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Operations\Everything\EverythingOperationParameterNames.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Features\Operations\Everything\EverythingOperationContinuationToken.cs" />
Expand Down
Loading