Skip to content

Conversation

@CodeWithKyrian
Copy link
Contributor

Reorganizes the internal server structure to improve code organization and maintainability.

Changes

Handler Reorganization

  • Moved: JsonRpc/HandlerServer/Handler/JsonRpcHandler
  • Reorganized: Server/RequestHandler/Server/Handler/Request/
  • Reorganized: Server/NotificationHandler/Server/Handler/Notification/
  • Moved: MethodHandlerInterfaceServer/Handler/ namespace

Transport Structure

  • Moved: TransportInterfaceServer/Transport/ namespace
  • Consolidated: All transport-related code in one location

Data Structure

  • Moved: Page.phpSchema/ namespace
  • Rationale: It's a data structure concern, not server logic

Why

Better Organization

  • Logical grouping: Related code is now co-located
  • Predictable structure: Easier to find and understand code
  • Cleaner namespaces: More intuitive import statements

Separation of Concerns

  • Server-specific: JsonRpcHandler is server implementation, not global protocol
  • Protocol-agnostic: MessageFactory stays in JsonRpc/ for reusability
  • Data vs Logic: Page is a data structure, not server logic

Breaking Changes

  • Internal only: No public API changes
  • Import updates: All internal references updated
  • Namespace changes: Internal classes moved to new namespaces

@CodeWithKyrian CodeWithKyrian force-pushed the refactor/reorganize-server-handlers-and-transports branch from 5de7aa1 to 7b136fd Compare September 28, 2025 04:39
@CodeWithKyrian CodeWithKyrian force-pushed the refactor/reorganize-server-handlers-and-transports branch from 5330c2e to c1b41af Compare September 28, 2025 17:57
@CodeWithKyrian CodeWithKyrian force-pushed the refactor/reorganize-server-handlers-and-transports branch from c1b41af to c48732c Compare September 28, 2025 17:58
@chr-hertel
Copy link
Member

in general i'm fine with this change, couple of thoughts tho:

  • the JsonRpcHandler is quite a central service - even if currently internal to the server i was expecting it to be useful with the client as well, but we can tackle that then anyways.
  • moving the Page to the schema def makes sense - next step might be to have that PaginatedRequest base class :D
  • the MessageFactory feels lonely, but maybe also something to revisit when working on the client.

@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Sep 28, 2025
@chr-hertel chr-hertel merged commit 41486bf into modelcontextprotocol:main Sep 28, 2025
10 checks passed
@CodeWithKyrian
Copy link
Contributor Author

CodeWithKyrian commented Sep 29, 2025

  • Yeah true, its internal to the server at the moment that's why I moved it. For the client, the handling process will be very much different I think, but as you said, we'll cross that bridge when we get there. .
  • You read my mind!! +1 on that
  • Sure!

@CodeWithKyrian CodeWithKyrian deleted the refactor/reorganize-server-handlers-and-transports branch September 29, 2025 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants