Merge pull request #2803 from FirelyTeam/fix/snapshotsource-reuse-sna…

…pshot

fix: #2802 - let SnapshotSource reuse self-generated snapshot

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

@@ -2225,8 +2225,10 @@ private async Tasks.Task<bool> ensureSnapshot(StructureDefinition sd, string pro
             try
             {
                 if (_settings.GenerateSnapshotForExternalProfiles
+#pragma warning disable CS0618 // Type or member is obsolete
                 && (!sd.HasSnapshot || (_settings.ForceRegenerateSnapshots && !sd.Snapshot.IsCreatedBySnapshotGenerator()))
                 )
+#pragma warning restore CS0618 // Type or member is obsolete
                 {
                     // Automatically expand external profiles on demand
                     // Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(ensureSnapshot)}] Recursively generate snapshot for type profile with url: '{sd.Url}' ...");
@@ -2310,7 +2312,9 @@ private async Tasks.Task<ElementDefinition> getSnapshotRootElement(StructureDefi
 #endif
 
             // 2. Return root element definition from existing (pre-generated) snapshot, if it exists
+#pragma warning disable CS0618 // Type or member is obsolete
             if (sd.HasSnapshot && (sd.Snapshot.IsCreatedBySnapshotGenerator() || !_settings.ForceRegenerateSnapshots))
+#pragma warning restore CS0618 // Type or member is obsolete
             {
                 // Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(getSnapshotRootElement)}] {nameof(profileUri)} = '{profileUri}' - use existing root element definition from snapshot: #{sd.Snapshot.Element[0].GetHashCode()}");
                 // No need to save root ElemDef annotation, as the snapshot has already been fully expanded

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

@@ -1941,7 +1941,7 @@ private async Tasks.Task<bool> ensureSnapshot(StructureDefinition sd, string pro
             try
             {
                 if (_settings.GenerateSnapshotForExternalProfiles
-                && (!sd.HasSnapshot || (_settings.ForceRegenerateSnapshots && !sd.Snapshot.IsCreatedBySnapshotGenerator()))
+                && (!sd.HasSnapshot || (_settings.RegenerationBehaviour == RegenerationSettings.FORCE_REGENERATE && !sd.Snapshot.IsCreatedBySnapshotGenerator()))
                 )
                 {
                     // Automatically expand external profiles on demand
@@ -2026,7 +2026,7 @@ private async Tasks.Task<ElementDefinition> getSnapshotRootElement(StructureDefi
 #endif
 
             // 2. Return root element definition from existing (pre-generated) snapshot, if it exists
-            if (sd.HasSnapshot && (sd.Snapshot.IsCreatedBySnapshotGenerator() || !_settings.ForceRegenerateSnapshots))
+            if (sd.HasSnapshot && (sd.Snapshot.IsCreatedBySnapshotGenerator() || _settings.RegenerationBehaviour != RegenerationSettings.FORCE_REGENERATE))
             {
                 // Debug.Print($"[{nameof(SnapshotGenerator)}.{nameof(getSnapshotRootElement)}] {nameof(profileUri)} = '{profileUri}' - use existing root element definition from snapshot: #{sd.Snapshot.Element[0].GetHashCode()}");
                 // No need to save root ElemDef annotation, as the snapshot has already been fully expanded

src/Hl7.Fhir.Shims.Base/Hl7.Fhir.Shims.Base.projitems

@@ -10,6 +10,7 @@
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(MSBuildThisFileDirectory)Model\ValueSetExpansionExtensions.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Specification\Snapshot\SnapshotGeneratorSettings.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Specification\Source\SnapshotSource.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Specification\Terminology\CodeSystemFilterProcessor.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Specification\Terminology\ExpandParameters.cs" />

src/Hl7.Fhir.Conformance/Specification/Snapshot/SnapshotGeneratorSettings.cs → src/Hl7.Fhir.Shims.Base/Specification/Snapshot/SnapshotGeneratorSettings.cs

@@ -1,4 +1,4 @@
-/* 
+/* 
  * Copyright (c) 2017, Firely (info@fire.ly) and contributors
  * See the file CONTRIBUTORS for details.
  * 
@@ -7,6 +7,7 @@
  */
 
 using Hl7.Fhir.Utility;
+using System;
 
 namespace Hl7.Fhir.Specification.Snapshot
 {
@@ -34,7 +35,9 @@ public void CopyTo(SnapshotGeneratorSettings other)
         {
             if (other == null) { throw Error.ArgumentNull(nameof(other)); }
             other.GenerateSnapshotForExternalProfiles = GenerateSnapshotForExternalProfiles;
+#pragma warning disable CS0618 // Type or member is obsolete
             other.ForceRegenerateSnapshots = ForceRegenerateSnapshots;
+#pragma warning restore CS0618 // Type or member is obsolete
             other.GenerateExtensionsOnConstraints = GenerateExtensionsOnConstraints;
             other.GenerateAnnotationsOnConstraints = GenerateAnnotationsOnConstraints;
             other.GenerateElementIds = GenerateElementIds;
@@ -55,7 +58,18 @@ public void CopyTo(SnapshotGeneratorSettings other)
         /// Re-generated snapshots are annotated to prevent duplicate re-generation (assuming the provided resource resolver uses caching).
         /// If disabled (default), then the snapshot generator relies on existing snapshot components, if they exist.
         /// </summary>
-        public bool ForceRegenerateSnapshots { get; set; } = false; // ForceExpandAll
+        [Obsolete(
+            "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")]
+        public bool ForceRegenerateSnapshots
+        {
+            get { return this.RegenerationBehaviour == RegenerationSettings.FORCE_REGENERATE; } 
+            set { this.RegenerationBehaviour = value ? RegenerationSettings.FORCE_REGENERATE : RegenerationSettings.REGENERATE_ONCE; }
+        } // ForceExpandAll
+
+        /// <summary>
+        /// Setting for the regeneration behaviour of the snapshot generator. see <see cref="RegenerationSettings"/>.
+        /// </summary>
+        public RegenerationSettings RegenerationBehaviour { get; set; }
 
         /// <summary>
         /// Enable this setting to add a custom <see cref="SnapshotGeneratorExtensions.CONSTRAINED_BY_DIFF_EXT"/> extension
@@ -84,4 +98,23 @@ public void CopyTo(SnapshotGeneratorSettings other)
         // <remarks>See GForge #9791</remarks>
         // public bool MergeTypeProfiles { get; set; }
     }
+
+    /// <summary>
+    /// Settings for defining the behaviour of the snapshot generator with respect to regenerating snapshots.
+    /// </summary>
+    public enum RegenerationSettings
+    {
+        /// <summary>
+        /// Try to use an existing snapshot, if available.
+        /// </summary>
+        TRY_USE_EXISTING,
+        /// <summary>
+        /// Regenerate the snapshot once, to ensure it is up-to-date.
+        /// </summary>
+        REGENERATE_ONCE,
+        /// <summary>
+        /// Regenerate the snapshot every time. This is useful for debugging and testing purposes.
+        /// </summary>
+        FORCE_REGENERATE,
+    }
 }

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

@@ -54,7 +54,9 @@ public SnapshotSource(ISyncOrAsyncResourceResolver source, bool regenerate)
         private static SnapshotGeneratorSettings createSettings(bool regenerate)
         {
             var settings = SnapshotGeneratorSettings.CreateDefault();
+#pragma warning disable CS0618 // Type or member is obsolete
             settings.ForceRegenerateSnapshots = regenerate;
+#pragma warning restore CS0618 // Type or member is obsolete
             return settings;
         }
 
@@ -70,19 +72,19 @@ public SnapshotSource(ISyncOrAsyncResourceResolver source)
 
         #region IResourceResolver
 
-        private IAsyncResourceResolver _resolver => Generator.AsyncResolver;
+        private IAsyncResourceResolver Resolver => Generator.AsyncResolver;
 
         /// <summary>Find a resource based on its relative or absolute uri.</summary>
         /// <remarks>The source ensures that resolved <see cref="StructureDefinition"/> instances have a snapshot component.</remarks>
-        public async Tasks.Task<Resource> ResolveByUriAsync(string uri) => await ensureSnapshot(await _resolver.ResolveByUriAsync(uri).ConfigureAwait(false)).ConfigureAwait(false);
+        public async Tasks.Task<Resource> ResolveByUriAsync(string uri) => await ensureSnapshot(await Resolver.ResolveByUriAsync(uri).ConfigureAwait(false)).ConfigureAwait(false);
 
         /// <inheritdoc cref="ResolveByUriAsync(string)"/>
         [Obsolete("SnapshotSource now works best with asynchronous resolvers. Use ResolveByUriAsync() instead.")]
         public Resource ResolveByUri(string uri) => TaskHelper.Await(() => ResolveByUriAsync(uri));
 
         /// <summary>Find a (conformance) resource based on its canonical uri.</summary>
         /// <remarks>The source ensures that resolved <see cref="StructureDefinition"/> instances have a snapshot component.</remarks>
-        public async Tasks.Task<Resource> ResolveByCanonicalUriAsync(string uri) => await ensureSnapshot(await _resolver.ResolveByCanonicalUriAsync(uri).ConfigureAwait(false)).ConfigureAwait(false);
+        public async Tasks.Task<Resource> ResolveByCanonicalUriAsync(string uri) => await ensureSnapshot(await Resolver.ResolveByCanonicalUriAsync(uri).ConfigureAwait(false)).ConfigureAwait(false);
 
         /// <inheritdoc cref="ResolveByCanonicalUriAsync(string)"/>
         [Obsolete("SnapshotSource now works best with asynchronous resolvers. Use ResolveByCanonicalUriAsync() instead.")]
@@ -96,7 +98,14 @@ private async Tasks.Task<Resource> ensureSnapshot(Resource res)
         {
             if (res is StructureDefinition sd)
             {
-                if (!sd.HasSnapshot || Generator.Settings.ForceRegenerateSnapshots || !sd.Snapshot.IsCreatedBySnapshotGenerator())
+                if (
+                    !sd.HasSnapshot || 
+                    Generator.Settings.RegenerationBehaviour == RegenerationSettings.FORCE_REGENERATE || 
+                    (
+                        !sd.Snapshot.IsCreatedBySnapshotGenerator() && 
+                        Generator.Settings.RegenerationBehaviour == RegenerationSettings.REGENERATE_ONCE
+                    )
+                )
                 {
                     await Generator.UpdateAsync(sd).ConfigureAwait(false);
                 }
@@ -107,6 +116,6 @@ private async Tasks.Task<Resource> ensureSnapshot(Resource res)
         // Allow derived classes to override
         // http://blogs.msdn.com/b/jaredpar/archive/2011/03/18/debuggerdisplay-attribute-best-practices.aspx
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
-        internal protected virtual string DebuggerDisplay => $"{GetType().Name} for {_resolver.DebuggerDisplayString()}";
+        internal protected virtual string DebuggerDisplay => $"{GetType().Name} for {Resolver.DebuggerDisplayString()}";
     }
 }