From 2906727b5e8b69349caeeba1e803459a2cea652f Mon Sep 17 00:00:00 2001 From: Patrick Pan Date: Fri, 29 Nov 2024 15:34:14 +1100 Subject: [PATCH] add lock on SetReferrerState Signed-off-by: Patrick Pan --- ...s => ReferrersStateAlreadySetException.cs} | 8 ++--- .../Remote/HttpResponseMessageExtensions.cs | 2 +- .../Registry/Remote/ManifestStore.cs | 6 ++-- .../Registry/Remote/Referrers.cs | 12 +++---- .../Registry/Remote/Repository.cs | 36 ++++++++++--------- .../Exceptions/ExceptionTest.cs | 6 ++-- .../Remote/ManifestStoreTest.cs | 28 +++++++-------- .../Remote/ReferrersTest.cs | 11 ------ .../Remote/RepositoryTest.cs | 34 +++++++++--------- 9 files changed, 65 insertions(+), 78 deletions(-) rename src/OrasProject.Oras/Exceptions/{ReferrersSupportLevelAlreadySetException.cs => ReferrersStateAlreadySetException.cs} (72%) diff --git a/src/OrasProject.Oras/Exceptions/ReferrersSupportLevelAlreadySetException.cs b/src/OrasProject.Oras/Exceptions/ReferrersStateAlreadySetException.cs similarity index 72% rename from src/OrasProject.Oras/Exceptions/ReferrersSupportLevelAlreadySetException.cs rename to src/OrasProject.Oras/Exceptions/ReferrersStateAlreadySetException.cs index 87ffd00..4c0b7ad 100644 --- a/src/OrasProject.Oras/Exceptions/ReferrersSupportLevelAlreadySetException.cs +++ b/src/OrasProject.Oras/Exceptions/ReferrersStateAlreadySetException.cs @@ -15,18 +15,18 @@ namespace OrasProject.Oras.Exceptions; -public class ReferrersSupportLevelAlreadySetException : Exception +public class ReferrersStateAlreadySetException : Exception { - public ReferrersSupportLevelAlreadySetException() + public ReferrersStateAlreadySetException() { } - public ReferrersSupportLevelAlreadySetException(string? message) + public ReferrersStateAlreadySetException(string? message) : base(message) { } - public ReferrersSupportLevelAlreadySetException(string? message, Exception? inner) + public ReferrersStateAlreadySetException(string? message, Exception? inner) : base(message, inner) { } diff --git a/src/OrasProject.Oras/Registry/Remote/HttpResponseMessageExtensions.cs b/src/OrasProject.Oras/Registry/Remote/HttpResponseMessageExtensions.cs index 354d3ac..0e4585c 100644 --- a/src/OrasProject.Oras/Registry/Remote/HttpResponseMessageExtensions.cs +++ b/src/OrasProject.Oras/Registry/Remote/HttpResponseMessageExtensions.cs @@ -112,7 +112,7 @@ public static void CheckOCISubjectHeader(this HttpResponseMessage response, Repo if (response.Headers.TryGetValues("OCI-Subject", out var values)) { // Set it to ReferrerSupported when the response header contains OCI-Subject - repository.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersSupported); + repository.SetReferrersState(Referrers.ReferrersState.ReferrersSupported); } // If the "OCI-Subject" header is NOT set, it means that either the manifest diff --git a/src/OrasProject.Oras/Registry/Remote/ManifestStore.cs b/src/OrasProject.Oras/Registry/Remote/ManifestStore.cs index 5f55f99..c4675c1 100644 --- a/src/OrasProject.Oras/Registry/Remote/ManifestStore.cs +++ b/src/OrasProject.Oras/Registry/Remote/ManifestStore.cs @@ -177,7 +177,7 @@ private async Task PushWithIndexingAsync(Descriptor expected, Stream content, Re { case MediaType.ImageManifest: case MediaType.ImageIndex: - if (Repository.ReferrersSupportLevel == Referrers.ReferrersSupportLevel.ReferrersSupported) + if (Repository.ReferrersState == Referrers.ReferrersState.ReferrersSupported) { // Push the manifest straightaway when the registry supports referrers API await DoPushAsync(expected, content, reference, cancellationToken).ConfigureAwait(false); @@ -190,7 +190,7 @@ private async Task PushWithIndexingAsync(Descriptor expected, Stream content, Re // Push the manifest when ReferrerState is Unknown or NotSupported await DoPushAsync(expected, contentDuplicate, reference, cancellationToken).ConfigureAwait(false); } - if (Repository.ReferrersSupportLevel == Referrers.ReferrersSupportLevel.ReferrersSupported) + if (Repository.ReferrersState == Referrers.ReferrersState.ReferrersSupported) { // Early exit when the registry supports Referrers API // No need to index referrers list @@ -244,7 +244,7 @@ private async Task ProcessReferrersAndPushIndex(Descriptor desc, Stream content, return; } - Repository.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersNotSupported); + Repository.SetReferrersState(Referrers.ReferrersState.ReferrersNotSupported); await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(desc, Referrers.ReferrerOperation.ReferrerAdd), cancellationToken); } diff --git a/src/OrasProject.Oras/Registry/Remote/Referrers.cs b/src/OrasProject.Oras/Registry/Remote/Referrers.cs index 061127c..cacaaee 100644 --- a/src/OrasProject.Oras/Registry/Remote/Referrers.cs +++ b/src/OrasProject.Oras/Registry/Remote/Referrers.cs @@ -20,11 +20,11 @@ namespace OrasProject.Oras.Registry.Remote; public class Referrers { - internal enum ReferrersSupportLevel + internal enum ReferrersState { - ReferrersUnknown, - ReferrersSupported, - ReferrersNotSupported + ReferrersUnknown = 0, + ReferrersSupported = 1, + ReferrersNotSupported = 2 } internal record ReferrerChange(Descriptor Referrer, ReferrerOperation ReferrerOperation); @@ -51,10 +51,6 @@ internal static string BuildReferrersTag(Descriptor descriptor) /// The updated referrers list, updateRequired internal static (IList, bool) ApplyReferrerChanges(IList oldReferrers, ReferrerChange referrerChange) { - if (oldReferrers == null || referrerChange == null) - { - return (new List(), false); - } // updatedReferrers is a list to store the updated referrers var updatedReferrers = new List(); // referrerIndexMap is a Dictionary to store referrer as the key diff --git a/src/OrasProject.Oras/Registry/Remote/Repository.cs b/src/OrasProject.Oras/Registry/Remote/Repository.cs index 44f6f20..e413b80 100644 --- a/src/OrasProject.Oras/Registry/Remote/Repository.cs +++ b/src/OrasProject.Oras/Registry/Remote/Repository.cs @@ -47,8 +47,14 @@ public class Repository : IRepository public RepositoryOptions Options => _opts; - internal Referrers.ReferrersSupportLevel ReferrersSupportLevel { get; set; } = Referrers.ReferrersSupportLevel.ReferrersUnknown; - + private int _referrersState = (int) Referrers.ReferrersState.ReferrersUnknown; + + internal Referrers.ReferrersState ReferrersState + { + get => (Referrers.ReferrersState) _referrersState; + private set => _referrersState = (int) value; + } + internal static readonly string[] DefaultManifestMediaTypes = [ Docker.MediaType.Manifest, @@ -88,28 +94,26 @@ public Repository(RepositoryOptions options) } /// - /// SetReferrerSupportLevel indicates the Referrers API support level of the remote repository. + /// SetReferrersState indicates the Referrers API state of the remote repository. /// - /// SetReferrerSupportLevel is valid only when it is called for the first time. - /// SetReferrerSupportLevel returns ReferrersSupportLevelAlreadySetException if the - /// Referrers API support level has been already set. - /// - When the level is set to ReferrersSupported, the Referrers() function will always + /// SetReferrersState is valid only when it is called for the first time. + /// SetReferrersState returns ReferrersStateAlreadySetException if the + /// Referrers API state has been already set. + /// - When the state is set to ReferrersSupported, the Referrers() function will always /// request the Referrers API. Reference: https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#listing-referrers - /// - When the level is set to ReferrersNotSupported, the Referrers() function will always + /// - When the state is set to ReferrersNotSupported, the Referrers() function will always /// request the Referrers Tag. Reference: https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#referrers-tag-schema - /// - When the capability is not set, the Referrers() function will automatically + /// - When the state is not set, the Referrers() function will automatically /// determine which API to use. /// /// - /// - internal void SetReferrerSupportLevel(Referrers.ReferrersSupportLevel level) + /// + internal void SetReferrersState(Referrers.ReferrersState level) { - if (ReferrersSupportLevel == Referrers.ReferrersSupportLevel.ReferrersUnknown) - { - ReferrersSupportLevel = level; - } else if (ReferrersSupportLevel != level) + if (Interlocked.CompareExchange(ref _referrersState, (int) level, (int) Referrers.ReferrersState.ReferrersUnknown) == (int) Referrers.ReferrersState.ReferrersUnknown) + {} else if (_referrersState != (int) level) { - throw new ReferrersSupportLevelAlreadySetException($"current support level: {ReferrersSupportLevel}, latest support level: {level}"); + throw new ReferrersStateAlreadySetException($"current referrers state: {ReferrersState}, latest referrers state: {level}"); } } diff --git a/tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs b/tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs index 13888f3..d9fb8a5 100644 --- a/tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs +++ b/tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs @@ -53,8 +53,8 @@ public async Task NotFoundException() [Fact] public async Task ReferrersSupportLevelAlreadySetException() { - await Assert.ThrowsAsync(() => throw new ReferrersSupportLevelAlreadySetException()); - await Assert.ThrowsAsync(() => throw new ReferrersSupportLevelAlreadySetException("Referrers support level has already been set")); - await Assert.ThrowsAsync(() => throw new ReferrersSupportLevelAlreadySetException("Referrers support level has already been set", null)); + await Assert.ThrowsAsync(() => throw new ReferrersStateAlreadySetException()); + await Assert.ThrowsAsync(() => throw new ReferrersStateAlreadySetException("Referrers state has already been set")); + await Assert.ThrowsAsync(() => throw new ReferrersStateAlreadySetException("Referrers state has already been set", null)); } } diff --git a/tests/OrasProject.Oras.Tests/Remote/ManifestStoreTest.cs b/tests/OrasProject.Oras.Tests/Remote/ManifestStoreTest.cs index 350ae65..e0721e9 100644 --- a/tests/OrasProject.Oras.Tests/Remote/ManifestStoreTest.cs +++ b/tests/OrasProject.Oras.Tests/Remote/ManifestStoreTest.cs @@ -167,14 +167,14 @@ public async Task ManifestStore_PushAsyncWithoutSubject() var cancellationToken = new CancellationToken(); var store = new ManifestStore(repo); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); await store.PushAsync(expectedManifestDesc, new MemoryStream(expectedManifestBytes), cancellationToken); Assert.Equal(expectedManifestBytes, receivedManifest); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); await store.PushAsync(expectedConfigDesc, new MemoryStream(expectedConfigBytes), cancellationToken); Assert.Equal(expectedConfigBytes, receivedManifest); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); } @@ -254,15 +254,15 @@ public async Task ManifestStore_PushAsyncWithSubjectAndReferrerSupported() var store = new ManifestStore(repo); // first push with image manifest - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); await store.PushAsync(expectedManifestDesc, new MemoryStream(expectedManifestBytes), cancellationToken); Assert.Equal(expectedManifestBytes, receivedManifest); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersSupported, repo.ReferrersState); // second push with index manifest await store.PushAsync(expectedIndexManifestDesc, new MemoryStream(expectedIndexManifestBytes), cancellationToken); Assert.Equal(expectedIndexManifestBytes, receivedManifest); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersSupported, repo.ReferrersState); } @@ -377,18 +377,18 @@ public async Task ManifestStore_PushAsyncWithSubjectAndReferrerNotSupported() var store = new ManifestStore(repo); // First push with referrer tag schema - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); await store.PushAsync(firstExpectedManifestDesc, new MemoryStream(firstExpectedManifestBytes), cancellationToken); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersNotSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersNotSupported, repo.ReferrersState); Assert.Equal(firstExpectedManifestBytes, receivedManifestContent); Assert.True(oldIndexDeleted); Assert.Equal(firstExpectedIndexReferrersBytes, receivedIndexContent); // Second push with referrer tag schema - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersNotSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersNotSupported, repo.ReferrersState); await store.PushAsync(secondExpectedManifestDesc, new MemoryStream(secondExpectedManifestBytes), cancellationToken); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersNotSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersNotSupported, repo.ReferrersState); Assert.Equal(secondExpectedManifestBytes, receivedManifestContent); Assert.True(firstIndexDeleted); Assert.Equal(secondExpectedIndexReferrersBytes, receivedIndexContent); @@ -460,9 +460,9 @@ public async Task ManifestStore_PushAsyncWithSubjectAndReferrerNotSupportedWitho var cancellationToken = new CancellationToken(); var store = new ManifestStore(repo); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); await store.PushAsync(expectedIndexManifestDesc, new MemoryStream(expectedIndexManifestBytes), cancellationToken); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersNotSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersNotSupported, repo.ReferrersState); Assert.Equal(expectedIndexManifestBytes, receivedIndexManifestContent); Assert.Equal(expectedIndexReferrersBytes, receivedIndexReferrersContent); } @@ -547,9 +547,9 @@ public async Task ManifestStore_PushAsyncWithSubjectAndNoUpdateRequired() var cancellationToken = new CancellationToken(); var store = new ManifestStore(repo); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); await store.PushAsync(expectedManifestDesc, new MemoryStream(expectedManifestBytes), cancellationToken); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersNotSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersNotSupported, repo.ReferrersState); Assert.Equal(expectedManifestBytes, receivedManifestContent); } } diff --git a/tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs b/tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs index 5f39fd5..215d5a8 100644 --- a/tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs +++ b/tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs @@ -155,17 +155,6 @@ public void ApplyReferrerChanges_ShouldNotKeepOldEmptyReferrers() Assert.True(updateRequired); } - [Fact] - public void ApplyReferrerChanges_ThrowsWhenOldAndNewReferrersAreNull() - { - IList oldReferrers = null; - Referrers.ReferrerChange referrerChange = null; - - var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange); - Assert.Empty(updatedReferrers); - Assert.False(updateRequired); - } - [Fact] public void ApplyReferrerChanges_ThrowsWhenOldAndNewReferrersAreEmpty() { diff --git a/tests/OrasProject.Oras.Tests/Remote/RepositoryTest.cs b/tests/OrasProject.Oras.Tests/Remote/RepositoryTest.cs index 5efe889..6aae7e1 100644 --- a/tests/OrasProject.Oras.Tests/Remote/RepositoryTest.cs +++ b/tests/OrasProject.Oras.Tests/Remote/RepositoryTest.cs @@ -2546,40 +2546,38 @@ public async Task Repository_MountAsync_Fallback_GetContentError() } [Fact] - public void SetReferrersSupportLevel_ShouldSet_WhenInitiallyUnknown() + public void SetReferrersState_ShouldSet_WhenInitiallyUnknown() { var repo = new Repository("localhost:5000/test2"); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); - repo.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersSupported); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); + repo.SetReferrersState(Referrers.ReferrersState.ReferrersSupported); + Assert.Equal(Referrers.ReferrersState.ReferrersSupported, repo.ReferrersState); } [Fact] - public void SetReferrersSupportLevel_ShouldThrowException_WhenChangingAfterSet() + public void SetReferrersState_ShouldThrowException_WhenChangingAfterSet() { var repo = new Repository("localhost:5000/test2"); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); - repo.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersSupported); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); + repo.SetReferrersState(Referrers.ReferrersState.ReferrersSupported); + Assert.Equal(Referrers.ReferrersState.ReferrersSupported, repo.ReferrersState); - var exception = Assert.Throws(() => - repo.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersNotSupported) + var exception = Assert.Throws(() => + repo.SetReferrersState(Referrers.ReferrersState.ReferrersNotSupported) ); - Assert.Equal("current support level: ReferrersSupported, latest support level: ReferrersNotSupported", exception.Message); + Assert.Equal("current referrers state: ReferrersSupported, latest referrers state: ReferrersNotSupported", exception.Message); } [Fact] - public void SetReferrersSupportLevel_ShouldNotThrowException_WhenSettingSameValue() + public void SetReferrersState_ShouldNotThrowException_WhenSettingSameValue() { var repo = new Repository("localhost:5000/test2"); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersUnknown, repo.ReferrersSupportLevel); - repo.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersSupported); - Assert.Equal(Referrers.ReferrersSupportLevel.ReferrersSupported, repo.ReferrersSupportLevel); + Assert.Equal(Referrers.ReferrersState.ReferrersUnknown, repo.ReferrersState); + repo.SetReferrersState(Referrers.ReferrersState.ReferrersSupported); + Assert.Equal(Referrers.ReferrersState.ReferrersSupported, repo.ReferrersState); - var exception = Record.Exception(() => repo.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersSupported)); + var exception = Record.Exception(() => repo.SetReferrersState(Referrers.ReferrersState.ReferrersSupported)); Assert.Null(exception); } - - }