diff --git a/article/tests/test_article_by_collection.py b/article/tests/test_article_by_collection.py new file mode 100644 index 000000000..c11c4caa1 --- /dev/null +++ b/article/tests/test_article_by_collection.py @@ -0,0 +1,222 @@ +""" +Um Article por CollectionPidV2 (XML + coleção, mesmo pid v3): filtros por +coleção, relatórios por coleção, um documento por Article no índice de +busca e vínculo entre as instâncias do mesmo artigo no OAI. +""" +from importlib import import_module + +from django.contrib.auth import get_user_model +from django.test import TestCase + +from article.models import Article +from article.search_indexes import ArticleIndex, ArticleOAIIndex +from article.tests.test_mixins import make_article_in_collection +from collection.models import Collection +from journal.models import Journal, SciELOJournal +from report import data_availability + +User = get_user_model() + +PID_V3 = "CVcmNHzKpFmYM77VQBJwHYt" +PID_V2_SCL = "S0103-65642009000300003" +PID_V2_PSI = "S1678-51772009000300003" + + +class ArticleByCollectionDataMixin: + """ + Psicologia USP: scl (pusp, 0103-6564) e psi (psicousp, 1678-5177) + Journal Single: somente scl (single, 0000-0001) + """ + + @classmethod + def setUpTestData(cls): + cls.user = User.objects.create_user(username="by-collection", password="x") + cls.scl = Collection.objects.create( + acron3="scl", + network_classification=["scielonetwork"], + domain="www.scielo.br", + is_active=True, + creator=cls.user, + ) + cls.psi = Collection.objects.create( + acron3="psi", + domain="pepsic.bvsalud.org", + is_active=True, + creator=cls.user, + ) + cls.pusp = Journal.objects.create(title="Psicologia USP", creator=cls.user) + cls.single = Journal.objects.create(title="Journal Single", creator=cls.user) + for journal, collection, acron, issn_scielo in ( + (cls.pusp, cls.scl, "pusp", "0103-6564"), + (cls.pusp, cls.psi, "psicousp", "1678-5177"), + (cls.single, cls.scl, "single", "0000-0001"), + ): + SciELOJournal.objects.create( + journal=journal, + collection=collection, + journal_acron=acron, + issn_scielo=issn_scielo, + creator=cls.user, + ) + cls.scl_item = make_article_in_collection( + cls.user, PID_V3, cls.scl, PID_V2_SCL, "pusp", + journal=cls.pusp, is_classic_public=True, + ) + cls.psi_item = make_article_in_collection( + cls.user, PID_V3, cls.psi, PID_V2_PSI, "psicousp", + journal=cls.pusp, is_classic_public=True, + ) + + +class SelectItemsByCollectionTest(ArticleByCollectionDataMixin, TestCase): + def test_collection_acron_list(self): + self.assertEqual( + set(Article.select_items(collection_acron_list=["psi"])), + {self.psi_item}, + ) + self.assertEqual( + set(Article.select_items(collection_acron_list=["scl", "psi"])), + {self.scl_item, self.psi_item}, + ) + + def test_journal_acron_of_the_collection(self): + self.assertEqual( + set(Article.select_items(journal_acron_list=["psicousp"])), + {self.psi_item}, + ) + self.assertEqual( + set(Article.select_items(journal_acron_list=["pusp"])), + {self.scl_item}, + ) + + def test_journal_acron_and_collection(self): + self.assertEqual( + list(Article.select_items( + collection_acron_list=["scl"], journal_acron_list=["psicousp"] + )), + [], + ) + + def test_data_availability_report_counts_by_collection(self): + for collection, expected in ((self.scl, 1), (self.psi, 1), (None, 2)): + qs = data_availability.filter_articles(collection=collection) + self.assertEqual(qs.count(), expected, collection) + + +class ArticleIndexByCollectionTest(ArticleByCollectionDataMixin, TestCase): + """Um documento por Article (coleção), relacionados pelo pid v3.""" + + def test_index_queryset_has_one_item_by_collection(self): + self.assertEqual( + set(ArticleIndex().index_queryset()), {self.scl_item, self.psi_item} + ) + + def test_collection_fields_of_the_article_collection(self): + index = ArticleIndex() + self.assertEqual(index.prepare_collection(self.psi_item), ["psi"]) + self.assertEqual(index.prepare_ids(self.psi_item)[:2], [PID_V2_PSI, PID_V3]) + self.assertEqual(index.prepare_ta_cluster(self.psi_item), ["psicousp"]) + + def test_oai_identifier_has_collection(self): + index = ArticleOAIIndex() + self.assertEqual( + index.prepare_id(self.psi_item), f"oai:scielo:psi_{PID_V2_PSI}" + ) + self.assertEqual( + index.prepare_item_id(self.scl_item), f"oai:scielo:scl_{PID_V2_SCL}" + ) + self.assertEqual(index.prepare_collections(self.psi_item), {"1678-5177"}) + + def test_identical_items_urls(self): + self.assertEqual( + self.scl_item.identical_items_urls, + [f"{self.psi.base_url}/scielo.php?script=sci_arttext&pid={PID_V2_PSI}"], + ) + + def test_oai_compile_relates_identical_items(self): + compiled = ArticleOAIIndex().fields["compile"].prepare(self.scl_item) + self.assertIn('', compiled) + self.assertIn( + f"{self.psi.base_url}/scielo.php?script=sci_arttext&pid={PID_V2_PSI}", + compiled, + ) + self.assertIn( + f'scl_{PID_V2_SCL}', compiled + ) + self.assertIn(f'{PID_V3}', compiled) + # o identificador local vem primeiro + self.assertLess( + compiled.index(f"scl_{PID_V2_SCL}"), + compiled.index(f'{PID_V3}'), + ) + # identificadores pid v2 e pid v3 qualificados + self.assertRegex( + compiled, + r'\s*\s*' + + PID_V2_SCL + + "", + ) + self.assertRegex( + compiled, + r'\s*\s*' + + PID_V3 + + "", + ) + + +class MigrationIdentifyCollectionTest(TestCase): + """article.0054: identificação da coleção dos artigos""" + + def setUp(self): + self.migration = import_module( + "article.migrations.0054_migrate_article_collection_pid_v2" + ) + main = ["scielonetwork"] + self.journal_collections = { + # periódico em uma coleção + 1: [{"collection_id": 10, "issn_scielo": "0000-0001", + "journal_acron": "single", "collection__network_classification": main}], + # issn_scielo diferente em cada coleção + 2: [ + {"collection_id": 10, "issn_scielo": "0103-6564", + "journal_acron": "pusp", "collection__network_classification": main}, + {"collection_id": 20, "issn_scielo": "1678-5177", + "journal_acron": "psicousp", "collection__network_classification": None}, + ], + # mesmo issn_scielo nas duas coleções + 3: [ + {"collection_id": 20, "issn_scielo": "0102-311X", + "journal_acron": "csp", "collection__network_classification": None}, + {"collection_id": 10, "issn_scielo": "0102-311X", + "journal_acron": "csp", "collection__network_classification": main}, + ], + # mesmo issn_scielo, nenhuma coleção principal + 4: [ + {"collection_id": 20, "issn_scielo": "0254-9247", + "journal_acron": "rp", "collection__network_classification": None}, + {"collection_id": 30, "issn_scielo": "0254-9247", + "journal_acron": "rp", "collection__network_classification": None}, + ], + } + + def identify(self, journal_id, pid_v2=None, collection_id=None): + return self.migration.identify_collection_id( + {"journal_id": journal_id, "pid_v2": pid_v2, "collection_id": collection_id}, + self.journal_collections, + ) + + def test_article_collection(self): + self.assertEqual(self.identify(2, PID_V2_SCL, collection_id=20), 20) + + def test_single_collection_journal(self): + self.assertEqual(self.identify(1), 10) + + def test_issn_scielo_of_pid_v2(self): + self.assertEqual(self.identify(2, PID_V2_SCL), 10) + self.assertEqual(self.identify(2, PID_V2_PSI), 20) + + def test_same_issn_scielo_uses_main_collection(self): + self.assertEqual(self.identify(3, "S0102-311X2019000104001"), 10) + + def test_not_identified(self): + self.assertIsNone(self.identify(4, "S0254-92472019000104001")) diff --git a/article/tests/test_migration_fill_article_collection.py b/article/tests/test_migration_fill_article_collection.py new file mode 100644 index 000000000..2b47f4db2 --- /dev/null +++ b/article/tests/test_migration_fill_article_collection.py @@ -0,0 +1,126 @@ +from importlib import import_module +from types import SimpleNamespace +from unittest.mock import patch + +from django.contrib.auth import get_user_model +from django.db import connection +from django.test import TestCase + +from article.models import AMArticle, Article +from collection.models import Collection +from journal.models import Journal, SciELOJournal + +User = get_user_model() + +migration = import_module("article.migrations.0055_fill_article_collection") + +PID_V2_SCL = "S0103-65642009000300003" +PID_V2_PSI = "S1678-51772009000300003" + + +class FillArticleCollectionMigrationTest(TestCase): + def setUp(self): + self.user = User.objects.create_user(username="fill_collection_user") + self.scl = Collection.objects.create( + acron3="scl", network_classification=["scielonetwork"], creator=self.user + ) + self.psi = Collection.objects.create(acron3="psi", creator=self.user) + self.rve = Collection.objects.create(acron3="rve", creator=self.user) + + # Psicologia USP: issn_scielo diferente em cada coleção + self.pusp = self.make_journal( + "pusp", ((self.scl, "0103-6564"), (self.psi, "1678-5177")) + ) + # somente em scl + self.single = self.make_journal("single", ((self.scl, "0000-0001"),)) + # mesmo issn_scielo em scl e psi + self.csp = self.make_journal( + "csp", ((self.psi, "0102-311X"), (self.scl, "0102-311X")) + ) + # mesmo issn_scielo, nenhuma coleção principal + self.rp = self.make_journal( + "rp", ((self.psi, "0254-9247"), (self.rve, "0254-9247")) + ) + + def make_journal(self, title, collections): + journal = Journal.objects.create(title=title, creator=self.user) + for collection, issn_scielo in collections: + SciELOJournal.objects.create( + journal=journal, + collection=collection, + issn_scielo=issn_scielo, + creator=self.user, + ) + return journal + + def make_article(self, pid_v3, journal, pid_v2=None, collection=None): + return Article.objects.create( + pid_v3=pid_v3, + pid_v2=pid_v2, + journal=journal, + collection=collection, + creator=self.user, + ) + + def run_migration(self, batch_size=2): + with patch.object(migration, "BATCH_SIZE", batch_size), patch("builtins.print"): + migration.fill_article_collection( + None, SimpleNamespace(connection=connection) + ) + + def assert_collection(self, article, expected): + article.refresh_from_db() + self.assertEqual(article.collection, expected, article.pid_v3) + + def test_legacy_article_with_same_pid_v2(self): + article = self.make_article("v3-legacy", self.pusp, PID_V2_PSI) + AMArticle.objects.create( + pid=PID_V2_SCL, collection=self.scl, new_record=article, creator=self.user + ) + AMArticle.objects.create( + pid=PID_V2_PSI, collection=self.psi, new_record=article, creator=self.user + ) + self.run_migration() + self.assert_collection(article, self.psi) + + def test_issn_scielo_of_pid_v2(self): + scl_item = self.make_article("v3-scl", self.pusp, PID_V2_SCL) + psi_item = self.make_article("v3-psi", self.pusp, PID_V2_PSI) + self.run_migration() + self.assert_collection(scl_item, self.scl) + self.assert_collection(psi_item, self.psi) + + def test_same_issn_scielo_uses_main_collection(self): + article = self.make_article("v3-csp", self.csp, "S0102-311X2019000104001") + self.run_migration() + self.assert_collection(article, self.scl) + + def test_single_collection_journal(self): + article = self.make_article("v3-single", self.single) + self.run_migration() + self.assert_collection(article, self.scl) + + def test_main_collection_of_the_journal(self): + article = self.make_article("v3-main", self.pusp) + self.run_migration() + self.assert_collection(article, self.scl) + + def test_not_identified(self): + article = self.make_article("v3-rp", self.rp, "S0254-92472019000104001") + no_journal = self.make_article("v3-no-journal", None) + self.run_migration() + self.assert_collection(article, None) + self.assert_collection(no_journal, None) + + def test_keeps_existing_collection(self): + article = self.make_article("v3-kept", self.pusp, PID_V2_SCL, self.psi) + self.run_migration() + self.assert_collection(article, self.psi) + + def test_many_batches(self): + articles = [ + self.make_article(f"v3-{i}", self.pusp, PID_V2_PSI) for i in range(7) + ] + self.run_migration(batch_size=3) + for article in articles: + self.assert_collection(article, self.psi) diff --git a/pid_provider/base_pid_provider.py b/pid_provider/base_pid_provider.py index e7f6a10e4..962d39574 100644 --- a/pid_provider/base_pid_provider.py +++ b/pid_provider/base_pid_provider.py @@ -408,7 +408,7 @@ def is_registered_xml_zip(cls, zip_xml_file_path): ), }, ) - return { + yield { "error_msg": f"Unable to check whether {zip_xml_file_path} is registered {e}", "error_type": str(type(e)), } diff --git a/pid_provider/exceptions.py b/pid_provider/exceptions.py index a3c83d076..0a9845b4d 100644 --- a/pid_provider/exceptions.py +++ b/pid_provider/exceptions.py @@ -48,4 +48,9 @@ class APIPidProviderFixPidV2Error(Exception): ... class UnmatchedPidProviderXMLError(Exception): ... -class RequiredCollectionErrorToGetPidProviderXMLError(Exception): ... \ No newline at end of file +class RequiredCollectionErrorToGetPidProviderXMLError(Exception): ... + + +class XMLCollectionNotFoundErrorToGetPidProviderXMLError( + RequiredCollectionErrorToGetPidProviderXMLError +): ... diff --git a/pid_provider/migrations/0021_collectionpidv2_sps_pkg_name.py b/pid_provider/migrations/0021_collectionpidv2_sps_pkg_name.py new file mode 100644 index 000000000..14c61f083 --- /dev/null +++ b/pid_provider/migrations/0021_collectionpidv2_sps_pkg_name.py @@ -0,0 +1,26 @@ +# Generated by Django 5.2.7 on 2026-10-28 13:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("pid_provider", "0020_collectionpidv2"), + ] + + operations = [ + migrations.AddField( + model_name="collectionpidv2", + name="sps_pkg_name", + field=models.CharField( + blank=True, max_length=100, null=True, verbose_name="SPS package name" + ), + ), + migrations.AddIndex( + model_name="collectionpidv2", + index=models.Index( + fields=["sps_pkg_name"], name="pid_provide_sps_pkg_8e5c4f_idx" + ), + ), + ] diff --git a/pid_provider/models.py b/pid_provider/models.py index 472dba67e..f37f37562 100644 --- a/pid_provider/models.py +++ b/pid_provider/models.py @@ -10,7 +10,7 @@ from django.core.files.base import ContentFile from django.core.exceptions import FieldError -from django.db import IntegrityError, models +from django.db import IntegrityError, models, transaction from django.db.models import Prefetch, Q, Count from django.utils.translation import gettext_lazy as _ from modelcluster.fields import ParentalKey @@ -34,7 +34,9 @@ from pid_provider import choices, exceptions from pid_provider.multi_collection import ( get_journal_pid_from_v2, + get_xml_collection_acron, get_xml_collections, + normalize_acron, ) from pid_provider.query_params import ( zero_to_none, @@ -343,7 +345,9 @@ def __str__(self): @classmethod @profile_classmethod def get_or_create(cls, pid_type, pid_in_xml, version, user, pid_provider_xml): - if pid_in_xml and pid_type and version and user and pid_provider_xml: + # version é opcional: na falta da coleção principal, + # PidProviderXML.current_version fica vazio (ver CollectionPidV2) + if pid_in_xml and pid_type and user and pid_provider_xml: try: obj = cls.objects.get( pid_provider_xml=pid_provider_xml, @@ -362,7 +366,7 @@ def get_or_create(cls, pid_type, pid_in_xml, version, user, pid_provider_xml): return obj raise ValueError( - f"OtherPid.get_or_create requires pid_in_xml ({pid_in_xml}) and pid_type ({pid_type}) and version ({version}) and user ({user}) and pid_provider_xml ({pid_provider_xml})" + f"OtherPid.get_or_create requires pid_in_xml ({pid_in_xml}) and pid_type ({pid_type}) and user ({user}) and pid_provider_xml ({pid_provider_xml})" ) @property @@ -404,11 +408,16 @@ class CollectionPidV2(CommonControlField): current_version = models.ForeignKey( XMLVersion, on_delete=models.SET_NULL, null=True, blank=True ) + # nome do pacote SPS obtido de current_version + sps_pkg_name = models.CharField( + _("SPS package name"), max_length=100, null=True, blank=True + ) panels = [ FieldPanel("collection", read_only=True), FieldPanel("journal_acron", read_only=True), FieldPanel("pid_v2", read_only=True), + FieldPanel("sps_pkg_name", read_only=True), AutocompletePanel("current_version", read_only=True), ] @@ -416,14 +425,72 @@ class Meta: unique_together = [("pid_provider_xml", "collection")] indexes = [ models.Index(fields=["pid_v2"]), + models.Index(fields=["sps_pkg_name"], name="pid_provide_sps_pkg_8e5c4f_idx"), ] def __str__(self): return f"{self.collection} {self.journal_acron} {self.pid_v2}" + def set_sps_pkg_name(self, raise_error=True): + """ + Obtém sps_pkg_name de current_version.xml_with_pre, que é a versão + do XML de pid_provider_xml na coleção + """ + try: + self.sps_pkg_name = ( + self.current_version + and self.current_version.xml_with_pre.sps_pkg_name + ) or None + except Exception as e: + if raise_error: + raise + logging.exception(e) + self.sps_pkg_name = None + + @classmethod + def get_ids_to_complete_sps_pkg_name(cls, force_update=False): + """ + Ids dos registros cujo sps_pkg_name deve ser obtido de current_version + """ + items = cls.objects.filter(current_version__isnull=False) + if not force_update: + items = items.filter(sps_pkg_name__isnull=True) + return items.order_by("id").values_list("id", flat=True) + + @classmethod + def complete_sps_pkg_name(cls, user, ids): + """ + Preenche sps_pkg_name dos registros (ids) a partir de + current_version.xml_with_pre.sps_pkg_name + + A leitura do XML é lenta, por isso é feita por lotes de ids + (ver task_complete_collection_pid_v2_sps_pkg_name) + """ + items = cls.objects.filter(id__in=ids, current_version__isnull=False) + result = {"updated": 0, "failed": 0} + for item in items.select_related("current_version").iterator(): + try: + item.set_sps_pkg_name() + item.updated_by = user + item.save() + result["updated"] += 1 + except Exception as e: + result["failed"] += 1 + exc_type, exc_value, exc_traceback = sys.exc_info() + UnexpectedEvent.create( + exception=e, + exc_traceback=exc_traceback, + detail={ + "operation": "CollectionPidV2.complete_sps_pkg_name", + "collection_pid_v2_id": item.id, + "pid_v2": item.pid_v2, + }, + ) + return result + @staticmethod def is_equal_to(pid_provider_xml, xml_with_pre): - if not pid_provider_xml: + if not pid_provider_xml or not pid_provider_xml.pk: return False for item in pid_provider_xml.collection_pids_v2.filter(collection__acron3=xml_with_pre.collection): version = item.current_version @@ -440,6 +507,35 @@ def journal_pid(self): def get(cls, pid_provider_xml, collection): return cls.objects.get(pid_provider_xml=pid_provider_xml, collection=collection) + @classmethod + def get_or_create( + cls, user, pid_provider_xml_id, collection, pid_v2=None, journal_acron=None + ): + """ + Obtém ou cria o registro do documento na coleção + + Não altera o registro existente; current_version é completado no + registro do XML da coleção (ver PidProviderXML.add_collections) + """ + if not (user and pid_provider_xml_id and collection): + raise ValueError( + f"CollectionPidV2.get_or_create requires user ({user}) and pid_provider_xml_id ({pid_provider_xml_id}) and collection ({collection})" + ) + params = dict(pid_provider_xml_id=pid_provider_xml_id, collection=collection) + try: + return cls.objects.get(**params) + except cls.DoesNotExist: + try: + with transaction.atomic(): + return cls.objects.create( + creator=user, + pid_v2=pid_v2, + journal_acron=journal_acron, + **params, + ) + except IntegrityError: + return cls.objects.get(**params) + @classmethod def create( cls, @@ -458,6 +554,7 @@ def create( obj.pid_provider_xml = pid_provider_xml obj.collection = collection obj.current_version = current_version + obj.set_sps_pkg_name(raise_error=False) obj.save() return obj except IntegrityError: @@ -476,7 +573,7 @@ def create_or_update( ): if not (user and pid_provider_xml and collection and pid_v2 and journal_acron and current_version): raise ValueError( - f"CollectionPidV2.create_or_update requires user ({user}) and pid_provider_xml ({pid_provider_xml}) and collection ({collection}) and pid_v2 ({pid_v2})" + f"CollectionPidV2.create_or_update requires user ({user}) and pid_provider_xml ({pid_provider_xml}) and collection ({collection}) and pid_v2 ({pid_v2}) and journal_acron ({journal_acron}) and current_version ({current_version})" ) try: obj = cls.get(pid_provider_xml, collection) @@ -484,6 +581,7 @@ def create_or_update( obj.journal_acron = journal_acron obj.updated_by = user obj.current_version = current_version + obj.set_sps_pkg_name(raise_error=False) obj.save() return obj except cls.DoesNotExist: @@ -507,6 +605,8 @@ def get_current_version(pid_provider_xml, collection=None, collection_acron=None return if not collection and not collection_acron: return pid_provider_xml.current_version + if not pid_provider_xml.pk: + return params = {"current_version__isnull": False} if collection: @@ -1034,6 +1134,32 @@ def register( raise exceptions.RequiredCollectionErrorToGetPidProviderXMLError( "Unable to identify collection from XML. Please provide collection_acron or ensure XML has valid collection information." ) + if not xml_with_pre.collection: + # o XML não informa a coleção de origem (custom-meta): + # identifica-a entre as coleções do periódico, senão o pid v2 + # e a versão do XML da coleção não seriam registrados + # (ver add_collections) + collection_acron = get_xml_collection_acron( + xml_with_pre, xml_collections + ) + if not collection_acron: + raise exceptions.RequiredCollectionErrorToGetPidProviderXMLError( + "Unable to identify the XML collection among the journal collections " + f"{[item['collection'].acron3 for item in xml_collections]}. " + "Please provide collection_acron or ensure XML has valid collection information." + ) + xml_with_pre.collection = collection_acron + # a coleção de origem informada no XML (custom-meta) deve ser uma + # das coleções do periódico, senão o pid v2 e a versão do XML da + # coleção não seriam registrados (ver add_collections) + if xml_with_pre.collection and not any( + item["collection"].acron3 == xml_with_pre.collection + for item in xml_collections + ): + raise exceptions.XMLCollectionNotFoundErrorToGetPidProviderXMLError( + f"Journal not found in the XML collection ({xml_with_pre.collection}). " + f"Journal collections: {[item['collection'].acron3 for item in xml_collections]}" + ) input_data.update(fix_get_article_data(xml_with_pre)) input_data["origin"] = origin response["input_data"] = input_data @@ -1096,15 +1222,18 @@ def register( origin_date, registered_in_core, ) - registered = cls._save( - registered, - xml_adapter, - user, - origin_date, - available_since, - registered_in_core, - xml_collections, - ) + # evita registro parcial (ex.: pid v2 da coleção principal + # gravado sem o CollectionPidV2) em caso de falha + with transaction.atomic(): + registered = cls._save( + registered, + xml_adapter, + user, + origin_date, + available_since, + registered_in_core, + xml_collections, + ) # data to return response.update(registered.data) # garante que o valor de v2 seja o da entrada, da coleção processada, e não o da coleção principal (que pode ser diferente) @@ -1276,7 +1405,12 @@ def add_collections(self, user, xml_adapter, xml_collections): self, collection, xml_with_pre.v2, - journal_acron=xml_collection["journal_acron"], + # na falta do acrônimo do periódico na coleção, usa o do XML, + # que é o da coleção de origem do XML + journal_acron=( + xml_collection["journal_acron"] + or normalize_acron(xml_with_pre.journal_acron) + ), current_version=current_version, ) @@ -1485,12 +1619,17 @@ def _add_current_version( @profile_method def check_registered_pids_changed(self, xml_with_pre, ignore_v2=False): registered_changed = [] + # versão da coleção do XML ou, na falta dela, a da coleção principal + current_version = ( + self.get_current_version(collection_acron=xml_with_pre.collection) + or self.current_version + ) if self.v3 != xml_with_pre.v3: registered_changed.append( { "pid_type": "pid_v3", "pid_in_xml": xml_with_pre.v3, - "version": self.current_version, + "version": current_version, "registered": self.v3, } ) @@ -1511,7 +1650,7 @@ def check_registered_pids_changed(self, xml_with_pre, ignore_v2=False): { "pid_type": "aop_pid", "pid_in_xml": xml_with_pre.aop_pid, - "version": self.current_version, + "version": current_version, "registered": self.aop_pid, } ) @@ -1879,7 +2018,10 @@ class Meta: ] def __str__(self): - return f"{self.pid_provider_xml.v3}" + if self.pid_provider_xml and self.pid_provider_xml.v3: + return f"{self.pid_provider_xml.v3}" + + return self.correct_pid_v2 or self.incorrect_pid_v2 or "-" @staticmethod @profile_staticmethod @@ -1888,7 +2030,7 @@ def autocomplete_custom_queryset_filter(search_term): @profile_method def autocomplete_label(self): - return f"{self.pid_provider_xml.v3}" + return str(self) @classmethod @profile_classmethod diff --git a/pid_provider/multi_collection.py b/pid_provider/multi_collection.py index e27a362db..7d2ff5833 100644 --- a/pid_provider/multi_collection.py +++ b/pid_provider/multi_collection.py @@ -35,8 +35,8 @@ def normalize_acron(acron): def get_xml_collections(xml_with_pre): """ Retorna os dados do periódico em cada coleção em que está presente, - identificado pelos ISSNs e pelo PID do periódico contido no pid v2, - com a coleção principal primeiro + identificado pelos ISSNs, pelo PID do periódico contido no pid v2 ou + pelo acrônimo, com a coleção principal primeiro Returns ------- @@ -44,8 +44,8 @@ def get_xml_collections(xml_with_pre): [{"collection": Collection, "journal_acron": str, "journal_pid": str, "is_main": bool}] - Se o XML informa a coleção de origem (custom-meta), retorna somente - os dados do periódico nesta coleção + Não filtra pela coleção de origem do XML (custom-meta): retorna todas + as coleções do periódico (ver PidProviderXML.add_collections) """ issn_print = xml_with_pre.journal_issn_print @@ -99,3 +99,41 @@ def get_xml_collections(xml_with_pre): items.insert(0, main_item) return items + + +def get_xml_collection_acron(xml_with_pre, xml_collections): + """ + Identifica a coleção de origem do XML que não a informa (custom-meta) + entre as coleções do periódico (ver get_xml_collections) + + Restringe as coleções do periódico, nesta ordem: + - pelo PID do periódico contido no pid v2 (issn_scielo) + - pelo acrônimo do periódico + + Returns + ------- + str + acrônimo da coleção ou None, se não houver uma única coleção candidata + (ex.: periódico com o mesmo PID e acrônimo em mais de uma coleção) + """ + journal_pid = get_journal_pid_from_v2(xml_with_pre.v2) + criteria = ( + ("journal_pid", journal_pid and journal_pid.upper()), + ("journal_acron", normalize_acron(xml_with_pre.journal_acron)), + ) + candidates = list(xml_collections) + for key, value in criteria: + if len(candidates) <= 1: + break + if not value: + continue + matched = [ + item + for item in candidates + if normalize_acron(item[key]) == value.lower() + ] + if matched: + candidates = matched + if len(candidates) == 1: + return candidates[0]["collection"].acron3 + return None diff --git a/pid_provider/scripts/complete_collection_pid_v2_sps_pkg_name.py b/pid_provider/scripts/complete_collection_pid_v2_sps_pkg_name.py new file mode 100644 index 000000000..da6f4750d --- /dev/null +++ b/pid_provider/scripts/complete_collection_pid_v2_sps_pkg_name.py @@ -0,0 +1,10 @@ +from pid_provider.tasks import task_complete_collection_pid_v2_sps_pkg_name + + +def run(username, force_update=None): + task_complete_collection_pid_v2_sps_pkg_name.apply_async( + kwargs={ + "username": username, + "force_update": force_update in ("true", "True", "1"), + } + ) diff --git a/pid_provider/tasks.py b/pid_provider/tasks.py index 0977eee10..bd8ce7f0e 100644 --- a/pid_provider/tasks.py +++ b/pid_provider/tasks.py @@ -8,7 +8,7 @@ ) # ajuste o import conforme sua estrutura from core.utils.utils import _get_user from pid_provider.provider import PidProvider -from pid_provider.models import PidProviderXML +from pid_provider.models import CollectionPidV2, PidProviderXML from journal.models import Journal, SciELOJournal from tracker.models import UnexpectedEvent @@ -300,3 +300,43 @@ def task_fix_journal_pid_provider_xmls_status( }, ) raise + + +@celery_app.task(bind=True) +def task_complete_collection_pid_v2_sps_pkg_name( + self, + username=None, + user_id=None, + force_update=False, + batch_size=100, +): + """ + Preenche CollectionPidV2.sps_pkg_name a partir de + current_version.xml_with_pre.sps_pkg_name + + Divide os registros em lotes processados por subtarefas, para que cada + uma termine dentro do limite de tempo de tarefa do celery + """ + user = _get_user(self.request, username=username, user_id=user_id) + ids = list( + CollectionPidV2.get_ids_to_complete_sps_pkg_name(force_update=force_update) + ) + for start in range(0, len(ids), batch_size): + subtask_complete_collection_pid_v2_sps_pkg_name.apply_async( + kwargs=dict( + user_id=user.id, + ids=ids[start : start + batch_size], + ) + ) + return {"total": len(ids), "batch_size": batch_size} + + +@celery_app.task(bind=True) +def subtask_complete_collection_pid_v2_sps_pkg_name( + self, + username=None, + user_id=None, + ids=None, +): + user = _get_user(self.request, username=username, user_id=user_id) + return CollectionPidV2.complete_sps_pkg_name(user, ids) diff --git a/pid_provider/tests/test_collection_pid_v2.py b/pid_provider/tests/test_collection_pid_v2.py index 852554526..43329e2ea 100644 --- a/pid_provider/tests/test_collection_pid_v2.py +++ b/pid_provider/tests/test_collection_pid_v2.py @@ -22,6 +22,7 @@ from pid_provider.models import CollectionPidV2, PidProviderXML, XMLVersion from pid_provider.multi_collection import ( get_journal_pid_from_v2, + get_xml_collection_acron, get_xml_collections, normalize_acron, ) @@ -133,6 +134,75 @@ def test_normalize_acron(self): self.assertIsNone(normalize_acron(None)) +class GetXMLCollectionAcronTest(SimpleTestCase): + def xml_collections(self, *items): + return [ + { + "collection": SimpleNamespace(acron3=acron3), + "journal_acron": journal_acron, + "journal_pid": journal_pid, + "is_main": is_main, + } + for acron3, journal_acron, journal_pid, is_main in items + ] + + def test_by_journal_pid_of_pid_v2(self): + xml_collections = self.xml_collections( + ("scl", "pusp", ISSN_PRINT, True), + ("psi", "psicousp", ISSN_ELECTRONIC, False), + ) + for v2, expected in ((SCL_V2, "scl"), (PSI_V2, "psi")): + with self.subTest(v2=v2): + self.assertEqual( + get_xml_collection_acron( + make_xml_with_pre(v2, "pusp"), xml_collections + ), + expected, + ) + + def test_by_journal_acron_when_journal_pid_is_the_same(self): + xml_collections = self.xml_collections( + ("scl", "pusp", ISSN_PRINT, True), + ("psi", "psicousp", ISSN_PRINT, False), + ) + self.assertEqual( + get_xml_collection_acron( + make_xml_with_pre(SCL_V2, " PSICOUSP "), xml_collections + ), + "psi", + ) + + def test_by_journal_acron_when_journal_pid_is_unknown(self): + xml_collections = self.xml_collections( + ("scl", "pusp", ISSN_PRINT, True), + ("psi", "psicousp", ISSN_ELECTRONIC, False), + ) + self.assertEqual( + get_xml_collection_acron( + make_xml_with_pre("S0000-00002009000300003", "psicousp"), + xml_collections, + ), + "psi", + ) + + def test_single_collection(self): + xml_collections = self.xml_collections(("psi", "psicousp", ISSN_ELECTRONIC, False)) + self.assertEqual( + get_xml_collection_acron(make_xml_with_pre(None, None), xml_collections), + "psi", + ) + + def test_none_when_ambiguous(self): + xml_collections = self.xml_collections( + ("scl", "pusp", ISSN_PRINT, True), + ("psi", "pusp", ISSN_PRINT, False), + ) + self.assertIsNone( + get_xml_collection_acron(make_xml_with_pre(SCL_V2, "pusp"), xml_collections) + ) + self.assertIsNone(get_xml_collection_acron(make_xml_with_pre(SCL_V2, "pusp"), [])) + + class GetXMLCollectionsTest(MultiCollectionJournalTestBase): def result(self, xml_with_pre): return [ diff --git a/pid_provider/tests/test_other_pid.py b/pid_provider/tests/test_other_pid.py index 75e4ea46b..f0c791a79 100644 --- a/pid_provider/tests/test_other_pid.py +++ b/pid_provider/tests/test_other_pid.py @@ -11,8 +11,6 @@ class OtherPidGetOrCreateTests(TestCase): def setUp(self): self.user = User.objects.create_user(username="otherpid-user", password="x") self.ppx = PidProviderXML.objects.create(creator=self.user, v3="ABC") - # version é obrigatório (truthy) para get_or_create -- um FK vazio - # (None) faz a validação falhar e levantar ValueError. self.version = XMLVersion.objects.create(creator=self.user, pid_provider_xml=self.ppx) def test_raises_value_error_when_any_required_argument_is_missing(self): @@ -25,15 +23,17 @@ def test_raises_value_error_when_any_required_argument_is_missing(self): pid_provider_xml=self.ppx, ) - def test_raises_value_error_when_version_is_missing(self): - with self.assertRaises(ValueError): - OtherPid.get_or_create( - pid_type="pid_v3", - pid_in_xml="OLD-V3", - version=None, - user=self.user, - pid_provider_xml=self.ppx, - ) + def test_creates_when_version_is_missing(self): + # sem a coleção principal, PidProviderXML.current_version é vazio + obj = OtherPid.get_or_create( + pid_type="pid_v3", + pid_in_xml="OLD-V3", + version=None, + user=self.user, + pid_provider_xml=self.ppx, + ) + self.assertIsNone(obj.version) + self.assertEqual(obj.pid_in_xml, "OLD-V3") def test_creates_new_when_none_exists(self): obj = OtherPid.get_or_create( diff --git a/pid_provider/tests/test_pid_provider_xml_maintenance.py b/pid_provider/tests/test_pid_provider_xml_maintenance.py index 6a0979997..87aba30aa 100644 --- a/pid_provider/tests/test_pid_provider_xml_maintenance.py +++ b/pid_provider/tests/test_pid_provider_xml_maintenance.py @@ -223,13 +223,10 @@ def test_keeps_most_recent_and_records_other_pid_for_the_rest(self): ).exists() ) - def test_swallows_error_when_older_item_has_no_current_version(self): + def test_creates_other_pid_when_older_item_has_no_current_version(self): """ - Documenta o comportamento real: sem current_version, o OtherPid do - item mais antigo não pode ser criado (version obrigatório) -- - fix_duplicated_pkg_name captura a exceção internamente (via - UnexpectedEvent.create) e não propaga, mas também não cria o - OtherPid nem levanta erro para o chamador. + Sem current_version (ex.: sem a coleção principal), o OtherPid do + item mais antigo é criado sem version. """ PidProviderXML.objects.create( creator=self.user, v3="V3-OLDER-NOVERSION", pkg_name="pkg-fix-noversion" @@ -239,13 +236,15 @@ def test_swallows_error_when_older_item_has_no_current_version(self): ) newer.save() - PidProviderXML.fix_duplicated_pkg_name("pkg-fix-noversion", self.user) # não deve propagar + PidProviderXML.fix_duplicated_pkg_name("pkg-fix-noversion", self.user) newer.refresh_from_db() self.assertEqual(newer.proc_status, choices.PPXML_STATUS_DEDUPLICATED) - self.assertFalse( + self.assertTrue( OtherPid.objects.filter( - pid_provider_xml=newer, pid_in_xml="V3-OLDER-NOVERSION" + pid_provider_xml=newer, + pid_in_xml="V3-OLDER-NOVERSION", + version__isnull=True, ).exists() ) diff --git a/pid_provider/tests/test_register.py b/pid_provider/tests/test_register.py index d631469bb..9408f09b2 100644 --- a/pid_provider/tests/test_register.py +++ b/pid_provider/tests/test_register.py @@ -164,14 +164,19 @@ def setUp(self): self.addCleanup(self.p_record.stop) # register() exige que a coleção do XML seja identificável - # (RequiredCollectionErrorToGetPidProviderXMLError); _save está + # (RequiredCollectionErrorToGetPidProviderXMLError) e que seja uma + # das coleções do periódico + # (XMLCollectionNotFoundErrorToGetPidProviderXMLError); _save está # mockado nos testes, então o conteúdo da lista não é consumido. + self.xml.collection = "scl" + collection = MagicMock(name="collection") + collection.acron3 = "scl" self.p_xml_collections = patch(f"{PATCH_BASE}.get_xml_collections") self.m_xml_collections = self.p_xml_collections.start() self.m_xml_collections.return_value = [ { "is_main": True, - "collection": MagicMock(name="collection"), + "collection": collection, "journal_acron": "acron", "journal_pid": "0000-0000", } @@ -510,6 +515,20 @@ def test_unidentified_collection_becomes_error_response(self): ) self.assert_recorded_status("error") + def test_journal_not_found_in_the_xml_collection_becomes_error_response(self): + self.xml.collection = "psi" + with patch(f"{PATCH_BASE}.select_record") as m_select, \ + patch(f"{PATCH_BASE}.PidProviderXML._save") as m_save: + response = PidProviderXML.register(self.xml, "file.xml", self.user) + + m_select.assert_not_called() + m_save.assert_not_called() + self.assertEqual( + response["error_type"], + str(exceptions.XMLCollectionNotFoundErrorToGetPidProviderXMLError), + ) + self.assert_recorded_status("error") + class UnexpectedErrorPathTest(RegisterTestBase): """ diff --git a/pid_provider/tests/test_register_multi_collection.py b/pid_provider/tests/test_register_multi_collection.py index 901f53b3d..e2c343251 100644 --- a/pid_provider/tests/test_register_multi_collection.py +++ b/pid_provider/tests/test_register_multi_collection.py @@ -34,6 +34,7 @@ PidProviderXML, XMLVersion, ) +from pid_provider.tasks import task_complete_collection_pid_v2_sps_pkg_name User = get_user_model() @@ -481,17 +482,63 @@ class RegisterWithoutCollectionInXMLTest( ): """XML não informa a coleção de origem""" - def test_no_version_is_registered(self): - response = self.register(make_xml_with_pre(SCL_V2, "pusp"), "scl") + def test_collection_is_identified_by_journal_pid_of_pid_v2(self): + response = self.register(make_xml_with_pre(PSI_V2, "pusp"), "psi") registered = PidProviderXML.objects.get() - self.assertEqual(response["v2"], SCL_V2) + self.assertEqual(response["v2"], PSI_V2) self.assertEqual( set(registered.collections.values_list("acron3", flat=True)), {"scl", "psi"}, ) - self.assertFalse(CollectionPidV2.objects.exists()) + self.assertEqual(registered.collection_pids_v2_data, [PSI_DATA]) + # a versão registrada informa a coleção identificada + self.assertEqual( + registered.get_current_version(self.psi).xml_with_pre.collection, "psi" + ) self.assertIsNone(registered.current_version) + + def test_main_collection_xml_is_the_current_version(self): + self.register(make_xml_with_pre(SCL_V2, "pusp"), "scl") + + registered = PidProviderXML.objects.get() + self.assertEqual(registered.v2, SCL_V2) + self.assertEqual(registered.collection_pids_v2_data, [SCL_DATA]) + self.assert_current_version_is_from(registered, self.scl) + + def test_collection_is_identified_by_journal_acron(self): + # PID do periódico desconhecido: identifica pelo acrônimo + SciELOJournal.objects.filter(collection=self.psi).update( + issn_scielo=ISSN_PRINT + ) + self.register(make_xml_with_pre(SCL_V2, "psicousp"), "psi") + + registered = PidProviderXML.objects.get() + self.assertEqual( + registered.get_current_version(self.psi).xml_with_pre.collection, "psi" + ) + self.assertIsNone(registered.get_current_version(self.scl)) + + def test_ambiguous_collection_is_not_registered(self): + # mesmo PID e acrônimo do periódico nas coleções + SciELOJournal.objects.filter(collection=self.psi).update( + issn_scielo=ISSN_PRINT, journal_acron="pusp" + ) + + response = PidProviderXML.register( + make_xml_with_pre(SCL_V2, "pusp"), + "scl", + self.user, + origin_date="2026-01-01", + ) + + self.assertEqual(response["event_status"], "error") + self.assertEqual( + response["error_type"], + str(exceptions.RequiredCollectionErrorToGetPidProviderXMLError), + ) + self.assertFalse(PidProviderXML.objects.exists()) + self.assertFalse(CollectionPidV2.objects.exists()) self.assertFalse(XMLVersion.objects.exists()) def test_unidentified_collection_is_not_registered(self): @@ -534,6 +581,64 @@ def test_each_collection_has_its_own_current_version(self): self.assertEqual(scl_version.xml_with_pre.collection, "scl") self.assertEqual(psi_version.xml_with_pre.collection, "psi") + def test_each_collection_has_sps_pkg_name_of_its_current_version(self): + self.register(scl_xml(), "scl") + self.register(psi_xml(), "psi") + + for item in CollectionPidV2.objects.all(): + with self.subTest(collection=item.collection.acron3): + self.assertIsNotNone(item.sps_pkg_name) + self.assertEqual( + item.sps_pkg_name, item.current_version.xml_with_pre.sps_pkg_name + ) + + def test_complete_sps_pkg_name(self): + self.register(scl_xml(), "scl") + self.register(psi_xml(), "psi") + expected = dict( + CollectionPidV2.objects.values_list("id", "sps_pkg_name") + ) + CollectionPidV2.objects.update(sps_pkg_name=None) + + ids = CollectionPidV2.get_ids_to_complete_sps_pkg_name() + result = CollectionPidV2.complete_sps_pkg_name(self.user, list(ids)) + + self.assertEqual(result, {"updated": 2, "failed": 0}) + self.assertEqual( + dict(CollectionPidV2.objects.values_list("id", "sps_pkg_name")), + expected, + ) + + def test_get_ids_to_complete_sps_pkg_name(self): + self.register(scl_xml(), "scl") + self.register(psi_xml(), "psi") + all_ids = list(CollectionPidV2.objects.order_by("id").values_list("id", flat=True)) + + self.assertEqual(list(CollectionPidV2.get_ids_to_complete_sps_pkg_name()), []) + self.assertEqual( + list(CollectionPidV2.get_ids_to_complete_sps_pkg_name(force_update=True)), + all_ids, + ) + + def test_task_complete_sps_pkg_name_dispatches_batches(self): + self.register(scl_xml(), "scl") + self.register(psi_xml(), "psi") + CollectionPidV2.objects.update(sps_pkg_name=None) + ids = list(CollectionPidV2.objects.order_by("id").values_list("id", flat=True)) + + with patch( + "pid_provider.tasks.subtask_complete_collection_pid_v2_sps_pkg_name.apply_async" + ) as mock_apply_async: + result = task_complete_collection_pid_v2_sps_pkg_name( + user_id=self.user.id, batch_size=1 + ) + + self.assertEqual(result, {"total": 2, "batch_size": 1}) + self.assertEqual( + [call.kwargs["kwargs"]["ids"] for call in mock_apply_async.call_args_list], + [[ids[0]], [ids[1]]], + ) + def test_current_version_is_the_main_collection_version(self): self.register(scl_xml(), "scl") self.register(psi_xml(), "psi") @@ -869,3 +974,79 @@ def test_erratum_does_not_change_the_article(self): }, article_versions, ) + + +class RegisterCollectionPidV2GuaranteesTest( + RegisterSameArticleInDifferentCollectionsTestBase +): + """ + O XML de scl registra o pid v2 de scl (CollectionPidV2) ou o registro + falha, sem registro parcial + """ + + def setUp(self): + super().setUp() + self.register(psi_xml(), "psi") + self.psi_registered = PidProviderXML.objects.get() + + def register_with_error(self, xml_with_pre, filename): + return PidProviderXML.register( + xml_with_pre, filename, self.user, origin_date="2026-01-01" + ) + + def assert_psi_registered_unchanged(self): + registered = PidProviderXML.objects.get() + self.assertEqual(registered.v3, self.psi_registered.v3) + self.assertIsNone(registered.v2) + self.assertIsNone(registered.current_version) + self.assertEqual(registered.collection_pids_v2_data, [PSI_DATA]) + self.assertFalse(OtherPid.objects.exists()) + self.assertEqual(XMLVersion.objects.count(), 1) + + def test_journal_not_found_in_the_xml_collection_is_an_error(self): + SciELOJournal.objects.filter(collection=self.scl).delete() + + response = self.register_with_error(scl_xml(), "scl") + + self.assertEqual(response["event_status"], "error") + self.assertEqual( + response["error_type"], + str(exceptions.XMLCollectionNotFoundErrorToGetPidProviderXMLError), + ) + self.assertIn("scl", response["error_msg"]) + self.assert_psi_registered_unchanged() + + def test_journal_acron_of_the_xml_is_used_when_missing_in_collection(self): + SciELOJournal.objects.filter(collection=self.scl).update(journal_acron=None) + + self.register(scl_xml(), "scl") + + registered = PidProviderXML.objects.get() + self.assertEqual(registered.v2, SCL_V2) + self.assert_pids_v2_by_collection(registered) + + def test_failure_to_register_collection_pid_v2_does_not_leave_partial_record(self): + with patch.object( + CollectionPidV2, "create_or_update", side_effect=ValueError("failure") + ): + response = self.register_with_error(scl_xml(), "scl") + + # o documento foi identificado (event_status "updated"), mas o + # registro falhou + self.assertEqual(response["error_type"], str(ValueError)) + self.assertIn("failure", response["error_msg"]) + # v2 e current_version de scl não ficam gravados sem o CollectionPidV2 + self.assert_psi_registered_unchanged() + + def test_scl_xml_is_registered_after_failure(self): + with patch.object( + CollectionPidV2, "create_or_update", side_effect=ValueError("failure") + ): + self.register_with_error(scl_xml(), "scl") + + response = self.register(scl_xml(), "scl") + + registered = PidProviderXML.objects.get() + self.assertEqual(response["v3"], self.psi_registered.v3) + self.assertEqual(registered.v2, SCL_V2) + self.assert_pids_v2_by_collection(registered)