Repository navigation
Conversation
- 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 modelcontextprotocol#463
rdimitrov
left a comment
rdimitrov
left a comment
There was a problem hiding this comment.
@jalateras - Thanks for finding and addressing this issue! 💯 The solution works well, added a small suggestion to make it even simpler 👍
| ) | ||
|
|
||
| // MarshalJSON implements custom JSON marshaling for ServerJSON to properly handle empty repositories | ||
| func (s ServerJSON) MarshalJSON() ([]byte, error) { |
There was a problem hiding this comment.
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?
|
If I'm reading this correctly, it looks like it will address #613 ( FWIW, the empty fields are a schema violation and there are ~90 such servers currently published. |
|
I will close this one as it got addressed by #717 |
Summary
This PR fixes an issue where remote-only servers (servers with no repository) were still including an empty repository object with blank URL and source fields in their JSON output. This adds unnecessary noise to the API responses.
What Changed
Added omitempty tags to Repository struct fields: The
URLandSourcefields in the Repository struct now haveomitemptytags to prevent empty string values from being serialized.Implemented custom MarshalJSON for ServerJSON: A custom marshaling method was added to properly handle the case where a Repository struct is initialized but contains only empty values. This ensures the entire repository field is omitted from the JSON output when all its fields are empty.
Added comprehensive tests: Test coverage was added for both the Repository struct marshaling and the ServerJSON marshaling to verify the behavior works correctly.
Technical Details
The fix uses a two-pronged approach:
omitemptytags so individual empty fields are omittedThis ensures backward compatibility while fixing the issue for remote-only servers.
Testing
Fixes #463