Merge pull request #2721 from FirelyTeam/feature/2164-zip-cache-fix

#2164 reinforce zip cache validity check

src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs

@@ -8,6 +8,7 @@
  * available at https://github.com/FirelyTeam/firely-net-sdk/blob/master/LICENSE
  */
 
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.IO.Compression;
@@ -67,11 +68,11 @@ public bool IsActual()
 
             if (!dir.Exists) return false;
 
-            // Sometimes unzipping fails after creating the directory, try to fix that by
-            // checking if there are any files at all.
-            var dirIsEmpty = !dir.EnumerateFileSystemInfos().Any();
-            if (dirIsEmpty) return false;
-
+            // zip unpacking fails sometimes (issue #2164). if this cache is supposed to contain the specification, it should have at least a bunch of files in there (notably, we test for profiles-types.xml)
+            var isSpecificationZip = ZipPath.EndsWith(Path.Combine("specification.zip"));
+            var doesNotHaveCriticalFiles = !File.Exists(Path.Combine(CachePath, "specification", "profiles-types.xml"));
+            if (isSpecificationZip && doesNotHaveCriticalFiles) return false;
+            
             var currentZipFileTime = File.GetLastWriteTimeUtc(ZipPath);
 
             return dir.CreationTimeUtc >= currentZipFileTime;
@@ -107,8 +108,14 @@ public void Refresh()
 
             dir.Create();
 
-
-            ZipFile.ExtractToDirectory(ZipPath, dir.FullName);
+            try
+            {
+                ZipFile.ExtractToDirectory(ZipPath, dir.FullName);
+            }
+            catch
+            {
+                Clear();
+            }
 
             // and also extract the contained zip in there too with all the xsds in there
             if (File.Exists(Path.Combine(dir.FullName, "fhir-all-xsd.zip")))
@@ -130,9 +137,9 @@ public void Clear()
 
 
         /// <summary>
-        /// Gets the cache directory, but does not create one if it does not yet exist
+        /// Gets the cache directory, or creates an empty cache directory if it does not exist
         /// </summary>
-        /// <returns></returns>
+        /// <returns>Information about the existing (or new) cache directory</returns>
         private DirectoryInfo getCachedZipDirectory()
         {
             // First, create the main "cache" directory.

src/Hl7.Fhir.Conformance/Specification/Source/CommonZipSource.cs

@@ -22,7 +22,7 @@ namespace Hl7.Fhir.Specification.Source
     /// <summary>Reads FHIR artifacts (Profiles, ValueSets, ...) from a ZIP archive. Thread-safe.</summary>
     /// <remarks>Extracts the ZIP archive to a temporary folder and delegates to the <see cref="CommonDirectorySource"/>.</remarks>
     [DebuggerDisplay(@"\{{DebuggerDisplay,nq}}")]
-    public class CommonZipSource : ISummarySource, ICommonConformanceSource, IArtifactSource, IResourceResolver, IAsyncResourceResolver
+    public class CommonZipSource : ISummarySource, ICommonConformanceSource, IArtifactSource, IAsyncResourceResolver
     {
 #pragma warning disable IDE1006 // Naming Styles - cannot fix this name because of bw-compatibility
         public const string SpecificationZipFileName = "specification.zip";

src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs

@@ -1,9 +1,13 @@
 using FluentAssertions;
+using FluentAssertions.Extensions;
+using Hl7.Fhir.Model;
 using Hl7.Fhir.Specification.Source;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using System;
 using System.IO;
+using System.IO.Compression;
 using System.Linq;
+using Task = System.Threading.Tasks.Task;
 
 namespace Hl7.Fhir.Specification.Tests.Source
 {
@@ -30,7 +34,7 @@ public void ListSummariesExcludingSubdirectories()
         {
             var zip = unpackTestData(new DirectorySourceSettings { IncludeSubDirectories = false });
             var summaries = zip.ListSummaries();
-            summaries.First().Origin.StartsWith(zip.ExtractPath);
+            summaries.First().Origin.StartsWith(zip.ExtractPath).Should().BeTrue();
 
             Assert.IsNotNull(summaries, "Collection of summaries should not be null");
             Assert.AreEqual(1, summaries.Count(), "In the zipfile there is 1 resource in the root folder.");
@@ -52,5 +56,16 @@ public void UnpacksToSpecificDirectory()
             var summaries = zip.ListSummaries();
             summaries.First().Origin.Should().StartWith(extractDir);
         }
+
+        [TestMethod]
+        // If this test fails, the specification might have changed. This filename is hardcoded into the validation of a successful ZIP unpack
+        // (firely-net-sdk/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs:74)
+        public void TestFilePrescence()
+        {
+            var zip = ZipSource.CreateValidationSource();
+            zip.ListSummaries(); // make sure the zip is unpacked, we don't need the return value
+            // if extractpath is null, something went seriously wrong
+            File.Exists(Path.Combine(zip.ExtractPath!, "profiles-types.xml")).Should().BeTrue();
+        }
     }
 }