Skip to content

Commit a4230b7

Browse files
ewoutkramerewoutkramer
authored
Merge pull request #2943 from FirelyTeam/feature/use-proper-snapshot-generation-settings
Use proper snapshot generation settings, fixed obsolete warnings
2 parents 962f408 + f3b4806 commit a4230b7

File tree

20 files changed

+136
-98
lines changed

20 files changed

+136
-98
lines changed

‎src/Hl7.Fhir.Base/CompatibilitySuppressions.xml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@
77
<Left>lib/netstandard2.0/Hl7.Fhir.Base.dll</Left>
88
<Right>lib/net8.0/Hl7.Fhir.Base.dll</Right>
99
</Suppression>
10+
<Suppression>
11+
<DiagnosticId>CP0002</DiagnosticId>
12+
<Target>M:Hl7.Fhir.FhirPath.FhirEvaluationContext.WithResourceOverrides(Hl7.Fhir.ElementModel.ITypedElement,Hl7.Fhir.ElementModel.ITypedElement)</Target>
13+
<Left>lib/net8.0/Hl7.Fhir.Base.dll</Left>
14+
<Right>lib/net8.0/Hl7.Fhir.Base.dll</Right>
15+
<IsBaselineSuppression>true</IsBaselineSuppression>
16+
</Suppression>
1017
<Suppression>
1118
<DiagnosticId>CP0002</DiagnosticId>
1219
<Target>M:Hl7.Fhir.Rest.ContentType.BuildContentType(Hl7.Fhir.Rest.ResourceFormat,System.String)</Target>
@@ -21,6 +28,20 @@
2128
<Right>lib/net8.0/Hl7.Fhir.Base.dll</Right>
2229
<IsBaselineSuppression>true</IsBaselineSuppression>
2330
</Suppression>
31+
<Suppression>
32+
<DiagnosticId>CP0002</DiagnosticId>
33+
<Target>M:Hl7.FhirPath.EvaluationContext.WithResourceOverrides(Hl7.Fhir.ElementModel.ITypedElement,Hl7.Fhir.ElementModel.ITypedElement)</Target>
34+
<Left>lib/net8.0/Hl7.Fhir.Base.dll</Left>
35+
<Right>lib/net8.0/Hl7.Fhir.Base.dll</Right>
36+
<IsBaselineSuppression>true</IsBaselineSuppression>
37+
</Suppression>
38+
<Suppression>
39+
<DiagnosticId>CP0002</DiagnosticId>
40+
<Target>M:Hl7.Fhir.FhirPath.FhirEvaluationContext.WithResourceOverrides(Hl7.Fhir.ElementModel.ITypedElement,Hl7.Fhir.ElementModel.ITypedElement)</Target>
41+
<Left>lib/netstandard2.0/Hl7.Fhir.Base.dll</Left>
42+
<Right>lib/netstandard2.0/Hl7.Fhir.Base.dll</Right>
43+
<IsBaselineSuppression>true</IsBaselineSuppression>
44+
</Suppression>
2445
<Suppression>
2546
<DiagnosticId>CP0002</DiagnosticId>
2647
<Target>M:Hl7.Fhir.Rest.ContentType.BuildContentType(Hl7.Fhir.Rest.ResourceFormat,System.String)</Target>
@@ -35,4 +56,11 @@
3556
<Right>lib/netstandard2.0/Hl7.Fhir.Base.dll</Right>
3657
<IsBaselineSuppression>true</IsBaselineSuppression>
3758
</Suppression>
59+
<Suppression>
60+
<DiagnosticId>CP0002</DiagnosticId>
61+
<Target>M:Hl7.FhirPath.EvaluationContext.WithResourceOverrides(Hl7.Fhir.ElementModel.ITypedElement,Hl7.Fhir.ElementModel.ITypedElement)</Target>
62+
<Left>lib/netstandard2.0/Hl7.Fhir.Base.dll</Left>
63+
<Right>lib/netstandard2.0/Hl7.Fhir.Base.dll</Right>
64+
<IsBaselineSuppression>true</IsBaselineSuppression>
65+
</Suppression>
3866
</Suppressions>

