From 021635e6a818cb351ee9a169edea7093d7c5b969 Mon Sep 17 00:00:00 2001 From: jalateras Date: Sat, 13 Sep 2025 09:06:56 +1000 Subject: [PATCH] fix: omit empty repository from JSON output for remote-only servers - Add omitempty tags to Repository struct fields (URL and Source) - Implement custom MarshalJSON for ServerJSON to handle empty repositories - Add comprehensive tests for JSON marshaling behavior - Ensure backward compatibility with existing data This prevents empty repository objects with blank URL and source fields from being included in the JSON output for remote-only servers, reducing unnecessary noise in the API responses. Fixes #463 --- pkg/api/v0/marshal.go | 49 ++++++++++++++++ pkg/api/v0/types_test.go | 124 +++++++++++++++++++++++++++++++++++++++ pkg/model/types.go | 4 +- pkg/model/types_test.go | 113 +++++++++++++++++++++++++++++++++++ 4 files changed, 288 insertions(+), 2 deletions(-) create mode 100644 pkg/api/v0/marshal.go create mode 100644 pkg/api/v0/types_test.go create mode 100644 pkg/model/types_test.go diff --git a/pkg/api/v0/marshal.go b/pkg/api/v0/marshal.go new file mode 100644 index 00000000..73ecf10d --- /dev/null +++ b/pkg/api/v0/marshal.go @@ -0,0 +1,49 @@ +package v0 + +import ( + "encoding/json" + + "github.com/modelcontextprotocol/registry/pkg/model" +) + +// MarshalJSON implements custom JSON marshaling for ServerJSON to properly handle empty repositories +func (s ServerJSON) MarshalJSON() ([]byte, error) { + // Check if repository is empty (all fields are zero values) + isEmptyRepo := s.Repository.URL == "" && s.Repository.Source == "" && + s.Repository.ID == "" && s.Repository.Subfolder == "" + + // Create an alias type to avoid infinite recursion + type Alias ServerJSON + + if isEmptyRepo { + // Create a version without the repository field + type ServerJSONNoRepo struct { + Schema string `json:"$schema,omitempty"` + Name string `json:"name"` + Description string `json:"description"` + Status model.Status `json:"status,omitempty"` + Version string `json:"version"` + WebsiteURL string `json:"website_url,omitempty"` + Packages []model.Package `json:"packages,omitempty"` + Remotes []model.Transport `json:"remotes,omitempty"` + Meta *ServerMeta `json:"_meta,omitempty"` + } + + noRepo := ServerJSONNoRepo{ + Schema: s.Schema, + Name: s.Name, + Description: s.Description, + Status: s.Status, + Version: s.Version, + WebsiteURL: s.WebsiteURL, + Packages: s.Packages, + Remotes: s.Remotes, + Meta: s.Meta, + } + + return json.Marshal(noRepo) + } + + // If repository is not empty, use default marshaling + return json.Marshal(Alias(s)) +} \ No newline at end of file diff --git a/pkg/api/v0/types_test.go b/pkg/api/v0/types_test.go new file mode 100644 index 00000000..54eb1059 --- /dev/null +++ b/pkg/api/v0/types_test.go @@ -0,0 +1,124 @@ +package v0_test + +import ( + "encoding/json" + "testing" + + apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" + "github.com/modelcontextprotocol/registry/pkg/model" +) + +func TestServerJSON_OmitsEmptyRepository(t *testing.T) { + tests := []struct { + name string + server apiv0.ServerJSON + wantRepo bool // whether repository field should be in the JSON + }{ + { + name: "server with empty repository", + server: apiv0.ServerJSON{ + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + Repository: model.Repository{}, // empty repository + Remotes: []model.Transport{ + { + Type: "streamable-http", + URL: "https://example.com/mcp", + }, + }, + }, + wantRepo: false, + }, + { + name: "server with repository containing empty strings", + server: apiv0.ServerJSON{ + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + Repository: model.Repository{ + URL: "", + Source: "", + }, + Remotes: []model.Transport{ + { + Type: "streamable-http", + URL: "https://example.com/mcp", + }, + }, + }, + wantRepo: false, + }, + { + name: "server with valid repository", + server: apiv0.ServerJSON{ + Name: "com.example/test-server", + Description: "A test server", + Version: "1.0.0", + Repository: model.Repository{ + URL: "https://github.com/owner/repo", + Source: "github", + }, + }, + wantRepo: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data, err := json.Marshal(tt.server) + if err != nil { + t.Fatalf("Failed to marshal server: %v", err) + } + + var result map[string]interface{} + err = json.Unmarshal(data, &result) + if err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + _, hasRepo := result["repository"] + if hasRepo != tt.wantRepo { + if tt.wantRepo { + t.Errorf("Expected repository field to be present in JSON, but it was missing") + } else { + t.Errorf("Expected repository field to be omitted from JSON, but it was present: %s", string(data)) + } + } + }) + } +} + +func TestServerJSON_RemoteOnlyServer(t *testing.T) { + // This test specifically addresses issue #463 + server := apiv0.ServerJSON{ + Name: "com.example/remote-server", + Description: "A remote-only MCP server", + Version: "1.0.0", + Remotes: []model.Transport{ + { + Type: "streamable-http", + URL: "https://api.example.com/mcp", + }, + }, + // No repository field set - should be omitted from JSON + } + + data, err := json.Marshal(server) + if err != nil { + t.Fatalf("Failed to marshal server: %v", err) + } + + jsonStr := string(data) + + // Check that the JSON doesn't contain a repository field + var result map[string]interface{} + err = json.Unmarshal(data, &result) + if err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if _, hasRepo := result["repository"]; hasRepo { + t.Errorf("Remote-only server should not have repository field in JSON output.\nGot: %s", jsonStr) + } +} \ No newline at end of file diff --git a/pkg/model/types.go b/pkg/model/types.go index bcb3baf1..7882dbc6 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -35,8 +35,8 @@ type Package struct { // Repository represents a source code repository as defined in the spec type Repository struct { - URL string `json:"url"` - Source string `json:"source"` + URL string `json:"url,omitempty"` + Source string `json:"source,omitempty"` ID string `json:"id,omitempty"` Subfolder string `json:"subfolder,omitempty"` } diff --git a/pkg/model/types_test.go b/pkg/model/types_test.go new file mode 100644 index 00000000..21855da7 --- /dev/null +++ b/pkg/model/types_test.go @@ -0,0 +1,113 @@ +package model_test + +import ( + "encoding/json" + "testing" + + "github.com/modelcontextprotocol/registry/pkg/model" +) + +func TestRepository_JSONMarshal_OmitsEmptyFields(t *testing.T) { + tests := []struct { + name string + repo model.Repository + expected string + }{ + { + name: "empty repository", + repo: model.Repository{}, + expected: `{}`, + }, + { + name: "repository with only URL", + repo: model.Repository{ + URL: "https://github.com/owner/repo", + }, + expected: `{"url":"https://github.com/owner/repo"}`, + }, + { + name: "repository with only source", + repo: model.Repository{ + Source: "github", + }, + expected: `{"source":"github"}`, + }, + { + name: "repository with all fields", + repo: model.Repository{ + URL: "https://github.com/owner/repo", + Source: "github", + ID: "owner/repo", + Subfolder: "src", + }, + expected: `{"url":"https://github.com/owner/repo","source":"github","id":"owner/repo","subfolder":"src"}`, + }, + { + name: "repository with empty strings", + repo: model.Repository{ + URL: "", + Source: "", + }, + expected: `{}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data, err := json.Marshal(tt.repo) + if err != nil { + t.Fatalf("Failed to marshal repository: %v", err) + } + + actual := string(data) + if actual != tt.expected { + t.Errorf("Expected JSON %s, got %s", tt.expected, actual) + } + }) + } +} + +func TestRepository_JSONUnmarshal_HandlesEmptyFields(t *testing.T) { + tests := []struct { + name string + json string + expected model.Repository + }{ + { + name: "empty JSON object", + json: `{}`, + expected: model.Repository{}, + }, + { + name: "JSON with only URL", + json: `{"url":"https://github.com/owner/repo"}`, + expected: model.Repository{ + URL: "https://github.com/owner/repo", + }, + }, + { + name: "JSON with all fields", + json: `{"url":"https://github.com/owner/repo","source":"github","id":"owner/repo","subfolder":"src"}`, + expected: model.Repository{ + URL: "https://github.com/owner/repo", + Source: "github", + ID: "owner/repo", + Subfolder: "src", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var repo model.Repository + err := json.Unmarshal([]byte(tt.json), &repo) + if err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if repo != tt.expected { + t.Errorf("Expected repository %+v, got %+v", tt.expected, repo) + } + }) + } +} \ No newline at end of file