Skip to content

Conversation

@soyuka
Copy link
Contributor

@soyuka soyuka commented Dec 29, 2025

Summary

Introduces interface-based abstractions for schema generation and discovery components, enabling extensibility and proper dependency injection patterns. This allows integration with external schema libraries (e.g., API Platform) and custom discovery implementations.

Motivation and Context

The current implementation tightly couples schema generation to PHP attributes and hardcodes discoverer instantiation within loaders. This makes it difficult to:

This PR decouples these concerns by introducing interfaces and moving object construction responsibilities to the Builder.

Breaking Changes

Minimal breaking changes:

  • SchemaGenerator::generate() now accepts \Reflector instead of \ReflectionMethod|\ReflectionFunction
  • DiscoveryLoader constructor signature changed (internal usage only, constructed by Builder)

Users relying on default Builder behavior are not affected.

I suggest that the Discoverer (and probably the DiscoveryLoader) should be marked @internal and some of the classes be marked as final. I'm unsure if you'd merge this changes as they may break current implementations, let me know.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Changes:

  • Added SchemaGeneratorInterface with generate(\Reflector): array method
  • Added DiscovererInterface (marked @internal) with discover() method
  • SchemaGenerator now implements SchemaGeneratorInterface
  • Discoverer and CachedDiscoverer now implement DiscovererInterface (marked @internal and @final)
  • Builder now constructs and injects Discoverer into DiscoveryLoader (proper DI)
  • Builder automatically decorates Discoverer with CachedDiscoverer when cache is configured
  • DiscoveryLoader simplified to pure loader (no longer factory)
  • Added Builder::setSchemaGenerator() and Builder::setDiscoverer() for custom implementations

Future possibilities:

  • Custom schema providers (e.g., ApiPlatformSchemaProvider)
  • Alternative discovery strategies
  • Class-based schema generation (currently throws \BadMethodCallException) (note that currently a tool is defined using a method or a function but in the future we). open the possibility that a tool could be referenced as an invoke-able class or any other method).

Also relates to #153 (indeed the #153 PR is missing the ArrayLoader change and looking at the code its unclear that the schema generator is used there as well as in the discoverer).

* to improve performance when discovery is called multiple times.
*
* @internal
* @final
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should I mark the class final directly?

@soyuka soyuka force-pushed the dependency-injection branch 2 times, most recently from b50fd39 to 8179fb4 Compare December 29, 2025 09:22
@soyuka soyuka force-pushed the dependency-injection branch from 8179fb4 to fd0aa5f Compare December 29, 2025 09:32
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.

1 participant