Skip to content

Commit cd6a614

Browse files
committed
chore: refactor source filter validation
Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>
1 parent b6966d7 commit cd6a614

3 files changed

Lines changed: 106 additions & 5 deletions

File tree

catalog/internal/catalog/loader.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ func (l *Loader) updateSources(path string, config *sourceConfig) error {
194194
return fmt.Errorf("invalid source: duplicate id %s", id)
195195
}
196196

197-
if _, err := NewModelFilterFromSource(&source, nil, nil); err != nil {
197+
// Validate includedModels/excludedModels patterns early
198+
if err := ValidateSourceFilters(&source); err != nil {
198199
return fmt.Errorf("invalid source %s: %w", id, err)
199200
}
200201

catalog/internal/catalog/model_filter.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,31 @@ func (f *ModelFilter) Allows(name string) bool {
136136
return true
137137
}
138138

139+
// ValidateSourceFilters checks that the includedModels and excludedModels patterns
140+
// in a source are valid (non-empty, compilable, non-conflicting) without constructing
141+
// the full ModelFilter. This is useful for early validation at configuration load time.
142+
func ValidateSourceFilters(source *Source) error {
143+
if source == nil {
144+
return fmt.Errorf("source cannot be nil")
145+
}
146+
147+
// Check for conflicting patterns
148+
if err := detectConflictingPatterns(source.IncludedModels, source.ExcludedModels); err != nil {
149+
return fmt.Errorf("source %s: %w", source.Id, err)
150+
}
151+
152+
// Validate that all patterns compile successfully
153+
if _, err := compilePatterns("includedModels", source.IncludedModels); err != nil {
154+
return fmt.Errorf("source %s: %w", source.Id, err)
155+
}
156+
157+
if _, err := compilePatterns("excludedModels", source.ExcludedModels); err != nil {
158+
return fmt.Errorf("source %s: %w", source.Id, err)
159+
}
160+
161+
return nil
162+
}
163+
139164
// NewModelFilterFromSource composes a ModelFilter using the source-level configuration and any legacy additions.
140165
func NewModelFilterFromSource(source *Source, extraIncluded, extraExcluded []string) (*ModelFilter, error) {
141166
if source == nil {

catalog/internal/catalog/model_filter_test.go

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,87 @@ func TestModelFilterWithWildcardInMiddle(t *testing.T) {
5858
assert.False(t, filter.Allows("DeepSeek/empty-old-v1"))
5959
assert.False(t, filter.Allows("Foo/old"))
6060
assert.False(t, filter.Allows("Bar/deprecated"))
61-
61+
6262
// Test that */pattern* requires the pattern immediately after /
6363
filter2, err := NewModelFilter(nil, []string{"*/deprecated", "*/old*"})
6464
require.NoError(t, err)
65-
65+
6666
assert.True(t, filter2.Allows("Mistral/empty-deprecated")) // doesn't match */deprecated (no immediate match after /)
67-
assert.False(t, filter2.Allows("Foo/deprecated")) // matches */deprecated
68-
assert.False(t, filter2.Allows("Bar/old-model")) // matches */old*
67+
assert.False(t, filter2.Allows("Foo/deprecated")) // matches */deprecated
68+
assert.False(t, filter2.Allows("Bar/old-model")) // matches */old*
69+
}
70+
71+
func TestValidateSourceFilters(t *testing.T) {
72+
t.Run("valid source with no filters", func(t *testing.T) {
73+
source := &Source{
74+
CatalogSource: apimodels.CatalogSource{
75+
Id: "test",
76+
Name: "Test source",
77+
},
78+
}
79+
err := ValidateSourceFilters(source)
80+
assert.NoError(t, err)
81+
})
82+
83+
t.Run("valid source with patterns", func(t *testing.T) {
84+
source := &Source{
85+
CatalogSource: apimodels.CatalogSource{
86+
Id: "test",
87+
Name: "Test source",
88+
IncludedModels: []string{"Granite/*", "Meta/*"},
89+
ExcludedModels: []string{"*-beta"},
90+
},
91+
}
92+
err := ValidateSourceFilters(source)
93+
assert.NoError(t, err)
94+
})
95+
96+
t.Run("conflicting patterns", func(t *testing.T) {
97+
source := &Source{
98+
CatalogSource: apimodels.CatalogSource{
99+
Id: "test",
100+
Name: "Test source",
101+
IncludedModels: []string{"Granite/*"},
102+
ExcludedModels: []string{"Granite/*"},
103+
},
104+
}
105+
err := ValidateSourceFilters(source)
106+
require.Error(t, err)
107+
assert.Contains(t, err.Error(), "source test")
108+
assert.Contains(t, err.Error(), "Granite/*")
109+
})
110+
111+
t.Run("empty pattern in includedModels", func(t *testing.T) {
112+
source := &Source{
113+
CatalogSource: apimodels.CatalogSource{
114+
Id: "test",
115+
Name: "Test source",
116+
IncludedModels: []string{"Granite/*", ""},
117+
},
118+
}
119+
err := ValidateSourceFilters(source)
120+
require.Error(t, err)
121+
assert.Contains(t, err.Error(), "source test")
122+
assert.Contains(t, err.Error(), "pattern cannot be empty")
123+
})
124+
125+
t.Run("invalid regex pattern", func(t *testing.T) {
126+
// This shouldn't happen with our glob-to-regex conversion,
127+
// but validates the error path exists
128+
source := &Source{
129+
CatalogSource: apimodels.CatalogSource{
130+
Id: "test",
131+
Name: "Test source",
132+
IncludedModels: []string{"valid/*"},
133+
},
134+
}
135+
err := ValidateSourceFilters(source)
136+
assert.NoError(t, err) // Our conversion always produces valid regex
137+
})
138+
139+
t.Run("nil source", func(t *testing.T) {
140+
err := ValidateSourceFilters(nil)
141+
require.Error(t, err)
142+
assert.Contains(t, err.Error(), "source cannot be nil")
143+
})
69144
}

0 commit comments

Comments
 (0)