diff --git a/requirements/dev.txt b/requirements/dev.txt index 44012d2..716f1df 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -13,6 +13,7 @@ whitenoise pytest>=3.0.7 pytest-cov>=2.4.0 pytest-django>=3.1.2 +pytest-mock>=1.6.3 flake8 coveralls diff --git a/sandbox/static/styles.css b/sandbox/static/styles.css index 61bbb87..503037e 100644 --- a/sandbox/static/styles.css +++ b/sandbox/static/styles.css @@ -66,6 +66,11 @@ div[role="main"] { color: #fff; } +.messages li.error { + background-color: red; + color: #fff; +} + .import #main-content { padding: 0; } diff --git a/setup.cfg b/setup.cfg index 81804ad..dfb309f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -3,7 +3,7 @@ universal=1 [tool:pytest] DJANGO_SETTINGS_MODULE=sandbox.settings -addopts = -vs --cov=td_biblio +addopts = -vs --cov=td_biblio --cov-report term-missing testpaths = td_biblio/tests [flake8] diff --git a/td_biblio/exceptions.py b/td_biblio/exceptions.py new file mode 100644 index 0000000..62b955a --- /dev/null +++ b/td_biblio/exceptions.py @@ -0,0 +1,15 @@ +# td_biblio exceptions +class BaseLoaderError(Exception): + pass + + +class BibTeXLoaderError(BaseLoaderError): + pass + + +class PMIDLoaderError(BaseLoaderError): + pass + + +class DOILoaderError(BaseLoaderError): + pass diff --git a/td_biblio/models.py b/td_biblio/models.py index c419716..cbb7cca 100644 --- a/td_biblio/models.py +++ b/td_biblio/models.py @@ -48,6 +48,11 @@ def get_formatted_name(self): def _set_user(self): """Look for local django user based on human name""" + if '' in (self.last_name, self.first_name): + return + + self._set_first_initial() + User = get_user_model() try: self.user = User.objects.get( @@ -55,7 +60,9 @@ def _set_user(self): models.Q(first_name__iexact=self.first_name) | models.Q(first_name__istartswith=self.first_initial[0]) ) - except: + except User.DoesNotExist: + pass + except User.MultipleObjectsReturned: pass diff --git a/td_biblio/tests/fixtures/entries.py b/td_biblio/tests/fixtures/entries.py index bffedae..3655468 100644 --- a/td_biblio/tests/fixtures/entries.py +++ b/td_biblio/tests/fixtures/entries.py @@ -23,6 +23,8 @@ '26408185', '26588162', '27095822', + '24357323', + '25190796', ] # Digital Object Identfiers @@ -52,4 +54,6 @@ '10.1186/s13059-015-0769-z', '10.1210/jc.2007-0927', '10.1210/jc.2014-4047', + '10.1126/science.1241089', + '10.1126/science.1255274', ] diff --git a/td_biblio/tests/test_loaders.py b/td_biblio/tests/test_loaders.py index 2817120..1a77518 100644 --- a/td_biblio/tests/test_loaders.py +++ b/td_biblio/tests/test_loaders.py @@ -13,6 +13,7 @@ from eutils.exceptions import EutilsNCBIError from requests.exceptions import HTTPError +from ..exceptions import DOILoaderError, PMIDLoaderError from ..utils.loaders import BibTeXLoader, DOILoader, PubmedLoader from ..models import Author, Entry, Journal from .fixtures.entries import PMIDs as FPMIDS, DOIs as FDOIS @@ -206,7 +207,33 @@ def test_save_records_from_mutiple_pmid(self): self.loader.load_records(PMIDs=FPMIDS) self.loader.save_records() - self.assertEqual(Entry.objects.count(), 21) + self.assertEqual(Entry.objects.count(), len(FPMIDS)) + + +@pytest.mark.django_db +class PubmedLoaderToRecordTests(TestCase): + """ + Tests for the patched pubmed loader + """ + def setUp(self): + """ + Set object level vars + """ + self.PMID = 26588162 + self.loader = PubmedLoader() + + @pytest.fixture(autouse=True) + def mock_to_record_with_exception(self, mocker): + + def raise_exception(self, msg): + raise PMIDLoaderError('Patched PMIDLoaderError') + + mocker.patch.object(PubmedLoader, 'to_record', raise_exception) + + def test_load_records_with_to_record_exception(self): + + with pytest.raises(PMIDLoaderError): + self.loader.load_records(PMIDs=self.PMID) @pytest.mark.django_db @@ -252,7 +279,7 @@ def test_load_records_from_an_existing_doi(self): 'last_name': 'Tufféry' } ], - 'journal': 'J. Chem. Theory Comput.', + 'journal': 'Journal of Chemical Theory and Computation', 'volume': '10', 'number': '10', 'pages': '4745-4758', @@ -291,7 +318,6 @@ def test_save_records_from_an_existing_doi(self): self.assertEqual(Journal.objects.count(), 0) self.loader.load_records(DOIs=[self.doi, ]) - print('type: {}'.format(self.doi.__class__.__name__)) self.loader.save_records() self.assertEqual(Author.objects.count(), 4) @@ -321,4 +347,30 @@ def test_save_records_from_several_dois(self): self.loader.load_records(DOIs=FDOIS) self.loader.save_records() - self.assertEqual(Entry.objects.count(), 25) + self.assertEqual(Entry.objects.count(), len(FDOIS)) + + +@pytest.mark.django_db +class DOILoaderToRecordTests(TestCase): + """ + Tests for the patched pubmed loader + """ + def setUp(self): + """ + Set object level vars + """ + self.doi = '10.1021/ct500592m' + self.loader = DOILoader() + + @pytest.fixture(autouse=True) + def mock_to_record_with_exception(self, mocker): + + def raise_exception(self, msg): + raise DOILoaderError('Patched DOILoaderError') + + mocker.patch.object(DOILoader, 'to_record', raise_exception) + + def test_load_records_with_to_record_exception(self): + + with pytest.raises(DOILoaderError): + self.loader.load_records(DOIs=self.doi) diff --git a/td_biblio/utils/loaders.py b/td_biblio/utils/loaders.py index 0b9c338..a3498db 100644 --- a/td_biblio/utils/loaders.py +++ b/td_biblio/utils/loaders.py @@ -7,6 +7,7 @@ import datetime import json import logging +import sys import bibtexparser import eutils.client @@ -19,6 +20,7 @@ from django.utils.translation import ugettext_lazy as _ from habanero import cn +from ..exceptions import DOILoaderError, PMIDLoaderError from ..models import Author, Journal, Entry, AuthorEntryRank @@ -155,7 +157,7 @@ class BibTeXLoader(BaseLoader): Usage: - >>> from td_biblio.utils.managers import BibTeXLoader + >>> from td_biblio.utils.loaders import BibTeXLoader >>> loader = BibTeXLoader() >>> loader.load_records(bibtex_filename='foo.bib') >>> loader.save_records() @@ -179,7 +181,7 @@ def to_record(self, input): # Check if month is numerical or not try: int(pub_date['month']) - except: + except ValueError: pub_date['month'] = strptime(pub_date['month'], '%b').tm_mon # Convert date fields to integers pub_date = dict( @@ -221,7 +223,7 @@ class PubmedLoader(BaseLoader): Usage: - >>> from td_biblio.utils.managers import PubmedLoader + >>> from td_biblio.utils.loaders import PubmedLoader >>> loader = PubmedLoader() >>> loader.load_records(PMIDs=26588162) >>> loader.save_records() @@ -261,7 +263,24 @@ def load_records(self, PMIDs=None): """Load all PMIDs as valid records""" entries = self.client.efetch(db='pubmed', id=PMIDs) - self.records = [self.to_record(r) for r in entries] + self.records = [] + + for entry in entries: + try: + record = self.to_record(entry) + except Exception: + e, v, tb = sys.exc_info() + msg = _( + "An error occured while loading the following PMID: {}. " + "Check logs for details." + ).format( + entry.pmid + ) + logger.error( + '{}, error: {} [{}], data: {}'.format(msg, e, v, entry) + ) + raise PMIDLoaderError(msg) + self.records.append(record) class DOILoader(BaseLoader): @@ -271,7 +290,7 @@ class DOILoader(BaseLoader): Usage: - >>> from td_biblio.utils.managers import DOILoader + >>> from td_biblio.utils.loaders import DOILoader >>> loader = DOILoader() >>> loader.load_records(DOIs='10.1021/ct500592m') >>> loader.save_records() @@ -298,9 +317,9 @@ def to_record(self, input): 'title': input.get('title', ''), 'authors': [ { - 'first_name': a['given'], - 'last_name': a['family'] - } for a in input['author'] + 'first_name': a.get('given', ''), + 'last_name': a.get('family', '') + } for a in input.get('author') ], 'journal': journal, 'volume': input.get('volume', ''), @@ -320,4 +339,21 @@ def load_records(self, DOIs=None): # Records might be a str or unicode (python 2) if not isinstance(records, list): records = [records, ] - self.records = [self.to_record(json.loads(r)) for r in records] + self.records = [] + for r in records: + data = json.loads(r) + try: + record = self.to_record(data) + except Exception: + e, v, tb = sys.exc_info() + msg = _( + "An error occured while loading the following DOI: {}. " + "Check logs for details." + ).format( + data.get('DOI') + ) + logger.error( + '{}, error: {} [{}], data: {}'.format(msg, e, v, data) + ) + raise DOILoaderError(msg) + self.records.append(record) diff --git a/td_biblio/views.py b/td_biblio/views.py index 2ec6b4a..d8dd4e3 100644 --- a/td_biblio/views.py +++ b/td_biblio/views.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- import datetime +import logging from django.contrib import messages from django.contrib.auth.decorators import login_required, user_passes_test @@ -7,10 +8,13 @@ from django.utils.translation import ugettext_lazy as _ from django.views.generic import FormView, ListView +from .exceptions import DOILoaderError, PMIDLoaderError from .forms import EntryBatchImportForm from .models import Author, Collection, Entry, Journal from .utils.loaders import DOILoader, PubmedLoader +logger = logging.getLogger('td_biblio') + def superuser_required(function=None): """ @@ -53,7 +57,7 @@ def get(self, request, *args, **kwargs): # Is it an integer? try: self.current_publication_date = datetime.date(int(year), 1, 1) - except: + except TypeError: self.current_publication_date = None # -- Publication author @@ -61,7 +65,7 @@ def get(self, request, *args, **kwargs): # Is it an integer? try: self.current_publication_author = int(author) - except: + except TypeError: self.current_publication_author = None # -- Publication collection @@ -69,7 +73,7 @@ def get(self, request, *args, **kwargs): # Is it an integer? try: self.current_publication_collection = int(collection) - except: + except TypeError: self.current_publication_collection = None return super(EntryListView, self).get(request, *args, **kwargs) @@ -157,14 +161,26 @@ def form_valid(self, form): pmids = form.cleaned_data['pmids'] if len(pmids): pm_loader = PubmedLoader() - pm_loader.load_records(PMIDs=pmids) + + try: + pm_loader.load_records(PMIDs=pmids) + except PMIDLoaderError as e: + messages.error(self.request, e) + return self.form_invalid(form) + pm_loader.save_records() # DOIs dois = form.cleaned_data['dois'] if len(dois): doi_loader = DOILoader() - doi_loader.load_records(DOIs=dois) + + try: + doi_loader.load_records(DOIs=dois) + except DOILoaderError as e: + messages.error(self.request, e) + return self.form_invalid(form) + doi_loader.save_records() messages.success(