Skip to content

Commit d15206f

Browse files
dmartinoldmjb
dmartinol
and
dmjb
authored
Remove API handler for Toolhive format (#146)
* using UpstreamRegistry in test functions Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * aligned toolhive test builder Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * aligned toolhive test builder Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * forcing API sources to have upstream format Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * updated docs Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * forcing API sources to have upstream format Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * restored taskfile Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * updated docs Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * reduced cyclomatic complexity Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * fixed test error, removed unneeded file Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * restored v0 route to not break tests Signed-off-by: Daniele Martinoli <dmartino@redhat.com> --------- Signed-off-by: Daniele Martinoli <dmartino@redhat.com> Co-authored-by: Don Browne <dmjb@users.noreply.github.com>
1 parent 9744f94 commit d15206f

File tree

8 files changed

+70
-860
lines changed

8 files changed

+70
-860
lines changed

‎examples/config-api.yaml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ source:
1919
# Source type: git, or api
2020
type: api
2121

22-
# Data format: toolhive (native) or upstream (MCP registry format)
23-
# Use 'upstream' for the official MCP registry format
22+
# Data format: only 'upstream' is supported for API sources
2423
format: upstream
2524

2625
# API endpoint configuration

‎internal/config/config.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,25 @@ func (c *Config) validate() error {
290290
return fmt.Errorf("source.type is required")
291291
}
292292

293+
if err := c.validateSourceConfigByType(); err != nil {
294+
return err
295+
}
296+
297+
// Validate sync policy
298+
if c.SyncPolicy == nil || c.SyncPolicy.Interval == "" {
299+
return fmt.Errorf("syncPolicy.interval is required")
300+
}
301+
302+
// Try to parse the interval to ensure it's valid
303+
if _, err := time.ParseDuration(c.SyncPolicy.Interval); err != nil {
304+
return fmt.Errorf("syncPolicy.interval must be a valid duration (e.g., '30m', '1h'): %w", err)
305+
}
306+
307+
return nil
308+
}
309+
310+
// validateSourceConfigByType validates the source configuration by the source type
311+
func (c *Config) validateSourceConfigByType() error {
293312
// Validate source-specific settings
294313
switch c.Source.Type {
295314
case SourceTypeGit:
@@ -307,6 +326,9 @@ func (c *Config) validate() error {
307326
if c.Source.API.Endpoint == "" {
308327
return fmt.Errorf("source.api.endpoint is required")
309328
}
329+
if c.Source.Format != "" && c.Source.Format != SourceFormatUpstream {
330+
return fmt.Errorf("source.format must be either empty or %s when type is api, got %s", SourceFormatUpstream, c.Source.Format)
331+
}
310332

311333
case SourceTypeFile:
312334
if c.Source.File == nil {
@@ -320,15 +342,5 @@ func (c *Config) validate() error {
320342
return fmt.Errorf("unsupported source type: %s", c.Source.Type)
321343
}
322344

323-
// Validate sync policy
324-
if c.SyncPolicy == nil || c.SyncPolicy.Interval == "" {
325-
return fmt.Errorf("syncPolicy.interval is required")
326-
}
327-
328-
// Try to parse the interval to ensure it's valid
329-
if _, err := time.ParseDuration(c.SyncPolicy.Interval); err != nil {
330-
return fmt.Errorf("syncPolicy.interval must be a valid duration (e.g., '30m', '1h'): %w", err)
331-
}
332-
333345
return nil
334346
}

‎internal/config/config_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,20 @@ func TestConfigValidate(t *testing.T) {
377377
wantErr: true,
378378
errMsg: "source.file.path is required",
379379
},
380+
{
381+
name: "invalid_format_when_type_is_api",
382+
config: &Config{
383+
Source: SourceConfig{
384+
Type: "api",
385+
Format: "toolhive",
386+
API: &APIConfig{
387+
Endpoint: "http://example.com",
388+
},
389+
},
390+
},
391+
wantErr: true,
392+
errMsg: "source.format must be either empty or upstream",
393+
},
380394
{
381395
name: "unsupported_source_type",
382396
config: &Config{

‎internal/sources/api.go‎

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,10 @@ import (
1111
)
1212

1313
// APISourceHandler handles registry data from API endpoints
14-
// It detects the format (ToolHive vs Upstream) and delegates to the appropriate handler
14+
// It validates the Upstream format and delegates to the appropriate handler
1515
type APISourceHandler struct {
1616
httpClient httpclient.Client
1717
validator SourceDataValidator
18-
toolhiveHandler *ToolHiveAPIHandler
1918
upstreamHandler *UpstreamAPIHandler
2019
}
2120

@@ -26,7 +25,6 @@ func NewAPISourceHandler() *APISourceHandler {
2625
return &APISourceHandler{
2726
httpClient: httpClient,
2827
validator: NewSourceDataValidator(),
29-
toolhiveHandler: NewToolHiveAPIHandler(httpClient),
3028
upstreamHandler: NewUpstreamAPIHandler(httpClient),
3129
}
3230
}
@@ -38,6 +36,11 @@ func (*APISourceHandler) Validate(source *config.SourceConfig) error {
3836
config.SourceTypeAPI, source.Type)
3937
}
4038

39+
if source.Format != "" && source.Format != config.SourceFormatUpstream {
40+
return fmt.Errorf("unsupported format: expected %s or empty, got %s",
41+
config.SourceFormatUpstream, source.Format)
42+
}
43+
4144
if source.API == nil {
4245
return fmt.Errorf("api configuration is required for source type %s",
4346
config.SourceTypeAPI)
@@ -51,7 +54,7 @@ func (*APISourceHandler) Validate(source *config.SourceConfig) error {
5154
}
5255

5356
// FetchRegistry retrieves registry data from the API endpoint
54-
// It auto-detects the format and delegates to the appropriate handler
57+
// It validates the Upstream format and delegates to the appropriate handler
5558
func (h *APISourceHandler) FetchRegistry(ctx context.Context, cfg *config.Config) (*FetchResult, error) {
5659
logger := log.FromContext(ctx)
5760

@@ -60,14 +63,13 @@ func (h *APISourceHandler) FetchRegistry(ctx context.Context, cfg *config.Config
6063
return nil, fmt.Errorf("source validation failed: %w", err)
6164
}
6265

63-
// Detect format and get appropriate handler
64-
handler, format, err := h.detectFormatAndGetHandler(ctx, cfg)
66+
// Validate Upstream format and get appropriate handler
67+
handler, err := h.validateUstreamFormat(ctx, cfg)
6568
if err != nil {
66-
return nil, fmt.Errorf("format detection failed: %w", err)
69+
return nil, fmt.Errorf("upstream format validation failed: %w", err)
6770
}
6871

69-
logger.Info("Detected API format, delegating to handler",
70-
"format", format)
72+
logger.Info("Validated Upstream format, delegating to handler")
7173

7274
// Delegate to the appropriate handler
7375
return handler.FetchRegistry(ctx, cfg)
@@ -80,48 +82,33 @@ func (h *APISourceHandler) CurrentHash(ctx context.Context, cfg *config.Config)
8082
return "", fmt.Errorf("source validation failed: %w", err)
8183
}
8284

83-
// Detect format and get appropriate handler
84-
handler, _, err := h.detectFormatAndGetHandler(ctx, cfg)
85+
// Validate Upstream format and get appropriate handler
86+
handler, err := h.validateUstreamFormat(ctx, cfg)
8587
if err != nil {
86-
return "", fmt.Errorf("format detection failed: %w", err)
88+
return "", fmt.Errorf("upstream format validation failed: %w", err)
8789
}
8890

8991
// Delegate to the appropriate handler
9092
return handler.CurrentHash(ctx, cfg)
9193
}
9294

93-
// apiFormatHandler is an internal interface for format-specific handlers
94-
type apiFormatHandler interface {
95-
Validate(ctx context.Context, endpoint string) error
96-
FetchRegistry(ctx context.Context, cfg *config.Config) (*FetchResult, error)
97-
CurrentHash(ctx context.Context, cfg *config.Config) (string, error)
98-
}
99-
100-
// detectFormatAndGetHandler detects the API format and returns the appropriate handler
101-
func (h *APISourceHandler) detectFormatAndGetHandler(
95+
// validateUstreamFormat validates the Upstream format and returns the appropriate handler
96+
func (h *APISourceHandler) validateUstreamFormat(
10297
ctx context.Context,
10398
cfg *config.Config,
104-
) (apiFormatHandler, string, error) {
99+
) (*UpstreamAPIHandler, error) {
105100
logger := log.FromContext(ctx)
106101
endpoint := h.getBaseURL(cfg)
107102

108-
// Try ToolHive format first (/v0/info)
109-
toolhiveErr := h.toolhiveHandler.Validate(ctx, endpoint)
110-
if toolhiveErr == nil {
111-
logger.Info("Validated as ToolHive format")
112-
return h.toolhiveHandler, "toolhive", nil
113-
}
114-
logger.V(1).Info("ToolHive format validation failed", "error", toolhiveErr.Error())
115-
116103
// Try upstream format (/openapi.yaml)
117104
upstreamErr := h.upstreamHandler.Validate(ctx, endpoint)
118105
if upstreamErr == nil {
119106
logger.Info("Validated as upstream MCP Registry format")
120-
return h.upstreamHandler, "upstream", nil
107+
return h.upstreamHandler, nil
121108
}
122109
logger.V(1).Info("Upstream format validation failed", "error", upstreamErr.Error())
123110

124-
return nil, "", fmt.Errorf("unable to detect valid API format (tried toolhive and upstream)")
111+
return nil, fmt.Errorf("unable to validate Upstream format")
125112
}
126113

127114
// getBaseURL extracts and normalizes the base URL

‎internal/sources/api_test.go‎

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ import (
1414
)
1515

1616
const (
17-
toolhiveInfoPath = "/v0/info"
18-
toolhiveServersPath = "/v0/servers"
19-
openapiPath = "/openapi.yaml"
17+
openapiPath = "/openapi.yaml"
2018
)
2119

2220
func TestAPISources(t *testing.T) {
@@ -54,6 +52,17 @@ var _ = Describe("APISourceHandler", func() {
5452
Expect(err.Error()).To(ContainSubstring("invalid source type"))
5553
})
5654

55+
It("should reject non-Upstream format", func() {
56+
source := &config.SourceConfig{
57+
Type: config.SourceTypeAPI,
58+
Format: config.SourceFormatToolHive,
59+
}
60+
61+
err := handler.Validate(source)
62+
Expect(err).To(HaveOccurred())
63+
Expect(err.Error()).To(ContainSubstring("unsupported format:"))
64+
})
65+
5766
It("should reject missing API configuration", func() {
5867
source := &config.SourceConfig{
5968
Type: config.SourceTypeAPI,
@@ -91,49 +100,11 @@ var _ = Describe("APISourceHandler", func() {
91100
})
92101
})
93102

94-
Describe("Format Detection", func() {
95-
Context("ToolHive Format", func() {
96-
BeforeEach(func() {
97-
mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
98-
switch r.URL.Path {
99-
case toolhiveInfoPath:
100-
w.Header().Set("Content-Type", "application/json")
101-
w.WriteHeader(http.StatusOK)
102-
_, _ = w.Write([]byte(`{"version":"1.0.0","last_updated":"2025-01-14T00:00:00Z","source":"file:/data/registry.json","total_servers":5}`))
103-
case toolhiveServersPath:
104-
w.Header().Set("Content-Type", "application/json")
105-
w.WriteHeader(http.StatusOK)
106-
_, _ = w.Write([]byte(`{"servers":[],"total":0}`))
107-
default:
108-
w.WriteHeader(http.StatusNotFound)
109-
}
110-
}))
111-
})
112-
113-
It("should detect and validate ToolHive format", func() {
114-
registryConfig := &config.Config{
115-
Source: config.SourceConfig{
116-
Type: config.SourceTypeAPI,
117-
API: &config.APIConfig{
118-
Endpoint: mockServer.URL,
119-
},
120-
},
121-
}
122-
result, err := handler.FetchRegistry(ctx, registryConfig)
123-
Expect(err).NotTo(HaveOccurred())
124-
Expect(result).NotTo(BeNil())
125-
Expect(result.Format).To(Equal(config.SourceFormatToolHive))
126-
})
127-
})
128-
103+
Describe("Upstream Format Validation", func() {
129104
Context("Upstream Format", func() {
130105
BeforeEach(func() {
131106
mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
132107
switch r.URL.Path {
133-
case toolhiveInfoPath:
134-
// Return 404 for /v0/info (upstream doesn't have this)
135-
w.WriteHeader(http.StatusNotFound)
136-
_, _ = w.Write([]byte(`{"detail":"Endpoint not found"}`))
137108
case openapiPath:
138109
w.Header().Set("Content-Type", "application/x-yaml")
139110
w.WriteHeader(http.StatusOK)
@@ -189,7 +160,7 @@ openapi: 3.1.0
189160

190161
_, err := handler.FetchRegistry(ctx, registryConfig)
191162
Expect(err).To(HaveOccurred())
192-
Expect(err.Error()).To(ContainSubstring("format detection failed"))
163+
Expect(err.Error()).To(ContainSubstring("upstream format validation failed"))
193164
})
194165
})
195166
})

0 commit comments

Comments
 (0)