resolve comments

Signed-off-by: Patrick Pan <[email protected]>

src/OrasProject.Oras/Content/Digest.cs

@@ -46,19 +46,6 @@ internal static string Validate(string? digest)
 
         return digest;
     }
-
-    internal static string GetAlgorithm(string digest)
-    {
-        var validatedDigest = Validate(digest);
-        return validatedDigest.Split(':')[0];
-    }
-
-    internal static string GetRef(string digest)
-    {
-        var validatedDigest = Validate(digest);
-        return validatedDigest.Split(':')[1];
-    }
-
     /// <summary>
     /// Generates a SHA-256 digest from a byte array.
     /// </summary>

src/OrasProject.Oras/Oci/Index.cs

@@ -51,14 +51,7 @@ internal static (Descriptor, byte[]) GenerateIndex(IList<Descriptor> manifests)
             MediaType = Oci.MediaType.ImageIndex,
             SchemaVersion = 2
         };
-        var indexContent = Encoding.UTF8.GetBytes(JsonSerializer.Serialize(index));
-        var indexDesc = new Descriptor()
-        {
-            Digest = Digest.ComputeSHA256(indexContent),
-            MediaType = Oci.MediaType.ImageIndex,
-            Size = indexContent.Length
-        };
-
-        return (indexDesc, indexContent);
+        var indexContent = JsonSerializer.SerializeToUtf8Bytes(index);
+        return (Descriptor.Create(indexContent, Oci.MediaType.ImageIndex), indexContent);
     }
 }

src/OrasProject.Oras/Registry/Remote/HttpResponseMessageExtensions.cs

@@ -107,12 +107,19 @@ public static void VerifyContentDigest(this HttpResponseMessage response, string
     /// </summary>
     /// <param name="response"></param>
     /// <param name="repository"></param>
-    public static void CheckOciSubjectHeader(this HttpResponseMessage response, Repository repository)
+    public static void CheckOCISubjectHeader(this HttpResponseMessage response, Repository repository)
     {
         if (response.Headers.TryGetValues("OCI-Subject", out var values))
         {
+            // Set it to ReferrerSupported when the response header contains OCI-Subject
             repository.ReferrerState = Referrers.ReferrerState.ReferrerSupported;
         }
+
+        // If the "OCI-Subject" header is NOT set, it means that either the manifest
+        // has no subject OR the referrers API is NOT supported by the registry.
+        //
+        // Since we don't know whether the pushed manifest has a subject or not,
+        // we do not set the ReferrerState to ReferrerNotSupported here.
     }
 
     /// <summary>

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

@@ -180,23 +180,30 @@ private async Task PushWithIndexingAsync(Descriptor expected, Stream content, st
             case MediaType.ImageIndex:
                 if (Repository.ReferrerState == Referrers.ReferrerState.ReferrerSupported)
                 { 
+                    // Push the manifest straightaway when the registry supports referrers API
                     await InternalPushAsync(expected, content, reference, cancellationToken).ConfigureAwait(false);
                     return;
                 }
-
+                
                 var contentBytes = await content.ReadAllAsync(expected, cancellationToken);
                 using (var contentDuplicate = new MemoryStream(contentBytes))
                 {
+                    // Push the manifest when ReferrerState is Unknown or NotSupported
                     await InternalPushAsync(expected, contentDuplicate, reference, cancellationToken).ConfigureAwait(false);
                 }
                 if (Repository.ReferrerState == Referrers.ReferrerState.ReferrerSupported)
                 {
+                    // Early exit when the registry supports Referrers API
+                    // No need to index referrers list
                     return;
                 }
 
                 using (var contentDuplicate = new MemoryStream(contentBytes))
                 {
-                    await ProcessReferrersAndPushIndex(expected, contentDuplicate);
+                    // 1. Index the referrers list using referrers tag schema when manifest contains a subject field
+                    //    And the ReferrerState is not supported
+                    // 2. Or do nothing when the manifest does not contain a subject field when ReferrerState is not supported/unknown
+                    await ProcessReferrersAndPushIndex(expected, contentDuplicate, cancellationToken);
                 }
                 break;
             default:
@@ -239,13 +246,16 @@ private async Task ProcessReferrersAndPushIndex(Descriptor desc, Stream content,
         }
 
         Repository.ReferrerState = Referrers.ReferrerState.ReferrerNotSupported;
-        await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(desc, Referrers.ReferrerOperation.ReferrerAdd));
+        await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(desc, Referrers.ReferrerOperation.ReferrerAdd), cancellationToken);
     }
 
     /// <summary>
     /// UpdateReferrersIndex updates the referrers index for a given subject by applying the specified referrer changes.
     /// If the referrers index is updated, the new index is pushed to the repository. If referrers 
     /// garbage collection is not skipped, the old index is deleted.
+    /// References:
+    ///  - 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
     /// </summary>
     /// <param name="subject"></param>
     /// <param name="referrerChange"></param>
@@ -256,25 +266,36 @@ private async Task UpdateReferrersIndex(Descriptor subject,
     {
         try
         {
+            // 1. pull the original referrers index list using referrers tag schema
             var referrersTag = Referrers.BuildReferrersTag(subject);
-            var (oldDesc, oldReferrers) = await PullReferrersIndexList(referrersTag);
+            var (oldDesc, oldReferrers) = await PullReferrersIndexList(referrersTag, cancellationToken);
+
+            // 2. apply the referrer change to referrers list
             var updatedReferrers =
                 Referrers.ApplyReferrerChanges(oldReferrers,  referrerChange);
 
+            // 3. push the updated referrers list using referrers tag schema
             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
+                // 2. OR the updated referrers list is empty but referrers GC
+                //    is skipped, in this case an empty index should still be pushed
+                //    as the old index won't get deleted
                 var (indexDesc, indexContent) = Index.GenerateIndex(updatedReferrers);
                 using (var content = new MemoryStream(indexContent))
                 {
                     await InternalPushAsync(indexDesc, content, referrersTag, cancellationToken).ConfigureAwait(false);
                 }
             }
-
+            
             if (repository.Options.SkipReferrersGc || Descriptor.IsEmptyOrNull(oldDesc))
             {
+                // Skip the delete process if SkipReferrersGc is set to true or the old Descriptor is empty or null
                 return;
             }
-
+
+            // 4. delete the dangling original referrers index, if applicable
             await DeleteAsync(oldDesc, cancellationToken).ConfigureAwait(false);
         }
         catch (NoReferrerUpdateException)
@@ -296,7 +317,7 @@ private async Task UpdateReferrersIndex(Descriptor subject,
     {
         try
         {
-            var (desc, content) = await FetchAsync(referrersTag);
+            var (desc, content) = await FetchAsync(referrersTag, cancellationToken);
             var index = JsonSerializer.Deserialize<Index>(content);
             if (index == null)
             {
@@ -318,8 +339,7 @@ private async Task UpdateReferrersIndex(Descriptor subject,
     /// <param name="stream"></param>
     /// <param name="contentReference"></param>
     /// <param name="cancellationToken"></param>
-    private async Task InternalPushAsync(Descriptor expected, Stream stream, string contentReference,
-        CancellationToken cancellationToken)
+    private async Task InternalPushAsync(Descriptor expected, Stream stream, string contentReference, CancellationToken cancellationToken)
     {
         var remoteReference = Repository.ParseReference(contentReference);
         var url = new UriFactory(remoteReference, Repository.Options.PlainHttp).BuildRepositoryManifest();
@@ -333,7 +353,7 @@ private async Task InternalPushAsync(Descriptor expected, Stream stream, string
         {
             throw await response.ParseErrorResponseAsync(cancellationToken).ConfigureAwait(false);
         }
-        response.CheckOciSubjectHeader(Repository);
+        response.CheckOCISubjectHeader(Repository);
         response.VerifyContentDigest(expected.Digest);
     }
 

src/OrasProject.Oras/Registry/Remote/Referrers.cs

@@ -37,7 +37,8 @@ internal enum ReferrerOperation
 
     internal static string BuildReferrersTag(Descriptor descriptor)
     {
-        return Digest.GetAlgorithm(descriptor.Digest) + "-" + Digest.GetRef(descriptor.Digest);
+        var validatedDigest = Digest.Validate(descriptor.Digest);
+        return validatedDigest.Substring(0, validatedDigest.IndexOf(':')) + "-" + validatedDigest.Substring(validatedDigest.IndexOf(':') + 1);
     }
 
     /// <summary>

tests/OrasProject.Oras.Tests/Remote/ReferrersTest.cs

@@ -22,6 +22,23 @@ namespace OrasProject.Oras.Tests.Remote;
 
 public class ReferrersTest
 {
+    [Fact]
+    public void BuildReferrersTag_ShouldReturnReferrersTagSuccessfully()
+    {
+        var desc = RandomDescriptor();
+        var index = desc.Digest.IndexOf(':');
+        var expected = desc.Digest.Substring(0, index) + "-" + desc.Digest.Substring(index + 1);
+        Assert.Equal(expected, Referrers.BuildReferrersTag(desc));
+    }
+
+    [Fact]
+    public void BuildReferrersTag_ShouldThrowInvalidDigestException()
+    {
+        var desc = RandomDescriptor();
+        desc.Digest = "sha123321";
+        Assert.Throws<InvalidDigestException>(() => Referrers.BuildReferrersTag(desc));
+    }
+
     [Fact]
     public void ApplyReferrerChanges_ShouldAddNewReferrers()
     {