Skip to content

Commit

Permalink
add lock on SetReferrerState
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick Pan <[email protected]>
  • Loading branch information
Patrick Pan committed Nov 29, 2024
1 parent 50e696e commit 2906727
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/OrasProject.Oras/Registry/Remote/ManifestStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -244,7 +244,7 @@ private async Task ProcessReferrersAndPushIndex(Descriptor desc, Stream content,
return;

Check warning on line 244 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs#L244

Added line #L244 was not covered by tests
}

Repository.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersNotSupported);
Repository.SetReferrersState(Referrers.ReferrersState.ReferrersNotSupported);
await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(desc, Referrers.ReferrerOperation.ReferrerAdd), cancellationToken);
}

Expand Down
12 changes: 4 additions & 8 deletions src/OrasProject.Oras/Registry/Remote/Referrers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -51,10 +51,6 @@ internal static string BuildReferrersTag(Descriptor descriptor)
/// <returns>The updated referrers list, updateRequired</returns>
internal static (IList<Descriptor>, bool) ApplyReferrerChanges(IList<Descriptor> oldReferrers, ReferrerChange referrerChange)
{
if (oldReferrers == null || referrerChange == null)
{
return (new List<Descriptor>(), false);
}
// updatedReferrers is a list to store the updated referrers
var updatedReferrers = new List<Descriptor>();
// referrerIndexMap is a Dictionary to store referrer as the key
Expand Down
36 changes: 20 additions & 16 deletions src/OrasProject.Oras/Registry/Remote/Repository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Check warning on line 55 in src/OrasProject.Oras/Registry/Remote/Repository.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/Repository.cs#L55

Added line #L55 was not covered by tests
}

internal static readonly string[] DefaultManifestMediaTypes =
[
Docker.MediaType.Manifest,
Expand Down Expand Up @@ -88,28 +94,26 @@ public Repository(RepositoryOptions options)
}

/// <summary>
/// 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.
/// </summary>
/// <param name="level"></param>
/// <exception cref="ReferrersSupportLevelAlreadySetException"></exception>
internal void SetReferrerSupportLevel(Referrers.ReferrersSupportLevel level)
/// <exception cref="ReferrersStateAlreadySetException"></exception>
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}");
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ public async Task NotFoundException()
[Fact]
public async Task ReferrersSupportLevelAlreadySetException()
{
await Assert.ThrowsAsync<ReferrersSupportLevelAlreadySetException>(() => throw new ReferrersSupportLevelAlreadySetException());
await Assert.ThrowsAsync<ReferrersSupportLevelAlreadySetException>(() => throw new ReferrersSupportLevelAlreadySetException("Referrers support level has already been set"));
await Assert.ThrowsAsync<ReferrersSupportLevelAlreadySetException>(() => throw new ReferrersSupportLevelAlreadySetException("Referrers support level has already been set", null));
await Assert.ThrowsAsync<ReferrersStateAlreadySetException>(() => throw new ReferrersStateAlreadySetException());
await Assert.ThrowsAsync<ReferrersStateAlreadySetException>(() => throw new ReferrersStateAlreadySetException("Referrers state has already been set"));
await Assert.ThrowsAsync<ReferrersStateAlreadySetException>(() => throw new ReferrersStateAlreadySetException("Referrers state has already been set", null));
}
}
28 changes: 14 additions & 14 deletions tests/OrasProject.Oras.Tests/Remote/ManifestStoreTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down Expand Up @@ -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);
}


Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}
11 changes: 0 additions & 11 deletions tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,17 +155,6 @@ public void ApplyReferrerChanges_ShouldNotKeepOldEmptyReferrers()
Assert.True(updateRequired);
}

[Fact]
public void ApplyReferrerChanges_ThrowsWhenOldAndNewReferrersAreNull()
{
IList<Descriptor> oldReferrers = null;
Referrers.ReferrerChange referrerChange = null;

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Assert.Empty(updatedReferrers);
Assert.False(updateRequired);
}

[Fact]
public void ApplyReferrerChanges_ThrowsWhenOldAndNewReferrersAreEmpty()
{
Expand Down
34 changes: 16 additions & 18 deletions tests/OrasProject.Oras.Tests/Remote/RepositoryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReferrersSupportLevelAlreadySetException>(() =>
repo.SetReferrerSupportLevel(Referrers.ReferrersSupportLevel.ReferrersNotSupported)
var exception = Assert.Throws<ReferrersStateAlreadySetException>(() =>
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);
}


}

0 comments on commit 2906727

Please sign in to comment.