fix(convert): fix the update rule of download count for images (#1802)

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

pkg/extensions/search/convert/convert_test.go

@@ -980,3 +980,95 @@ func TestGetOneManifestAnnotations(t *testing.T) {
 		So(*imageSummary.Source, ShouldBeBlank)
 	})
 }
+
+func TestDownloadCount(t *testing.T) {
+	Convey("manifest", t, func() {
+		repoMeta, manifestMetaMap, indexDataMap := test.GetMetadataForRepos(
+			test.Repo{
+				Name: "repo",
+				Images: []test.RepoImage{
+					{
+						Image: test.CreateRandomImage(),
+						Tag:   "10-downloads",
+						Statistics: mTypes.DescriptorStatistics{
+							DownloadCount: 10,
+						},
+					},
+				},
+			},
+		)
+
+		repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap,
+			convert.SkipQGLField{}, nil)
+		So(*repoSummary.DownloadCount, ShouldEqual, 10)
+		So(*repoSummary.NewestImage.DownloadCount, ShouldEqual, 10)
+	})
+
+	Convey("index", t, func() {
+		img1, img2, img3 := test.CreateRandomImage(), test.CreateRandomImage(), test.CreateRandomImage()
+		multiArch := test.CreateMultiarchWith().Images([]test.Image{img1, img2, img3}).Build()
+
+		repoMeta, manifestMetaMap, indexDataMap := test.GetMetadataForRepos(
+			test.Repo{
+				Name: "repo",
+				MultiArchImages: []test.RepoMultiArchImage{
+					{
+						MultiarchImage: multiArch,
+						Tag:            "160-multiarch",
+						ImageStatistics: map[string]mTypes.DescriptorStatistics{
+							img1.DigestStr():      {DownloadCount: 10},
+							img2.DigestStr():      {DownloadCount: 20},
+							img3.DigestStr():      {DownloadCount: 30},
+							multiArch.DigestStr(): {DownloadCount: 100},
+						},
+					},
+				},
+			},
+		)
+
+		repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap,
+			convert.SkipQGLField{}, nil)
+		So(*repoSummary.DownloadCount, ShouldEqual, 100)
+		So(*repoSummary.NewestImage.DownloadCount, ShouldEqual, 100)
+	})
+
+	Convey("index + manifest mixed", t, func() {
+		img1 := test.CreateRandomImage()
+		img2 := test.CreateRandomImage()
+		img3 := test.CreateImageWith().DefaultLayers().ImageConfig(
+			ispec.Image{Created: test.DateRef(2020, 1, 1, 1, 1, 1, 0, time.UTC)},
+		).Build()
+
+		multiArch := test.CreateMultiarchWith().Images([]test.Image{img1, img2, img3}).Build()
+
+		repoMeta, manifestMetaMap, indexDataMap := test.GetMetadataForRepos(
+			test.Repo{
+				Name: "repo",
+				Images: []test.RepoImage{
+					{
+						Image:      test.CreateRandomImage(),
+						Tag:        "5-downloads",
+						Statistics: mTypes.DescriptorStatistics{DownloadCount: 5},
+					},
+				},
+				MultiArchImages: []test.RepoMultiArchImage{
+					{
+						MultiarchImage: multiArch,
+						Tag:            "160-multiarch",
+						ImageStatistics: map[string]mTypes.DescriptorStatistics{
+							img1.DigestStr():      {DownloadCount: 10},
+							img2.DigestStr():      {DownloadCount: 20},
+							img3.DigestStr():      {DownloadCount: 30},
+							multiArch.DigestStr(): {DownloadCount: 100},
+						},
+					},
+				},
+			},
+		)
+
+		repoSummary := convert.RepoMeta2RepoSummary(context.Background(), repoMeta[0], manifestMetaMap, indexDataMap,
+			convert.SkipQGLField{}, nil)
+		So(*repoSummary.DownloadCount, ShouldEqual, 105)
+		So(*repoSummary.NewestImage.DownloadCount, ShouldEqual, 100)
+	})
+}

pkg/extensions/search/convert/metadb.go

@@ -70,17 +70,15 @@ func RepoMeta2RepoSummary(ctx context.Context, repoMeta mTypes.RepoMetadata,
 				platformString := strings.TrimSpace(fmt.Sprintf("%s %s", opSys, arch))
 				repoPlatformsSet[platformString] = &gql_generated.Platform{Os: &opSys, Arch: &arch}
 			}
-
-			repoDownloadCount += manifestMetaMap[*manifestSummary.Digest].DownloadCount
 		}
 
+		repoDownloadCount += *imageSummary.DownloadCount
+
 		if *imageSummary.Vendor != "" {
 			repoVendorsSet[*imageSummary.Vendor] = true
 		}
 
 		lastUpdatedImageSummary = UpdateLastUpdatedTimestamp(&repoLastUpdatedTimestamp, lastUpdatedImageSummary, imageSummary)
-
-		repoDownloadCount += repoMeta.Statistics[descriptor.Digest].DownloadCount
 	}
 
 	// calculate repo size = sum all manifest, config and layer blobs sizes
@@ -248,6 +246,8 @@ func ImageIndex2ImageSummary(ctx context.Context, repo, tag string, indexDigest
 		manifestSummaries = append(manifestSummaries, manifestSummary)
 	}
 
