Fix inverted logic on registry cache expiration

[ZachNagengast]

In RegistryClient.swift there is an in-memory package metadata cache expiration check

        let cacheKey = MetadataCacheKey(registry: registry, package: package)
        if let cached = self.metadataCache[cacheKey], cached.expires < .now() {
            return cached.metadata
        }

The logic for cached.expires < .now() appears to be inverted, e.g. if a cache is found for the cacheKey, the initial values look like this 2389992062152 < 2303592072235 which equates to false, because cached.expires is set with a TTL of 60*60 seconds in the future. This leads to the cache only being used after it has expired and may be causing downstream fingerprint TOFU issues when new package releases are published and the metadata cache is in-memory.

Motivation:

This issue was identified via the use of a private SPM registry conforming to the registry server specification. Disclaimer: This issue was also occurring in previous versions of Xcode but now that they are sharing code with SPM, I hope this example is still relevant. The scenario it is occurring in is as follows:

1. Have Xcode open for 1 hour or more after resolving dependencies at least once to populate the cache.
2. Release a new package version to the registry
3. Attempt to resolve the new version in Xcode
4. At this stage, my belief is that the previous release's metadata (including the checksum) from the "expired" cache is stored to disk as the fingerprint associated with the new release for TOFU purposes.
   • Code path:
   • -> Get expected checksum for new version
   • -> Get metadata for version
   • -> Check if we should hit the server or use cache
   • -> Use cached metadata if it's been in memory for 1 hour
   • -> Write cached (old) checksum to fingerprint storage for new version
5. Attempting even swift package resolve via CLI at this point results the following error:
   • invalid registry source archive checksum 'newchecksum', expected 'previouschecksum'

Because the checksum fingerprint is stored on disk in ~/Library/org.swift.swiftpm/security/fingerprints/, it takes manual intervention to recover from this by purging various caches.
Example of a common workaround: https://stackoverflow.com/questions/79417322/invalid-registry-source-archive-checksum-a-expected-b
Related github issue: #8981

In addition to that, there are probably excessive requests being sent to various registries due to the cache not being used during the initial TTL.

Modifications:

Two main changes in this PR that should help with this:

1. Adding version property to the MetadataCacheKey, so that even if the previous metadata is in cache and hasn't expired, it won't be applied to the new version.
2. Reversing the logic for checking expiration, so that it will appropriately fetch new data upon expiration, rather than only fetching new data until expiration.

In addition to that, this PR also:

• Fixes a similar cache expiration issue for the availabilityCache
• Adds tests for both caches
• Updates withAvailabilityCheck to internal (from private) for testability.

Result:

The hopeful result of this is that package registry consumers will no longer require manual workarounds to fix their local state when they hit this edge case, as well as avoid hitting github and other package registries too often (my guess for the original intent of the cache).

That said, it will be a bit tricky to validate in its entirety because this cache is only relevant for long-running tasks via libSwiftPM I presume.

One big thing to note is that this may be a noticeable change to existing behavior since the cache is not currently being used as far as I can tell, and once fixed, it will significantly reduce the freshness of the metadata compared to before.

Curious to hear the maintainers' thoughts on this.

[ZachNagengast]

This section is just moved below to group internal scoped functions.

[plemarquand]

Thanks so much for this contribution, this is a great catch. I really appreciate the well laid out issue description as well as the test coverage.

[ZachNagengast]

Thanks @plemarquand! Would you be able to trigger the ci tests?

[plemarquand]

@swift-ci test

[plemarquand]

CI blocked by #9150

[ZachNagengast]

@swift-ci test

[plemarquand]

@swift-ci test

[plemarquand]

@swift-ci test windows