Hi Andrew, >> try: >> + person = Person.objects.get_or_create(email__iexact=email, >> + defaults={'name': name, >> + 'email': email})[0] >> + except IntegrityError: >> + # we lost the race to create the person >> person = Person.objects.get(email__iexact=email) > > get_or_create() should handle the IntegrityError in there for you.
Upon inspection, yes it does, although *not* - annoyingly - in get_or_create itself, but in _create_object_from_params. Indeed, if you just read g_o_c, you can easily convince yourself it's not parallel-safe, even though it is. *sigh* Anyway, you are right and I have fixed this up for v2. Regards, Daniel > >> - if name: # use the latest provided name >> - person.name = name >> - except Person.DoesNotExist: >> - person = Person(name=name, email=email) >> + >> + if name: # use the latest provided name >> + person.name = name >> + person.save() >> >> return person >> >> @@ -947,7 +952,6 @@ def parse_mail(mail, list_id=None): >> raise ValueError("Broken 'Message-Id' header") >> msgid = msgid[:255] >> >> - author = find_author(mail) >> subject = mail.get('Subject') >> name, prefixes = clean_subject(subject, [project.linkname]) >> is_comment = subject_check(subject) >> @@ -973,7 +977,7 @@ def parse_mail(mail, list_id=None): >> >> if not is_comment and (diff or pull_url): # patches or pull requests >> # we delay the saving until we know we have a patch. >> - author.save() >> + author = get_or_create_author(mail) >> >> delegate = find_delegate_by_header(mail) >> if not delegate and diff: >> @@ -984,7 +988,7 @@ def parse_mail(mail, list_id=None): >> # series to match against. >> series = None >> if n: >> - series = find_series(project, mail) >> + series = find_series(project, mail, author) >> else: >> x = n = 1 >> >> @@ -1061,7 +1065,7 @@ def parse_mail(mail, list_id=None): >> is_cover_letter = True >> >> if is_cover_letter: >> - author.save() >> + author = get_or_create_author(mail) >> >> # we don't use 'find_series' here as a cover letter will >> # always be the first item in a thread, thus the references >> @@ -1109,7 +1113,7 @@ def parse_mail(mail, list_id=None): >> if not submission: >> return >> >> - author.save() >> + author = get_or_create_author(mail) >> >> comment = Comment( >> submission=submission, >> diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py >> index 68bcb937b273..6cfe7a484443 100644 >> --- a/patchwork/tests/test_parser.py >> +++ b/patchwork/tests/test_parser.py >> @@ -34,7 +34,7 @@ from patchwork.models import Patch >> from patchwork.models import Person >> from patchwork.models import State >> from patchwork.parser import clean_subject >> -from patchwork.parser import find_author >> +from patchwork.parser import get_or_create_author >> from patchwork.parser import find_patch_content as find_content >> from patchwork.parser import find_project_by_header >> from patchwork.parser import find_series >> @@ -225,7 +225,7 @@ class SenderEncodingTest(TestCase): >> >> def _test_encoding(self, from_header, sender_name, sender_email): >> email = self._create_email(from_header) >> - person = find_author(email) >> + person = get_or_create_author(email) >> person.save() >> >> # ensure it was parsed correctly >> @@ -241,7 +241,7 @@ class SenderEncodingTest(TestCase): >> def test_empty(self): >> email = self._create_email('') >> with self.assertRaises(ValueError): >> - find_author(email) >> + get_or_create_author(email) >> >> def test_ascii_encoding(self): >> from_header = 'example user <u...@example.com>' >> @@ -269,7 +269,7 @@ class SenderEncodingTest(TestCase): >> >> >> class SenderCorrelationTest(TestCase): >> - """Validate correct behavior of the find_author case. >> + """Validate correct behavior of the get_or_create_author case. >> >> Relies of checking the internal state of a Django model object. >> >> @@ -284,25 +284,16 @@ class SenderCorrelationTest(TestCase): >> 'test\n' >> return message_from_string(mail) >> >> - def test_non_existing_sender(self): >> - sender = 'Non-existing Sender <nonexist...@example.com>' >> - mail = self._create_email(sender) >> - >> - # don't create the person - attempt to find immediately >> - person = find_author(mail) >> - self.assertEqual(person._state.adding, True) >> - self.assertEqual(person.id, None) >> - >> def test_existing_sender(self): >> sender = 'Existing Sender <exist...@example.com>' >> mail = self._create_email(sender) >> >> # create the person first >> - person_a = find_author(mail) >> + person_a = get_or_create_author(mail) >> person_a.save() >> >> # then attempt to parse email with the same 'From' line >> - person_b = find_author(mail) >> + person_b = get_or_create_author(mail) >> self.assertEqual(person_b._state.adding, False) >> self.assertEqual(person_b.id, person_a.id) >> >> @@ -311,12 +302,12 @@ class SenderCorrelationTest(TestCase): >> mail = self._create_email(sender) >> >> # create the person first >> - person_a = find_author(mail) >> + person_a = get_or_create_author(mail) >> person_a.save() >> >> # then attempt to parse email with a new 'From' line >> mail = self._create_email('exist...@example.com') >> - person_b = find_author(mail) >> + person_b = get_or_create_author(mail) >> self.assertEqual(person_b._state.adding, False) >> self.assertEqual(person_b.id, person_a.id) >> >> @@ -324,11 +315,11 @@ class SenderCorrelationTest(TestCase): >> sender = 'Existing Sender <exist...@example.com>' >> mail = self._create_email(sender) >> >> - person_a = find_author(mail) >> + person_a = get_or_create_author(mail) >> person_a.save() >> >> mail = self._create_email(sender.upper()) >> - person_b = find_author(mail) >> + person_b = get_or_create_author(mail) >> self.assertEqual(person_b._state.adding, False) >> self.assertEqual(person_b.id, person_a.id) >> >> @@ -361,7 +352,8 @@ class SeriesCorrelationTest(TestCase): >> email = self._create_email(msgid) >> project = create_project() >> >> - self.assertIsNone(find_series(project, email)) >> + self.assertIsNone(find_series(project, email, >> + get_or_create_author(email))) >> >> def test_first_reply(self): >> msgid_a = make_msgid() >> @@ -371,7 +363,8 @@ class SeriesCorrelationTest(TestCase): >> # assume msgid_a was already handled >> ref = create_series_reference(msgid=msgid_a) >> >> - series = find_series(ref.series.project, email) >> + series = find_series(ref.series.project, email, >> + get_or_create_author(email)) >> self.assertEqual(series, ref.series) >> >> def test_nested_series(self): >> @@ -395,7 +388,7 @@ class SeriesCorrelationTest(TestCase): >> # ...and the "first patch" of this new series >> msgid = make_msgid() >> email = self._create_email(msgid, msgids) >> - series = find_series(project, email) >> + series = find_series(project, email, get_or_create_author(email)) >> >> # this should link to the second series - not the first >> self.assertEqual(len(msgids), 4 + 1) # old series + new cover >> > > -- > Andrew Donnellan OzLabs, ADL Canberra > andrew.donnel...@au1.ibm.com IBM Australia Limited _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork