From 9f341fbc39576ff42e27addd1062067859b12cf2 Mon Sep 17 00:00:00 2001 From: Kasdejong Date: Thu, 15 Feb 2024 17:38:22 +0100 Subject: [PATCH 1/3] stash --- .../Specification/Source/ZipCacher.cs | 4 ++- .../Specification/Source/CommonZipSource.cs | 2 +- .../Source/ZipSourceTests.cs | 35 ++++++++++++++++++- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs b/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs index ce2bc21fa8..8d14f4fb52 100644 --- a/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs +++ b/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; @@ -70,7 +71,8 @@ public bool IsActual() // 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; + var dirContents = dir.EnumerateFileSystemInfos(); + Console.Error.WriteLine(dirIsEmpty); var currentZipFileTime = File.GetLastWriteTimeUtc(ZipPath); diff --git a/src/Hl7.Fhir.Conformance/Specification/Source/CommonZipSource.cs b/src/Hl7.Fhir.Conformance/Specification/Source/CommonZipSource.cs index 2c3dba0b87..4edd9b1eeb 100644 --- a/src/Hl7.Fhir.Conformance/Specification/Source/CommonZipSource.cs +++ b/src/Hl7.Fhir.Conformance/Specification/Source/CommonZipSource.cs @@ -22,7 +22,7 @@ namespace Hl7.Fhir.Specification.Source /// Reads FHIR artifacts (Profiles, ValueSets, ...) from a ZIP archive. Thread-safe. /// Extracts the ZIP archive to a temporary folder and delegates to the . [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"; diff --git a/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs b/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs index bb3f34164d..c4afa9cc0e 100644 --- a/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs +++ b/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,34 @@ public void UnpacksToSpecificDirectory() var summaries = zip.ListSummaries(); summaries.First().Origin.Should().StartWith(extractDir); } + + [TestMethod] + public void TestConcurrentUnpackAttempts() + { + var zipFile = Path.Combine("TestData", "ResourcesInSubfolder.zip"); + var extractDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + var startingSecond = (System.DateTime.UtcNow.Second) % 60; + Console.Error.WriteLine(extractDir); + + var tasks = Enumerable.Range(0, 10).Select(i => new Task(() => _doUnpackAttempt(startingSecond, zipFile, extractDir))); + + Assert.IsTrue(Task.WaitAll(tasks.ToArray(), 5000)); + } + + private static bool _canStart(int seconds) + { + return System.DateTime.UtcNow.Second >= seconds; + } + + private static void _doUnpackAttempt(int seconds, string zipFile, string extractDir) + { + var zip = new ZipSource(zipFile, extractDir, + new DirectorySourceSettings() { IncludeSubDirectories = false }); + + while (!_canStart(seconds)) {} + + var summaries = zip.ListSummaries(); + Console.Error.WriteLine(summaries); + } } } From 02b58359da63ae941fe9d24fc6d3649bcf589fd3 Mon Sep 17 00:00:00 2001 From: Kasdejong Date: Tue, 20 Feb 2024 14:32:11 +0100 Subject: [PATCH 2/3] Reworked zip unpacking actuality check to classify a cache directory without files as non-actual (this should now trigger re-unpacking) --- .../Specification/Source/ZipCacher.cs | 27 ++++++++++------ .../Source/ZipSourceTests.cs | 32 ++++--------------- 2 files changed, 24 insertions(+), 35 deletions(-) diff --git a/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs b/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs index 8d14f4fb52..989886ff60 100644 --- a/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs +++ b/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs @@ -68,12 +68,13 @@ 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(); - var dirContents = dir.EnumerateFileSystemInfos(); - Console.Error.WriteLine(dirIsEmpty); - + // zip unpacking fails sometimes + // 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, "profiles-types.xml")); + if (isSpecificationZip && doesNotHaveCriticalFiles) return false; + var currentZipFileTime = File.GetLastWriteTimeUtc(ZipPath); return dir.CreationTimeUtc >= currentZipFileTime; @@ -109,8 +110,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"))) @@ -132,9 +139,9 @@ public void Clear() /// - /// 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 /// - /// + /// Information about the existing (or new) cache directory private DirectoryInfo getCachedZipDirectory() { // First, create the main "cache" directory. diff --git a/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs b/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs index c4afa9cc0e..f143232f28 100644 --- a/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs +++ b/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs @@ -58,32 +58,14 @@ public void UnpacksToSpecificDirectory() } [TestMethod] - public void TestConcurrentUnpackAttempts() + // 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 zipFile = Path.Combine("TestData", "ResourcesInSubfolder.zip"); - var extractDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); - var startingSecond = (System.DateTime.UtcNow.Second) % 60; - Console.Error.WriteLine(extractDir); - - var tasks = Enumerable.Range(0, 10).Select(i => new Task(() => _doUnpackAttempt(startingSecond, zipFile, extractDir))); - - Assert.IsTrue(Task.WaitAll(tasks.ToArray(), 5000)); - } - - private static bool _canStart(int seconds) - { - return System.DateTime.UtcNow.Second >= seconds; - } - - private static void _doUnpackAttempt(int seconds, string zipFile, string extractDir) - { - var zip = new ZipSource(zipFile, extractDir, - new DirectorySourceSettings() { IncludeSubDirectories = false }); - - while (!_canStart(seconds)) {} - - var summaries = zip.ListSummaries(); - Console.Error.WriteLine(summaries); + var zip = ZipSource.CreateValidationSource(); + zip.ListSummaries(); // make sure it is unpacked, 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(); } } } From 2339fc73a8ce7bb9ed69a849c12baf80b90d7309 Mon Sep 17 00:00:00 2001 From: Kasdejong Date: Tue, 20 Feb 2024 14:45:55 +0100 Subject: [PATCH 3/3] updated comments --- src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs | 6 ++---- .../Source/ZipSourceTests.cs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs b/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs index 989886ff60..80b29bbeea 100644 --- a/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs +++ b/src/Hl7.Fhir.Base/Specification/Source/ZipCacher.cs @@ -68,11 +68,9 @@ public bool IsActual() if (!dir.Exists) return false; - // zip unpacking fails sometimes - // 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) + // 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, "profiles-types.xml")); + var doesNotHaveCriticalFiles = !File.Exists(Path.Combine(CachePath, "specification", "profiles-types.xml")); if (isSpecificationZip && doesNotHaveCriticalFiles) return false; var currentZipFileTime = File.GetLastWriteTimeUtc(ZipPath); diff --git a/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs b/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs index f143232f28..3884e1017c 100644 --- a/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs +++ b/src/Hl7.Fhir.Specification.Shared.Tests/Source/ZipSourceTests.cs @@ -63,7 +63,7 @@ public void UnpacksToSpecificDirectory() public void TestFilePrescence() { var zip = ZipSource.CreateValidationSource(); - zip.ListSummaries(); // make sure it is unpacked, don't need the return value + 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(); }