fix: aceitar version inteiro no envelope de evento - #1
Merged
Conversation
Rails serializes `version` as a JSON number, so Jason.decode/1 hands the
consumers the integer 1. Both consumers compared it against the string list
["1"], which never matched:
- RedisSubscriber.resolve_topics/1 returned [], so the event reached no
topic at all, not even org_events. The only trace was a Logger.warning
that says "missing required fields" while printing an event that has
them all.
- EventsController rejected the request as an unsupported version.
Nothing breaks today because the publisher does not send the field yet. The
day it does, delivery drops to zero with no error on either side - the same
symptom as "not implemented". The mechanism that exists to protect against
schema incompatibility would be the one causing the outage.
Measured against a real Redis and a real Phoenix.PubSub, publishing the
envelope build_envelope/0 actually generates: 2 messages delivered with no
`version`, 0 with `version: 1`, 2 with `version: "1"`.
The four existing version tests all use strings, so they pass on both sides
of this bug. The two added here fail before the fix.
- accept both forms in redis_subscriber.ex and events_controller.ex
- comment why the list must not be narrowed back
- cover the integer path, accepted and rejected
Deploy order matters: this must reach production before prostaff-api starts
sending the field. It is backwards compatible (integer, string and absent all
work), so it can ship on its own.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
O que esta PR faz?
O Rails serializa
versioncomo número JSON, então oJason.decode/1entrega o inteiro1aosdois consumidores. Os dois comparavam contra a lista de strings
["1"], que nunca casa. Esta PRpassa a aceitar as duas formas.
Nada quebra hoje, porque o publisher ainda não envia o campo. No dia em que enviar, a entrega cai a
zero sem erro em nenhuma das duas pontas - o mesmo sintoma de "não implementado". O mecanismo que
existe justamente para proteger contra incompatibilidade de schema seria o que causaria a queda.
Tipo da PR
Bugfix.
O que esta PR adiciona, remove ou atualiza?
redis_subscriber.exeevents_controller.ex:@supported_versionspassa de["1"]para[1, "1"].isso, é o tipo de coisa que alguém "limpa" numa refatoração futura.
versionquejá existiam usam string e passam dos dois lados do bug.
Como testar?
Os dois testes novos falham no commit anterior:
O
right: []é o bug: envelope completo, com todos os campos obrigatórios, e nenhum tópico.Medição anterior contra Redis real e
Phoenix.PubSubreal, publicando o envelope que obuild_envelopedo Rails realmente gera:version(comportamento de hoje)version: 1inteiroversion: "1"stringResultado esperado
mix testem 55 testes, 0 falhas. Portões de qualidade verdes:format,compile --warnings-as-errors,credo --strict,sobelow --config --exitedialyzer.Ordem de deploy
Esta PR precisa chegar em produção antes de o
prostaff-apicomeçar a enviarversion.Inverter a ordem derruba a entrega de eventos pela janela inteira de deploy, sem alarme.
É retrocompatível por construção - inteiro, string e ausência continuam funcionando - então pode ir
sozinha, sem coordenação.
Fora de escopo
Ficam para as próximas: separar os dois motivos pelos quais
resolve_topics/1devolve[](hoje olog diz "missing required fields" para os dois), subir o ramo não roteado de
Logger.debugparawarning, e dar destino assinável aos tipos que hoje só chegam emorg_events.