Add caching for provider metadata #600
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently we download these files and cache them locally with a hardcoded lifetime. Once the file is expired, we attempt to download a new copy; however, if that request fails, we just fail the operation. With this change, we'll at least fall back to the already-stored copy, which may be stale but should still work in most cases.
RobJohnston
reviewed
Sep 14, 2020
RobJohnston
reviewed
Sep 14, 2020
RobJohnston
reviewed
Sep 14, 2020
tlmii
approved these changes
Sep 15, 2020
This change reduces the repentitiveness of implementing request-and-cache or check-cache-else-request patterns when requesting provider metadata. This simplified code in each provider, and also centralized testing that behavior on the CacheService class rather than duplicating cache behavior testing. It also removes the GetCatalogAsync and GetMetadataAsync as those names were Cdnjs-specifc, and the catalog is just another form of metadata. Testing was also largely refactored to apply fakes at the ICacheService layer instead of the underlying IWebRequestHandler, as that should be transparent to the Catalog implementations. There are a couple exceptions to verify that no web requests are issued at all during some test scenarios.
- Add ConfigureAwait(false) to the files changed in this PR - Remove unnecessary using in UnpkgCatalogTest - Remove unused sample string CdnjsProviderTest
jimmylewis
force-pushed
the
cacheMetadata
branch
from
bf6bc67 to
7ecebc2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This affects the scenarios where we need to fetch the latest library versions or the file lists for a given library. For Cdnjs, because they offer a full catalog, it also affects that.
When fetching versions (or catalog), we want to have live data if possible, as new versions (or packages) can be released. These operations will initially attempt a web request, and if it fails, fall back to the cached contents on disk (if any). Note that there is now a 1-day cache lifetime applied to the initial request, which should cut down on repetitive traffic.
When fetching file lists, we assume that released libraries are static. In this case, we first check for the information on disk, and then fallback to a web request if it's missing. There is no lifetime applied in this case. If the library is mutated, a user will have to clear the cache to get the updated metadata for the library (but, they'd have to do the same to get updated versions of any cached library assets as well).
This resolves #598, which should in turn help with some of the reliability issues reported in #370.