+	totalDownloadCount += repoMeta.Statistics[indexDigestStr].DownloadCount
+
 	for _, signatures := range repoMeta.Signatures[indexDigest.String()] {
 		if len(signatures) > 0 {
 			isSigned = true
@@ -511,7 +511,7 @@ func ImageManifest2ManifestSummary(ctx context.Context, repo, tag string, descri
 		configSize        = manifestContent.Config.Size
 		artifactType      = zcommon.GetManifestArtifactType(manifestContent)
 		imageLastUpdated  = zcommon.GetImageLastUpdated(configContent)
-		downloadCount     = manifestMeta.DownloadCount
+		downloadCount     = repoMeta.Statistics[digest.String()].DownloadCount
 		isSigned          = false
 	)
 

pkg/test/repo.go

@@ -8,12 +8,14 @@ import (
 
 type RepoImage struct {
 	Image
-	Tag string
+	Tag        string
+	Statistics mTypes.DescriptorStatistics
 }
 
 type RepoMultiArchImage struct {
 	MultiarchImage
-	Tag string
+	ImageStatistics map[string]mTypes.DescriptorStatistics
+	Tag             string
 }
 
 type Repo struct {
@@ -38,45 +40,32 @@ func GetMetadataForRepos(repos ...Repo) ([]mTypes.RepoMetadata, map[string]mType
 			Name:         repo.Name,
 			Tags:         map[string]mTypes.Descriptor{},
 			Signatures:   map[string]mTypes.ManifestSignatures{},
+			Statistics:   map[string]mTypes.DescriptorStatistics{},
 			IsStarred:    repo.IsStarred,
 			IsBookmarked: repo.IsBookmarked,
 		}
 
 		for _, image := range repo.Images {
-			if image.Tag != "" {
+			addImageMetaToMetaDB(image, repoMeta, manifestMetadataMap)
-				repoMeta.Tags[image.Tag] = mTypes.Descriptor{
-					MediaType: ispec.MediaTypeImageManifest,
-					Digest:    image.DigestStr(),
-				}
-			}
-			// here we can do many more checks about the images like check for referrers, signatures but it's not needed yet
-			// I need just the tags for now and the fake signature.
-
-			// This is done just to mark a manifest as signed in the resulted RepoMeta
-			if image.Manifest.ArtifactType == TestFakeSignatureArtType && image.Manifest.Subject != nil {
-				signedManifestDig := image.Manifest.Subject.Digest.String()
-				repoMeta.Signatures[signedManifestDig] = mTypes.ManifestSignatures{
-					"fakeSignature": []mTypes.SignatureInfo{{SignatureManifestDigest: image.ManifestDescriptor.Digest.String()}},
-				}
-			}
-
-			manifestMetadataMap[image.ManifestDescriptor.Digest.String()] = mTypes.ManifestMetadata{
-				ManifestBlob: image.ManifestDescriptor.Data,
-				ConfigBlob:   image.ConfigDescriptor.Data,
-			}
 		}
 
 		for _, multiArch := range repo.MultiArchImages {
+			if multiArch.ImageStatistics == nil {
+				multiArch.ImageStatistics = map[string]mTypes.DescriptorStatistics{}
+			}
+
 			repoMeta.Tags[multiArch.Tag] = mTypes.Descriptor{
 				MediaType: ispec.MediaTypeImageIndex,
 				Digest:    multiArch.DigestStr(),
 			}
 
+			repoMeta.Statistics[multiArch.DigestStr()] = multiArch.ImageStatistics[multiArch.DigestStr()]
+
 			for _, image := range multiArch.Images {
-				manifestMetadataMap[image.ManifestDescriptor.Digest.String()] = mTypes.ManifestMetadata{
+				addImageMetaToMetaDB(RepoImage{
-					ManifestBlob: image.ManifestDescriptor.Data,
+					Image:      image,
-					ConfigBlob:   image.ConfigDescriptor.Data,
+					Statistics: multiArch.ImageStatistics[image.DigestStr()],
-				}
+				}, repoMeta, manifestMetadataMap)
 			}
 
 			indexDataMap[multiArch.indexDescriptor.Digest.String()] = mTypes.IndexData{
@@ -89,3 +78,32 @@ func GetMetadataForRepos(repos ...Repo) ([]mTypes.RepoMetadata, map[string]mType
 
 	return reposMetadata, manifestMetadataMap, indexDataMap
 }
+
+func addImageMetaToMetaDB(image RepoImage, repoMeta mTypes.RepoMetadata,
+	manifestMetadataMap map[string]mTypes.ManifestMetadata,
+) {
+	if image.Tag != "" {
+		repoMeta.Tags[image.Tag] = mTypes.Descriptor{
+			MediaType: ispec.MediaTypeImageManifest,
+			Digest:    image.DigestStr(),
+		}
+	}
+	// here we can do many more checks about the images like check for referrers, signatures but it's not needed yet
+	// I need just the tags for now and the fake signature.
+
+	// This is done just to mark a manifest as signed in the resulted RepoMeta
+	if image.Manifest.ArtifactType == TestFakeSignatureArtType && image.Manifest.Subject != nil {
+		signedManifestDig := image.Manifest.Subject.Digest.String()
+		repoMeta.Signatures[signedManifestDig] = mTypes.ManifestSignatures{
+			"fakeSignature": []mTypes.SignatureInfo{{SignatureManifestDigest: image.ManifestDescriptor.Digest.String()}},
+		}
+	}
+
+	repoMeta.Statistics[image.DigestStr()] = image.Statistics
+
+	manifestMetadataMap[image.DigestStr()] = mTypes.ManifestMetadata{
+		ManifestBlob:  image.ManifestDescriptor.Data,
+		ConfigBlob:    image.ConfigDescriptor.Data,
+		DownloadCount: image.Statistics.DownloadCount,
+	}
+}