Do not blindly delete duplicate schedules

django_celery_beat/models.py

@@ -122,8 +122,9 @@ def from_schedule(cls, schedule):
         except cls.DoesNotExist:
             return cls(**spec)
         except MultipleObjectsReturned:
-            cls.objects.filter(**spec).delete()
-            return cls(**spec)
+            # unique_together constraint should not permit reaching this code,
+            # but just in case, we return the first schedule found
+            return cls.objects.filter(**spec).first()
 
     def __str__(self):
         return '{0} ({1}, {2})'.format(
@@ -183,8 +184,7 @@ def from_schedule(cls, schedule, period=SECONDS):
         except cls.DoesNotExist:
             return cls(every=every, period=period)
         except MultipleObjectsReturned:
-            cls.objects.filter(every=every, period=period).delete()
-            return cls(every=every, period=period)
+            return cls.objects.filter(every=every, period=period).first()
 
     def __str__(self):
         readable_period = None
@@ -236,8 +236,7 @@ def from_schedule(cls, schedule):
         except cls.DoesNotExist:
             return cls(**spec)
         except MultipleObjectsReturned:
-            cls.objects.filter(**spec).delete()
-            return cls(**spec)
+            return cls.objects.filter(**spec).first()
 
 
 class CrontabSchedule(models.Model):
@@ -350,8 +349,7 @@ def from_schedule(cls, schedule):
         except cls.DoesNotExist:
             return cls(**spec)
         except MultipleObjectsReturned:
-            cls.objects.filter(**spec).delete()
-            return cls(**spec)
+            return cls.objects.filter(**spec).first()
 
 
 class PeriodicTasks(models.Model):

t/unit/test_models.py

@@ -7,13 +7,17 @@
 from django.db.migrations.autodetector import MigrationAutodetector
 from django.db.migrations.loader import MigrationLoader
 from django.db.migrations.questioner import NonInteractiveMigrationQuestioner
+from django.utils import timezone
 
 import timezone_field
 
 from django_celery_beat import migrations as beat_migrations
 from django_celery_beat.models import (
     crontab_schedule_celery_timezone,
     SolarSchedule,
+    CrontabSchedule,
+    ClockedSchedule,
+    IntervalSchedule,
 )
 
 
@@ -74,6 +78,20 @@ def test_default_timezone_without_settings_config(self):
     def test_default_timezone_with_settings_config(self):
         assert crontab_schedule_celery_timezone() == self.FIRST_VALID_TIMEZONE
 
+    def test_duplicate_schedules(self):
+        # See: https://github.com/celery/django-celery-beat/issues/322
+        # create 2 duplicates schedules
+        sched1 = CrontabSchedule.objects.create(hour="4")
+        CrontabSchedule.objects.create(hour="4")
+        self.assertEqual(CrontabSchedule.objects.count(), 2)
+        # try to create a duplicate CrontabSchedule from a celery schedule
+        schedule = schedules.crontab(hour="4")
+        sched3 = CrontabSchedule.from_schedule(schedule)
+        # the schedule should be the first of the 2 previous duplicates
+        self.assertEqual(sched3, sched1)
+        # and the duplicates should not be deleted !
+        self.assertEqual(CrontabSchedule.objects.count(), 2)
+
 
 class SolarScheduleTestCase(TestCase):
     EVENT_CHOICES = SolarSchedule._meta.get_field("event").choices
@@ -99,3 +117,43 @@ def test_celery_solar_schedules_included_as_event_choices(self):
 
         for event_choice in event_choices_values:
             assert event_choice in schedules.solar._all_events
+
+    def test_duplicate_schedules(self):
+        # Duplicates cannot be tested for solar schedules because of the
+        # unique constraints in the SolarSchedule model
+        pass
+
+
+class IntervalScheduleTestCase(TestCase):
+
+    def test_duplicate_schedules(self):
+        # See: https://github.com/celery/django-celery-beat/issues/322
+        kwargs = {'every': 1, 'period': IntervalSchedule.SECONDS}
+        # create 2 duplicates schedules
+        sched1 = IntervalSchedule.objects.create(**kwargs)
+        IntervalSchedule.objects.create(**kwargs)
+        self.assertEqual(IntervalSchedule.objects.count(), 2)
+        # try to create a duplicate IntervalSchedule from a celery schedule
+        schedule = schedules.schedule(run_every=1.0)
+        sched3 = IntervalSchedule.from_schedule(schedule)
+        # the schedule should be the first of the 2 previous duplicates
+        self.assertEqual(sched3, sched1)
+        # and the duplicates should not be deleted !
+        self.assertEqual(IntervalSchedule.objects.count(), 2)
+
+
+class ClockedScheduleTestCase(TestCase):
+
+    def test_duplicate_schedules(self):
+        # See: https://github.com/celery/django-celery-beat/issues/322
+        d = timezone.now()
+        # create 2 duplicates schedules
+        sched1 = ClockedSchedule.objects.create(clocked_time=d)
+        sched2 = ClockedSchedule.objects.create(clocked_time=d)
+        self.assertEqual(ClockedSchedule.objects.count(), 2)
+        # try to create a duplicate ClockedSchedule from a previous schedule
+        sched3 = ClockedSchedule.from_schedule(sched2.schedule)
+        # the schedule should be the first of the 2 previous duplicates
+        self.assertEqual(sched3, sched1)
+        # and the duplicates should not be deleted !
+        self.assertEqual(ClockedSchedule.objects.count(), 2)