Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

2555 Add support for multiple valuesets and system filters to ValueSetExpander #2558

Merged
merged 4 commits into from
Aug 3, 2023
Merged

2555 Add support for multiple valuesets and system filters to ValueSetExpander #2558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b303200
Add cycle detection
ewoutkramer Jul 31, 2023
89a035b
Merge branch 'develop' into feature/expand-ts-capabilities
ewoutkramer Jul 31, 2023
b2d25eb
Add support for multiple compose.ValueSet and compose.System.
ewoutkramer Jul 31, 2023
9e93e69
Reworked the expander to more closely follow the expansion algorithm …
ewoutkramer Aug 1, 2023
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
11 changes: 9 additions & 2 deletions src/Hl7.Fhir.Base/Specification/Terminology/ValueSetExpanderSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ public class ValueSetExpanderSettings
/// </summary>
public bool IncludeDesignations { get; set; }

//// <summary>
//// Controls whether concepts are included that are marked as inactive. This setting overrides
//// <c>ValueSet.compose.inactive</c>.
//// </summary>
//public bool IncludeInactive { get; set; }

/// <summary>Default constructor. Creates a new <see cref="ValueSetExpanderSettings"/> instance with default property values.</summary>
public ValueSetExpanderSettings() { }

Expand All @@ -57,13 +63,14 @@ public void CopyTo(ValueSetExpanderSettings other)
other.MaxExpansionSize = MaxExpansionSize;
other.ValueSetSource = ValueSetSource;
other.IncludeDesignations = IncludeDesignations;
// other.IncludeInactive = IncludeInactive;
ewoutkramer marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>Creates a new <see cref="ValueSetExpanderSettings"/> object that is a copy of the current instance.</summary>
public ValueSetExpanderSettings Clone() => new ValueSetExpanderSettings(this);
public ValueSetExpanderSettings Clone() => new(this);

