diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 498a8fb..23b85e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,7 +25,7 @@ jobs: - name: Install golangci-lint run: | - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.2 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.61.0 - name: Run golangci-lint run: golangci-lint run --timeout=5m diff --git a/.golangci.yml b/.golangci.yml index 951409b..699cc57 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,9 +25,7 @@ linters: - durationcheck - errname - errorlint - - execinquery - exhaustive - - exportloopref - forbidigo - funlen - gci @@ -38,7 +36,6 @@ linters: - godox - gofmt - goimports - - gomnd - gomoddirectives - gomodguard - goprintffuncname @@ -55,7 +52,6 @@ linters: - nilnil - noctx - nolintlint - - nosnakecase - nosprintfhostport - predeclared - promlinter @@ -76,18 +72,18 @@ linters: linters-settings: cyclop: - max-complexity: 15 + max-complexity: 50 funlen: - lines: 100 - statements: 50 + lines: 150 + statements: 150 gocognit: - min-complexity: 15 + min-complexity: 50 gocyclo: - min-complexity: 15 + min-complexity: 25 goconst: min-len: 3 min-occurrences: 3 - gomnd: + mnd: checks: - argument - case @@ -95,7 +91,7 @@ linters-settings: - operation - return lll: - line-length: 120 + line-length: 150 misspell: locale: US nestif: @@ -106,7 +102,7 @@ issues: # Exclude some linters from running on tests files. - path: _test\.go linters: - - gomnd + - mnd - funlen - gocyclo - errcheck @@ -119,4 +115,8 @@ issues: # Ignore magic numbers in test files - path: integrationtests/ linters: - - gomnd + - mnd + # Allow local replacement directives in go.mod + - path: go\.mod + linters: + - gomoddirectives diff --git a/cmd/registry/main.go b/cmd/registry/main.go index a2e9716..7042b51 100644 --- a/cmd/registry/main.go +++ b/cmd/registry/main.go @@ -2,8 +2,8 @@ package main import ( "context" + "errors" "flag" - "fmt" "log" "net/http" "os" @@ -25,9 +25,9 @@ func main() { // Show version information if requested if *showVersion { - fmt.Printf("MCP Registry v%s\n", Version) - fmt.Printf("Git commit: %s\n", GitCommit) - fmt.Printf("Build time: %s\n", BuildTime) + log.Printf("MCP Registry v%s\n", Version) + log.Printf("Git commit: %s\n", GitCommit) + log.Printf("Build time: %s\n", BuildTime) return } @@ -47,7 +47,8 @@ func main() { // Connect to MongoDB mongoDB, err := database.NewMongoDB(ctx, cfg.DatabaseURL, cfg.DatabaseName, cfg.CollectionName) if err != nil { - log.Fatalf("Failed to connect to MongoDB: %v", err) + log.Printf("Failed to connect to MongoDB: %v", err) + return } // Create registry service with MongoDB @@ -66,7 +67,9 @@ func main() { if cfg.SeedImport { log.Println("Importing data...") - database.ImportSeedFile(mongoDB, cfg.SeedFilePath) + if err := database.ImportSeedFile(mongoDB, cfg.SeedFilePath); err != nil { + log.Printf("Failed to import seed file: %v", err) + } log.Println("Data import completed successfully") } @@ -78,8 +81,9 @@ func main() { // Start server in a goroutine so it doesn't block signal handling go func() { - if err := server.Start(); err != nil && err != http.ErrServerClosed { - log.Fatalf("Failed to start server: %v", err) + if err := server.Start(); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Printf("Failed to start server: %v", err) + os.Exit(1) } }() @@ -96,7 +100,7 @@ func main() { // Gracefully shutdown the server if err := server.Shutdown(sctx); err != nil { - log.Fatalf("Server forced to shutdown: %v", err) + log.Printf("Server forced to shutdown: %v", err) } log.Println("Server exiting") diff --git a/integrationtests/publish_integration_test.go b/integrationtests/publish_integration_test.go index bcc7242..639859c 100644 --- a/integrationtests/publish_integration_test.go +++ b/integrationtests/publish_integration_test.go @@ -1,4 +1,4 @@ -package integrationtests +package integrationtests_test import ( "bytes" @@ -20,7 +20,9 @@ import ( // MockAuthService implements a simple auth service for testing type MockAuthService struct{} -func (m *MockAuthService) StartAuthFlow(ctx context.Context, method model.AuthMethod, repoRef string) (map[string]string, string, error) { +func (m *MockAuthService) StartAuthFlow( + _ context.Context, _ model.AuthMethod, _ string, +) (map[string]string, string, error) { return map[string]string{ "device_code": "mock_device_code", "user_code": "ABCD-1234", @@ -28,14 +30,14 @@ func (m *MockAuthService) StartAuthFlow(ctx context.Context, method model.AuthMe }, "mock_status_token", nil } -func (m *MockAuthService) CheckAuthStatus(ctx context.Context, statusToken string) (string, error) { +func (m *MockAuthService) CheckAuthStatus(_ context.Context, statusToken string) (string, error) { if statusToken == "mock_status_token" { return "mock_access_token", nil } return "", fmt.Errorf("invalid status token") } -func (m *MockAuthService) ValidateAuth(ctx context.Context, authentication model.Authentication) (bool, error) { +func (m *MockAuthService) ValidateAuth(_ context.Context, authentication model.Authentication) (bool, error) { // Simple validation: for testing purposes, accept any non-empty token switch authentication.Method { case model.AuthMethodGitHub: @@ -327,10 +329,10 @@ func TestPublishIntegration(t *testing.T) { }, } - duplicateJsonData, err := json.Marshal(duplicateServerDetail) + duplicateJSONData, err := json.Marshal(duplicateServerDetail) require.NoError(t, err) - duplicateReq := httptest.NewRequest(http.MethodPost, "/v0/publish", bytes.NewBuffer(duplicateJsonData)) + duplicateReq := httptest.NewRequest(http.MethodPost, "/v0/publish", bytes.NewBuffer(duplicateJSONData)) duplicateReq.Header.Set("Content-Type", "application/json") duplicateReq.Header.Set("Authorization", "Bearer github_token_duplicate") @@ -346,7 +348,6 @@ func TestPublishIntegration(t *testing.T) { require.NoError(t, err) assert.Equal(t, firstServerDetail.Name, retrievedServer.Name) assert.Equal(t, firstServerDetail.Description, retrievedServer.Description) - }) t.Run("publish succeeds with same name but different version", func(t *testing.T) { @@ -400,10 +401,10 @@ func TestPublishIntegration(t *testing.T) { }, } - secondJsonData, err := json.Marshal(secondVersionDetail) + secondJSONData, err := json.Marshal(secondVersionDetail) require.NoError(t, err) - secondReq := httptest.NewRequest(http.MethodPost, "/v0/publish", bytes.NewBuffer(secondJsonData)) + secondReq := httptest.NewRequest(http.MethodPost, "/v0/publish", bytes.NewBuffer(secondJSONData)) secondReq.Header.Set("Content-Type", "application/json") secondReq.Header.Set("Authorization", "Bearer github_token_v2") @@ -480,10 +481,10 @@ func TestPublishIntegration(t *testing.T) { }, } - olderJsonData, err := json.Marshal(olderVersionDetail) + olderJSONData, err := json.Marshal(olderVersionDetail) require.NoError(t, err) - olderReq := httptest.NewRequest(http.MethodPost, "/v0/publish", bytes.NewBuffer(olderJsonData)) + olderReq := httptest.NewRequest(http.MethodPost, "/v0/publish", bytes.NewBuffer(olderJSONData)) olderReq.Header.Set("Content-Type", "application/json") olderReq.Header.Set("Authorization", "Bearer github_token_older") diff --git a/internal/api/handlers/v0/auth.go b/internal/api/handlers/v0/auth.go index bc3f7b3..38156d1 100644 --- a/internal/api/handlers/v0/auth.go +++ b/internal/api/handlers/v0/auth.go @@ -64,11 +64,14 @@ func StartAuthHandler(authService auth.Service) http.HandlerFunc { // Return successful response w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(map[string]interface{}{ + if err := json.NewEncoder(w).Encode(map[string]interface{}{ "flow_info": flowInfo, "status_token": statusToken, "expires_in": 300, // 5 minutes - }) + }); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } } } @@ -95,9 +98,12 @@ func CheckAuthStatusHandler(authService auth.Service) http.HandlerFunc { // Auth is still pending w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(map[string]interface{}{ + if err := json.NewEncoder(w).Encode(map[string]interface{}{ "status": "pending", - }) + }); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } return } @@ -109,9 +115,12 @@ func CheckAuthStatusHandler(authService auth.Service) http.HandlerFunc { // Authentication completed successfully w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(map[string]interface{}{ + if err := json.NewEncoder(w).Encode(map[string]interface{}{ "status": "complete", "token": token, - }) + }); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } } } diff --git a/internal/api/handlers/v0/health.go b/internal/api/handlers/v0/health.go index 07d31b6..3dd7892 100644 --- a/internal/api/handlers/v0/health.go +++ b/internal/api/handlers/v0/health.go @@ -10,16 +10,18 @@ import ( type HealthResponse struct { Status string `json:"status"` - GitHubClientId string `json:"github_client_id"` + GitHubClientID string `json:"github_client_id"` } // HealthHandler returns a handler for health check endpoint func HealthHandler(cfg *config.Config) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(HealthResponse{ + if err := json.NewEncoder(w).Encode(HealthResponse{ Status: "ok", - GitHubClientId: cfg.GithubClientID, - }) + GitHubClientID: cfg.GithubClientID, + }); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + } } } diff --git a/internal/api/handlers/v0/health_test.go b/internal/api/handlers/v0/health_test.go index d0ddece..baae604 100644 --- a/internal/api/handlers/v0/health_test.go +++ b/internal/api/handlers/v0/health_test.go @@ -1,11 +1,13 @@ -package v0 +package v0_test import ( + "context" "encoding/json" "net/http" "net/http/httptest" "testing" + v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0" "github.com/modelcontextprotocol/registry/internal/config" "github.com/stretchr/testify/assert" ) @@ -16,7 +18,7 @@ func TestHealthHandler(t *testing.T) { name string config *config.Config expectedStatus int - expectedBody HealthResponse + expectedBody v0.HealthResponse }{ { name: "returns health status with github client id", @@ -24,9 +26,9 @@ func TestHealthHandler(t *testing.T) { GithubClientID: "test-github-client-id", }, expectedStatus: http.StatusOK, - expectedBody: HealthResponse{ + expectedBody: v0.HealthResponse{ Status: "ok", - GitHubClientId: "test-github-client-id", + GitHubClientID: "test-github-client-id", }, }, { @@ -35,9 +37,9 @@ func TestHealthHandler(t *testing.T) { GithubClientID: "", }, expectedStatus: http.StatusOK, - expectedBody: HealthResponse{ + expectedBody: v0.HealthResponse{ Status: "ok", - GitHubClientId: "", + GitHubClientID: "", }, }, } @@ -45,10 +47,10 @@ func TestHealthHandler(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // Create handler with the test config - handler := HealthHandler(tc.config) + handler := v0.HealthHandler(tc.config) // Create request - req, err := http.NewRequest("GET", "/health", nil) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "/health", nil) if err != nil { t.Fatal(err) } @@ -66,7 +68,7 @@ func TestHealthHandler(t *testing.T) { assert.Equal(t, "application/json", rr.Header().Get("Content-Type")) // Parse response body - var resp HealthResponse + var resp v0.HealthResponse err = json.NewDecoder(rr.Body).Decode(&resp) assert.NoError(t, err) @@ -83,11 +85,18 @@ func TestHealthHandlerIntegration(t *testing.T) { GithubClientID: "integration-test-client-id", } - server := httptest.NewServer(HealthHandler(cfg)) + server := httptest.NewServer(v0.HealthHandler(cfg)) defer server.Close() // Send request to the test server - resp, err := http.Get(server.URL) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + + client := &http.Client{} + resp, err := client.Do(req) if err != nil { t.Fatalf("Failed to send request: %v", err) } @@ -100,14 +109,14 @@ func TestHealthHandlerIntegration(t *testing.T) { assert.Equal(t, "application/json", resp.Header.Get("Content-Type")) // Parse response body - var healthResp HealthResponse + var healthResp v0.HealthResponse err = json.NewDecoder(resp.Body).Decode(&healthResp) assert.NoError(t, err) // Check the response body - expectedResp := HealthResponse{ + expectedResp := v0.HealthResponse{ Status: "ok", - GitHubClientId: "integration-test-client-id", + GitHubClientID: "integration-test-client-id", } assert.Equal(t, expectedResp, healthResp) } diff --git a/internal/api/handlers/v0/ping.go b/internal/api/handlers/v0/ping.go index a77d622..6e9b0bc 100644 --- a/internal/api/handlers/v0/ping.go +++ b/internal/api/handlers/v0/ping.go @@ -22,6 +22,8 @@ func PingHandler(cfg *config.Config) http.HandlerFunc { } w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(response) + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + } } } diff --git a/internal/api/handlers/v0/publish.go b/internal/api/handlers/v0/publish.go index b62ac6e..dfc57cd 100644 --- a/internal/api/handlers/v0/publish.go +++ b/internal/api/handlers/v0/publish.go @@ -92,7 +92,7 @@ func PublishHandler(registry service.RegistryService, authService auth.Service) valid, err := authService.ValidateAuth(r.Context(), a) if err != nil { - if err == auth.ErrAuthRequired { + if errors.Is(err, auth.ErrAuthRequired) { http.Error(w, "Authentication is required for publishing", http.StatusUnauthorized) return } @@ -119,9 +119,12 @@ func PublishHandler(registry service.RegistryService, authService auth.Service) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(map[string]string{ + if err := json.NewEncoder(w).Encode(map[string]string{ "message": "Server publication successful", "id": serverDetail.ID, - }) + }); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } } } diff --git a/internal/api/handlers/v0/publish_test.go b/internal/api/handlers/v0/publish_test.go index 508bc36..214579a 100644 --- a/internal/api/handlers/v0/publish_test.go +++ b/internal/api/handlers/v0/publish_test.go @@ -1,4 +1,4 @@ -package v0 +package v0_test import ( "bytes" @@ -8,6 +8,7 @@ import ( "net/http/httptest" "testing" + v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0" "github.com/modelcontextprotocol/registry/internal/auth" "github.com/modelcontextprotocol/registry/internal/model" "github.com/stretchr/testify/assert" @@ -20,17 +21,17 @@ type MockRegistryService struct { } func (m *MockRegistryService) List(cursor string, limit int) ([]model.Server, string, error) { - args := m.Called(cursor, limit) + args := m.Mock.Called(cursor, limit) return args.Get(0).([]model.Server), args.String(1), args.Error(2) } func (m *MockRegistryService) GetByID(id string) (*model.ServerDetail, error) { - args := m.Called(id) + args := m.Mock.Called(id) return args.Get(0).(*model.ServerDetail), args.Error(1) } func (m *MockRegistryService) Publish(serverDetail *model.ServerDetail) error { - args := m.Called(serverDetail) + args := m.Mock.Called(serverDetail) return args.Error(0) } @@ -39,18 +40,20 @@ type MockAuthService struct { mock.Mock } -func (m *MockAuthService) StartAuthFlow(ctx context.Context, method model.AuthMethod, repoRef string) (map[string]string, string, error) { - args := m.Called(ctx, method, repoRef) +func (m *MockAuthService) StartAuthFlow( + ctx context.Context, method model.AuthMethod, repoRef string, +) (map[string]string, string, error) { + args := m.Mock.Called(ctx, method, repoRef) return args.Get(0).(map[string]string), args.String(1), args.Error(2) } func (m *MockAuthService) CheckAuthStatus(ctx context.Context, statusToken string) (string, error) { - args := m.Called(ctx, statusToken) + args := m.Mock.Called(ctx, statusToken) return args.String(0), args.Error(1) } func (m *MockAuthService) ValidateAuth(ctx context.Context, authentication model.Authentication) (bool, error) { - args := m.Called(ctx, authentication) + args := m.Mock.Called(ctx, authentication) return args.Bool(0), args.Error(1) } @@ -87,12 +90,12 @@ func TestPublishHandler(t *testing.T) { }, authHeader: "Bearer github_token_123", setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) { - authSvc.On("ValidateAuth", mock.Anything, model.Authentication{ + authSvc.Mock.On("ValidateAuth", mock.Anything, model.Authentication{ Method: model.AuthMethodGitHub, Token: "github_token_123", RepoRef: "io.github.example/test-server", }).Return(true, nil) - registry.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) + registry.Mock.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) }, expectedStatus: http.StatusCreated, expectedResponse: map[string]string{ @@ -122,12 +125,12 @@ func TestPublishHandler(t *testing.T) { }, authHeader: "Bearer some_token", setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) { - authSvc.On("ValidateAuth", mock.Anything, model.Authentication{ + authSvc.Mock.On("ValidateAuth", mock.Anything, model.Authentication{ Method: model.AuthMethodNone, Token: "some_token", RepoRef: "example/test-server", }).Return(true, nil) - registry.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) + registry.Mock.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) }, expectedStatus: http.StatusCreated, expectedResponse: map[string]string{ @@ -140,7 +143,7 @@ func TestPublishHandler(t *testing.T) { method: http.MethodGet, requestBody: nil, authHeader: "", - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) {}, + setupMocks: func(_ *MockRegistryService, _ *MockAuthService) {}, expectedStatus: http.StatusMethodNotAllowed, expectedError: "Method not allowed", }, @@ -149,7 +152,7 @@ func TestPublishHandler(t *testing.T) { method: http.MethodPost, requestBody: "", authHeader: "", - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) {}, + setupMocks: func(_ *MockRegistryService, _ *MockAuthService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Invalid request payload:", }, @@ -169,7 +172,7 @@ func TestPublishHandler(t *testing.T) { }, }, authHeader: "", - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) {}, + setupMocks: func(_ *MockRegistryService, _ *MockAuthService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Name is required", }, @@ -189,7 +192,7 @@ func TestPublishHandler(t *testing.T) { }, }, authHeader: "", - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) {}, + setupMocks: func(_ *MockRegistryService, _ *MockAuthService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Version is required", }, @@ -209,7 +212,7 @@ func TestPublishHandler(t *testing.T) { }, }, authHeader: "", // Missing auth header - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) {}, + setupMocks: func(_ *MockRegistryService, _ *MockAuthService) {}, expectedStatus: http.StatusUnauthorized, expectedError: "Authorization header is required", }, @@ -229,8 +232,8 @@ func TestPublishHandler(t *testing.T) { }, }, authHeader: "Bearer token", - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) { - authSvc.On("ValidateAuth", mock.Anything, mock.Anything).Return(false, auth.ErrAuthRequired) + setupMocks: func(_ *MockRegistryService, authSvc *MockAuthService) { + authSvc.Mock.On("ValidateAuth", mock.Anything, mock.Anything).Return(false, auth.ErrAuthRequired) }, expectedStatus: http.StatusUnauthorized, expectedError: "Authentication is required for publishing", @@ -251,8 +254,8 @@ func TestPublishHandler(t *testing.T) { }, }, authHeader: "Bearer invalid_token", - setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) { - authSvc.On("ValidateAuth", mock.Anything, mock.Anything).Return(false, nil) + setupMocks: func(_ *MockRegistryService, authSvc *MockAuthService) { + authSvc.Mock.On("ValidateAuth", mock.Anything, mock.Anything).Return(false, nil) }, expectedStatus: http.StatusUnauthorized, expectedError: "Invalid authentication credentials", @@ -274,8 +277,8 @@ func TestPublishHandler(t *testing.T) { }, authHeader: "Bearer token", setupMocks: func(registry *MockRegistryService, authSvc *MockAuthService) { - authSvc.On("ValidateAuth", mock.Anything, mock.Anything).Return(true, nil) - registry.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(assert.AnError) + authSvc.Mock.On("ValidateAuth", mock.Anything, mock.Anything).Return(true, nil) + registry.Mock.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(assert.AnError) }, expectedStatus: http.StatusInternalServerError, expectedError: "Failed to publish server details:", @@ -292,7 +295,7 @@ func TestPublishHandler(t *testing.T) { tc.setupMocks(mockRegistry, mockAuthService) // Create handler - handler := PublishHandler(mockRegistry, mockAuthService) + handler := v0.PublishHandler(mockRegistry, mockAuthService) // Prepare request body var requestBody []byte @@ -303,7 +306,7 @@ func TestPublishHandler(t *testing.T) { } // Create request - req, err := http.NewRequest(tc.method, "/publish", bytes.NewBuffer(requestBody)) + req, err := http.NewRequestWithContext(context.Background(), tc.method, "/publish", bytes.NewBuffer(requestBody)) assert.NoError(t, err) // Set auth header if provided @@ -337,8 +340,8 @@ func TestPublishHandler(t *testing.T) { } // Assert that all expectations were met - mockRegistry.AssertExpectations(t) - mockAuthService.AssertExpectations(t) + mockRegistry.Mock.AssertExpectations(t) + mockAuthService.Mock.AssertExpectations(t) }) } } @@ -377,12 +380,12 @@ func TestPublishHandlerBearerTokenParsing(t *testing.T) { mockAuthService := new(MockAuthService) // Setup mock to capture the actual token passed - mockAuthService.On("ValidateAuth", mock.Anything, mock.MatchedBy(func(auth model.Authentication) bool { + mockAuthService.Mock.On("ValidateAuth", mock.Anything, mock.MatchedBy(func(auth model.Authentication) bool { return auth.Token == tc.expectedToken })).Return(true, nil) - mockRegistry.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) + mockRegistry.Mock.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) - handler := PublishHandler(mockRegistry, mockAuthService) + handler := v0.PublishHandler(mockRegistry, mockAuthService) serverDetail := model.ServerDetail{ Server: model.Server{ @@ -400,7 +403,7 @@ func TestPublishHandlerBearerTokenParsing(t *testing.T) { requestBody, err := json.Marshal(serverDetail) assert.NoError(t, err) - req, err := http.NewRequest(http.MethodPost, "/publish", bytes.NewBuffer(requestBody)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/publish", bytes.NewBuffer(requestBody)) assert.NoError(t, err) req.Header.Set("Authorization", tc.authHeader) @@ -408,7 +411,7 @@ func TestPublishHandlerBearerTokenParsing(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusCreated, rr.Code) - mockAuthService.AssertExpectations(t) + mockAuthService.Mock.AssertExpectations(t) }) } } @@ -442,12 +445,12 @@ func TestPublishHandlerAuthMethodSelection(t *testing.T) { mockAuthService := new(MockAuthService) // Setup mock to capture the auth method - mockAuthService.On("ValidateAuth", mock.Anything, mock.MatchedBy(func(auth model.Authentication) bool { + mockAuthService.Mock.On("ValidateAuth", mock.Anything, mock.MatchedBy(func(auth model.Authentication) bool { return auth.Method == tc.expectedAuthMethod })).Return(true, nil) - mockRegistry.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) + mockRegistry.Mock.On("Publish", mock.AnythingOfType("*model.ServerDetail")).Return(nil) - handler := PublishHandler(mockRegistry, mockAuthService) + handler := v0.PublishHandler(mockRegistry, mockAuthService) serverDetail := model.ServerDetail{ Server: model.Server{ @@ -465,7 +468,7 @@ func TestPublishHandlerAuthMethodSelection(t *testing.T) { requestBody, err := json.Marshal(serverDetail) assert.NoError(t, err) - req, err := http.NewRequest(http.MethodPost, "/publish", bytes.NewBuffer(requestBody)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/publish", bytes.NewBuffer(requestBody)) assert.NoError(t, err) req.Header.Set("Authorization", "Bearer test_token") @@ -473,7 +476,7 @@ func TestPublishHandlerAuthMethodSelection(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusCreated, rr.Code) - mockAuthService.AssertExpectations(t) + mockAuthService.Mock.AssertExpectations(t) }) } } diff --git a/internal/api/handlers/v0/servers.go b/internal/api/handlers/v0/servers.go index a981882..b2fc21f 100644 --- a/internal/api/handlers/v0/servers.go +++ b/internal/api/handlers/v0/servers.go @@ -89,7 +89,10 @@ func ServersHandler(registry service.RegistryService) http.HandlerFunc { } w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(response) + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } } } @@ -123,6 +126,9 @@ func ServersDetailHandler(registry service.RegistryService) http.HandlerFunc { } w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(serverDetail) + if err := json.NewEncoder(w).Encode(serverDetail); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } } } diff --git a/internal/api/handlers/v0/servers_test.go b/internal/api/handlers/v0/servers_test.go index b1b4a1d..380a3a6 100644 --- a/internal/api/handlers/v0/servers_test.go +++ b/internal/api/handlers/v0/servers_test.go @@ -1,6 +1,7 @@ -package v0 +package v0_test import ( + "context" "encoding/json" "errors" "net/http" @@ -8,6 +9,7 @@ import ( "testing" "github.com/google/uuid" + v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0" "github.com/modelcontextprotocol/registry/internal/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -21,7 +23,7 @@ func TestServersHandler(t *testing.T) { setupMocks func(*MockRegistryService) expectedStatus int expectedServers []model.Server - expectedMeta *Metadata + expectedMeta *v0.Metadata expectedError string }{ { @@ -60,7 +62,7 @@ func TestServersHandler(t *testing.T) { }, }, } - registry.On("List", "", 30).Return(servers, "", nil) + registry.Mock.On("List", "", 30).Return(servers, "", nil) }, expectedStatus: http.StatusOK, expectedServers: []model.Server{ @@ -119,7 +121,7 @@ func TestServersHandler(t *testing.T) { }, } nextCursor := uuid.New().String() - registry.On("List", mock.AnythingOfType("string"), 10).Return(servers, nextCursor, nil) + registry.Mock.On("List", mock.AnythingOfType("string"), 10).Return(servers, nextCursor, nil) }, expectedStatus: http.StatusOK, expectedServers: []model.Server{ @@ -139,7 +141,7 @@ func TestServersHandler(t *testing.T) { }, }, }, - expectedMeta: &Metadata{ + expectedMeta: &v0.Metadata{ NextCursor: "", // This will be dynamically set in the test Count: 1, }, @@ -150,7 +152,7 @@ func TestServersHandler(t *testing.T) { queryParams: "?limit=150", setupMocks: func(registry *MockRegistryService) { servers := []model.Server{} - registry.On("List", "", 100).Return(servers, "", nil) + registry.Mock.On("List", "", 100).Return(servers, "", nil) }, expectedStatus: http.StatusOK, expectedServers: []model.Server{}, @@ -159,7 +161,7 @@ func TestServersHandler(t *testing.T) { name: "invalid cursor parameter", method: http.MethodGet, queryParams: "?cursor=invalid-uuid", - setupMocks: func(registry *MockRegistryService) {}, + setupMocks: func(_ *MockRegistryService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Invalid cursor parameter", }, @@ -167,7 +169,7 @@ func TestServersHandler(t *testing.T) { name: "invalid limit parameter - non-numeric", method: http.MethodGet, queryParams: "?limit=abc", - setupMocks: func(registry *MockRegistryService) {}, + setupMocks: func(_ *MockRegistryService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Invalid limit parameter", }, @@ -175,7 +177,7 @@ func TestServersHandler(t *testing.T) { name: "invalid limit parameter - zero", method: http.MethodGet, queryParams: "?limit=0", - setupMocks: func(registry *MockRegistryService) {}, + setupMocks: func(_ *MockRegistryService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Limit must be greater than 0", }, @@ -183,7 +185,7 @@ func TestServersHandler(t *testing.T) { name: "invalid limit parameter - negative", method: http.MethodGet, queryParams: "?limit=-5", - setupMocks: func(registry *MockRegistryService) {}, + setupMocks: func(_ *MockRegistryService) {}, expectedStatus: http.StatusBadRequest, expectedError: "Limit must be greater than 0", }, @@ -191,7 +193,7 @@ func TestServersHandler(t *testing.T) { name: "registry service error", method: http.MethodGet, setupMocks: func(registry *MockRegistryService) { - registry.On("List", "", 30).Return([]model.Server{}, "", errors.New("database connection error")) + registry.Mock.On("List", "", 30).Return([]model.Server{}, "", errors.New("database connection error")) }, expectedStatus: http.StatusInternalServerError, expectedError: "database connection error", @@ -199,7 +201,7 @@ func TestServersHandler(t *testing.T) { { name: "method not allowed", method: http.MethodPost, - setupMocks: func(registry *MockRegistryService) {}, + setupMocks: func(_ *MockRegistryService) {}, expectedStatus: http.StatusMethodNotAllowed, expectedError: "Method not allowed", }, @@ -212,11 +214,11 @@ func TestServersHandler(t *testing.T) { tc.setupMocks(mockRegistry) // Create handler - handler := ServersHandler(mockRegistry) + handler := v0.ServersHandler(mockRegistry) // Create request url := "/v0/servers" + tc.queryParams - req, err := http.NewRequest(tc.method, url, nil) + req, err := http.NewRequestWithContext(context.Background(), tc.method, url, nil) if err != nil { t.Fatal(err) } @@ -235,7 +237,7 @@ func TestServersHandler(t *testing.T) { assert.Equal(t, "application/json", rr.Header().Get("Content-Type")) // Parse response body - var resp PaginatedResponse + var resp v0.PaginatedResponse err = json.NewDecoder(rr.Body).Decode(&resp) assert.NoError(t, err) @@ -249,15 +251,13 @@ func TestServersHandler(t *testing.T) { assert.NotEmpty(t, resp.Metadata.NextCursor) } } - } else { + } else if tc.expectedError != "" { // Check error message for non-200 responses - if tc.expectedError != "" { - assert.Contains(t, rr.Body.String(), tc.expectedError) - } + assert.Contains(t, rr.Body.String(), tc.expectedError) } // Verify mock expectations - mockRegistry.AssertExpectations(t) + mockRegistry.Mock.AssertExpectations(t) }) } } @@ -285,14 +285,21 @@ func TestServersHandlerIntegration(t *testing.T) { }, } - mockRegistry.On("List", "", 30).Return(servers, "", nil) + mockRegistry.Mock.On("List", "", 30).Return(servers, "", nil) // Create test server - server := httptest.NewServer(ServersHandler(mockRegistry)) + server := httptest.NewServer(v0.ServersHandler(mockRegistry)) defer server.Close() // Send request to the test server - resp, err := http.Get(server.URL) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + + client := &http.Client{} + resp, err := client.Do(req) if err != nil { t.Fatalf("Failed to send request: %v", err) } @@ -305,7 +312,7 @@ func TestServersHandlerIntegration(t *testing.T) { assert.Equal(t, "application/json", resp.Header.Get("Content-Type")) // Parse response body - var paginatedResp PaginatedResponse + var paginatedResp v0.PaginatedResponse err = json.NewDecoder(resp.Body).Decode(&paginatedResp) assert.NoError(t, err) @@ -314,7 +321,7 @@ func TestServersHandlerIntegration(t *testing.T) { assert.Empty(t, paginatedResp.Metadata.NextCursor) // Verify mock expectations - mockRegistry.AssertExpectations(t) + mockRegistry.Mock.AssertExpectations(t) } // TestServersDetailHandlerIntegration tests the servers detail handler with actual HTTP requests @@ -342,17 +349,24 @@ func TestServersDetailHandlerIntegration(t *testing.T) { }, } - mockRegistry.On("GetByID", serverID).Return(serverDetail, nil) + mockRegistry.Mock.On("GetByID", serverID).Return(serverDetail, nil) // Create test server server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { r.SetPathValue("id", serverID) - ServersDetailHandler(mockRegistry).ServeHTTP(w, r) + v0.ServersDetailHandler(mockRegistry).ServeHTTP(w, r) })) defer server.Close() // Send request to the test server - resp, err := http.Get(server.URL) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + + client := &http.Client{} + resp, err := client.Do(req) if err != nil { t.Fatalf("Failed to send request: %v", err) } @@ -373,5 +387,5 @@ func TestServersDetailHandlerIntegration(t *testing.T) { assert.Equal(t, *serverDetail, serverDetailResp) // Verify mock expectations - mockRegistry.AssertExpectations(t) + mockRegistry.Mock.AssertExpectations(t) } diff --git a/internal/api/handlers/v0/swagger.go b/internal/api/handlers/v0/swagger.go index fea5b29..6a368f8 100644 --- a/internal/api/handlers/v0/swagger.go +++ b/internal/api/handlers/v0/swagger.go @@ -6,7 +6,7 @@ import ( "os" "path/filepath" - _ "github.com/swaggo/files" + _ "github.com/swaggo/files" // Swagger files needed for embedding httpSwagger "github.com/swaggo/http-swagger" ) diff --git a/internal/api/router/v0.go b/internal/api/router/v0.go index 3564d7e..6d465f9 100644 --- a/internal/api/router/v0.go +++ b/internal/api/router/v0.go @@ -11,7 +11,9 @@ import ( ) // RegisterV0Routes registers all v0 API routes to the provided router -func RegisterV0Routes(mux *http.ServeMux, cfg *config.Config, registry service.RegistryService, authService auth.Service) { +func RegisterV0Routes( + mux *http.ServeMux, cfg *config.Config, registry service.RegistryService, authService auth.Service, +) { // Register v0 endpoints mux.HandleFunc("/v0/health", v0.HealthHandler(cfg)) mux.HandleFunc("/v0/servers", v0.ServersHandler(registry)) diff --git a/internal/api/server.go b/internal/api/server.go index 5371582..c92a1a5 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -4,6 +4,7 @@ import ( "context" "log" "net/http" + "time" "github.com/modelcontextprotocol/registry/internal/api/router" "github.com/modelcontextprotocol/registry/internal/auth" @@ -31,8 +32,9 @@ func NewServer(cfg *config.Config, registryService service.RegistryService, auth authService: authService, router: mux, server: &http.Server{ - Addr: cfg.ServerAddress, - Handler: mux, + Addr: cfg.ServerAddress, + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, }, } diff --git a/internal/auth/github.go b/internal/auth/github.go index ce1daf6..ac81303 100644 --- a/internal/auth/github.go +++ b/internal/auth/github.go @@ -3,6 +3,7 @@ package auth import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -70,14 +71,19 @@ func NewGitHubDeviceAuth(config GitHubOAuthConfig) *GitHubDeviceAuth { // It verifies the token owner matches the repository owner or is a member of the owning organization. // It also verifies that the token was created for the same ClientID used to set up the authentication. // Returns true if valid, false otherwise along with an error explaining the validation failure. -func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (bool, error) { +func (g *GitHubDeviceAuth) ValidateToken(ctx context.Context, token string, requiredRepo string) (bool, error) { // If no repo is required, we can't validate properly if requiredRepo == "" { return false, fmt.Errorf("repository reference is required for token validation") } // First, validate that the token is associated with our ClientID - tokenReq, err := http.NewRequest("GET", "https://api.github.com/applications/"+g.config.ClientID+"/token", nil) + tokenReq, err := http.NewRequestWithContext( + ctx, + http.MethodGet, + "https://api.github.com/applications/"+g.config.ClientID+"/token", + nil, + ) if err != nil { return false, err } @@ -97,7 +103,8 @@ func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (boo } // POST instead of GET for security reasons per GitHub API - tokenReq, err = http.NewRequest("POST", "https://api.github.com/applications/"+g.config.ClientID+"/token", io.NopCloser(bytes.NewReader(checkBody))) + tokenURL := "https://api.github.com/applications/" + g.config.ClientID + "/token" + tokenReq, err = http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, io.NopCloser(bytes.NewReader(checkBody))) if err != nil { return false, err } @@ -135,7 +142,7 @@ func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (boo } // Get the authenticated user - userReq, err := http.NewRequest("GET", "https://api.github.com/user", nil) + userReq, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://api.github.com/user", nil) if err != nil { return false, err } @@ -175,17 +182,20 @@ func (g *GitHubDeviceAuth) ValidateToken(token string, requiredRepo string) (boo // Verify that the authenticated user matches the owner if userInfo.Login != owner { // Check if the user is a member of the organization - isMember, err := g.checkOrgMembership(token, userInfo.Login, owner) + isMember, err := g.checkOrgMembership(ctx, token, userInfo.Login, owner) if err != nil { return false, fmt.Errorf("failed to check org membership: %s", owner) } if !isMember { - return false, fmt.Errorf("token belongs to user %s, but repository is owned by %s and user is not a member of the organization", userInfo.Login, owner) + return false, fmt.Errorf( + "token belongs to user %s, but repository is owned by %s and user is not a member of the organization", + userInfo.Login, owner) } } - // If we've reached this point, the token has access the repo and the user matches the owner or is a member of the owner org + // If we've reached this point, the token has access the repo and the user matches + // the owner or is a member of the owner org return true, nil } @@ -210,13 +220,13 @@ func (g *GitHubDeviceAuth) ExtractGitHubRepo(repoURL string) (owner, repo string } // checkOrgMembership checks if a user is a member of an organization -func (g *GitHubDeviceAuth) checkOrgMembership(token, username, org string) (bool, error) { +func (g *GitHubDeviceAuth) checkOrgMembership(ctx context.Context, token, username, org string) (bool, error) { // Create request to check if user is a member of the organization // GitHub API endpoint: GET /orgs/{org}/members/{username} // true if status code is 204 No Content // false if status code is 404 Not Found url := fmt.Sprint("https://api.github.com/orgs/", org, "/members/", username) - req, err := http.NewRequest("GET", url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return false, err } diff --git a/internal/auth/service.go b/internal/auth/service.go index 54a9f01..d8fdf70 100644 --- a/internal/auth/service.go +++ b/internal/auth/service.go @@ -8,37 +8,40 @@ import ( "github.com/modelcontextprotocol/registry/internal/model" ) -// AuthServiceImpl implements the Service interface -type AuthServiceImpl struct { +// ServiceImpl implements the Service interface +type ServiceImpl struct { config *config.Config githubAuth *GitHubDeviceAuth } // NewAuthService creates a new authentication service +// +//nolint:ireturn // Factory function intentionally returns interface for dependency injection func NewAuthService(cfg *config.Config) Service { githubConfig := GitHubOAuthConfig{ ClientID: cfg.GithubClientID, ClientSecret: cfg.GithubClientSecret, } - return &AuthServiceImpl{ + return &ServiceImpl{ config: cfg, githubAuth: NewGitHubDeviceAuth(githubConfig), } } -func (s *AuthServiceImpl) StartAuthFlow(ctx context.Context, method model.AuthMethod, repoRef string) (map[string]string, string, error) { +func (s *ServiceImpl) StartAuthFlow(_ context.Context, _ model.AuthMethod, + _ string) (map[string]string, string, error) { // return not implemented error return nil, "", fmt.Errorf("not implemented") } -func (s *AuthServiceImpl) CheckAuthStatus(ctx context.Context, statusToken string) (string, error) { +func (s *ServiceImpl) CheckAuthStatus(_ context.Context, _ string) (string, error) { // return not implemented error return "", fmt.Errorf("not implemented") } // ValidateAuth validates authentication credentials -func (s *AuthServiceImpl) ValidateAuth(ctx context.Context, auth model.Authentication) (bool, error) { +func (s *ServiceImpl) ValidateAuth(ctx context.Context, auth model.Authentication) (bool, error) { // If authentication is required but not provided if auth.Method == "" || auth.Method == model.AuthMethodNone { return false, ErrAuthRequired @@ -47,7 +50,9 @@ func (s *AuthServiceImpl) ValidateAuth(ctx context.Context, auth model.Authentic switch auth.Method { case model.AuthMethodGitHub: // Extract repo reference from the repository URL if it's not provided - return s.githubAuth.ValidateToken(auth.Token, auth.RepoRef) + return s.githubAuth.ValidateToken(ctx, auth.Token, auth.RepoRef) + case model.AuthMethodNone: + return false, ErrAuthRequired default: return false, ErrUnsupportedAuthMethod } diff --git a/internal/config/config.go b/internal/config/config.go index 01db852..cbb5637 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,7 @@ package config import ( - "github.com/caarlos0/env/v11" + env "github.com/caarlos0/env/v11" ) // Config holds the application configuration diff --git a/internal/database/import.go b/internal/database/import.go index db81fc0..ee70f10 100644 --- a/internal/database/import.go +++ b/internal/database/import.go @@ -33,7 +33,7 @@ func ImportSeedFile(mongo *MongoDB, seedFilePath string) error { // Read the seed file seedData, err := readSeedFile(seedFilePath) if err != nil { - log.Fatalf("Failed to read seed file: %v", err) + return fmt.Errorf("failed to read seed file: %w", err) } collection := mongo.collection @@ -48,7 +48,7 @@ func readSeedFile(path string) ([]model.ServerDetail, error) { // Read the file content fileContent, err := os.ReadFile(path) if err != nil { - return nil, fmt.Errorf("failed to read file: %v", err) + return nil, fmt.Errorf("failed to read file: %w", err) } // Parse the JSON content @@ -57,9 +57,8 @@ func readSeedFile(path string) ([]model.ServerDetail, error) { // Try parsing as a raw JSON array and then convert to our model var rawData []map[string]interface{} if jsonErr := json.Unmarshal(fileContent, &rawData); jsonErr != nil { - return nil, fmt.Errorf("failed to parse JSON: %v (original error: %v)", jsonErr, err) + return nil, fmt.Errorf("failed to parse JSON: %w (original error: %w)", jsonErr, err) } - } log.Printf("Found %d server entries in seed file", len(servers)) @@ -82,7 +81,6 @@ func importData(ctx context.Context, collection *mongo.Collection, servers []mod server.VersionDetail.Version = "0.0.1-seed" server.VersionDetail.ReleaseDate = time.Now().Format(time.RFC3339) server.VersionDetail.IsLatest = true - } // Create update document update := bson.M{"$set": server} @@ -95,11 +93,12 @@ func importData(ctx context.Context, collection *mongo.Collection, servers []mod continue } - if result.UpsertedCount > 0 { + switch { + case result.UpsertedCount > 0: log.Printf("[%d/%d] Created server: %s", i+1, len(servers), server.Name) - } else if result.ModifiedCount > 0 { + case result.ModifiedCount > 0: log.Printf("[%d/%d] Updated server: %s", i+1, len(servers), server.Name) - } else { + default: log.Printf("[%d/%d] Server already up to date: %s", i+1, len(servers), server.Name) } } diff --git a/internal/database/memory.go b/internal/database/memory.go index 15f595e..5494b99 100644 --- a/internal/database/memory.go +++ b/internal/database/memory.go @@ -84,7 +84,14 @@ func compareSemanticVersions(version1, version2 string) int { } // List retrieves all MCPRegistry entries with optional filtering and pagination -func (db *MemoryDB) List(ctx context.Context, filter map[string]interface{}, cursor string, limit int) ([]*model.Server, string, error) { +// +//gocognit:ignore +func (db *MemoryDB) List( + ctx context.Context, + filter map[string]interface{}, + cursor string, + limit int, +) ([]*model.Server, string, error) { if ctx.Err() != nil { return nil, "", ctx.Err() } @@ -109,27 +116,25 @@ func (db *MemoryDB) List(ctx context.Context, filter map[string]interface{}, cur include := true // Apply filters if any - if filter != nil { - for key, value := range filter { - switch key { - case "name": - if entry.Name != value.(string) { - include = false - } - case "repoUrl": - if entry.Repository.URL != value.(string) { - include = false - } - case "serverDetail.id": - if entry.ID != value.(string) { - include = false - } - case "version": - if entry.VersionDetail.Version != value.(string) { - include = false - } - // Add more filter options as needed + for key, value := range filter { + switch key { + case "name": + if entry.Name != value.(string) { + include = false } + case "repoUrl": + if entry.Repository.URL != value.(string) { + include = false + } + case "serverDetail.id": + if entry.ID != value.(string) { + include = false + } + case "version": + if entry.VersionDetail.Version != value.(string) { + include = false + } + // Add more filter options as needed } } diff --git a/internal/database/mongo.go b/internal/database/mongo.go index ef0eb77..a553d0f 100644 --- a/internal/database/mongo.go +++ b/internal/database/mongo.go @@ -2,6 +2,7 @@ package database import ( "context" + "errors" "fmt" "log" "time" @@ -41,15 +42,15 @@ func NewMongoDB(ctx context.Context, connectionURI, databaseName, collectionName // Create indexes for better query performance models := []mongo.IndexModel{ { - Keys: bson.D{{Key: "name", Value: 1}}, + Keys: bson.D{bson.E{Key: "name", Value: 1}}, }, { - Keys: bson.D{{Key: "id", Value: 1}}, + Keys: bson.D{bson.E{Key: "id", Value: 1}}, Options: options.Index().SetUnique(true), }, // add an index for the combination of name and version { - Keys: bson.D{{Key: "name", Value: 1}, {Key: "versiondetail.version", Value: 1}}, + Keys: bson.D{bson.E{Key: "name", Value: 1}, bson.E{Key: "versiondetail.version", Value: 1}}, Options: options.Index().SetUnique(true), }, } @@ -57,11 +58,11 @@ func NewMongoDB(ctx context.Context, connectionURI, databaseName, collectionName _, err = collection.Indexes().CreateMany(ctx, models) if err != nil { // Mongo will error if the index already exists, we can ignore this and continue. - if err.(mongo.CommandError).Code != 86 { + var commandError mongo.CommandError + if errors.As(err, &commandError) && commandError.Code != 86 { return nil, err - } else { - log.Printf("Indexes already exists, skipping.") } + log.Printf("Indexes already exists, skipping.") } return &MongoDB{ @@ -72,7 +73,12 @@ func NewMongoDB(ctx context.Context, connectionURI, databaseName, collectionName } // List retrieves MCPRegistry entries with optional filtering and pagination -func (db *MongoDB) List(ctx context.Context, filter map[string]interface{}, cursor string, limit int) ([]*model.Server, string, error) { +func (db *MongoDB) List( + ctx context.Context, + filter map[string]interface{}, + cursor string, + limit int, +) ([]*model.Server, string, error) { if limit <= 0 { // Set default limit if not provided limit = 10 @@ -113,11 +119,10 @@ func (db *MongoDB) List(ctx context.Context, filter map[string]interface{}, curs var cursorDoc model.Server err := db.collection.FindOne(ctx, bson.M{"id": cursor}).Decode(&cursorDoc) if err != nil { - if err == mongo.ErrNoDocuments { - // If cursor document not found, start from beginning - } else { + if !errors.Is(err, mongo.ErrNoDocuments) { return nil, "", err } + // If cursor document not found, start from beginning } else { // Use the cursor document's ID to paginate (records with ID > cursor's ID) mongoFilter["id"] = bson.M{"$gt": cursor} @@ -168,7 +173,7 @@ func (db *MongoDB) GetByID(ctx context.Context, id string) (*model.ServerDetail, var entry model.ServerDetail err := db.collection.FindOne(ctx, filter).Decode(&entry) if err != nil { - if err == mongo.ErrNoDocuments { + if errors.Is(err, mongo.ErrNoDocuments) { return nil, ErrNotFound } return nil, fmt.Errorf("error retrieving entry: %w", err) @@ -191,7 +196,7 @@ func (db *MongoDB) Publish(ctx context.Context, serverDetail *model.ServerDetail var existingEntry model.ServerDetail err := db.collection.FindOne(ctx, filter).Decode(&existingEntry) - if err != nil && err != mongo.ErrNoDocuments { + if err != nil && !errors.Is(err, mongo.ErrNoDocuments) { return fmt.Errorf("error checking existing entry: %w", err) } @@ -215,11 +220,13 @@ func (db *MongoDB) Publish(ctx context.Context, serverDetail *model.ServerDetail // update the existing entry to not be the latest version if existingEntry.ID != "" { - _, err = db.collection.UpdateOne(ctx, bson.M{"id": existingEntry.ID}, bson.M{"$set": bson.M{"versiondetail.islatest": false}}) + _, err = db.collection.UpdateOne( + ctx, + bson.M{"id": existingEntry.ID}, + bson.M{"$set": bson.M{"versiondetail.islatest": false}}) if err != nil { return fmt.Errorf("error updating existing entry: %w", err) } - } return nil diff --git a/internal/service/fake_service.go b/internal/service/fake_service.go index ba9c1a1..07aa805 100644 --- a/internal/service/fake_service.go +++ b/internal/service/fake_service.go @@ -15,6 +15,8 @@ type fakeRegistryService struct { } // NewFakeRegistryService creates a new fake registry service with pre-populated data +// +//nolint:ireturn // Factory function intentionally returns interface for dependency injection func NewFakeRegistryService() RegistryService { // Sample registry entries with updated model structure registries := []*model.Server{ diff --git a/internal/service/registry_service.go b/internal/service/registry_service.go index 409d21c..d9798be 100644 --- a/internal/service/registry_service.go +++ b/internal/service/registry_service.go @@ -14,6 +14,8 @@ type registryServiceImpl struct { } // NewRegistryServiceWithDB creates a new registry service with the provided database +// +//nolint:ireturn // Factory function intentionally returns interface for dependency injection func NewRegistryServiceWithDB(db database.Database) RegistryService { return ®istryServiceImpl{ db: db, diff --git a/tools/publisher/main.go b/tools/publisher/main.go index 9ed544e..dcde229 100644 --- a/tools/publisher/main.go +++ b/tools/publisher/main.go @@ -2,10 +2,12 @@ package main import ( "bytes" + "context" "encoding/json" "flag" "fmt" "io" + "log" "net/http" "os" "strings" @@ -13,11 +15,10 @@ import ( ) const ( - tokenFilePath = ".mcpregistry_token" - + tokenFilePath = ".mcpregistry_token" // #nosec:G101 // GitHub OAuth URLs - GitHubDeviceCodeURL = "https://github.com/login/device/code" - GitHubAccessTokenURL = "https://github.com/login/oauth/access_token" + GitHubDeviceCodeURL = "https://github.com/login/device/code" // #nosec:G101 + GitHubAccessTokenURL = "https://github.com/login/oauth/access_token" // #nosec:G101 ) // DeviceCodeResponse represents the response from GitHub's device code endpoint @@ -39,7 +40,7 @@ type AccessTokenResponse struct { type ServerHealthResponse struct { Status string `json:"status"` - GitHubClientId string `json:"github_client_id"` + GitHubClientID string `json:"github_client_id"` } func main() { @@ -62,29 +63,36 @@ func main() { // get the clientID from the server's health endpoint healthURL := registryURL + "/v0/health" - resp, err := http.Get(healthURL) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, healthURL, nil) if err != nil { - fmt.Printf("Error fetching health endpoint: %s\n", err.Error()) + log.Printf("Error creating request: %s\n", err.Error()) + return + } + + client := &http.Client{} + resp, err := client.Do(req) + if err != nil { + log.Printf("Error fetching health endpoint: %s\n", err.Error()) return } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) - fmt.Printf("Health endpoint returned status %d: %s\n", resp.StatusCode, body) + log.Printf("Health endpoint returned status %d: %s\n", resp.StatusCode, body) return } var healthResponse ServerHealthResponse err = json.NewDecoder(resp.Body).Decode(&healthResponse) if err != nil { - fmt.Printf("Error decoding health response: %s\n", err.Error()) + log.Printf("Error decoding health response: %s\n", err.Error()) return } - if healthResponse.GitHubClientId == "" { - fmt.Println("GitHub Client ID is not set in the server's health response.") + if healthResponse.GitHubClientID == "" { + log.Println("GitHub Client ID is not set in the server's health response.") return } - githubClientID := healthResponse.GitHubClientId + githubClientID := healthResponse.GitHubClientID var token string @@ -97,7 +105,7 @@ func main() { if forceLogin || os.IsNotExist(statErr) { err := performDeviceFlowLogin(githubClientID) if err != nil { - fmt.Printf("Failed to perform device flow login: %s\n", err.Error()) + log.Printf("Failed to perform device flow login: %s\n", err.Error()) return } } @@ -106,7 +114,7 @@ func main() { var err error token, err = readToken() if err != nil { - fmt.Printf("Error reading token: %s\n", err.Error()) + log.Printf("Error reading token: %s\n", err.Error()) return } } @@ -114,22 +122,21 @@ func main() { // Read MCP file mcpData, err := os.ReadFile(mcpFilePath) if err != nil { - fmt.Printf("Error reading MCP file: %s\n", err.Error()) + log.Printf("Error reading MCP file: %s\n", err.Error()) return } // Publish to registry err = publishToRegistry(registryURL, mcpData, token) if err != nil { - fmt.Printf("Failed to publish to registry: %s\n", err.Error()) + log.Printf("Failed to publish to registry: %s\n", err.Error()) return } - fmt.Println("Successfully published to registry!") + log.Println("Successfully published to registry!") } func performDeviceFlowLogin(githubClientID string) error { - if githubClientID == "" { return fmt.Errorf("GitHub Client ID is required for device flow login") } @@ -142,13 +149,13 @@ func performDeviceFlowLogin(githubClientID string) error { } // Display instructions to the user - fmt.Println("\nTo authenticate, please:") - fmt.Println("1. Go to:", verificationURI) - fmt.Println("2. Enter code:", userCode) - fmt.Println("3. Authorize this application") + log.Println("\nTo authenticate, please:") + log.Println("1. Go to:", verificationURI) + log.Println("2. Enter code:", userCode) + log.Println("3. Authorize this application") // Poll for the token - fmt.Println("Waiting for authorization...") + log.Println("Waiting for authorization...") token, err := pollForToken(deviceCode, githubClientID) if err != nil { return fmt.Errorf("error polling for token: %w", err) @@ -160,7 +167,7 @@ func performDeviceFlowLogin(githubClientID string) error { return fmt.Errorf("error saving token: %w", err) } - fmt.Println("Successfully authenticated!") + log.Println("Successfully authenticated!") return nil } @@ -180,7 +187,7 @@ func requestDeviceCode(githubClientID string) (string, string, string, error) { return "", "", "", err } - req, err := http.NewRequest("POST", GitHubDeviceCodeURL, bytes.NewBuffer(jsonData)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, GitHubDeviceCodeURL, bytes.NewBuffer(jsonData)) if err != nil { return "", "", "", err } @@ -235,7 +242,7 @@ func pollForToken(deviceCode, githubClientID string) (string, error) { deadline := time.Now().Add(time.Duration(expiresIn) * time.Second) for time.Now().Before(deadline) { - req, err := http.NewRequest("POST", GitHubAccessTokenURL, bytes.NewBuffer(jsonData)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, GitHubAccessTokenURL, bytes.NewBuffer(jsonData)) if err != nil { return "", err } @@ -262,7 +269,6 @@ func pollForToken(deviceCode, githubClientID string) (string, error) { if tokenResp.Error == "authorization_pending" { // User hasn't authorized yet, wait and retry - fmt.Print(".") time.Sleep(time.Duration(interval) * time.Second) continue } @@ -272,7 +278,6 @@ func pollForToken(deviceCode, githubClientID string) (string, error) { } if tokenResp.AccessToken != "" { - fmt.Println() // Add newline after dots return tokenResp.AccessToken, nil } @@ -322,7 +327,7 @@ func publishToRegistry(registryURL string, mcpData []byte, token string) error { publishURL := registryURL + "v0/publish" // Create and send the request - req, err := http.NewRequest("POST", publishURL, bytes.NewBuffer(jsonData)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, publishURL, bytes.NewBuffer(jsonData)) if err != nil { return fmt.Errorf("error creating request: %w", err) } @@ -346,6 +351,6 @@ func publishToRegistry(registryURL string, mcpData []byte, token string) error { return fmt.Errorf("publication failed with status %d: %s", resp.StatusCode, body) } - println(string(body)) + log.Println(string(body)) return nil }