As duas abordagens de reescrita de BBCode do Discourse devem compartilhar uma API central?

Duas PRs abertas introduzem abordagens diferentes para editar atributos de BBCode no texto bruto das publicações:

Durante a revisão da #43187, Martin sugeriu extrair a funcionalidade genérica de manipulação de BBCode do serviço específico de Eventos.

Inicialmente, incluí algumas funcionalidades relacionadas baseadas em tokens da outra PR, mas isso introduziu dois mecanismos de reescrita dentro da #43187. Como a API baseada em tokens não tinha nenhum chamador em produção naquele branch, eu a removi no commit 6d13371.

A API baseada em tokens continua sendo útil independentemente para a #42932.

Perguntei-me se essas duas abordagens deveriam eventualmente convergir em uma única API central reutilizável, ou se seus requisitos diferentes justificam a retenção de ambas.

Comparação das implementações atuais

A tabela descreve o que o código atualmente implementa, em vez de sugerir que qualquer uma das abordagens seja inerentemente incorreta.

Capacidade BbcodeRewriter (#43187) API baseada em tokens (#42932) Significado e importância
1. Usa o parser de BBCode do Discourse Varre tags de abertura candidatas usando parseBBCodeTag, a mesma gramática de tags de abertura usada pelo mecanismo de Markdown. Usa DiscourseMarkdownIt.parse para obter tokens de Markdown e encontrar um bloco de BBCode correspondente. Ambas as abordagens entendem a sintaxe de BBCode, em vez de depender exclusivamente de uma expressão regular independente. Elas diferem em como identificam a tag a ser editada.
2. Seleciona o bloco de BBCode alvo Marca temporariamente tags de abertura candidatas, renderiza a publicação e identifica o candidato correspondente a um elemento HTML selecionado, como div.discourse-post-event. Seleciona o primeiro token de BBCode analisado cujo meta.bbcode corresponde ao nome da tag solicitado. A identidade do alvo é importante. A #43187 precisa associar o BBCode bruto ao Evento que o Discourse realmente renderizou. O fluxo de trabalho da #42932 usa o primeiro bloco de Evento analisado correspondente.
3. Lida com alvos ausentes ou ambíguos Lança TargetNotFound, AmbiguousTarget ou UnsupportedRenderer quando um alvo não pode ser identificado com confiança. Retorna null se não existir um bloco analisado correspondente; caso contrário, seleciona a primeira correspondência. O comportamento em caso de falha difere. A #43187 recusa deliberadamente edições automáticas ambíguas. Selecionar o primeiro bloco analisado pode ser suficiente quando o chamador espera apenas um bloco relevante.
4. Verifica o mapeamento de bruto para renderizado Renderiza o conteúdo marcado, verifica o marcador no elemento HTML pretendido e confirma que inserir e remover o marcador deixa o HTML renderizado inalterado. Usa os metadados de tokens do parser de Markdown para localizar a tag de abertura sem uma verificação adicional de identidade do HTML renderizado. Isso protege contra a edição do alvo renderizado errado. É particularmente relevante quando um job em segundo plano arquiva automaticamente um Evento existente, mas envolve renderização e complexidade adicionais.
5. Verifica a tag de abertura selecionada Antes de reescrever, varre novamente o texto bruto fornecido e verifica se o candidato selecionado ainda corresponde ao texto esperado da tag de abertura. Localiza a tag de abertura correspondente durante a chamada de atualização, sem uma comparação separada de tag de abertura esperada. Este é um check de consistência interna. Ajuda a evitar a reescrita de um candidato que não corresponde mais à seleção anterior. Por si só, não é proteção contra edições simultâneas no banco de dados.
6. Trata nomes de atributos e valores existentes Reescreve os atributos selecionados usando as fronteiras de atributos do parser. Também normaliza nomes de atributos renderizados equivalentes, permitindo que variantes como Recurrence e recurrence sejam removidas consistentemente. Usa correspondências de tokens de atributo para substituir ou remover nomes solicitados e o serializador compartilhado de atributos de BBCode para inserir valores. Ambas visam preservar atributos não relacionados. Normalizar nomes equivalentes é especialmente importante ao arquivar um Evento, pois deixar um alias de recorrência por trás poderia fazer com que o Evento concluído permanecesse recorrente.
7. Valida a tag reescrita Analisa novamente a tag de abertura reescrita e verifica se seus atributos correspondem aos valores pretendidos. Uma ida-e-volta inválida lança um erro. Produz diretamente a tag de abertura reescrita, sem uma etapa equivalente de análise final e comparação. A validação de ida-e-volta captura problemas de serialização, como valores de atributos alterados ou fronteiras malformadas. A abordagem baseada em tokens é mais simples, mas depende de sua lógica de reescrita e testes, em vez desta verificação adicional em tempo de execução.
8. Preserva o conteúdo não relacionado da publicação Altera a tag de abertura selecionada, mantendo o conteúdo bruto circundante. Também verifica se os valores de atributos não relacionados sobrevivem à reescrita. Substitui a substring da tag de abertura selecionada, mantendo o conteúdo bruto circundante e os atributos não solicitados para modificação. Ambas precisam evitar alterações não intencionais na publicação. Uma edição relacionada a um Evento não deve modificar silenciosamente a discussão circundante ou atributos de BBCode não relacionados.
9. Custo de implementação Requer varredura de candidatos, marcadores temporários, renderização da publicação, inspeção de HTML, reescrita e validação de atributos. Analisa tokens de Markdown, seleciona a primeira tag de abertura correspondente e a reescreve diretamente. Verificações em tempo de execução mais fortes têm um custo. A questão é se as salvaguardas extras devem estar disponíveis para todos os chamadores, serem opcionais ou serem implementadas apenas onde a aplicação as requer.

Por que os requisitos diferem

Para a #43187, a rolagem do sucessor é executada automaticamente quando uma ocorrência termina. O Evento original é arquivado como uma ocorrência não recorrente, e um novo Tópico é criado para a próxima ocorrência.

Se o bloco [event] errado for modificado, ou se os atributos reescritos não representarem mais o Evento pretendido, o erro poderia persistir entre as tentativas de rolagem automática.

Para a #42932, a ação do fluxo de trabalho solicita explicitamente que um Evento seja aberto ou fechado. Sua implementação atual baseada em tokens modifica o primeiro bloco de Evento analisado correspondente.

Portanto, a abordagem baseada em tokens não é necessariamente inadequada para esse caso de uso. Ela simplesmente fornece garantias diferentes de seleção e validação.

Ambas as abordagens já compartilham alguma infraestrutura subjacente, incluindo parseBBCodeTag e serializeBBCodeAttr.

Possível direção para uma abstração compartilhada

Uma possibilidade seria separar três responsabilidades:

  1. Encontrar uma tag de abertura de BBCode — usando tokens do parser, seleção explícita de candidatos ou mapeamento de elementos renderizados.

  2. Reescrever seus atributos — usando uma implementação compartilhada para fronteiras de atributos, serialização e preservação de valores não relacionados.

  3. Validar o resultado — opcionalmente aplicando identidade do alvo, detecção de ambiguidade, equivalência de conteúdo renderizado ou verificações de ida-e-volta de atributos, dependendo do chamador.

Isso permitiria que fluxos de trabalho mais simples usassem uma seleção de tokens direta, enquanto operações de edição automática mais sensíveis solicitariam salvaguardas mais fortes.

No entanto, também poderia introduzir complexidade desnecessária em comparação com a manutenção de duas APIs menores.

Pergunta para os mantenedores

Seria preferível que as duas PRs retivessem suas APIs de reescrita separadas, dado seus diferentes requisitos de seleção de alvo?

Alternativamente, um reescritor de atributos de baixo nível compartilhado com seleção de alvo e validação configuráveis seria uma melhor direção a longo prazo?

Estou particularmente interessado em saber se as verificações de segurança adicionais atualmente implementadas pelo BbcodeRewriter são consideradas funcionalidade central geralmente útil, ou se devem permanecer específicas para casos de uso que exigem garantias mais fortes.

Não estou propondo que nenhuma das PRs precise bloquear a outra; o objetivo é evitar duplicação desnecessária na infraestrutura central de BBCode do Discourse conforme ambos os recursos se desenvolvem.

1 curtida