From 11ffe863c6f7110e47558221db8af6d274dcbc66 Mon Sep 17 00:00:00 2001 From: Harsh Sunil Suryawanshi Date: Sat, 28 Mar 2026 20:17:27 +0530 Subject: [PATCH 1/3] [Registry] Cache shared manifest fetches during generation Signed-off-by: Harsh Sunil Suryawanshi --- registry/model.go | 64 +++++++++++++--- registry/model_generation_test.go | 118 ++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 12 deletions(-) diff --git a/registry/model.go b/registry/model.go index cec8a462..1bd11dcf 100644 --- a/registry/model.go +++ b/registry/model.go @@ -31,11 +31,61 @@ import ( "github.com/meshery/schemas/models/v1beta1/subcategory" log "github.com/sirupsen/logrus" "golang.org/x/sync/semaphore" + "golang.org/x/sync/singleflight" "google.golang.org/api/sheets/v4" ) var modelToCompGenerateTracker = store.NewGenericThreadSafeStore[compGenerateTracker]() +type generatorFactory func(registrant, url, packageName string) (models.PackageManager, error) + +type packageFetcher struct { + newGenerator generatorFactory + cache sync.Map + fetchGroup singleflight.Group +} + +func newPackageFetcher(newGenerator generatorFactory) *packageFetcher { + return &packageFetcher{ + newGenerator: newGenerator, + } +} + +func (pf *packageFetcher) getPackage(registrant, sourceURL, modelName string) (models.Package, error) { + cacheKey := fmt.Sprintf("%s\x00%s", utils.ReplaceSpacesAndConvertToLowercase(registrant), sourceURL) + if cachedPkg, ok := pf.cache.Load(cacheKey); ok { + return cachedPkg.(models.Package), nil + } + + fetchedPkg, err, _ := pf.fetchGroup.Do(cacheKey, func() (interface{}, error) { + if cachedPkg, ok := pf.cache.Load(cacheKey); ok { + return cachedPkg, nil + } + + generator, err := pf.newGenerator(registrant, sourceURL, modelName) + if err != nil { + return nil, err + } + + if utils.ReplaceSpacesAndConvertToLowercase(registrant) == artifactHub { + RateLimitArtifactHub() + } + + pkg, err := generator.GetPackage() + if err != nil { + return nil, err + } + + pf.cache.Store(cacheKey, pkg) + return pkg, nil + }) + if err != nil { + return nil, err + } + + return fetchedPkg.(models.Package), nil +} + type compGenerateTracker struct { totalComps int version string @@ -800,6 +850,7 @@ func InvokeGenerationFromSheet(wg *sync.WaitGroup, path string, modelsheetID, co // - Latest version only filtering func InvokeGenerationFromSheetWithOptions(wg *sync.WaitGroup, path string, modelsheetID, componentSheetID int64, spreadsheeetID string, modelName string, modelCSVFilePath, componentCSVFilePath, spreadsheeetCred, relationshipCSVFilePath string, relationshipSheetID int64, srv *sheets.Service, opts GenerationOptions) error { weightedSem := semaphore.NewWeighted(20) + packageFetcher := newPackageFetcher(generators.NewGenerator) url := GoogleSpreadSheetURL + spreadsheeetID totalAvailableModels := 0 spreadsheeetChan := make(chan SpreadsheetData) @@ -924,19 +975,8 @@ func InvokeGenerationFromSheetWithOptions(wg *sync.WaitGroup, path string, model } Log.Debug(fmt.Sprintf("Model %s: Creating generator for registrant: %s, source: %s", model.Model, model.Registrant, model.SourceURL)) - - generator, genErr := generators.NewGenerator(model.Registrant, model.SourceURL, model.Model) - if genErr != nil { - done <- ErrGenerateModel(genErr, model.Model) - return - } - - if utils.ReplaceSpacesAndConvertToLowercase(model.Registrant) == "artifacthub" { - RateLimitArtifactHub() - } - Log.Debug(fmt.Sprintf("Model %s: Fetching package from source", model.Model)) - pkg, genErr := generator.GetPackage() + pkg, genErr := packageFetcher.getPackage(model.Registrant, model.SourceURL, model.Model) if genErr != nil { done <- ErrGenerateModel(genErr, model.Model) return diff --git a/registry/model_generation_test.go b/registry/model_generation_test.go index b9d36cf6..9fdf41d9 100644 --- a/registry/model_generation_test.go +++ b/registry/model_generation_test.go @@ -4,12 +4,51 @@ import ( "context" "fmt" "sync" + "sync/atomic" "testing" "time" + "github.com/meshery/meshkit/generators/models" + "github.com/meshery/schemas/models/v1beta1/component" "github.com/stretchr/testify/assert" ) +type stubPackage struct { + name string + version string + sourceURL string +} + +func (sp stubPackage) GenerateComponents(string) ([]component.ComponentDefinition, error) { + return nil, nil +} + +func (sp stubPackage) GetVersion() string { + return sp.version +} + +func (sp stubPackage) GetSourceURL() string { + return sp.sourceURL +} + +func (sp stubPackage) GetName() string { + return sp.name +} + +type stubPackageManager struct { + pkg models.Package + callCount *atomic.Int32 + delay time.Duration +} + +func (spm stubPackageManager) GetPackage() (models.Package, error) { + spm.callCount.Add(1) + if spm.delay > 0 { + time.Sleep(spm.delay) + } + return spm.pkg, nil +} + func TestGenerationOptionsTimeoutBehavior(t *testing.T) { // Test that timeout value is respected when set tests := []struct { @@ -44,6 +83,85 @@ func TestGenerationOptionsTimeoutBehavior(t *testing.T) { } } +func TestPackageFetcherCachesByRegistrantAndSourceURL(t *testing.T) { + t.Parallel() + + callCount := &atomic.Int32{} + fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { + return stubPackageManager{ + pkg: &stubPackage{ + name: packageName, + version: "v1.0.0", + sourceURL: url, + }, + callCount: callCount, + }, nil + }) + + firstPkg, err := fetcher.getPackage("github", "https://example.com/aso.yaml", "azure-network") + assert.NoError(t, err) + + secondPkg, err := fetcher.getPackage("github", "https://example.com/aso.yaml", "azure-compute") + assert.NoError(t, err) + + assert.EqualValues(t, 1, callCount.Load()) + assert.Same(t, firstPkg, secondPkg) +} + +func TestPackageFetcherDoesNotShareAcrossRegistrants(t *testing.T) { + t.Parallel() + + callCount := &atomic.Int32{} + fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { + return stubPackageManager{ + pkg: &stubPackage{ + name: fmt.Sprintf("%s:%s", registrant, packageName), + version: "v1.0.0", + sourceURL: url, + }, + callCount: callCount, + }, nil + }) + + _, err := fetcher.getPackage("github", "https://example.com/shared.yaml", "azure-network") + assert.NoError(t, err) + + _, err = fetcher.getPackage("artifacthub", "https://example.com/shared.yaml", "azure-network") + assert.NoError(t, err) + + assert.EqualValues(t, 2, callCount.Load()) +} + +func TestPackageFetcherDeduplicatesConcurrentRequests(t *testing.T) { + t.Parallel() + + callCount := &atomic.Int32{} + fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { + return stubPackageManager{ + pkg: &stubPackage{ + name: packageName, + version: "v1.0.0", + sourceURL: url, + }, + callCount: callCount, + delay: 25 * time.Millisecond, + }, nil + }) + + var wg sync.WaitGroup + for range 10 { + wg.Add(1) + go func() { + defer wg.Done() + _, err := fetcher.getPackage("github", "https://example.com/aso.yaml", "azure-network") + assert.NoError(t, err) + }() + } + wg.Wait() + + assert.EqualValues(t, 1, callCount.Load()) +} + func TestProgressTrackerIntegration(t *testing.T) { // Simulate a model generation workflow totalModels := 50 From 9ab701d4e6361add68dca48e3e492bb02f02c6fc Mon Sep 17 00:00:00 2001 From: Harsh Sunil Suryawanshi Date: Sat, 28 Mar 2026 20:30:49 +0530 Subject: [PATCH 2/3] [Registry] Remove redundant package cache check Signed-off-by: Harsh Sunil Suryawanshi --- registry/model.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/registry/model.go b/registry/model.go index 1bd11dcf..a576c7bf 100644 --- a/registry/model.go +++ b/registry/model.go @@ -58,10 +58,6 @@ func (pf *packageFetcher) getPackage(registrant, sourceURL, modelName string) (m } fetchedPkg, err, _ := pf.fetchGroup.Do(cacheKey, func() (interface{}, error) { - if cachedPkg, ok := pf.cache.Load(cacheKey); ok { - return cachedPkg, nil - } - generator, err := pf.newGenerator(registrant, sourceURL, modelName) if err != nil { return nil, err From e89012b1776243024506c39d620644bd6d5b3c9c Mon Sep 17 00:00:00 2001 From: Harsh Sunil Suryawanshi Date: Thu, 2 Apr 2026 11:19:37 +0530 Subject: [PATCH 3/3] [Registry] Preserve model-specific package identity Signed-off-by: Harsh Sunil Suryawanshi --- registry/model.go | 36 ++++++++++- registry/model_generation_test.go | 103 +++++++++++++++++------------- 2 files changed, 91 insertions(+), 48 deletions(-) diff --git a/registry/model.go b/registry/model.go index a576c7bf..5b9d2db8 100644 --- a/registry/model.go +++ b/registry/model.go @@ -17,6 +17,7 @@ import ( "github.com/meshery/meshkit/errors" "github.com/meshery/meshkit/files" "github.com/meshery/meshkit/generators" + "github.com/meshery/meshkit/generators/github" "github.com/meshery/meshkit/generators/models" "github.com/meshery/meshkit/models/meshmodel/entity" "github.com/meshery/meshkit/utils" @@ -51,10 +52,39 @@ func newPackageFetcher(newGenerator generatorFactory) *packageFetcher { } } +func packageCacheKey(registrant, sourceURL, modelName string) string { + normalizedRegistrant := utils.ReplaceSpacesAndConvertToLowercase(registrant) + if normalizedRegistrant == artifactHub { + return fmt.Sprintf("%s\x00%s\x00%s", normalizedRegistrant, sourceURL, utils.ReplaceSpacesAndConvertToLowercase(modelName)) + } + + return fmt.Sprintf("%s\x00%s", normalizedRegistrant, sourceURL) +} + +// GitHub packages derive generated component metadata from the model name, so +// reuse the fetched content but return a per-model copy with the requested name. +func packageForModel(registrant, modelName string, pkg models.Package) models.Package { + if utils.ReplaceSpacesAndConvertToLowercase(registrant) != gitHub { + return pkg + } + + switch typedPkg := pkg.(type) { + case github.GitHubPackage: + typedPkg.Name = modelName + return typedPkg + case *github.GitHubPackage: + clonedPkg := *typedPkg + clonedPkg.Name = modelName + return &clonedPkg + default: + return pkg + } +} + func (pf *packageFetcher) getPackage(registrant, sourceURL, modelName string) (models.Package, error) { - cacheKey := fmt.Sprintf("%s\x00%s", utils.ReplaceSpacesAndConvertToLowercase(registrant), sourceURL) + cacheKey := packageCacheKey(registrant, sourceURL, modelName) if cachedPkg, ok := pf.cache.Load(cacheKey); ok { - return cachedPkg.(models.Package), nil + return packageForModel(registrant, modelName, cachedPkg.(models.Package)), nil } fetchedPkg, err, _ := pf.fetchGroup.Do(cacheKey, func() (interface{}, error) { @@ -79,7 +109,7 @@ func (pf *packageFetcher) getPackage(registrant, sourceURL, modelName string) (m return nil, err } - return fetchedPkg.(models.Package), nil + return packageForModel(registrant, modelName, fetchedPkg.(models.Package)), nil } type compGenerateTracker struct { diff --git a/registry/model_generation_test.go b/registry/model_generation_test.go index 9fdf41d9..9b863273 100644 --- a/registry/model_generation_test.go +++ b/registry/model_generation_test.go @@ -8,33 +8,12 @@ import ( "testing" "time" + artifacthubgen "github.com/meshery/meshkit/generators/artifacthub" + githubgen "github.com/meshery/meshkit/generators/github" "github.com/meshery/meshkit/generators/models" - "github.com/meshery/schemas/models/v1beta1/component" "github.com/stretchr/testify/assert" ) -type stubPackage struct { - name string - version string - sourceURL string -} - -func (sp stubPackage) GenerateComponents(string) ([]component.ComponentDefinition, error) { - return nil, nil -} - -func (sp stubPackage) GetVersion() string { - return sp.version -} - -func (sp stubPackage) GetSourceURL() string { - return sp.sourceURL -} - -func (sp stubPackage) GetName() string { - return sp.name -} - type stubPackageManager struct { pkg models.Package callCount *atomic.Int32 @@ -49,6 +28,22 @@ func (spm stubPackageManager) GetPackage() (models.Package, error) { return spm.pkg, nil } +func stubPackageForRegistrant(registrant, url, packageName string) models.Package { + switch registrant { + case "artifacthub": + return artifacthubgen.AhPackage{ + Name: fmt.Sprintf("%s:%s", registrant, packageName), + ChartUrl: url, + Version: "v1.0.0", + } + default: + return githubgen.GitHubPackage{ + Name: packageName, + SourceURL: url, + } + } +} + func TestGenerationOptionsTimeoutBehavior(t *testing.T) { // Test that timeout value is respected when set tests := []struct { @@ -83,17 +78,13 @@ func TestGenerationOptionsTimeoutBehavior(t *testing.T) { } } -func TestPackageFetcherCachesByRegistrantAndSourceURL(t *testing.T) { +func TestPackageFetcherCachesGitHubPackagesByRegistrantAndSourceURL(t *testing.T) { t.Parallel() callCount := &atomic.Int32{} fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { return stubPackageManager{ - pkg: &stubPackage{ - name: packageName, - version: "v1.0.0", - sourceURL: url, - }, + pkg: stubPackageForRegistrant(registrant, url, packageName), callCount: callCount, }, nil }) @@ -105,7 +96,28 @@ func TestPackageFetcherCachesByRegistrantAndSourceURL(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, 1, callCount.Load()) - assert.Same(t, firstPkg, secondPkg) + assert.Equal(t, "azure-network", firstPkg.GetName()) + assert.Equal(t, "azure-compute", secondPkg.GetName()) +} + +func TestPackageFetcherDoesNotShareArtifactHubPackagesAcrossModelNames(t *testing.T) { + t.Parallel() + + callCount := &atomic.Int32{} + fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { + return stubPackageManager{ + pkg: stubPackageForRegistrant(registrant, url, packageName), + callCount: callCount, + }, nil + }) + + _, err := fetcher.getPackage("artifacthub", "https://example.com/shared.yaml", "azure-network") + assert.NoError(t, err) + + _, err = fetcher.getPackage("artifacthub", "https://example.com/shared.yaml", "azure-compute") + assert.NoError(t, err) + + assert.EqualValues(t, 2, callCount.Load()) } func TestPackageFetcherDoesNotShareAcrossRegistrants(t *testing.T) { @@ -114,11 +126,7 @@ func TestPackageFetcherDoesNotShareAcrossRegistrants(t *testing.T) { callCount := &atomic.Int32{} fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { return stubPackageManager{ - pkg: &stubPackage{ - name: fmt.Sprintf("%s:%s", registrant, packageName), - version: "v1.0.0", - sourceURL: url, - }, + pkg: stubPackageForRegistrant(registrant, url, packageName), callCount: callCount, }, nil }) @@ -132,30 +140,35 @@ func TestPackageFetcherDoesNotShareAcrossRegistrants(t *testing.T) { assert.EqualValues(t, 2, callCount.Load()) } -func TestPackageFetcherDeduplicatesConcurrentRequests(t *testing.T) { +func TestPackageFetcherDeduplicatesConcurrentGitHubRequests(t *testing.T) { t.Parallel() callCount := &atomic.Int32{} fetcher := newPackageFetcher(func(registrant, url, packageName string) (models.PackageManager, error) { return stubPackageManager{ - pkg: &stubPackage{ - name: packageName, - version: "v1.0.0", - sourceURL: url, - }, + pkg: stubPackageForRegistrant(registrant, url, packageName), callCount: callCount, delay: 25 * time.Millisecond, }, nil }) + modelNames := []string{ + "azure-network", + "azure-compute", + "azure-storage", + "azure-network", + "azure-compute", + "azure-storage", + } var wg sync.WaitGroup - for range 10 { + for _, modelName := range modelNames { wg.Add(1) - go func() { + go func(modelName string) { defer wg.Done() - _, err := fetcher.getPackage("github", "https://example.com/aso.yaml", "azure-network") + pkg, err := fetcher.getPackage("github", "https://example.com/aso.yaml", modelName) assert.NoError(t, err) - }() + assert.Equal(t, modelName, pkg.GetName()) + }(modelName) } wg.Wait()