Skip to content

Commit

Permalink
resolve comments
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick Pan <[email protected]>
  • Loading branch information
Patrick Pan committed Dec 20, 2024
1 parent c30d6b2 commit b20a202
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/OrasProject.Oras/Oci/Descriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static Descriptor Create(Span<byte> data, string mediaType)

internal BasicDescriptor BasicDescriptor => new BasicDescriptor(MediaType, Digest, Size);

internal static bool IsEmptyOrInvalid(Descriptor? descriptor)
internal static bool IsNullOrInvalid(Descriptor? descriptor)
{
return descriptor == null || string.IsNullOrEmpty(descriptor.Digest) || string.IsNullOrEmpty(descriptor.MediaType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ public static void VerifyContentDigest(this HttpResponseMessage response, string
/// </summary>
/// <param name="response"></param>
/// <param name="repository"></param>
internal static void CheckOCISubjectHeader(this HttpResponseMessage response, Repository repository)
internal static void CheckOciSubjectHeader(this HttpResponseMessage response, Repository repository)
{
if (response.Headers.TryGetValues("OCI-Subject", out var values))
if (response.Headers.Contains("OCI-Subject"))
{
// Set it to Supported when the response header contains OCI-Subject
repository.ReferrersState = Referrers.ReferrersState.Supported;
Expand Down
19 changes: 12 additions & 7 deletions src/OrasProject.Oras/Registry/Remote/ManifestStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
using OrasProject.Oras.Oci;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Net;
using System.Net.Http;
using System.Runtime.InteropServices;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -251,7 +253,7 @@ private async Task ProcessReferrersAndPushIndex(Descriptor desc, Stream content,
}

Repository.ReferrersState = Referrers.ReferrersState.NotSupported;
await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(desc, Referrers.ReferrerOperation.ReferrerAdd), cancellationToken).ConfigureAwait(false);
await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(desc, Referrers.ReferrerOperation.Add), cancellationToken).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -276,10 +278,13 @@ private async Task UpdateReferrersIndex(Descriptor subject,
// 2. apply the referrer change to referrers list
var (updatedReferrers, updateRequired) =
Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
if (!updateRequired) return;
if (!updateRequired)
{
return;
}

// 3. push the updated referrers list using referrers tag schema
if (updatedReferrers.Count > 0 || repository.Options.SkipReferrersGC)
if (updatedReferrers.Count > 0 || repository.Options.SkipReferrersGc)
{
// push a new index in either case:
// 1. the referrers list has been updated with a non-zero size
Expand All @@ -293,9 +298,9 @@ private async Task UpdateReferrersIndex(Descriptor subject,
}
}

if (repository.Options.SkipReferrersGC || Descriptor.IsEmptyOrInvalid(oldDesc))
if (repository.Options.SkipReferrersGc || Descriptor.IsNullOrInvalid(oldDesc))
{
// Skip the delete process if SkipReferrersGC is set to true or the old Descriptor is empty or null
// Skip the delete process if SkipReferrersGc is set to true or the old Descriptor is empty or null
return;
}

Expand Down Expand Up @@ -326,7 +331,7 @@ private async Task UpdateReferrersIndex(Descriptor subject,
}
catch (NotFoundException)
{
return (null, new List<Descriptor>());
return (null, ImmutableArray<Descriptor>.Empty);
}
}

Expand All @@ -351,7 +356,7 @@ private async Task DoPushAsync(Descriptor expected, Stream stream, Reference rem
{
throw await response.ParseErrorResponseAsync(cancellationToken).ConfigureAwait(false);
}
response.CheckOCISubjectHeader(Repository);
response.CheckOciSubjectHeader(Repository);
response.VerifyContentDigest(expected.Digest);
}

Expand Down
14 changes: 6 additions & 8 deletions src/OrasProject.Oras/Registry/Remote/Referrers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ internal record ReferrerChange(Descriptor Referrer, ReferrerOperation ReferrerOp

internal enum ReferrerOperation
{
ReferrerAdd,
ReferrerDelete,
Add,
Delete,
}

internal static string BuildReferrersTag(Descriptor descriptor)
Expand All @@ -51,7 +51,7 @@ 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 (Descriptor.IsEmptyOrInvalid(referrerChange.Referrer))
if (Descriptor.IsNullOrInvalid(referrerChange.Referrer))
{
return (oldReferrers, false);
}
Expand All @@ -64,7 +64,7 @@ internal static (IList<Descriptor>, bool) ApplyReferrerChanges(IList<Descriptor>
var updateRequired = false;
foreach (var oldReferrer in oldReferrers)
{
if (Descriptor.IsEmptyOrInvalid(oldReferrer))
if (Descriptor.IsNullOrInvalid(oldReferrer))
{
// Skip any empty or null referrers
updateRequired = true;
Expand All @@ -79,7 +79,7 @@ internal static (IList<Descriptor>, bool) ApplyReferrerChanges(IList<Descriptor>
}
// Update the updatedReferrers list
// Add referrer index in the updatedReferrersSet
if (referrerChange.ReferrerOperation == ReferrerOperation.ReferrerDelete && Descriptor.Equals(basicDesc, referrerChange.Referrer.BasicDescriptor))
if (referrerChange.ReferrerOperation == ReferrerOperation.Delete && Descriptor.Equals(basicDesc, referrerChange.Referrer.BasicDescriptor))
{
updateRequired = true;
continue;
Expand All @@ -88,9 +88,8 @@ internal static (IList<Descriptor>, bool) ApplyReferrerChanges(IList<Descriptor>
updatedReferrersSet.Add(basicDesc);
}


var basicReferrerDesc = referrerChange.Referrer.BasicDescriptor;
if (referrerChange.ReferrerOperation == ReferrerOperation.ReferrerAdd)
if (referrerChange.ReferrerOperation == ReferrerOperation.Add)
{
if (!updatedReferrersSet.Contains(basicReferrerDesc))
{
Expand All @@ -99,7 +98,6 @@ internal static (IList<Descriptor>, bool) ApplyReferrerChanges(IList<Descriptor>
updatedReferrersSet.Add(basicReferrerDesc);
}
}


// Skip unnecessary update
if (!updateRequired && updatedReferrersSet.Count == oldReferrers.Count)
Expand Down
4 changes: 2 additions & 2 deletions src/OrasProject.Oras/Registry/Remote/RepositoryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ public struct RepositoryOptions
/// </summary>
public int TagListPageSize { get; set; }

// SkipReferrersGC specifies whether to delete the dangling referrers
// SkipReferrersGc specifies whether to delete the dangling referrers
// index when referrers tag schema is utilized.
// - If false, the old referrers index will be deleted after the new one is successfully uploaded.
// - If true, the old referrers index is kept.
// By default, it is disabled (set to false). See also:
// - https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#referrers-tag-schema
// - https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#pushing-manifests-with-subject
// - https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#deleting-manifests
public bool SkipReferrersGC { get; set; }
public bool SkipReferrersGc { get; set; }
}
16 changes: 8 additions & 8 deletions tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void ApplyReferrerChanges_ShouldAddNewReferrers()
};
var referrerChange = new Referrers.ReferrerChange(
newDescriptor,
Referrers.ReferrerOperation.ReferrerAdd
Referrers.ReferrerOperation.Add
);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Expand Down Expand Up @@ -92,7 +92,7 @@ public void ApplyReferrerChanges_ShouldDeleteReferrers()
};
var referrerChange = new Referrers.ReferrerChange(
oldDescriptor2,
Referrers.ReferrerOperation.ReferrerDelete
Referrers.ReferrerOperation.Delete
);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Expand Down Expand Up @@ -129,7 +129,7 @@ public void ApplyReferrerChanges_ShouldDeleteReferrersWithDuplicates()
};
var referrerChange = new Referrers.ReferrerChange(
oldDescriptor3,
Referrers.ReferrerOperation.ReferrerDelete
Referrers.ReferrerOperation.Delete
);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Expand Down Expand Up @@ -161,7 +161,7 @@ public void ApplyReferrerChanges_ShouldNotDeleteReferrersWhenNoUpdateRequired()
};
var referrerChange = new Referrers.ReferrerChange(
oldDescriptor3,
Referrers.ReferrerOperation.ReferrerDelete
Referrers.ReferrerOperation.Delete
);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Expand Down Expand Up @@ -196,7 +196,7 @@ public void ApplyReferrerChanges_ShouldDiscardDuplicateReferrers()
};
var referrerChange = new Referrers.ReferrerChange(
newDescriptor1,
Referrers.ReferrerOperation.ReferrerAdd
Referrers.ReferrerOperation.Add
);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Expand All @@ -220,7 +220,7 @@ public void ApplyReferrerChanges_ShouldNotAddNewDuplicateReferrers()
};
var referrerChange = new Referrers.ReferrerChange(
oldDescriptor1,
Referrers.ReferrerOperation.ReferrerAdd
Referrers.ReferrerOperation.Add
);
var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Assert.Equal(2, updatedReferrers.Count);
Expand All @@ -245,7 +245,7 @@ public void ApplyReferrerChanges_ShouldNotKeepOldEmptyReferrers()
};
var referrerChange = new Referrers.ReferrerChange(
newDescriptor,
Referrers.ReferrerOperation.ReferrerAdd
Referrers.ReferrerOperation.Add
);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);