‎src/Hl7.Fhir.Base/FhirPath/EvaluationContext.cs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,6 @@ public EvaluationContext(ITypedElement? resource, ITypedElement? rootResource, I
4141
{
4242
Environment = environment;
4343
}
44-
45-
/// <summary>
46-
/// Explicitly override the values of %resource and %rootResource in the evaluation context.
47-
/// </summary>
48-
public static EvaluationContext WithResourceOverrides(ITypedElement? resource, ITypedElement? rootResource = null) =>
49-
new EvaluationContext { Resource = resource, RootResource = rootResource ?? resource };
5044

5145
/// <summary>
5246
/// The data represented by <c>%rootResource</c>.
@@ -67,4 +61,14 @@ public static EvaluationContext WithResourceOverrides(ITypedElement? resource, I
6761
/// A delegate that handles the output for the <c>trace()</c> function.
6862
/// </summary>
6963
public Action<string, IEnumerable<ITypedElement>>? Tracer { get; set; }
64+
}
65+
66+
public static class EvaluationContextExtensions
67+
{
68+
public static T WithResourceOverrides<T>(this T context, ITypedElement? resource, ITypedElement? rootResource = null) where T : EvaluationContext
69+
{
70+
context.Resource = resource;
71+
context.RootResource = rootResource ?? resource;
72+
return context;
73+
}
7074
}

‎src/Hl7.Fhir.Base/FhirPath/FhirEvaluationContext.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,7 @@ public FhirEvaluationContext(ScopedNode node)
6363
{
6464
RootResource = Resource is ScopedNode sn ? sn.ResourceContext : node;
6565
}
66-
67-
/// <summary>
68-
/// Explicitly override the values of %resource and %rootResource in the evaluation context.
69-
/// </summary>
70-
public static new FhirEvaluationContext WithResourceOverrides(ITypedElement? resource, ITypedElement? rootResource = null) =>
71-
(FhirEvaluationContext)EvaluationContext.WithResourceOverrides(resource, rootResource);
66+
7267
public ITerminologyService? TerminologyService { get; set; }
7368

7469
private static ITypedElement toNearestResource(ScopedNode node)

‎src/Hl7.Fhir.Conformance/Specification/Snapshot/SnapshotGenerator.cs‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2224,11 +2224,17 @@ private async Tasks.Task<bool> ensureSnapshot(StructureDefinition sd, string pro
22242224

22252225
try
22262226
{
2227-
if (_settings.GenerateSnapshotForExternalProfiles
2227+
var shouldGenerate = _settings.RegenerationBehaviour switch
2228+
{
2229+
RegenerationSettings.TRY_USE_EXISTING => !sd.HasSnapshot,
2230+
RegenerationSettings.REGENERATE_ONCE => !sd.HasSnapshot || !sd.Snapshot.IsCreatedBySnapshotGenerator(),
22282231
#pragma warning disable CS0618 // Type or member is obsolete
2229-
&& (!sd.HasSnapshot || (_settings.ForceRegenerateSnapshots && !sd.Snapshot.IsCreatedBySnapshotGenerator()))
2230-
)
2232+
RegenerationSettings.FORCE_REGENERATE => true, // possible infinite recursion
22312233
#pragma warning restore CS0618 // Type or member is obsolete
2234+
_ => throw new InvalidOperationException($"Invalid RegenerationSettings value {_settings.RegenerationBehaviour}")
2235+
};
2236+
2237+
if (_settings.GenerateSnapshotForExternalProfiles && shouldGenerate)
22322238
{
22332239
// Automatically expand external profiles on demand
22342240
// Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(ensureSnapshot)}] Recursively generate snapshot for type profile with url: '{sd.Url}' ...");
@@ -2310,11 +2316,18 @@ private async Tasks.Task<ElementDefinition> getSnapshotRootElement(StructureDefi
23102316
var cachedRoot = sd.GetSnapshotRootElementAnnotation();
23112317
if (cachedRoot != null) { return cachedRoot; }
23122318
#endif
2313-
2314-
// 2. Return root element definition from existing (pre-generated) snapshot, if it exists
2319+
var hasValidRoot = _settings.RegenerationBehaviour switch
2320+
{
2321+
RegenerationSettings.TRY_USE_EXISTING => sd.HasSnapshot,
2322+
RegenerationSettings.REGENERATE_ONCE => sd.HasSnapshot && sd.Snapshot.IsCreatedBySnapshotGenerator(),
23152323
#pragma warning disable CS0618 // Type or member is obsolete
2316-
if (sd.HasSnapshot && (sd.Snapshot.IsCreatedBySnapshotGenerator() || !_settings.ForceRegenerateSnapshots))
2324+
RegenerationSettings.FORCE_REGENERATE => false,
23172325
#pragma warning restore CS0618 // Type or member is obsolete
2326+
_ => throw new InvalidOperationException($"Invalid RegenerationSettings value {_settings.RegenerationBehaviour}")
2327+
};
2328+
2329+
// 2. Return root element definition from existing (pre-generated) snapshot, if it exists
2330+
if (hasValidRoot)
23182331
{
23192332
// Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(getSnapshotRootElement)}] {nameof(profileUri)} = '{profileUri}' - use existing root element definition from snapshot: #{sd.Snapshot.Element[0].GetHashCode()}");
23202333
// No need to save root ElemDef annotation, as the snapshot has already been fully expanded

‎src/Hl7.Fhir.STU3.Tests/Model/ValidateAllExamplesSearchExtractionTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private static void ExtractExamplesFromResource(Dictionary<ModelInfo.SearchParam
154154

155155
try
156156
{
157-
var results = resourceModel.Select(index.Expression, new EvaluationContext(resourceModel));
157+
var results = resourceModel.Select(index.Expression, new EvaluationContext());
158158
if (results.Count() > 0)
159159
{
160160
foreach (var t2 in results)

‎src/Hl7.Fhir.STU3.Tests/Validation/SearchDataExtraction.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
using System.IO.Compression;
2121
using System.Linq;
2222
using System.Xml;
23+
using FhirEvaluationContext = Hl7.Fhir.FhirPath.FhirEvaluationContext;
2324

2425
namespace Hl7.Fhir.Test.Validation
2526
{
@@ -34,7 +35,7 @@ public class ValidateSearchExtractionAllExamplesTest
3435
[TestCategory("LongRunner")]
3536
public void SearchExtractionAllExamples()
3637
{
37-
string examplesZip = @"TestData\examples.zip";
38+
string examplesZip = @"TestData/examples.zip";
3839

3940
FhirXmlParser parser = new FhirXmlParser();
4041
int errorCount = 0;
@@ -130,7 +131,7 @@ private static void ExtractValuesForSearchParameterFromFile(Dictionary<string, i
130131

131132
private static void ExtractExamplesFromResource(Dictionary<string, int> exampleSearchValues, Resource resource, ModelInfo.SearchParamDefinition index, string key)
132133
{
133-
var results = resource.Select(index.Expression, new FhirEvaluationContext(resource.ToTypedElement()));
134+
var results = resource.Select(index.Expression, new FhirEvaluationContext());
134135
if (results.Any())
135136
{
136137
// we perform the Select on a Poco, because then we get the FHIR dialect of FhirPath as well.

‎src/Hl7.Fhir.STU3/Specification/Snapshot/SnapshotGenerator.cs‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,9 +1940,17 @@ private async Tasks.Task<bool> ensureSnapshot(StructureDefinition sd, string pro
19401940

19411941
try
19421942
{
1943-
if (_settings.GenerateSnapshotForExternalProfiles
1944-
&& (!sd.HasSnapshot || (_settings.RegenerationBehaviour == RegenerationSettings.FORCE_REGENERATE && !sd.Snapshot.IsCreatedBySnapshotGenerator()))
1945-
)
1943+
var shouldGenerate = _settings.RegenerationBehaviour switch
1944+
{
1945+
RegenerationSettings.TRY_USE_EXISTING => !sd.HasSnapshot,
1946+
RegenerationSettings.REGENERATE_ONCE => !sd.HasSnapshot || !sd.Snapshot.IsCreatedBySnapshotGenerator(),
1947+
#pragma warning disable CS0618 // Type or member is obsolete
1948+
RegenerationSettings.FORCE_REGENERATE => true,
1949+
#pragma warning restore CS0618 // Type or member is obsolete
1950+
_ => throw new InvalidOperationException($"Invalid RegenerationSettings value {_settings.RegenerationBehaviour}")
1951+
};
1952+
1953+
if (_settings.GenerateSnapshotForExternalProfiles && shouldGenerate)
19461954
{
19471955
// Automatically expand external profiles on demand
19481956
// Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(ensureSnapshot)}] Recursively generate snapshot for type profile with url: '{sd.Url}' ...");
@@ -2024,9 +2032,18 @@ private async Tasks.Task<ElementDefinition> getSnapshotRootElement(StructureDefi
20242032
var cachedRoot = sd.GetSnapshotRootElementAnnotation();
20252033
if (cachedRoot != null) { return cachedRoot; }
20262034
#endif
2027-
2035+
var hasValidRoot = _settings.RegenerationBehaviour switch
2036+
{
2037+
RegenerationSettings.TRY_USE_EXISTING => sd.HasSnapshot,
2038+
RegenerationSettings.REGENERATE_ONCE => sd.HasSnapshot && sd.Snapshot.IsCreatedBySnapshotGenerator(),
2039+
#pragma warning disable CS0618 // Type or member is obsolete
2040+
RegenerationSettings.FORCE_REGENERATE => false,
2041+
#pragma warning restore CS0618 // Type or member is obsolete
2042+
_ => throw new InvalidOperationException($"Invalid RegenerationSettings value {_settings.RegenerationBehaviour}")
2043+
};
2044+
20282045
// 2. Return root element definition from existing (pre-generated) snapshot, if it exists
2029-
if (sd.HasSnapshot && (sd.Snapshot.IsCreatedBySnapshotGenerator() || _settings.RegenerationBehaviour != RegenerationSettings.FORCE_REGENERATE))
2046+
if (hasValidRoot)
20302047
{
20312048
// Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(getSnapshotRootElement)}] {nameof(profileUri)} = '{profileUri}' - use existing root element definition from snapshot: #{sd.Snapshot.Element[0].GetHashCode()}");
20322049
// No need to save root ElemDef annotation, as the snapshot has already been fully expanded

‎src/Hl7.Fhir.Shared.Tests/Validation/SearchDataExtraction.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ private static void ExtractExamplesFromResource(Dictionary<string, int> exampleS
131131
try
132132
{
133133
// we perform the Select on a Poco, because then we get the FHIR dialect of FhirPath as well.
134-
results = resource.Select(index.Expression, new FhirEvaluationContext(resource.ToTypedElement()) { ElementResolver = mockResolver });
134+
results = resource.Select(index.Expression!, new FhirEvaluationContext { ElementResolver = mockResolver });
135135
}
136136
catch (Exception)
137137
{

‎src/Hl7.Fhir.Shims.Base/Specification/Snapshot/SnapshotGeneratorSettings.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ public void CopyTo(SnapshotGeneratorSettings other)
3535
{
3636
if (other == null) { throw Error.ArgumentNull(nameof(other)); }
3737
other.GenerateSnapshotForExternalProfiles = GenerateSnapshotForExternalProfiles;
38-
#pragma warning disable CS0618 // Type or member is obsolete
39-
other.ForceRegenerateSnapshots = ForceRegenerateSnapshots;
40-
#pragma warning restore CS0618 // Type or member is obsolete
38+
other.RegenerationBehaviour = RegenerationBehaviour;
4139
other.GenerateExtensionsOnConstraints = GenerateExtensionsOnConstraints;
4240
other.GenerateAnnotationsOnConstraints = GenerateAnnotationsOnConstraints;
4341
other.GenerateElementIds = GenerateElementIds;
@@ -59,11 +57,11 @@ public void CopyTo(SnapshotGeneratorSettings other)
5957
/// If disabled (default), then the snapshot generator relies on existing snapshot components, if they exist.
6058
/// </summary>
6159
[Obsolete(
62-
"This setting does not work as intended. When set to true, it regenerates a snapshot every time (which is not useful), and when set to false, it still regenerates a snapshot once, even if it already exists. We will consider removing it in a future major release. Use the new RegenerationBehaviour setting instead. See also https://github.com/FirelyTeam/firely-net-sdk/pull/2803")]
60+
"This setting does not work as intended. We will maintain the old behaviour for now, and we will consider removing it in a future major release. Use the new RegenerationBehaviour setting instead. See also https://github.com/FirelyTeam/firely-net-sdk/pull/2803")]
6361
public bool ForceRegenerateSnapshots
6462
{
65-
get { return this.RegenerationBehaviour == RegenerationSettings.FORCE_REGENERATE; }
66-
set { this.RegenerationBehaviour = value ? RegenerationSettings.FORCE_REGENERATE : RegenerationSettings.REGENERATE_ONCE; }
63+
get { return this.RegenerationBehaviour == RegenerationSettings.REGENERATE_ONCE; }
64+
set { this.RegenerationBehaviour = value ? RegenerationSettings.REGENERATE_ONCE : RegenerationSettings.TRY_USE_EXISTING; }
6765
} // ForceExpandAll
6866

6967
/// <summary>
@@ -115,6 +113,7 @@ public enum RegenerationSettings
115113
/// <summary>
116114
/// Regenerate the snapshot every time. This is useful for debugging and testing purposes.
117115
/// </summary>
116+
[Obsolete("Watch out when using this setting! it could lead to infinite recursion and is mainly meant for debugging and testing purposes. If you previously had ForceRegenerateSnapshots set to true, consider using REGENERATE_ONCE instead.")]
118117
FORCE_REGENERATE,
119118
}
120119
}

‎src/Hl7.Fhir.Shims.Base/Specification/Source/SnapshotSource.cs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,17 @@ private async Tasks.Task<Resource> ensureSnapshot(Resource res)
9898
{
9999
if (res is StructureDefinition sd)
100100
{
101-
if (
102-
!sd.HasSnapshot ||
103-
Generator.Settings.RegenerationBehaviour == RegenerationSettings.FORCE_REGENERATE ||
104-
(
105-
!sd.Snapshot.IsCreatedBySnapshotGenerator() &&
106-
Generator.Settings.RegenerationBehaviour == RegenerationSettings.REGENERATE_ONCE
107-
)
108-
)
101+
var shouldGenerate = Generator.Settings.RegenerationBehaviour switch
102+
{
103+
RegenerationSettings.TRY_USE_EXISTING => !sd.HasSnapshot,
104+
RegenerationSettings.REGENERATE_ONCE => !sd.HasSnapshot || !sd.Snapshot.IsCreatedBySnapshotGenerator(),
105+
#pragma warning disable CS0618 // Type or member is obsolete
106+
RegenerationSettings.FORCE_REGENERATE => true,
107+
#pragma warning restore CS0618 // Type or member is obsolete
108+
_ => throw Error.NotSupported($"Unknown regeneration behaviour: {Generator.Settings.RegenerationBehaviour}")
109+
};
110+
111+
if (shouldGenerate)
109112
{
110113
await Generator.UpdateAsync(sd).ConfigureAwait(false);
111114
}

0 commit comments

Comments
 (0)