/// <summary>Creates a new <see cref="ValueSetExpanderSettings"/> instance with default property values.</summary>
public static ValueSetExpanderSettings CreateDefault() => new ValueSetExpanderSettings();
public static ValueSetExpanderSettings CreateDefault() => new();

}
}
141 changes: 77 additions & 64 deletions src/Hl7.Fhir.Conformance/Specification/Terminology/ValueSetExpander.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/*
#nullable enable

/*
* Copyright (c) 2016, Firely (info@fire.ly) and contributors
* See the file CONTRIBUTORS for details.
*
Expand All @@ -16,19 +18,28 @@

namespace Hl7.Fhir.Specification.Terminology
{
/// <summary>
/// Expands valuesets by processing their <c>include</c> and <c>exclude</c> filters. Will create an in-place expansion.
/// </summary>
public class ValueSetExpander
{

//ValueSetExpander keeps throwing TerminologyService Exceptions to not change the public interface.
#pragma warning disable 0618

/// <summary>
/// Settings to control the behaviour of the expansion.
/// </summary>
public ValueSetExpanderSettings Settings { get; }

/// <summary>
/// Create a new expander with specific settings.
/// </summary>
/// <param name="settings"></param>
public ValueSetExpander(ValueSetExpanderSettings settings)
{
Settings = settings;
}

/// <summary>
/// Create a new expander with default settings
/// </summary>
public ValueSetExpander() : this(ValueSetExpanderSettings.CreateDefault())
{
// nothing
Expand All @@ -37,8 +48,14 @@ public ValueSetExpander() : this(ValueSetExpanderSettings.CreateDefault())
[Obsolete("ValueSetExpander now works best with asynchronous resolvers. Use ExpandAsync() instead.")]
public void Expand(ValueSet source) => TaskHelper.Await(() => ExpandAsync(source));

/// <summary>
/// Expand the <c>include</c> and <c>exclude</c> filters. Creates the <c></c>
/// </summary>
/// <param name="source"></param>
/// <returns></returns>
public T.Task ExpandAsync(ValueSet source) => ExpandAsync(source, new());

public async T.Task ExpandAsync(ValueSet source)
internal async T.Task ExpandAsync(ValueSet source, Stack<string> inclusionChain)
{
// Note we are expanding the valueset in-place, so it's up to the caller to decide whether
// to clone the valueset, depending on store and performance requirements.
Expand All @@ -47,15 +64,19 @@ public async T.Task ExpandAsync(ValueSet source)

try
{
await handleCompose(source).ConfigureAwait(false);
inclusionChain.Push(source.Url);
await handleCompose(source, inclusionChain).ConfigureAwait(false);
}
catch (Exception)
{
// Expansion failed - remove (partial) expansion
source.Expansion = null;
throw;
}

finally
{
inclusionChain.Pop();
}
}

private void setExpansionParameters(ValueSet vs)
Expand All @@ -72,42 +93,36 @@ private void setExpansionParameters(ValueSet vs)
//TODO add more parameters to the valuset here when we implement them.
}

private async T.Task handleCompose(ValueSet source, Stack<string> inclusionChain)
{
if (source.Compose == null) return;

//private int copyToExpansion(string system, string version, IEnumerable<ValueSet.ConceptDefinitionComponent> source, List<ValueSet.ContainsComponent> dest)
//{
// int added = 0;

// foreach (var concept in source)
// {
// bool isDeprecated = concept.GetDeprecated() ?? false;

// if (!isDeprecated)
// {
// var newContains = addToExpansion(system, version, concept.Code, concept.Display, concept.Abstract, dest);
// added += 1;

// if (concept.Concept != null && concept.Concept.Any())
// added += copyToExpansion(system, version, concept.Concept, newContains.Contains);
// }
// }
// handleImport(source);
await handleInclude(source, inclusionChain).ConfigureAwait(false);
await handleExclude(source, inclusionChain).ConfigureAwait(false);
}

// return added;
//}

private async T.Task handleCompose(ValueSet source)
private class SystemAndCodeComparer : IEqualityComparer<ValueSet.ContainsComponent>
{
if (source.Compose == null) return;
public bool Equals(ValueSet.ContainsComponent? x, ValueSet.ContainsComponent? y)
{
if (ReferenceEquals(x, y)) return true;
if (x is null || y is null) return false;

// handleImport(source);
await handleInclude(source).ConfigureAwait(false);
await handleExclude(source).ConfigureAwait(false);
return x.Code == y.Code && x.System == y.System;
}

public int GetHashCode(ValueSet.ContainsComponent obj) => (obj.Code ?? "").GetHashCode() ^ (obj.System ?? "").GetHashCode();
}

private static readonly IEqualityComparer<ValueSet.ContainsComponent> systemAndCodeComparer = new SystemAndCodeComparer();

private async T.Task<List<ValueSet.ContainsComponent>> collectConcepts(ValueSet.ConceptSetComponent conceptSet)
private async T.Task<List<ValueSet.ContainsComponent>> collectConcepts(ValueSet.ConceptSetComponent conceptSet, Stack<string> inclusionChain)
{
List<ValueSet.ContainsComponent> result = new List<ValueSet.ContainsComponent>();
List<ValueSet.ContainsComponent> result = new();

// vsd-1
if (!conceptSet.ValueSet.Any() && conceptSet.System == null)
throw Error.InvalidOperation($"Encountered a ConceptSet with neither a 'system' nor a 'valueset'");

Expand Down Expand Up @@ -143,14 +158,14 @@ private async T.Task handleCompose(ValueSet source)

if (conceptSet.ValueSet.Any())
{
if (conceptSet.ValueSet.Count() > 1)
throw new ValueSetExpansionTooComplexException($"ConceptSets with multiple valuesets are not yet supported.");
if (conceptSet.System != null)
throw new ValueSetExpansionTooComplexException($"ConceptSets with combined 'system' and 'valueset'(s) are not yet supported.");

var importedVs = conceptSet.ValueSet.Single();
var concepts = await getExpansionForValueSet(importedVs).ConfigureAwait(false);
import(result, concepts, importedVs);
var expanded = await T.Task.WhenAll(conceptSet.ValueSet.Select(vs => getExpansionForValueSet(vs, inclusionChain))).ConfigureAwait(false);
var concepts = expanded.SelectMany(concept => concept);

if (conceptSet.System is not null)
concepts = concepts.Where(c => c.System == conceptSet.System);

concepts = concepts.Distinct(systemAndCodeComparer);
import(result, concepts.ToList(), string.Join(",", conceptSet.ValueSet));
}

return result;
Expand All @@ -164,14 +179,14 @@ void import(List<ValueSet.ContainsComponent> dest, List<ValueSet.ContainsCompone
}
}

private async T.Task handleInclude(ValueSet source)
private async T.Task handleInclude(ValueSet source, Stack<string> inclusionChain)
{
if (!source.Compose.Include.Any()) return;

int csIndex = 0;
foreach (var include in source.Compose.Include)
{
var includedConcepts = await collectConcepts(include).ConfigureAwait(false);
var includedConcepts = await collectConcepts(include, inclusionChain).ConfigureAwait(false);

// Yes, exclusion could make this smaller again, but alas, before we have processed those we might have run out of memory
if (source.Expansion.Total + includedConcepts.Count > Settings.MaxExpansionSize)
Expand All @@ -186,13 +201,13 @@ private async T.Task handleInclude(ValueSet source)
}
}

private async T.Task handleExclude(ValueSet source)
private async T.Task handleExclude(ValueSet source, Stack<string> inclusionChain)
{
if (!source.Compose.Exclude.Any()) return;

foreach (var exclude in source.Compose.Exclude)
{
var excludedConcepts = await collectConcepts(exclude).ConfigureAwait(false);
var excludedConcepts = await collectConcepts(exclude, inclusionChain).ConfigureAwait(false);

source.Expansion.Contains.Remove(excludedConcepts);

Expand All @@ -201,22 +216,22 @@ private async T.Task handleExclude(ValueSet source)
}
}


private async T.Task<List<ValueSet.ContainsComponent>> getExpansionForValueSet(string uri)
private async T.Task<List<ValueSet.ContainsComponent>> getExpansionForValueSet(string uri, Stack<string> inclusionChain)
{
if (inclusionChain.Contains(uri))
throw new TerminologyServiceException($"ValueSet expansion encountered a cycling dependency from {inclusionChain.Peek()} back to {uri}.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was wondering how you would solve this, nice


if (Settings.ValueSetSource == null)
throw Error.InvalidOperation($"No valueset resolver available to resolve valueset '{uri}', " +
"set ValueSetExpander.Settings.ValueSetSource to fix.");

var importedVs = await Settings.ValueSetSource.AsAsync().FindValueSetAsync(uri).ConfigureAwait(false);
if (importedVs == null) throw new ValueSetUnknownException($"The ValueSet expander cannot find system '{uri}', so the expansion cannot be completed.");

if (!importedVs.HasExpansion) await ExpandAsync(importedVs).ConfigureAwait(false);
var importedVs = await Settings.ValueSetSource.AsAsync().FindValueSetAsync(uri).ConfigureAwait(false)
?? throw new ValueSetUnknownException($"The ValueSet expander cannot find system '{uri}', so the expansion cannot be completed.");
if (!importedVs.HasExpansion) await ExpandAsync(importedVs, inclusionChain).ConfigureAwait(false);

if (importedVs.HasExpansion)
return importedVs.Expansion.Contains;
else
throw new ValueSetUnknownException($"Expansion returned neither an error, nor an expansion for ValueSet with canonical reference '{uri}'");
return importedVs.HasExpansion
? importedVs.Expansion.Contains
: throw new ValueSetUnknownException($"Expansion returned neither an error, nor an expansion for ValueSet with canonical reference '{uri}'");
}

private async T.Task<List<ValueSet.ContainsComponent>> getConceptsFromCodeSystem(string uri)
Expand All @@ -225,20 +240,17 @@ private async T.Task handleExclude(ValueSet source)
throw Error.InvalidOperation($"No terminology service available to resolve references to codesystem '{uri}', " +
"set ValueSetExpander.Settings.ValueSetSource to fix.");

var importedCs = await Settings.ValueSetSource.AsAsync().FindCodeSystemAsync(uri).ConfigureAwait(false);
if (importedCs == null) throw new ValueSetUnknownException($"The ValueSet expander cannot find system '{uri}', so the expansion cannot be completed.");
var importedCs = await Settings.ValueSetSource.AsAsync().FindCodeSystemAsync(uri).ConfigureAwait(false)
?? throw new ValueSetUnknownException($"The ValueSet expander cannot find system '{uri}', so the expansion cannot be completed.");

var result = new List<ValueSet.ContainsComponent>();
result.AddRange(importedCs.Concept.Select(c => c.ToContainsComponent(importedCs, Settings)));

return result;
return importedCs.Concept.Select(c => c.ToContainsComponent(importedCs, Settings)).ToList();
}
}


public static class ContainsSetExtensions
{
public static ValueSet.ContainsComponent Add(this List<ValueSet.ContainsComponent> dest, string system, string version, string code, string display, List<ValueSet.DesignationComponent> designations = null, IEnumerable<ValueSet.ContainsComponent> children = null)
public static ValueSet.ContainsComponent Add(this List<ValueSet.ContainsComponent> dest, string system, string version, string code, string display, List<ValueSet.DesignationComponent>? designations = null, IEnumerable<ValueSet.ContainsComponent>? children = null)
{
var newContains = new ValueSet.ContainsComponent
{
Expand Down Expand Up @@ -331,5 +343,6 @@ private static ValueSet.DesignationComponent ToValueSetDesignation(this CodeSyst
}

}
#pragma warning restore
}

#nullable restore
Loading