Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Jul 11, 2025

Fix ReadResourceResult to comply with MCP schema

Summary

This PR restructures the ReadResourceResult class to comply with the MCP (Model Context Protocol) schema specification. The key change is moving from direct text, blob, and mimeType fields to a contents array containing ResourceContents objects.

Major Changes:

  • New class hierarchy: Created ResourceContents base class with TextResourceContents and BlobResourceContents implementations
  • Updated ReadResourceResult: Now uses contents array field instead of direct fields
  • API changes: Factory methods now require uri parameter for schema compliance
  • Updated all tests and examples: Modified to use new contents array structure
  • Removed deprecated methods: Eliminated signature conflicts while maintaining core functionality

The implementation follows the same pattern as the existing Content interface in the prompts module, ensuring consistency across the codebase.

Review & Testing Checklist for Human

⚠️ High Priority (4 items)

  • Test JSON serialization: Verify that the actual JSON output matches the MCP schema specification using MCP Inspector or similar tool
  • Check for breaking changes: Ensure existing code using the old API still works or fails gracefully with clear error messages
  • End-to-end testing: Test resource reading functionality with a real MCP client to verify the new structure works correctly
  • URI parameter handling: Verify that URI is correctly passed through all code paths, especially in AnnotationMcpResource

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    ReadResourceResult["ReadResourceResult.java<br/>(contents array)"]:::major-edit
    ResourceContents["ResourceContents.java<br/>(abstract base)"]:::major-edit
    TextResourceContents["TextResourceContents.java<br/>(text + uri + mimeType)"]:::major-edit
    BlobResourceContents["BlobResourceContents.java<br/>(blob + uri + mimeType)"]:::major-edit
    
    AnnotationMcpResource["AnnotationMcpResource.java<br/>(URI handling)"]:::minor-edit
    HelloResource["HelloResource.java<br/>(example usage)"]:::minor-edit
    Tests["*Test.java<br/>(updated assertions)"]:::minor-edit
    
    ReadResourceResult --> ResourceContents
    ResourceContents --> TextResourceContents
    ResourceContents --> BlobResourceContents
    AnnotationMcpResource --> ReadResourceResult
    HelloResource --> ReadResourceResult
    Tests --> ReadResourceResult
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

- Created mocapi-resources module with McpResource and McpResourceProvider interfaces
- Added @resource and @ResourceService annotations for marking resource methods and classes
- Implemented McpResourcesCapability with resources/list and resources/read JSON-RPC endpoints
- Added annotation processing with AnnotationMcpResource and factory classes
- Created MocapiResourcesAutoConfiguration for Spring Boot auto-configuration
- Added ReadResourceResult record for resource content (text/binary)
- Updated parent POM, autoconfigure, and starter dependencies
- Added example HelloResource service demonstrating usage
- Included comprehensive unit tests for all components
- Follows existing patterns from tools and prompts modules

Co-Authored-By: James Carman <[email protected]>
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

- Add enabled property to MocapiResourcesProperties with default true
- Add @ConditionalOnProperty annotation to mcpResourcesCapability bean
- Fix test method calls to use correct signatures (getMcpResources, listResources with cursor)
- All tests now passing with excellent coverage (95% overall, 100% annotation package)

Co-Authored-By: James Carman <[email protected]>
@jwcarman
Copy link
Contributor

It doesn't look like you've registered your auto configuration class here:

mocapi-autoconfigure/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports

- Add ReadResourceResultTest with comprehensive static factory method testing
- Add HelloResourceTest for example resource class coverage
- Add ResourceServiceMcpResourceProviderTest for provider functionality
- Enhance MocapiResourcesAutoConfigurationTest with additional bean tests
- Fix ResourceServiceMcpResourceProvider null pointer issue by initializing resources field
- Register MocapiResourcesAutoConfiguration in Spring Boot auto-configuration imports
- Add comprehensive AnnotationMcpResourceTest covering edge cases and error scenarios

These tests target the SonarCloud coverage gaps to achieve ≥80% coverage on new code.

Co-Authored-By: James Carman <[email protected]>
@jwcarman
Copy link
Contributor

Please address all sonarqube issues first before I review this PR.

devin-ai-integration bot and others added 2 commits July 11, 2025 19:42
- Replace Stream.peek() with filter() to avoid side effects
- Change RuntimeException to IllegalStateException for better error handling
- Use isEmpty() instead of equals("") in test assertions
- Update test expectations for IllegalStateException

Co-Authored-By: James Carman <[email protected]>
- Extract object creation outside lambda to have only one invocation that can throw runtime exception
- Addresses SonarQube maintainability issue in shouldThrowExceptionForInvalidReturnType test

Co-Authored-By: James Carman <[email protected]>
@jwcarman
Copy link
Contributor

I am trying to access resource using the MCP Inspector (by running the example application) and I am unable to read the resources. It looks like the ReadResourceResult record class is malformed. According to the canonical schema (in typescript):

https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-06-18/schema.ts

ReadResourceResult should have one array field called "contents" that can either be a TextResourceContents or BlobResourceContents object. Please use the schema for naming inspiration. Since we do not have the ability to do union types in Java, you'll need to create an interface similar to how we created an interface for Content in the mcp-prompts module.

devin-ai-integration bot and others added 2 commits July 11, 2025 20:15
- Restructure ReadResourceResult to use contents array field
- Create ResourceContents base class with TextResourceContents and BlobResourceContents implementations
- Follow same pattern as Content interface in prompts module
- Update all factory methods to require URI parameter for schema compliance
- Remove deprecated methods that conflicted with new signatures
- Update all tests to use new contents array structure
- Maintain backward compatibility where possible
- All tests passing with new schema-compliant structure

Co-Authored-By: James Carman <[email protected]>
- Remove unused lombok.Getter imports from content classes
- Fix field naming convention: _meta -> meta
- Fix method naming convention: get_meta() -> getMeta()
- Remove duplicate test methods in ReadResourceResultTest
- Update all test method calls to use new getMeta() API
- All tests passing, SonarQube issues resolved

Co-Authored-By: James Carman <[email protected]>
@sonarqubecloud
Copy link

@jwcarman jwcarman closed this Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants