filipedeschamps / tabnews.com.br

Conteúdos para quem trabalha com Programação e Tecnologia.
https://tabnews.com.br
GNU General Public License v3.0
5.32k stars 388 forks source link

`content(api)`: criar novo status `deleted` #348

Closed filipedeschamps closed 2 years ago

filipedeschamps commented 2 years ago

Contexto

Para fazer um "soft delete" de um content, vamos criar um novo status deleted e um novo campo deleted_at.

Execução

Hoje a tabela contents possui uma constraint entre owner_id e slug para que um usuário não possa criar posts com um mesmo identificador.

https://github.com/filipedeschamps/tabnews.com.br/blob/1d57c1ea7ffb11858f296b0b177f39524bcaaa36/infra/migrations/1649621949432_create-content-table.js#L70

O problema de manter essa constraint, é que acaba não se tornando mais possível deletar contents que acabem tendo o mesmo slug. Por exemplo:

  1. Eu crio um post /filipedeschamps/teste
  2. Em seguida me arrependo e deleto ele (soft delete, pois continua existindo na base).
  3. Ao tentar criar um outro post /filipedeschamps/teste, o backend vai reclamar, pois já existe essa combinação, mas uma delas (a primeira) está com o status deleted.

E se adicionar o status nessa constraint, não adianta também, pois daí eu não vou poder soft delete dois conteúdos com o mesmo indicador.

Então em conversas passadas com o @rodrigoKulb , ele fez uma sugestão de criar uma propriedade deleted_at e adicionar ela na constraint. Achei genial.


Fora isso, uma vez um content ganhando o status de deleted, não é mais possível voltar atrás.

tembra commented 2 years ago

Você acredita ter necessidade de criar um novo status deleted?

Assim ficaria organizado e teríamos acesso direto para consulta, mas o deleted_at resolve todos os problemas.

Isso nos ajudaria a manter o último status do conteúdo antes dele ter sido deletado e poderíamos contar com uma das maravilhas de se usar soft delete: análise de dados.

Com o tempo poderíamos saber se a maioria dos conteúdos são deletadas enquanto ainda estão com o status draft, pois as pessoas estão desistindo de postar algo. Ou então, se as pessoas se arrependeram de postar e estão deletadas depois que o status está como published.

Uma pergunta: ~Hoje existe a possibilidade de voltar um conteúdo pra draft após ele estar como published ?~ Se não existe, será adicionado no futuro?

Pergunto isso pois se não tiver e não formos adicionar esta possibilidade no futuro, o campo published_at já atenderia até o uso do status e na verdade nem precisaria ser utilizado, a não ser que depois surgissem status como pending-approval para aguardar a aprovação de alguém.

EDIT: Verifiquei na issue #349 que hoje o status draft não está acessível.

filipedeschamps commented 2 years ago

Ótimo ponto @tembra mas na minha visão fica menos semântico, por exemplo, imagina montar uma query que agrupa todos os posts pelos seus status ou o PATCH que será feito contra a API para mudar de um status para o outro, e também como consultar isso pela API.

Talvez se um dia quisermos fazer essa leitura dos dados, a modelagem do banco deveria ser INSERT ONLY (o que eu acharia ótimo), ou salvar essas ações em outro lugar para consulta. Dado a isso, voto em manter uma modelagem mais comum e previsível, mesmo que nesse caso seja um dado redundante.

EDIT: Verifiquei na issue #349 que hoje o status draft não está acessível.

Na verdade o status draft está disponível e funcional, porém hoje todos os endpoints forçam buscar os dados como published. Então não há pela API hoje ninguém que listem os posts em draft.

filipedeschamps commented 2 years ago

Sabe aquele tipo de problema que você acha que vai matar rapidão, mas quem te mata é o problema? 😂 então, quem está me matando é a CONSTRAINT UNIQUE envolvendo owner_id, slug e deleted_at, sendo que o deleted_at pode ser null e isso faz o Postgres aceitar múltiplas linhas iguais.

Por exemplo, considerando o que está em Produção, o segundo insert daria problema porque ele possui o mesmo owner_id e slug:

INSERT INTO contents (owner_id, slug, title, body, status) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published');
INSERT INTO contents (owner_id, slug, title, body, status) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published');

Resultado:

ERROR:  duplicate key value violates unique constraint "contents_uniqueness_fkey"
DETAIL:  Key (owner_id, slug)=(88717cd5-d255-4ada-a235-e8c38bffe173, slug) already exists.

