From d4987eb0cb9da0610dfd89d0ca2dbdf3c19dac53 Mon Sep 17 00:00:00 2001 From: Ritek Rounak Date: Wed, 4 Dec 2024 03:16:17 +0000 Subject: [PATCH] fix: [AH-489] Fix Docker & Helm DownloadCount | Gitness (#3032) * fix: [AH-489] Update Tag DB Queries * fix: [AH-489] Fix Docker Download Count in Get Manifests * add Download Count in Docker manifest Details * fix: [AH-489] Fix Docker DownloadCount in Details Page * fix: [AH-489] Fix Helm DownloadCount DB Query --- .../controller/metadata/artifact_mapper.go | 5 + .../get_artifacts_docker_manifests.go | 28 +++--- registry/app/api/openapi/api.yaml | 3 + .../openapi/contracts/artifact/types.gen.go | 9 +- registry/app/store/database/tag.go | 98 ++++++++++++++----- registry/types/tag.go | 14 +-- 6 files changed, 110 insertions(+), 47 deletions(-) diff --git a/registry/app/api/controller/metadata/artifact_mapper.go b/registry/app/api/controller/metadata/artifact_mapper.go index 18d110103..88fd862a2 100644 --- a/registry/app/api/controller/metadata/artifact_mapper.go +++ b/registry/app/api/controller/metadata/artifact_mapper.go @@ -114,6 +114,7 @@ func GetTagMetadata( isLatestVersion := latestTag == tag.Name command := GetPullCommand(image, tag.Name, string(tag.PackageType), registryURL) packageType, err := toPackageType(string(tag.PackageType)) + downloadCount := tag.DownloadCount if err != nil { log.Ctx(ctx).Error().Err(err).Msgf("Error converting package type %s", tag.PackageType) continue @@ -126,6 +127,7 @@ func GetTagMetadata( DigestCount: &digestCount, IslatestVersion: &isLatestVersion, PullCommand: &command, + DownloadsCount: &downloadCount, } artifactVersionMetadataList = append(artifactVersionMetadataList, *artifactVersionMetadata) } @@ -261,6 +263,7 @@ func GetDockerArtifactDetails( PullCommand: &pullCommand, Url: GetTagURL(tag.ImageName, tag.Name, registryURL), Size: &size, + DownloadsCount: &tag.DownloadCount, } response := &artifactapi.DockerArtifactDetailResponseJSONResponse{ @@ -282,6 +285,7 @@ func GetHelmArtifactDetails( createdAt := GetTimeInMs(tag.CreatedAt) modifiedAt := GetTimeInMs(tag.UpdatedAt) size := GetSize(manifest.TotalSize) + downloadCount := tag.DownloadCount artifactDetail := &artifactapi.HelmArtifactDetail{ Artifact: &tag.ImageName, Version: tag.Name, @@ -293,6 +297,7 @@ func GetHelmArtifactDetails( PullCommand: &pullCommand, Url: GetTagURL(tag.ImageName, tag.Name, registryURL), Size: &size, + DownloadsCount: &downloadCount, } response := &artifactapi.HelmArtifactDetailResponseJSONResponse{ diff --git a/registry/app/api/controller/metadata/get_artifacts_docker_manifests.go b/registry/app/api/controller/metadata/get_artifacts_docker_manifests.go index 572497457..395082381 100644 --- a/registry/app/api/controller/metadata/get_artifacts_docker_manifests.go +++ b/registry/app/api/controller/metadata/get_artifacts_docker_manifests.go @@ -74,7 +74,11 @@ func (c *APIController) GetDockerArtifactManifests( image := string(r.Artifact) version := string(r.Version) - manifestDetailsList, err := c.ProcessManifest(ctx, regInfo, image, version) + artifactMetadata, err := c.TagStore.GetLatestTagMetadata(ctx, regInfo.parentID, regInfo.RegistryIdentifier, image) + if err != nil { + return artifactManifestsErrorRs(err), nil + } + manifestDetailsList, err := c.ProcessManifest(ctx, regInfo, image, version, artifactMetadata.DownloadCount) if err != nil { return artifactManifestsErrorRs(err), nil } @@ -93,7 +97,7 @@ func (c *APIController) GetDockerArtifactManifests( func (c *APIController) getManifestList( ctx context.Context, reqManifest *ml.DeserializedManifestList, registry *types.Registry, image string, - regInfo *RegistryRequestBaseInfo, + regInfo *RegistryRequestBaseInfo, downloadCount int64, ) ([]artifact.DockerManifestDetails, error) { manifestDetailsList := []artifact.DockerManifestDetails{} for _, manifestEntry := range reqManifest.Manifests { @@ -118,7 +122,7 @@ func (c *APIController) getManifestList( if err != nil { return nil, err } - manifestDetailsList = append(manifestDetailsList, getManifestDetails(referencedManifest, mConfig)) + manifestDetailsList = append(manifestDetailsList, getManifestDetails(referencedManifest, mConfig, downloadCount)) } return manifestDetailsList, nil } @@ -131,14 +135,16 @@ func artifactManifestsErrorRs(err error) artifact.GetDockerArtifactManifestsResp } } -func getManifestDetails(m *types.Manifest, mConfig *manifestConfig) artifact.DockerManifestDetails { +func getManifestDetails( + m *types.Manifest, mConfig *manifestConfig, downloadsCount int64) artifact.DockerManifestDetails { createdAt := GetTimeInMs(m.CreatedAt) size := GetSize(m.TotalSize) manifestDetails := artifact.DockerManifestDetails{ - Digest: m.Digest.String(), - CreatedAt: &createdAt, - Size: &size, + Digest: m.Digest.String(), + CreatedAt: &createdAt, + Size: &size, + DownloadsCount: &downloadsCount, } if mConfig != nil { manifestDetails.OsArch = fmt.Sprintf("%s/%s", mConfig.Os, mConfig.Arch) @@ -153,7 +159,7 @@ func getManifestDetails(m *types.Manifest, mConfig *manifestConfig) artifact.Doc func (c *APIController) ProcessManifest( ctx context.Context, regInfo *RegistryRequestBaseInfo, - image, version string, + image, version string, downloadCount int64, ) ([]artifact.DockerManifestDetails, error) { registry, err := c.RegistryRepository.GetByParentIDAndName(ctx, regInfo.parentID, regInfo.RegistryIdentifier) if err != nil { @@ -178,15 +184,15 @@ func (c *APIController) ProcessManifest( if err != nil { return nil, err } - manifestDetailsList = append(manifestDetailsList, getManifestDetails(m, mConfig)) + manifestDetailsList = append(manifestDetailsList, getManifestDetails(m, mConfig, downloadCount)) case *ocischema.DeserializedManifest: mConfig, err := getManifestConfig(ctx, reqManifest.Config().Digest, regInfo.RootIdentifier, c.StorageDriver) if err != nil { return nil, err } - manifestDetailsList = append(manifestDetailsList, getManifestDetails(m, mConfig)) + manifestDetailsList = append(manifestDetailsList, getManifestDetails(m, mConfig, downloadCount)) case *ml.DeserializedManifestList: - manifestDetailsList, err = c.getManifestList(ctx, reqManifest, registry, image, regInfo) + manifestDetailsList, err = c.getManifestList(ctx, reqManifest, registry, image, regInfo, downloadCount) if err != nil { return nil, err } diff --git a/registry/app/api/openapi/api.yaml b/registry/app/api/openapi/api.yaml index 23f7f5434..7b8658f5d 100644 --- a/registry/app/api/openapi/api.yaml +++ b/registry/app/api/openapi/api.yaml @@ -1418,6 +1418,9 @@ components: type: string createdAt: type: string + downloadsCount: + type: integer + format: int64 required: - digest - layers diff --git a/registry/app/api/openapi/contracts/artifact/types.gen.go b/registry/app/api/openapi/contracts/artifact/types.gen.go index ee94a721e..5b335f734 100644 --- a/registry/app/api/openapi/contracts/artifact/types.gen.go +++ b/registry/app/api/openapi/contracts/artifact/types.gen.go @@ -210,10 +210,11 @@ type DockerLayersSummary struct { // DockerManifestDetails Harness Artifact Layers type DockerManifestDetails struct { - CreatedAt *string `json:"createdAt,omitempty"` - Digest string `json:"digest"` - OsArch string `json:"osArch"` - Size *string `json:"size,omitempty"` + CreatedAt *string `json:"createdAt,omitempty"` + Digest string `json:"digest"` + DownloadsCount *int64 `json:"downloadsCount,omitempty"` + OsArch string `json:"osArch"` + Size *string `json:"size,omitempty"` } // DockerManifests Harness Manifests diff --git a/registry/app/store/database/tag.go b/registry/app/store/database/tag.go index c1442180b..87a578f42 100644 --- a/registry/app/store/database/tag.go +++ b/registry/app/store/database/tag.go @@ -91,15 +91,17 @@ type tagMetadataDB struct { NonConformant bool `db:"manifest_non_conformant"` Payload []byte `db:"manifest_payload"` MediaType string `db:"mt_media_type"` + DownloadCount int64 `db:"download_count"` } type tagDetailDB struct { - ID int64 `db:"id"` - Name string `db:"name"` - ImageName string `db:"image_name"` - CreatedAt int64 `db:"created_at"` - UpdatedAt int64 `db:"updated_at"` - Size string `db:"size"` + ID int64 `db:"id"` + Name string `db:"name"` + ImageName string `db:"image_name"` + CreatedAt int64 `db:"created_at"` + UpdatedAt int64 `db:"updated_at"` + Size string `db:"size"` + DownloadCount int64 `db:"download_count"` } func (t tagDao) CreateOrUpdate(ctx context.Context, tag *types.Tag) error { @@ -466,15 +468,35 @@ func (t tagDao) GetTagDetail( ctx context.Context, repoID int64, imageName string, name string, ) (*types.TagDetail, error) { - q := databaseg.Builder.Select( - `tag_id as id, tag_name as name, - tag_image_name as image_name, tag_created_at as created_at, - tag_updated_at as updated_at, manifest_total_size as size`, - ). - From("tags"). - Join("manifests ON manifest_id = tag_manifest_id"). + // Define subquery for download counts + downloadCountSubquery := ` + SELECT + a.artifact_image_id, + COUNT(d.download_stat_id) AS download_count, + i.image_name, + i.image_registry_id + FROM artifacts a + JOIN download_stats d ON d.download_stat_artifact_id = a.artifact_id + JOIN images i ON i.image_id = a.artifact_image_id + GROUP BY a.artifact_image_id, i.image_name, i.image_registry_id + ` + // Build main query + q := databaseg.Builder. + Select(` + t.tag_id AS id, + t.tag_name AS name, + t.tag_image_name AS image_name, + t.tag_created_at AS created_at, + t.tag_updated_at AS updated_at, + m.manifest_total_size AS size, + COALESCE(dc.download_count, 0) AS download_count + `). + From("tags AS t"). + Join("manifests AS m ON m.manifest_id = t.tag_manifest_id"). + LeftJoin(fmt.Sprintf("(%s) AS dc ON t.tag_image_name = dc.image_name "+ + "AND t.tag_registry_id = dc.image_registry_id", downloadCountSubquery)). Where( - "tag_registry_id = ? AND tag_image_name = ? AND tag_name = ?", + "t.tag_registry_id = ? AND t.tag_image_name = ? AND t.tag_name = ?", repoID, imageName, name, ) @@ -765,16 +787,38 @@ func (t tagDao) GetAllTagsByRepoAndImage( image string, sortByField string, sortByOrder string, limit int, offset int, search string, ) (*[]types.TagMetadata, error) { - q := databaseg.Builder.Select( - `t.tag_name as name, m.manifest_total_size as size, - r.registry_package_type as package_type, t.tag_updated_at as modified_at, - m.manifest_schema_version, m.manifest_non_conformant, m.manifest_payload, - mt.mt_media_type `, - ). + // Define download count subquery + downloadCountSubquery := ` + SELECT + a.artifact_image_id, + COUNT(d.download_stat_id) AS download_count, + i.image_name, + i.image_registry_id + FROM artifacts a + JOIN download_stats d ON d.download_stat_artifact_id = a.artifact_id + JOIN images i ON i.image_id = a.artifact_image_id + GROUP BY a.artifact_image_id, i.image_name, i.image_registry_id + ` + + // Build the main query + q := databaseg.Builder. + Select(` + t.tag_name AS name, + m.manifest_total_size AS size, + r.registry_package_type AS package_type, + t.tag_updated_at AS modified_at, + m.manifest_schema_version, + m.manifest_non_conformant, + m.manifest_payload, + mt.mt_media_type, + COALESCE(dc.download_count, 0) AS download_count + `). From("tags t"). Join("registries r ON t.tag_registry_id = r.registry_id"). Join("manifests m ON t.tag_manifest_id = m.manifest_id"). Join("media_types mt ON mt.mt_id = m.manifest_media_type_id"). + LeftJoin(fmt.Sprintf("(%s) AS dc ON t.tag_image_name = dc.image_name "+ + "AND t.tag_registry_id = dc.image_registry_id", downloadCountSubquery)). Where( "r.registry_parent_id = ? AND r.registry_name = ? AND t.tag_image_name = ?", parentID, repoKey, image, @@ -977,6 +1021,7 @@ func (t tagDao) mapToTagMetadata( NonConformant: dst.NonConformant, MediaType: dst.MediaType, Payload: dst.Payload, + DownloadCount: dst.DownloadCount, }, nil } @@ -985,11 +1030,12 @@ func (t tagDao) mapToTagDetail( dst *tagDetailDB, ) (*types.TagDetail, error) { return &types.TagDetail{ - ID: dst.ID, - Name: dst.Name, - ImageName: dst.ImageName, - Size: dst.Size, - CreatedAt: time.UnixMilli(dst.CreatedAt), - UpdatedAt: time.UnixMilli(dst.UpdatedAt), + ID: dst.ID, + Name: dst.Name, + ImageName: dst.ImageName, + Size: dst.Size, + CreatedAt: time.UnixMilli(dst.CreatedAt), + UpdatedAt: time.UnixMilli(dst.UpdatedAt), + DownloadCount: dst.DownloadCount, }, nil } diff --git a/registry/types/tag.go b/registry/types/tag.go index bd62ea284..3a106ef1d 100644 --- a/registry/types/tag.go +++ b/registry/types/tag.go @@ -56,13 +56,15 @@ type TagMetadata struct { NonConformant bool Payload Payload MediaType string + DownloadCount int64 } type TagDetail struct { - ID int64 - Name string - ImageName string - CreatedAt time.Time - UpdatedAt time.Time - Size string + ID int64 + Name string + ImageName string + CreatedAt time.Time + UpdatedAt time.Time + Size string + DownloadCount int64 }