chore: refactor validation function

Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>

Author: Al-Pragliola

catalog/internal/catalog/loader.go

@@ -195,7 +195,7 @@ func (l *Loader) updateSources(path string, config *sourceConfig) error {
 		}
 
 		// Validate includedModels/excludedModels patterns early
-		if err := ValidateSourceFilters(&source); err != nil {
+		if err := ValidateSourceFilters(source.IncludedModels, source.ExcludedModels); err != nil {
 			return fmt.Errorf("invalid source %s: %w", id, err)
 		}
 

catalog/internal/catalog/model_filter.go

@@ -62,9 +62,28 @@ func compilePatterns(field string, patterns []string) ([]*compiledPattern, error
 	return compiled, nil
 }
 
+// ValidateSourceFilters validates that the includedModels and excludedModels patterns
+// are valid (non-empty, compilable, non-conflicting). This is useful for early validation
+// at configuration load time without constructing the full ModelFilter.
+func ValidateSourceFilters(included, excluded []string) error {
+	if err := detectConflictingPatterns(included, excluded); err != nil {
+		return err
+	}
+
+	if _, err := compilePatterns("includedModels", included); err != nil {
+		return err
+	}
+
+	if _, err := compilePatterns("excludedModels", excluded); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // NewModelFilter builds a ModelFilter from the provided include/exclude pattern lists.
 func NewModelFilter(included, excluded []string) (*ModelFilter, error) {
-	if err := detectConflictingPatterns(included, excluded); err != nil {
+	if err := ValidateSourceFilters(included, excluded); err != nil {
 		return nil, err
 	}
 
@@ -136,31 +155,6 @@ func (f *ModelFilter) Allows(name string) bool {
 	return true
 }
 
-// ValidateSourceFilters checks that the includedModels and excludedModels patterns
-// in a source are valid (non-empty, compilable, non-conflicting) without constructing
-// the full ModelFilter. This is useful for early validation at configuration load time.
-func ValidateSourceFilters(source *Source) error {
-	if source == nil {
-		return fmt.Errorf("source cannot be nil")
-	}
-
-	// Check for conflicting patterns
-	if err := detectConflictingPatterns(source.IncludedModels, source.ExcludedModels); err != nil {
-		return fmt.Errorf("source %s: %w", source.Id, err)
-	}
-
-	// Validate that all patterns compile successfully
-	if _, err := compilePatterns("includedModels", source.IncludedModels); err != nil {
-		return fmt.Errorf("source %s: %w", source.Id, err)
-	}
-
-	if _, err := compilePatterns("excludedModels", source.ExcludedModels); err != nil {
-		return fmt.Errorf("source %s: %w", source.Id, err)
-	}
-
-	return nil
-}
-
 // NewModelFilterFromSource composes a ModelFilter using the source-level configuration and any legacy additions.
 func NewModelFilterFromSource(source *Source, extraIncluded, extraExcluded []string) (*ModelFilter, error) {
 	if source == nil {

catalog/internal/catalog/model_filter_test.go

@@ -69,76 +69,36 @@ func TestModelFilterWithWildcardInMiddle(t *testing.T) {
 }
 
 func TestValidateSourceFilters(t *testing.T) {
-	t.Run("valid source with no filters", func(t *testing.T) {
-		source := &Source{
-			CatalogSource: apimodels.CatalogSource{
-				Id:   "test",
-				Name: "Test source",
-			},
-		}
-		err := ValidateSourceFilters(source)
+	t.Run("no filters", func(t *testing.T) {
+		err := ValidateSourceFilters(nil, nil)
 		assert.NoError(t, err)
 	})
 
-	t.Run("valid source with patterns", func(t *testing.T) {
-		source := &Source{
-			CatalogSource: apimodels.CatalogSource{
-				Id:             "test",
-				Name:           "Test source",
-				IncludedModels: []string{"Granite/*", "Meta/*"},
-				ExcludedModels: []string{"*-beta"},
-			},
-		}
-		err := ValidateSourceFilters(source)
+	t.Run("valid patterns", func(t *testing.T) {
+		err := ValidateSourceFilters([]string{"Granite/*", "Meta/*"}, []string{"*-beta"})
 		assert.NoError(t, err)
 	})
 
 	t.Run("conflicting patterns", func(t *testing.T) {
-		source := &Source{
-			CatalogSource: apimodels.CatalogSource{
-				Id:             "test",
-				Name:           "Test source",
-				IncludedModels: []string{"Granite/*"},
-				ExcludedModels: []string{"Granite/*"},
-			},
-		}
-		err := ValidateSourceFilters(source)
+		err := ValidateSourceFilters([]string{"Granite/*"}, []string{"Granite/*"})
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "source test")
 		assert.Contains(t, err.Error(), "Granite/*")
 	})
 
 	t.Run("empty pattern in includedModels", func(t *testing.T) {
-		source := &Source{
-			CatalogSource: apimodels.CatalogSource{
-				Id:             "test",
-				Name:           "Test source",
-				IncludedModels: []string{"Granite/*", ""},
-			},
-		}
-		err := ValidateSourceFilters(source)
+		err := ValidateSourceFilters([]string{"Granite/*", ""}, nil)
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "source test")
 		assert.Contains(t, err.Error(), "pattern cannot be empty")
 	})
 
-	t.Run("invalid regex pattern", func(t *testing.T) {
-		// This shouldn't happen with our glob-to-regex conversion,
-		// but validates the error path exists
-		source := &Source{
-			CatalogSource: apimodels.CatalogSource{
-				Id:             "test",
-				Name:           "Test source",
-				IncludedModels: []string{"valid/*"},
-			},
-		}
-		err := ValidateSourceFilters(source)
-		assert.NoError(t, err) // Our conversion always produces valid regex
+	t.Run("whitespace-only pattern", func(t *testing.T) {
+		err := ValidateSourceFilters([]string{"   "}, nil)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "pattern cannot be empty")
 	})
 
-	t.Run("nil source", func(t *testing.T) {
-		err := ValidateSourceFilters(nil)
-		require.Error(t, err)
-		assert.Contains(t, err.Error(), "source cannot be nil")
+	t.Run("valid glob patterns", func(t *testing.T) {
+		err := ValidateSourceFilters([]string{"valid/*"}, nil)
+		assert.NoError(t, err) // Our conversion always produces valid regex
 	})
 }