modelcontextprotocol / modelcontextprotocol/java-sdk

Un-deprecate/add no-arg builders

Abierto
#1,065 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

enhancement needs confirmation P3
Lenguaje dominante
Java
Estrellas
3.7k
Forks
1.1k
Merge medio
1 d 15 h
PR fusionados (30 d)
9

Descripción

#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));

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza con los puntos de entrada del builder de McpSchema nombrados aquí, incluidos CreateMessageRequest, SamplingMessage, TextContent, ProgressNotification y ModelPreferences. Compara las API con argumentos obligatorios y sin argumentos, y comprueba después si los valores del esquema pueden proporcionarse de forma incremental, incluidos los tokens de progreso gestionados por el framework; se considera terminado cuando el enfoque del builder es coherente en todos los tipos solicitados.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
java
Área
api, backend-api-design
Tipo de issue
Nueva funcionalidad
Dificultad
5/5
Tiempo estimado
Más de una semana
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
42/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.