Sincroniza o app pid_provider (compartilhado entre core e upload)#1448
Open
robertatakenaka wants to merge 20 commits into
Open
Sincroniza o app pid_provider (compartilhado entre core e upload)#1448robertatakenaka wants to merge 20 commits into
pid_provider (compartilhado entre core e upload)#1448robertatakenaka wants to merge 20 commits into
Conversation
Propósito: permitir que is_updated() sinalize 'não precisa salvar' via exceção em vez de retorno implícito, deixando o fluxo de register() explícito sobre por que parou (equal / já atualizado). Técnico: nova classe SkipSavePidProviderXML(Exception), usada em models.py dentro de is_updated() e capturada em register().
Propósito: formalizar os valores possíveis de status do XMLURL, que antes eram strings livres passadas soltas pelo código (base_pid_provider.py), sem validação nem enumeração no admin. Técnico: novas constantes XMLURL_STATUS_SUCCESS, XMLURL_STATUS_XML_FETCH_FAILED, XMLURL_STATUS_PID_PROVIDER_XML_FAILED e a tupla XMLURL_STATUS, usada como choices= no campo status de XMLURL em models.py.
…score percentual Propósito: substituir a comparação por soma de pontos fixos (get_score) por um score percentual comparável entre estratégias de busca (ids / journal+issue+artigo / journal+artigo), permitindo à PidProviderXML.get_best_match decidir com um único limiar (0.6) em vez de múltiplos pesos ad-hoc. Também centraliza a validação dos dados mínimos de entrada (validate_input_data), antes espalhada nas properties issn_query/article_data_query. Técnico: - Adiciona compare()/compare_items()/compare_lists() em substituição a get_score(); calcula percentual_score = total_score / nº de itens comparados. - QueryBuilderPidProviderXML passa a centralizar os dados via adapter_data (xml_adapter.data), compare_data (get_data_to_compare()) e xml_with_pre_data (get_article_data(300)), eliminando as ~20 cached_property individuais que replicavam atributos do xml_adapter. - Adiciona validate_input_data(), pkg_name_list e article_location_params como properties dedicadas. - article_data_query e issue_params passam a ler de adapter_data em vez de cached_property próprias.
…ion) e corrige bugs de matching/persistência
Propósito: (a) auditar todo o fluxo de PidProviderXML.register(),
inclusive quando não há PidProviderXML resultante (unmatched/erro/
conflito), coisa que antes só ia para UnexpectedEvent sem
padronização por status; (b) alinhar o matching de XML ao novo
QueryBuilderPidProviderXML/compare(); (c) corrigir dois bugs de
persistência encontrados na revisão.
Técnico:
- Novo modelo PidProviderXMLRegistration: FK pid_provider_xml
nullable (SET_NULL) pois unmatched/bad_request/error podem não ter
registro; event_status fechado (created/updated/skipped/forbidden/
conflict/unmatched/bad_request/error); LIGHTWEIGHT_STATUSES evita
gravar detail pesado nos casos comuns (created/updated/skip_update).
- register() reescrito: usa select_records()/select_record() (novos,
baseados em compare()) no lugar de get_records()/get_record();
passa a chamar PidProviderXMLRegistration.record(...) no finally.
- is_updated() vira staticmethod e levanta SkipSavePidProviderXML em
vez de retornar registered.data.
- get_best_match() substitui best_matches(): usa compare() com
percentual_score > 0.6 como corte, no lugar da soma de pontos fixos
de match()/title_similarity() (removidos).
- Novo campo readable_data (JSONField) e property data_to_compare,
evitando reparsing do XML a cada comparação.
- add_collections() extraído de _save(), com fallback de issn_path
via FieldError (compatibilidade entre app core e app upload, que
usam relations diferentes: scielojournal vs journalproc).
- Novo PidProviderXMLManager com select_related('current_version')
automático em PidProviderXML.objects.
- Correção de bug: fix_duplicated_pkg_name usava
other_pid.current_version (campo inexistente em OtherPid,
causaria AttributeError) — corrigido para other_pid.version.
- Correção de bug: mark_items_as_invalid calculava 'invalid' mas
nunca persistia; agora acumula items_to_update e salva via
bulk_update(['proc_status']).
- XMLURL ganha campos detail (JSONField) e is_public, status com
choices=XMLURL_STATUS, e o método record() centralizando a
gravação de sucesso/falha (fetch e pid_provider_xml_failed).
Propósito: eliminar a duplicação de lógica de log entre _handle_xml_fetch_failure, _handle_pid_provider_failure e _register_success, delegando tudo para o novo XMLURL.record(), que já resolve status, pid, detail e salvamento do zipfile num só lugar. Técnico: - Remove _handle_xml_fetch_failure, _handle_pid_provider_failure, _register_success e o helper _truncate_traceback (não é mais necessário truncar manualmente; detail agora é JSONField). - provide_pid_for_xml_uri passa a chamar XMLURL.record(...) diretamente nos três casos (fetch falhou / pid_provider falhou / sucesso), repassando document_item (novo parâmetro). - _handle_unexpected_error passa document_item adiante para o self.provide_pid_for_xml_uri(...) do detail de retry.
…n no admin
Propósito: dar visibilidade no Wagtail admin aos dois novos pontos de
auditoria (falhas de fetch/registro por URL, e eventos de registro
de PID por documento), que antes só existiam como dados sem tela.
Técnico:
- Novo XMLURLViewSet: list_display (url, status, pid), list_filter
por status e is_public, search_fields (url, status, pid).
- Novo PidProviderXMLRegistrationViewSet: list_display (pkg_name,
event_status, pid_provider_xml, created), list_filter
(event_status, created), search_fields incluindo
pid_provider_xml__v2/v3, list_per_page=50 e select_related
('pid_provider_xml') em get_queryset (volume alto esperado).
- Ambos adicionados a PidProviderViewSetGroup.items.
Propósito: Atualizar a versão do pacote packtools para incorporar correções e o novo recurso de nomeação de pacotes (sps_pkg_name) utilizado no registro de XMLURL. Solução técnica: Atualizado o pin de packtools de 4.15.0 para 4.16.8 em requirements/base.txt.
Propósito: Permitir distinguir falhas inesperadas de falhas de obtenção do XML e de falhas do pid provider, dando mais precisão ao diagnóstico de registros de XMLURL. Solução técnica: Adicionada a constante XMLURL_STATUS_UNEXPECTED_FAILURE e sua entrada correspondente na tupla XMLURL_STATUS.
Propósito: Simplificar e tornar mais robusto o registro de tentativas de obtenção/registro de XML, removendo duplicação entre o campo 'exceptions' e o campo 'detail', e centralizando a decisão de status. Solução técnica: - Removido o campo 'exceptions' do modelo (informação passa a ficar em 'detail'). - Reescrito o método 'record' para receber traceback_msg, is_public e params explicitamente. - Adicionado o método estático 'get_status', que determina o status (xml_fetch_failed, unexpected_failure, pid_provider_xml_failed ou success) a partir de xml_with_pre e response. - Adicionada a property 'data', que expõe de forma unificada o resultado (response ou exception + traceback). - Ajustado o nome do arquivo salvo para usar xml_with_pre.sps_pkg_name.
Propósito: Garantir que toda tentativa de obtenção/registro de XML (sucesso, falha de obtenção ou erro inesperado) passe por um único ponto de registro, e corrigir perda de erro em generator. Solução técnica: - Unificado o fluxo de try/except em provide_pid_for_xml_uri, removendo o método auxiliar _handle_unexpected_error e chamando XMLURL.record tanto no caminho de sucesso quanto no de exceção. - Tratado o caso em que XMLWithPre.create não retorna nenhum item, retornando dict de erro explícito (EmptyXMLWithPreError). - Corrigido 'return' para 'yield' no bloco de exceção de verificação de XML em ZIP, evitando que o erro fosse descartado dentro do generator. - Adicionados imports de traceback e do módulo choices.
Propósito: Aplicar no banco de dados as mudanças de modelo introduzidas em XMLURL: remoção do campo 'exceptions' (substituído por 'detail') e inclusão da nova opção 'unexpected_failure' no campo 'status'. Solução técnica: Gerada migration 0018 com as operações RemoveField (exceptions) e AlterField (status, incluindo XMLURL_STATUS_UNEXPECTED_FAILURE nas choices).
Propósito: Criar o pacote pid_provider/tests como ponto de entrada para a suíte de testes unitários do app pid_provider. Solução técnica: Adicionado arquivo __init__.py vazio.
Propósito: Cobrir o comportamento de XMLURL após a refatoração de record()/get_status()/data, incluindo os três cenários documentados (falha ao obter XML, sucesso completo, erro inesperado) e a idempotência via create_or_update. Solução técnica: Adicionados testes para get()/create()/create_or_update(), save_file() (incluindo caminho de exceção), get_status() (staticmethod, os 5 ramos de status) e a property data (resposta, exceção+traceback, e caso vazio).
Propósito: Cobrir a construção de queries (identifier_queries, issn_query, issue_params, article_location_params, article_data_query) e as funções de similaridade (compare, compare_items, compare_lists, get_score, zero_to_none) usadas na busca por candidatos de PID. Solução técnica: Adicionados testes unitários com mocks de xml_adapter e patch de how_similar, cobrindo validação de dados de entrada (validate_input_data), montagem de pkg_name_list e agregação de scores em compare().
Propósito: Cobrir a seleção do melhor candidato dentre múltiplos registros aprovados (score > 0.6), incluindo os casos de 0, 1, 2 e 3+ candidatos aprovados e o desempate por data de atualização. Solução técnica: Adicionados testes com mock de compare, verificando que 'matched' só aparece com 2+ aprovados, que 'registered' recebe sempre o de maior score, e que matched exclui o item já promovido a registered (matched[1:]).
Propósito: Cobrir a lógica de decisão de select_record (singular), que consome o generator select_records e resolve o melhor match por label, incluindo os bugs corrigidos de fatiamento duplicado de matched_items e de auto-referência em unmatched_items. Solução técnica: Adicionados testes com get_best_match mockado, cobrindo listas vazias (puladas via 'if not results'), retorno antecipado no primeiro label com 'registered', acumulação de unmatched_items reais entre labels, e chamada correta de get_best_match com candidatos e data_to_compare.
Propósito: Cobrir o novo comportamento de select_records como generator lazy que apenas monta e materializa (list()) os candidatos por estratégia de busca (ids, journal-issue-article, journal-article), sem mais orquestrar best_matches ou levantar DoesNotExist. Solução técnica: Adicionados testes com banco real (TestCase) e QueryBuilderPidProviderXML mockado, verificando a ordem dos labels yield-ados, que cada branch retorna list (não QuerySet) e a laziness do generator (nada executa até o primeiro next()).
Propósito: Cobrir a nova orquestração de register(), incluindo a mudança de is_updated (agora levanta exceções em vez de retornar dict), a correção do tratamento de ForbiddenPidProviderXMLRegistrationError (antes tratado incorretamente como bad_request) e a regra de quando a auditoria (PidProviderXMLRegistration.record) deve ou não ser gravada. Solução técnica: Adicionados testes cobrindo os caminhos created, updated, skipped 'limpos' (sem auditoria), os mesmos caminhos com matched_items (com auditoria), conflict/forbidden/unmatched/bad_request (sempre com auditoria), o fallback event_status='error' e um conjunto de testes de invariante garantindo que record() nunca é chamado mais de uma vez por execução.
Propósito: Cobrir provide_pid_for_xml_with_pre, provide_pid_for_xml_zip, provide_pid_for_xml_uri, is_registered_xml_with_pre/uri/zip, incluindo os bugs corrigidos de generator que retornava None silenciosamente (EmptyXMLWithPreError) e de 'return' vs 'yield' em is_registered_xml_zip. Solução técnica: Adicionados testes unitários com mocks de PidProviderXML/XMLURL/XMLWithPre/UnexpectedEvent, cobrindo o fluxo unificado try/except de provide_pid_for_xml_uri (obtenção do XML e registro no mesmo bloco), o repasse correto de argumentos em zip, a preservação de resultados parciais quando um generator falha no meio, e a confirmação de que 'except Exception as exception:' faz o binding correto da exceção capturada e a repassa para XMLURL.record.
Propósito: Corrigir incompatibilidade de assinatura do método get_queryset, que não recebia o parâmetro request exigido pela API do Wagtail (ModelViewSet), causando potencial erro em tempo de execução ao construir a listagem.
Solução técnica: Adiciona o parâmetro request à assinatura de get_queryset e repassa-o para super().get_queryset(request) (CommonControlFieldViewSet). Inclui fallback defensivo para self.model._default_manager.all() caso a superclasse retorne None, evitando AttributeError ao encadear select_related('pid_provider_xml').
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
2 participants
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 esse PR faz?
Sincroniza o app
pid_provider(compartilhado entrecoreeupload) trazendo para ocoreos avanços feitos no repositório doupload, fechando a divergência acumulada desde a última sincronização entre os dois projetos.Problemas que resolve:
PidProviderXML.register()falha antes de existir um registro (unmatched, conflito, bad_request, erro inesperado), não havia rastreabilidade padronizada — sóUnexpectedEventgenérico. Novo modeloPidProviderXMLRegistrationgrava sempre o resultado do fluxo, com FK nullable paraPidProviderXML(casos de erro podem não ter registro associado).best_matches/match/title_similarity) foi substituída por score percentual comparável entre estratégias de busca (select_records/select_record/get_best_match+compare()emquery_params.py).fix_duplicated_pkg_nameacessavaother_pid.current_version, campo inexistente emOtherPid(correto éversion) — lançariaAttributeErrorsempre que houvesse deduplicação comother_piddo tipopid_v3.mark_items_as_invalidcalculava o resultado mas nunca persistia — o método não tinha efeito nenhum no banco. Agora usabulk_update.is_registered_xml_uriretornavaNonesilenciosamente quandoXMLWithPre.create()não produzia nenhum item; agora retorna um dict de erro explícito (EmptyXMLWithPreError).is_registered_xml_zipusavareturndentro de um generator no blocoexcept, o que interrompia a iteração sem propagar o erro ao chamador; corrigido parayield.XMLURLguardava só uma string de exceção truncada manualmente. Agora temstatuscomchoices, campodetail(JSON), campois_public(inferido dedocument_item, novo parâmetro emprovide_pid_for_xml_uri) e métodorecord()centralizando a gravação — substituindo três handlers duplicados (_handle_xml_fetch_failure,_handle_pid_provider_failure,_handle_unexpected_error) embase_pid_provider.py.Feature adicionada:
PidProviderXMLManageraplicaselect_related("current_version")por padrão emPidProviderXML.objects, eliminando repetição manual em vários classmethods (get_xml_with_pre,public_items,get_by_pid_v3,mark_items_as_invalid, etc.).XMLURLViewSet,PidProviderXMLRegistrationViewSet) para dar visibilidade a essa auditoria no Wagtail.pid_provider/tests/), cobrindoBasePidProvider(uri/zip, casos de sucesso e falha),PidProviderXML.register/select_record/select_records/get_best_match,XMLURL(create/update/record/data) eQueryBuilderPidProviderXML/compare().packtoolsde4.15.0para4.16.8(dependência usada porget_complete_publication_date,get_article_data,deprecated_sps_pkg_name_list, entre outras APIs novas consumidas neste PR).Onde a revisão poderia começar?
pid_provider/models.py, pelo métodoPidProviderXML.register()— é onde a mudança de comportamento central acontece (uso deselect_records/select_record,SkipSavePidProviderXML, gravação emPidProviderXMLRegistration). Em seguida:pid_provider/exceptions.py— nova exceçãoSkipSavePidProviderXML.pid_provider/query_params.py—compare()/get_best_match(), a base do novo matching, e a simplificação deQueryBuilderPidProviderXML(menoscached_property, novovalidate_input_data()).pid_provider/models.py—PidProviderXMLRegistration,get_best_match(), correções de bug (fix_duplicated_pkg_name,mark_items_as_invalid),XMLURL.record(),PidProviderXMLManager.pid_provider/base_pid_provider.py— comoXMLURL.record()substituiu os handlers antigos, o novo parâmetrodocument_item, e as correções emis_registered_xml_uri/is_registered_xml_zip.pid_provider/wagtail_hooks.py— ViewSets novos (revisão mais rápida, é só exposição no admin).pid_provider/tests/— conferir se os cenários cobertos batem com o comportamento esperado descrito nos itens acima (os docstrings dos testes documentam bem as mudanças de contrato).requirements/base.txt— checar changelog dopacktoolsentre 4.15.0 e 4.16.8 quanto a breaking changes adicionais não usados aqui.Como este poderia ser testado manualmente?
PidProviderXML.register()com:PidProviderXMLRegistration.event_status == "created".force_update=False) → conferirevent_status == "skipped"e que nenhuma nova versão foi salva.unmatched) → conferir quePidProviderXMLRegistrationgravaevent_status == "unmatched"comdetailpopulado, mesmo sempid_provider_xmlassociado.event_status == "conflict".PidProviderXML.deduplicate_items(user, issns)(oufix_duplicated_pkg_namediretamente) em um cenário com documentos duplicados que tenhamother_piddo tipopid_v3— confirmar que não lança maisAttributeError.PidProviderXML.mark_items_as_invalid(issns)com ao menos um XML inválido/corrompido no set — verificar no banco queproc_statusfoi de fato atualizado paraPPXML_STATUS_INVALID(antes, nada era persistido).BasePidProvider.provide_pid_for_xml_uri(...), incluindo comdocument_item={"status": True/False}— conferir queXMLURLgravastatus,detail(com traceback/response),is_publiccorretamente inferido e, quando háxml_with_pre, o zipfile correspondente.BasePidProvider.is_registered_xml_uri(...)contra uma URI que não retorna nenhum XML — conferir que agora vem um dict de erro (EmptyXMLWithPreError) em vez deNone.BasePidProvider.is_registered_xml_zip(...)com um zip que causa exceção durante a leitura — conferir que o erro chega via iteração do generator (antes era perdido).python manage.py test pid_provider.tests.status/is_publiceevent_status/created) e busca funcionando.packtools, rodar a suíte completa dopid_provider(e idealmente deproc/migration) para pegar eventuais regressões de API do pacote.Algum cenário de contexto que queira dar?
O app
pid_providervive em dois repositórios (coreeupload) e evolui em paralelo nos dois — de tempos em tempos é preciso reconciliar manualmente os avanços de um lado para o outro, já que não há um pacote compartilhado único. Este PR é uma dessas reconciliações: ouploadavançou nesses pontos (auditoria, matching, correções, testes) e agora trazemos isso para ocore.Um ponto de contexto importante: o
corejá tem em produção o modeloXMLEvent, usado para auditar falhas no consumo do XML (na geração deArticle). O novoPidProviderXMLRegistrationaudita um momento diferente do ciclo de vida: o registro do PID em si, inclusive quando ele falha antes de qualquerArticleexistir. Avaliamos unificar os dois modelos e decidimos não fazer isso agora — a diferença de nullability/cascade entre eles (SET_NULLvsCASCADE) reflete uma diferença real de domínio, eXMLEventjá está em produção, então o custo de generalizar não compensa o ganho.Outro ponto: o bump de
packtools(4.15.0 → 4.16.8) é uma dependência direta das novas chamadas (get_complete_publication_date,get_article_data,deprecated_sps_pkg_name_list) e não é opcional — sem ele o código novo não roda.Screenshots
Não aplicável — mudanças de modelo, lógica de matching e auditoria; sem alteração de UI além dos novos ViewSets padrão do Wagtail admin (listagem tabular padrão, sem customização visual).
Quais são os tickets relevantes?
#1447
PR no Upload: scieloorg/scms-upload#1023
Referências
uploaddo apppid_provider.NSI.04 - Norma de Desenvolvimento Seguro(seção de segurança abaixo).packtools(4.15.0 → 4.16.8) — conferir antes do merge.Segurança da informação (NSI.04)
Este PR manipula dados sensíveis ou pessoais (LGPD)?
Este PR altera autenticação, autorização, controle de acesso ou gerenciamento de sessão?
Este PR introduz, atualiza ou remove dependências de terceiros?
Este PR foi validado pelo pipeline de segurança (SonarQube / Trivy)?
Este PR concatena, monta ou executa comandos SQL, HTML ou JavaScript a partir de entrada externa?
Q,Prefetch,filter,bulk_update), sem SQL bruto nem interpolação de entrada externa.Este PR expõe novos endpoints, telas ou serviços?
XMLURLViewSet,PidProviderXMLRegistrationViewSet), não são endpoints públicos; seguem o mesmo controle de acesso do admin já existente (autenticação/permissões do Wagtail), sem novo endpoint de API externa.Algum segredo, senha, chave ou token está sendo adicionado ao código-fonte?