modelcontextprotocol / modelcontextprotocol/java-sdk

Un-deprecate/add no-arg builders

Aberta
#1,065 0 comentários 0 reações 0 responsáveis Ver no GitHub

Ninguém assumiu esta issue ainda.

enhancement needs confirmation P3
Linguagem predominante
Java
Estrelas
3.7k
Forks
1.1k
Merge médio
1d 15h
PRs com merge (30d)
9

Descrição

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

Guia de contribuição

Abrir o guia de contribuição

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Direção de pesquisa

Comece pelos pontos de entrada do builder de McpSchema nomeados aqui, incluindo CreateMessageRequest, SamplingMessage, TextContent, ProgressNotification e ModelPreferences. Compare as APIs com argumentos obrigatórios e sem argumentos e, em seguida, verifique se os valores do schema podem ser fornecidos incrementalmente, incluindo tokens de progresso gerenciados pelo framework; está concluído quando a abordagem do builder for consistente entre os tipos solicitados.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
java
Domínio
api, backend-api-design
Tipo de issue
Funcionalidade
Dificuldade
5/5
Tempo estimado
Mais de uma semana
Status de atividade
Pouca atividade
Clareza
Razoavelmente clara
Facilidade para iniciantes
42/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.