Check warning on line 251 in tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs

View workflow job for this annotation

GitHub Actions / Analyze (8.0.x)

Argument of type 'List<Descriptor?>' cannot be used for parameter 'oldReferrers' of type 'IList<Descriptor>' in '(IList<Descriptor>, bool) Referrers.ApplyReferrerChanges(IList<Descriptor> oldReferrers, ReferrerChange referrerChange)' due to differences in the nullability of reference types.

Check warning on line 251 in tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs

View workflow job for this annotation

GitHub Actions / build (8.0.x)

Argument of type 'List<Descriptor?>' cannot be used for parameter 'oldReferrers' of type 'IList<Descriptor>' in '(IList<Descriptor>, bool) Referrers.ApplyReferrerChanges(IList<Descriptor> oldReferrers, ReferrerChange referrerChange)' due to differences in the nullability of reference types.
Expand All @@ -261,7 +261,7 @@ public void ApplyReferrerChanges_ShouldNotKeepOldEmptyReferrers()
public void ApplyReferrerChanges_NoUpdateWhenOldAndNewReferrersAreEmpty()
{
var oldReferrers = new List<Descriptor>();
var referrerChange = new Referrers.ReferrerChange(Descriptor.ZeroDescriptor(), Referrers.ReferrerOperation.ReferrerAdd);
var referrerChange = new Referrers.ReferrerChange(Descriptor.ZeroDescriptor(), Referrers.ReferrerOperation.Add);

var (updatedReferrers, updateRequired) = Referrers.ApplyReferrerChanges(oldReferrers, referrerChange);
Assert.Empty(updatedReferrers);
Expand Down

0 comments on commit b20a202

Please sign in to comment.