Isso porque temos isso de constraint:

image

Agora, rodando uma nova migration para considerar também o deleted_at:

exports.up = async (pgm) => {
  await pgm.addColumns('contents', {
    deleted_at: {
      type: 'timestamp with time zone',
      notNull: false,
    },
  });

  await pgm.dropConstraint('contents', 'contents_uniqueness_fkey');
  await pgm.addConstraint('contents', 'contents_uniqueness_fkey', 'UNIQUE (owner_id, slug, deleted_at)');
};

Show, atualizou a constraint:

image

Só que agora eu posso fazer múltiplos inserts, pois por padrão uma coluna NULL não faz dar clash nos valores:

image

Que !delicinha né?

E procurando na internet, esse é o comportamento mesmo e estão sugerindo uma alternativa com índices, mas puts, que bizarro demais:

E agora? Alguém conhece alguma alternativa mais elegante?

filipedeschamps commented 2 years ago

Bom, não consigo encontrar outra solução a não ser deletar a constraint e substituir por um índice. Fiz da seguinte forma no arquivo de migração:

  // pgm.createIndex(tablename, columns, options)

  await pgm.createIndex('contents', ['owner_id', 'slug', '(deleted_at IS NULL)'], {
    name: 'contents_owner_id_slug_deleted_at_unique_index',
    unique: true,
    where: 'deleted_at IS NULL',
  });

O que gera:

CREATE UNIQUE INDEX "contents_owner_id_slug_deleted_at_unique_index" ON "contents" ("owner_id", "slug", (deleted_at IS NULL)) WHERE deleted_at IS NULL;

E o comportamento de ir fazendo INSERTs em relação ao índice e o deleted_at:

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published', NULL);

-- OK

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug', 'title', 'body', 'published', NULL);

-- NOT OK
-- ERROR:  duplicate key value violates unique constraint "contents_owner_id_slug_deleted_at_unique_index"
-- DETAIL:  Key (owner_id, slug, (deleted_at IS NULL))=(88717cd5-d255-4ada-a235-e8c38bffe173, slug, t) already exists.

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'published', NULL);

-- OK

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'deleted', NULL);

-- NOT OK: ta certo, pois só mudei o `status` para `deleted` sem ter adicionado um `deleted_at`
-- ERROR:  duplicate key value violates unique constraint "contents_owner_id_slug_deleted_at_unique_index"
-- DETAIL:  Key (owner_id, slug, (deleted_at IS NULL))=(88717cd5-d255-4ada-a235-e8c38bffe173, slug-sera-deletado, t) already exists.

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'deleted', NOW());

-- OK: não esbarra mais por conta do `deleted_at`

INSERT INTO contents (owner_id, slug, title, body, status, deleted_at) VALUES ('88717cd5-d255-4ada-a235-e8c38bffe173', 'slug-sera-deletado', 'title', 'body', 'deleted', NOW());

-- OK: não esbarra mais por conta do `deleted_at`
image

O mesmo comportamento se garante se eu tentar sair fazendo UPDATE. Se ele não esbarrar com o índice único, tudo ok... se esbarrar, ele devolve um erro.

aprendendofelipe commented 2 years ago

Uma alternativa nada semântica seria usar uma data específica do passado no lugar de null e o artigo ser considerado deletado se deleted_at for diferente dessa data, por exemplo, 01/01/2000 00:... Quem aí já trabalhava na área na época do bug do milênio 😅

filipedeschamps commented 2 years ago

E se eu te falar que sempre preencher uma data foi a primeira alternativa que pensei quando comecei a entender que não daria para resolver usando constraint e null 😂 🤝👍

aprendendofelipe commented 2 years ago

E se eu te falar que sempre preencher uma data foi a primeira alternativa que pensei quando comecei a entender que não daria para resolver usando constraint e null 😂 🤝👍

Elegante? Nada! Funciona? Perfeitamente... hahaha

rodrigoKulb commented 2 years ago

@filipedeschamps aqui na empresa utilizamos 2 informações para registros apagados 01 - Quando = deleted_at (data) 02 - Quem = idUser (int) Agora com a funcionalidade liberada para outros membros apagarem artigos de outro usuários, acredito que seria muito importante essa informação até para segurança do portal.

E o campo int = 0 não daria o mesmo problema certo?

