Skip to content

Commit e75fc0f

Browse files
authored
feat: add config validation to NewCacheBackend (#204)
Signed-off-by: cryo <[email protected]>
1 parent 9819bb7 commit e75fc0f

File tree

2 files changed

+46
-47
lines changed

2 files changed

+46
-47
lines changed

src/semantic-router/pkg/cache/cache_factory.go

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import (
99

1010
// NewCacheBackend creates a cache backend instance from the provided configuration
1111
func NewCacheBackend(config CacheConfig) (CacheBackend, error) {
12+
if err := ValidateCacheConfig(config); err != nil {
13+
return nil, fmt.Errorf("invalid cache config: %w", err)
14+
}
15+
1216
if !config.Enabled {
1317
// Create a disabled cache backend
1418
observability.Debugf("Cache disabled - creating disabled in-memory cache backend")
@@ -34,17 +38,6 @@ func NewCacheBackend(config CacheConfig) (CacheBackend, error) {
3438
case MilvusCacheType:
3539
observability.Debugf("Creating Milvus cache backend - ConfigPath: %s, TTL: %ds, Threshold: %.3f",
3640
config.BackendConfigPath, config.TTLSeconds, config.SimilarityThreshold)
37-
if config.BackendConfigPath == "" {
38-
return nil, fmt.Errorf("backend_config_path is required for Milvus cache backend")
39-
}
40-
41-
// Ensure the Milvus configuration file exists
42-
if _, err := os.Stat(config.BackendConfigPath); os.IsNotExist(err) {
43-
observability.Debugf("Milvus config file not found: %s", config.BackendConfigPath)
44-
return nil, fmt.Errorf("Milvus config file not found: %s", config.BackendConfigPath)
45-
}
46-
observability.Debugf("Milvus config file found: %s", config.BackendConfigPath)
47-
4841
options := MilvusCacheOptions{
4942
Enabled: config.Enabled,
5043
SimilarityThreshold: config.SimilarityThreshold,
@@ -75,19 +68,22 @@ func ValidateCacheConfig(config CacheConfig) error {
7568
return fmt.Errorf("ttl_seconds cannot be negative, got: %d", config.TTLSeconds)
7669
}
7770

78-
// Check max entries for in-memory cache
79-
if config.BackendType == InMemoryCacheType || config.BackendType == "" {
71+
// Check backend-specific requirements
72+
switch config.BackendType {
73+
case InMemoryCacheType, "":
8074
if config.MaxEntries < 0 {
8175
return fmt.Errorf("max_entries cannot be negative for in-memory cache, got: %d", config.MaxEntries)
8276
}
83-
}
84-
85-
// Check backend-specific requirements
86-
switch config.BackendType {
8777
case MilvusCacheType:
8878
if config.BackendConfigPath == "" {
8979
return fmt.Errorf("backend_config_path is required for Milvus cache backend")
9080
}
81+
// Ensure the Milvus configuration file exists
82+
if _, err := os.Stat(config.BackendConfigPath); os.IsNotExist(err) {
83+
observability.Debugf("Milvus config file not found: %s", config.BackendConfigPath)
84+
return fmt.Errorf("milvus config file not found: %s", config.BackendConfigPath)
85+
}
86+
observability.Debugf("Milvus config file found: %s", config.BackendConfigPath)
9187
}
9288

9389
return nil

src/semantic-router/pkg/cache/cache_test.go

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -133,36 +133,6 @@ development:
133133
Expect(err).NotTo(HaveOccurred())
134134
})
135135

136-
It("should return error when backend_config_path is missing", func() {
137-
config := cache.CacheConfig{
138-
BackendType: cache.MilvusCacheType,
139-
Enabled: true,
140-
SimilarityThreshold: 0.8,
141-
TTLSeconds: 3600,
142-
// BackendConfigPath is missing
143-
}
144-
145-
backend, err := cache.NewCacheBackend(config)
146-
Expect(err).To(HaveOccurred())
147-
Expect(err.Error()).To(ContainSubstring("backend_config_path is required"))
148-
Expect(backend).To(BeNil())
149-
})
150-
151-
It("should return error when backend_config_path file doesn't exist", func() {
152-
config := cache.CacheConfig{
153-
BackendType: cache.MilvusCacheType,
154-
Enabled: true,
155-
SimilarityThreshold: 0.8,
156-
TTLSeconds: 3600,
157-
BackendConfigPath: "/nonexistent/milvus.yaml",
158-
}
159-
160-
backend, err := cache.NewCacheBackend(config)
161-
Expect(err).To(HaveOccurred())
162-
Expect(err.Error()).To(ContainSubstring("config file not found"))
163-
Expect(backend).To(BeNil())
164-
})
165-
166136
It("should create Milvus cache backend successfully with valid config", func() {
167137
config := cache.CacheConfig{
168138
BackendType: cache.MilvusCacheType,
@@ -221,6 +191,25 @@ development:
221191
Expect(backend).To(BeNil())
222192
})
223193
})
194+
195+
Context("with invalid config but valid backend type", func() {
196+
It("should return error due to validation when config has invalid values", func() {
197+
config := cache.CacheConfig{
198+
BackendType: cache.InMemoryCacheType, // valid backend type
199+
Enabled: true,
200+
SimilarityThreshold: -0.8, // invalid
201+
MaxEntries: 10,
202+
TTLSeconds: -1, // invalid
203+
}
204+
205+
backend, err := cache.NewCacheBackend(config)
206+
207+
Expect(err).To(HaveOccurred())
208+
Expect(err.Error()).To(ContainSubstring("invalid cache config")) // ensure from config validation
209+
Expect(backend).To(BeNil())
210+
})
211+
})
212+
224213
})
225214

226215
Describe("ValidateCacheConfig", func() {
@@ -319,6 +308,20 @@ development:
319308
Expect(err.Error()).To(ContainSubstring("backend_config_path is required for Milvus"))
320309
})
321310

311+
It("should return error when Milvus backend_config_path file doesn't exist", func() {
312+
config := cache.CacheConfig{
313+
BackendType: cache.MilvusCacheType,
314+
Enabled: true,
315+
SimilarityThreshold: 0.8,
316+
TTLSeconds: 3600,
317+
BackendConfigPath: "/nonexistent/milvus.yaml",
318+
}
319+
320+
err := cache.ValidateCacheConfig(config)
321+
Expect(err).To(HaveOccurred())
322+
Expect(err.Error()).To(ContainSubstring("config file not found"))
323+
})
324+
322325
It("should validate edge case values", func() {
323326
config := cache.CacheConfig{
324327
BackendType: cache.InMemoryCacheType,

0 commit comments

Comments
 (0)