change to strong ResourceContents type #33
Conversation
WalkthroughThis pull request refactors the resource reading and parsing logic across multiple components. The changes replace explicit JSON unmarshalling in client methods with dedicated parsing functions, and update return types from generic Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Method
participant Parser as mcp.ParseReadResourceResult
participant Utils as mcp.ParseResourceContents
Client->>Parser: Pass JSON response
Parser->>Utils: Parse each resource field
Utils-->>Parser: Return ResourceContent instance
Parser-->>Client: Return structured ReadResourceResult
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
examples/everything/main.go (1)
189-194: Validate dynamic resource parametersWhile the typed return is consistent, consider checking if
request.Params.URIis empty or malformed before returning the resource to handle potential user input issues gracefully.mcp/utils.go (2)
537-562: Extendable parsing for new resource typesCurrent checks for
textorblobare appropriate, but consider anticipating future extensions (e.g., JSON resource contents) or additional metadata fields if requirements evolve.
564-606: Robust parsing of read resource resultsThis end-to-end solution properly checks for array presence and object structure. For advanced debugging, you might log partial parsing failures or invalid entries before returning an error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/sse.go(1 hunks)client/stdio.go(1 hunks)examples/everything/main.go(1 hunks)mcp/types.go(2 hunks)mcp/utils.go(3 hunks)server/server.go(1 hunks)server/server_test.go(1 hunks)
🔇 Additional comments (12)
mcp/types.go (4)
376-376: Strong typing improved, replacing interface{} with ResourceContentsChanging the
Contentsfield from[]interface{}to[]ResourceContentssignificantly improves type safety and code clarity. This change enforces that only proper resource content types can be included in the results.
462-464: Well-designed interface approach for type safetyConverting
ResourceContentsfrom a concrete struct to an interface with a marker method is an excellent design choice. This pattern (sometimes called the "marker interface pattern") prevents arbitrary types from being used asResourceContentswhile enabling polymorphism. Only types that explicitly implementisResourceContents()can be used in this context.
476-476: Implementation satisfies interface contractThis marker method implementation correctly satisfies the
ResourceContentsinterface contract, enablingTextResourceContentsto be used whereResourceContentsis expected.
487-487: Implementation satisfies interface contractThis marker method implementation correctly satisfies the
ResourceContentsinterface contract, enablingBlobResourceContentsto be used whereResourceContentsis expected.client/stdio.go (1)
341-341: Improved encapsulation with centralized parsingReplacing direct JSON unmarshaling with a call to
mcp.ParseReadResourceResult(response)is a good refactoring that:
- Centralizes parsing logic, making it consistent across the codebase
- Reduces code duplication
- Makes the code more maintainable as parsing logic is in a single place
- Aligns with the new strongly-typed
ResourceContentsinterfaceThis change is consistent with similar changes in other client implementations.
client/sse.go (1)
415-415: Improved encapsulation with centralized parsingReplacing direct JSON unmarshaling with a call to
mcp.ParseReadResourceResult(response)maintains consistency with other client implementations and centralizes the parsing logic. This makes the code more maintainable and ensures uniform handling of the new strongly-typedResourceContentsinterface.server/server_test.go (2)
677-679: Function signature correctly updated to use strongly-typed returnThe function signature has been properly updated to return
[]mcp.ResourceContentsinstead of[]interface{}, which aligns with the type system improvements made elsewhere. This makes the test implementation type-safe and consistent with the production code.
680-683: Simplified return structure without unnecessary nestingThe return structure has been simplified by directly using
TextResourceContentsas an implementation of theResourceContentsinterface, rather than wrapping it in another struct. This makes the code cleaner and more direct while satisfying the interface requirements.server/server.go (1)
32-35: Promoting stronger typing for resource handlersSwitching from
[]interface{}to[]mcp.ResourceContentssignificantly improves type safety and clarity, eliminating the overhead of downcasting and reducing runtime errors.examples/everything/main.go (1)
176-181: Typed resource return enhances clarityReturning a slice of
mcp.ResourceContentsensures a well-defined structure for the resource, aiding maintainability and self-documentation of handler behavior.mcp/utils.go (2)
308-313: Typed contents improve predictabilityUsing
[]ResourceContentsinNewReadResourceResultaligns with the strongly-typed approach, reducing ambiguity and potential misuse compared to[]interface{}.
413-419: Consolidated resource parsingForwarding embedded resource maps to
ParseResourceContentskeeps the code organized, centralizes error handling, and streamlines content validation logic.
2 participants
similiar to #26, add strong type to ResourceContents
Summary by CodeRabbit
New Features
Refactor