aprendendofelipe commented 2 years ago

@rodrigoKulb o problema seria que o mesmo usuário não conseguiria apagar duas vezes a postagem com o mesmo slug e owner_id

Edit para melhor clareza: Esse problema é se a sugestão foi usar o idUser junto de owner_id e slug como unique.

filipedeschamps commented 2 years ago

Interessantíssimo @rodrigoKulb 👍 no nosso caso o id do usuário é um UUID, então não aceitaria 0, mas não importa porque o índice que estamos usando para controlar o unique não leva em consideração esse campo. Na migration que está no novo PR ele só considera o owner_id, slug e deleted_at.

E acho ótimo anotar esse tipo de informação, mas sugiro não fazer nessa versão (até porque vamos ter tudo isso registrado em logs caso a gente precise investigar). E daí numa próxima interação fazer a anotação de quem editou um conteúdo (não somente deletou), até porque esse "soft delete" é uma "edição de conteúdo" (que altera o status para deleted). Daí devemos anotar todas as atualizações de um conteúdo, inclusive para conseguir mostrar um histórico como o GitHub faz.

rodrigoKulb commented 2 years ago

Edit para melhor clareza: Esse problema é se a sugestão foi usar o idUser junto de owner_id e slug como unique.

@aprendendofelipe verdade! 😅️

id do usuário é um UUID

@filipedeschamps verdade 😅️ show! Seguimos quebrando a cabeça aqui!

rodrigoKulb commented 2 years ago

Só para jogar uma ideia aqui, se utilizarmos o campo: deleted_at (int) com o Unix Timestamp

Assim poderia utilizar o ZERO sem problemas = Jan 01 1970 00:00:00 GMT+0000

filipedeschamps commented 2 years ago

@rodrigoKulb gosto de você por estar com a mesma raiva que eu em ter que usar um índice para controlar o unique 😂

Mas apesar dessa ideia tecnicamente ser válida, semanticamente fica estranho o deleted_at ser num formato e os outros campos de data como created_at ou published_at ser em outro formato.

tembra commented 2 years ago

Comentando aqui somente pra registrar a normalidade de se usar unique index em soft deletes: Trabalho há +10 anos com grandes ERPs e todos os que possuem soft delete utilizam desta estratégia. Seja com banco de dados SQL Server, Oracle ou Postgres.

Sempre é criado um índice único contendo o flag do registro de deleção.

Para entendermos melhor o porquê do Postgres permitir múltiplos nulos devemos lembrar da tradução da palavra constraint que significa restrição.

Tendo isto em mente entendemos que a maioria das implementações dos SGBDs utiliza constraints como forma de referenciar alguma coisa, como por exemplo chaves estrangeiras ou valores válidos. Por outro lado sabemos que os índices são utilizados explicitamente para facilitar/agilizar a busca por dados.

Dessa forma agregando agora o conceito de unique entendemos que um valor NULL não impede nenhuma restrição, por isso alguns SGBDs como o Postgres o desconsideram quando falamos de constraints (lembrem: restrições). Não há o que ser restringido se o valor for NULL exceto se a própria coluna não o suportar é claro (mas isso não tem mais a ver com constraints). Por outro lado um índice único deve levar todas as possibilidades (incluindo o NULL) em consideração, pois se trata de como os dados serão organizados.

Ainda sim em alguns SGBDs (como no Postgres) é preciso informar explicitamente que determinada coluna poderá conter o valor NULL e, quando tiver, deve ser indexada de forma única, como feito pelo @filipedeschamps no comentário acima, replicado logo abaixo:

CREATE UNIQUE INDEX "contents_owner_id_slug_deleted_at_unique_index"
ON "contents" ("owner_id", "slug", (deleted_at IS NULL))
WHERE deleted_at IS NULL;
filipedeschamps commented 2 years ago

Explicação sensacional @tembra e isso me tranquiliza bastante sobre a escolha então de criarmos o índice para controlar isso 🤝

filipedeschamps commented 2 years ago

Fechado pelo PR #431 🤝

filipedeschamps commented 2 years ago

Turma, olha o que está por vir na versão 15 do Postgres 😍 https://blog.rustprooflabs.com/2022/07/postgres-15-unique-improvement-with-null

aprendendofelipe commented 2 years ago

Turma, olha o que está por vir na versão 15 do Postgres

UNIQUE NULLS NOT DISTINCT 😍😍😍