Un-deprecate/add no-arg builders

Offen
#1,065 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Anfängerfreundlichkeit
42/100
Issue-Typ
Feature
Klarheit
Größtenteils klar
Aktivitätsstatus
Ruhig
Tech-Stack
java

Rechercherichtung

Beginnen Sie mit den hier genannten Einstiegspunkten des McpSchema-Builders, einschließlich CreateMessageRequest, SamplingMessage, TextContent, ProgressNotification und ModelPreferences. Vergleichen Sie die APIs mit erforderlichen Argumenten und ohne Argumente und prüfen Sie anschließend, ob Schemawerte schrittweise bereitgestellt werden können, einschließlich der vom Framework verwalteten Fortschrittstoken; abgeschlossen ist die Arbeit, wenn der Builder-Ansatz über die angeforderten Typen hinweg konsistent ist.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

enhancement needs confirmation P3

#928 deprecated no-arg builders for schema types, e.g. McpSchema.Resource

I'd like you to consider reversing that decision, and also to add no-arg versions for types without them (e.g. ProgressNotification).

I understand the motivation that builders not be left in an invalid state. It's worth noting that it might achieve that aim currently (I didn't audit all the classes), but if the schema ever has any fields which are mutually exclusive or conditionally required, then using constructors will only offer partial protection.

The problem is that it makes it undermines one of the main advantages of the builder pattern, which is to make construction clearer.

Here was my attempt to write a CreateMessageRequest with only required properties:

var request = McpSchema.CreateMessageRequest.builder(
        List.of(McpSchema.SamplingMessage.builder(
            McpSchema.Role.USER,
            McpSchema.TextContent.builder("Test Sampling Message").build()).build()
        ),
        50
    )
    .build();

Maybe there's a better way to format/indent this, but I tried several variations and I thought this was the best one.

Compare that with the same thing constructed with named properties

var request = McpSchema.CreateMessageRequest.builder()
    .messages(List.of(
        McpSchema.SamplingMessage.builder()
            .role(McpSchema.Role.USER)
            .content(McpSchema.TextContent.builder("Test Sampling Message").build())
            .build()
    ))
    .maxTokens(50)
    .build();

It's also worth noting that some builders have APIs like ModelPreferences#addHint. If that pattern were applied consistently, it could be simplified a little more by replacing messages(List.of( with addMessage(

Beyond readability

I'm writing a framework and builders enforcing all required params at once makes them inflexible for some possible API designs.

For example, my framework automatically manages progress tokens. I considered a design such as

void sendProgress(Consumer<McpSchema.ProgressNotification.Builder> consumer);

// an example caller. mcp creates the builder and applies the token
mcp.sendProgress(p -> p.progress(5).total(10));

However, it's not possible for the framework to implement this with the current builder methods. The user of the framework knows the progress value, the framework itself knows the progress token, but the builder expects both at once.

So the builder has to be worked around, for example to this

void sendProgress(double progress, Consumer<McpSchema.ProgressNotification.Builder> consumer);
// an example caller
mcp.sendProgress(5, p -> p.total(10));
Vorherrschende Sprache
Java
Sterne
3.7k
Forks
1.1k
Ø Merge
1 T. 15 Std.
Gemergte PRs (30 T.)
9

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus modelcontextprotocol/java-sdk

Alle Issues in modelcontextprotocol/java-sdk

Ähnliche Issues

Weitere Issues zu Java

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.