Skip to content

Fix #463: Omit empty repository from JSON output for remote-only servers #466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+288 −2
Open

Fix #463: Omit empty repository from JSON output for remote-only servers #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions pkg/api/v0/marshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of custom marshalling could we handle this in the database retrieval methods? After unmarshalling ServerJSON in ./internal/database/postgres.go and ./internal/database/memory.go have something like:

// Clean empty repository to make omitempty work
if serverJSON.Repository.URL == "" && serverJSON.Repository.Source == "" &&
   serverJSON.Repository.ID == "" && serverJSON.Repository.Subfolder == "" {
    serverJSON.Repository = model.Repository{}
}

Your current approach works fine too though! My only reasoning is this would be simpler to maintain and leverage the existing:

Repository    model.Repository    `json:"repository,omitempty"`

rather than a custom marshalling logic. What do you think?

// 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))
}
124 changes: 124 additions & 0 deletions pkg/api/v0/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
}
}
4 changes: 2 additions & 2 deletions pkg/model/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
113 changes: 113 additions & 0 deletions pkg/model/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Loading