From 265cd3b631514b23e66c028b138011dd26879707 Mon Sep 17 00:00:00 2001 From: tom-price Date: Wed, 18 Jul 2018 15:58:51 +0200 Subject: [PATCH] Created gereralized Args/Kwargs model associated with each job type. Model (not DB) validation prevents more than one of the potential fields from being set. Additional Javascript hides all but one value field in admin interface. Tests need to be written, but according to Factory Boy documentation "All factories for a Django Model should use the DjangoModelFactory base class", so I'll likely address that first. --- scheduler/admin.py | 39 +++++++- .../0007_added_args_and_kwargs_models.py | 70 ++++++++++++++ scheduler/models.py | 94 ++++++++++++------- scheduler/static/scheduler/js/base.js | 39 ++++++++ scheduler/tests.py | 26 +---- 5 files changed, 207 insertions(+), 61 deletions(-) create mode 100644 scheduler/migrations/0007_added_args_and_kwargs_models.py create mode 100644 scheduler/static/scheduler/js/base.js diff --git a/scheduler/admin.py b/scheduler/admin.py index 04d0853..2fd9f6e 100644 --- a/scheduler/admin.py +++ b/scheduler/admin.py @@ -2,9 +2,11 @@ from django.conf import settings from django.contrib import admin +from django.contrib.contenttypes.admin import GenericStackedInline from django.utils.translation import ugettext_lazy as _ -from scheduler.models import CronJob, RepeatableJob, ScheduledJob +from scheduler.models import CronJob, RepeatableJob, ScheduledJob,\ + JobArg, JobKwarg QUEUES = [(key, key) for key in settings.RQ_QUEUES.keys()] @@ -32,6 +34,31 @@ def delete_model(self, request, obj): delete_model.short_description = _("Delete selected %(verbose_name_plural)s") +class JobArgInline(GenericStackedInline): + model = JobArg + extra = 0 + fieldsets = ( + (None, { + 'fields': ('arg_name', 'str_val', 'int_val', 'datetime_val',), + }), + ) + class Media: + js = ('scheduler/js/base.js',) + + +class JobKwargInline(GenericStackedInline): + model = JobKwarg + extra = 0 + fieldsets = ( + (None, { + 'fields': ('key', 'arg_name', 'str_val', 'int_val', 'datetime_val',), + }), + ) + + class Media: + js = ('scheduler/js/base.js',) + + @admin.register(ScheduledJob) class ScheduledJobAdmin(QueueMixin, admin.ModelAdmin): list_display = ( @@ -42,7 +69,7 @@ class ScheduledJobAdmin(QueueMixin, admin.ModelAdmin): readonly_fields = ('job_id', ) fieldsets = ( (None, { - 'fields': ('name', 'callable', 'callable_args', 'callable_kwargs', 'enabled', ), + 'fields': ('name', 'callable', 'enabled', ), }), (_('RQ Settings'), { 'fields': ('queue', 'job_id', ), @@ -55,6 +82,7 @@ class ScheduledJobAdmin(QueueMixin, admin.ModelAdmin): ), }), ) + inlines = [JobArgInline, JobKwargInline] @admin.register(RepeatableJob) @@ -68,7 +96,7 @@ class RepeatableJobAdmin(QueueMixin, admin.ModelAdmin): readonly_fields = ('job_id', ) fieldsets = ( (None, { - 'fields': ('name', 'callable', 'callable_args', 'callable_kwargs', 'enabled', ), + 'fields': ('name', 'callable', 'enabled', ), }), (_('RQ Settings'), { 'fields': ('queue', 'job_id', ), @@ -83,6 +111,7 @@ class RepeatableJobAdmin(QueueMixin, admin.ModelAdmin): ), }), ) + inlines = [JobArgInline, JobKwargInline] @admin.register(CronJob) @@ -95,7 +124,7 @@ class CronJobAdmin(QueueMixin, admin.ModelAdmin): readonly_fields = ('job_id', ) fieldsets = ( (None, { - 'fields': ('name', 'callable', 'callable_args', 'callable_kwargs', 'enabled', ), + 'fields': ('name', 'callable', 'enabled', ), }), (_('RQ Settings'), { 'fields': ('queue', 'job_id', ), @@ -108,3 +137,5 @@ class CronJobAdmin(QueueMixin, admin.ModelAdmin): ), }), ) + inlines = [JobArgInline, JobKwargInline] + diff --git a/scheduler/migrations/0007_added_args_and_kwargs_models.py b/scheduler/migrations/0007_added_args_and_kwargs_models.py new file mode 100644 index 0000000..748dc18 --- /dev/null +++ b/scheduler/migrations/0007_added_args_and_kwargs_models.py @@ -0,0 +1,70 @@ +# Generated by Django 2.0.7 on 2018-07-23 14:12 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('scheduler', '0006_added_args_kwargs'), + ] + + operations = [ + migrations.CreateModel( + name='JobArg', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('str_val', models.CharField(blank=True, max_length=255, verbose_name='String Value')), + ('int_val', models.IntegerField(blank=True, null=True, verbose_name='Int Value')), + ('datetime_val', models.DateTimeField(blank=True, null=True, verbose_name='Datetime Value')), + ('arg_name', models.CharField(choices=[('str_val', 'string'), ('int_val', 'int'), ('datetime_val', 'Datetime')], default='str_val', max_length=12, verbose_name='Argument Type')), + ('object_id', models.PositiveIntegerField()), + ('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType')), + ], + options={ + 'abstract': False, + }, + ), + migrations.CreateModel( + name='JobKwarg', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('str_val', models.CharField(blank=True, max_length=255, verbose_name='String Value')), + ('int_val', models.IntegerField(blank=True, null=True, verbose_name='Int Value')), + ('datetime_val', models.DateTimeField(blank=True, null=True, verbose_name='Datetime Value')), + ('arg_name', models.CharField(choices=[('str_val', 'string'), ('int_val', 'int'), ('datetime_val', 'Datetime')], default='str_val', max_length=12, verbose_name='Argument Type')), + ('object_id', models.PositiveIntegerField()), + ('key', models.CharField(max_length=255)), + ('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType')), + ], + options={ + 'abstract': False, + }, + ), + migrations.RemoveField( + model_name='cronjob', + name='callable_args', + ), + migrations.RemoveField( + model_name='cronjob', + name='callable_kwargs', + ), + migrations.RemoveField( + model_name='repeatablejob', + name='callable_args', + ), + migrations.RemoveField( + model_name='repeatablejob', + name='callable_kwargs', + ), + migrations.RemoveField( + model_name='scheduledjob', + name='callable_args', + ), + migrations.RemoveField( + model_name='scheduledjob', + name='callable_kwargs', + ), + ] diff --git a/scheduler/models.py b/scheduler/models.py index bc47f0b..f44f4af 100644 --- a/scheduler/models.py +++ b/scheduler/models.py @@ -3,9 +3,10 @@ from datetime import timedelta import croniter -import json from django.conf import settings +from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db import models from django.templatetags.tz import utc @@ -17,25 +18,64 @@ from model_utils.models import TimeStampedModel +@python_2_unicode_compatible +class BaseJobArg(models.Model): + + ARG_NAME = Choices( + ('str_val', _('string')), + ('int_val', _('int')), + ('datetime_val', _('Datetime')), + ) + str_val = models.CharField(_('String Value'), blank=True, max_length=255) + int_val = models.IntegerField(_('Int Value'), blank=True, null=True) + datetime_val = models.DateTimeField(_('Datetime Value'), blank=True, null=True) + + arg_name = models.CharField( + _('Argument Type'), max_length=12, choices=ARG_NAME, default=ARG_NAME.str_val + ) + + content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey() + + def clean(self): + self.clean_one_value() + + def clean_one_value(self): + count = 0 + count += 1 if self.str_val != '' else 0 + count += 1 if self.int_val else 0 + count += 1 if self.datetime_val else 0 + if count == 0: + raise ValidationError({ + 'arg_name': ValidationError( + _('There are at least one arg type must have a value'), code='invalid') + }) + if count > 1: + raise ValidationError({ + 'arg_name': ValidationError( + _('There are multiple arg types with values'), code='invalid') + }) + + class Meta: + abstract = True + + +class JobArg(BaseJobArg): + pass + + +class JobKwarg(BaseJobArg): + key = models.CharField(max_length=255) + + @python_2_unicode_compatible class BaseJob(TimeStampedModel): name = models.CharField(_('name'), max_length=128, unique=True) callable = models.CharField(_('callable'), max_length=2048) - callable_args = models.CharField( - _('args'), blank=True, max_length=2048, - help_text=_( - 'A comma separated list of arguments (to be passed as strings)' - ' in order e.g.: arg1, arg2' - ) - ) - callable_kwargs = models.CharField( - _('kwargs'), blank=True, max_length=2048, - help_text=_( - 'A json string to be parsed by json.loads e.g.: ' - '{"kwarg1": "a", "kwarg2": 2}' - ) - ) + callable_args = GenericRelation(JobArg) + callable_kwargs = GenericRelation(JobKwarg) enabled = models.BooleanField(_('enabled'), default=True) queue = models.CharField(_('queue'), max_length=16) job_id = models.CharField( @@ -70,7 +110,6 @@ def callable_func(self): def clean(self): self.clean_callable() self.clean_queue() - self.clean_kwargs() def clean_callable(self): try: @@ -90,16 +129,6 @@ def clean_queue(self): ', '.join(queue_keys))), code='invalid') }) - def clean_kwargs(self): - try: - if self.callable_kwargs: - json.loads(self.callable_kwargs) - except: - raise ValidationError({ - 'callable_kwargs': ValidationError( - _('Invalid kwargs, must be parsable by json.loads'), code='invalid') - }) - def is_scheduled(self): return self.job_id in self.scheduler() is_scheduled.short_description = _('is scheduled?') @@ -150,17 +179,15 @@ def schedule_time_utc(self): return utc(self.scheduled_time) def parse_args(self): - if self.callable_args is not "": - return [x.strip() for x in self.callable_args.split(',')] - return [] + args = self.callable_args.values().order_by('id') + return [arg[arg['arg_name']] for arg in args] def parse_kwargs(self): - if self.callable_kwargs is not "": - return json.loads(self.callable_kwargs) - return {} + kwargs = self.callable_kwargs.values().order_by('id') + return {kwarg['key']: kwarg[kwarg['arg_name']] for kwarg in kwargs} def function_string(self): - func = self.callable + "(\u200b{})" # zero-width space allows better text-wrap + func = self.callable + "(\u200b{})" # zero-width space allows textwrap args = self.parse_args() args_list = [repr(arg) for arg in args] kwargs = self.parse_kwargs() @@ -287,3 +314,4 @@ class Meta: verbose_name = _('Cron Job') verbose_name_plural = _('Cron Jobs') ordering = ('name', ) + diff --git a/scheduler/static/scheduler/js/base.js b/scheduler/static/scheduler/js/base.js new file mode 100644 index 0000000..06a243a --- /dev/null +++ b/scheduler/static/scheduler/js/base.js @@ -0,0 +1,39 @@ +(function($) { + $(function() { + var updateDisplayedFields = function(row) { + const selectField = row.find('.field-arg_name select'), + strVal = row.find('.field-str_val'), + intVal = row.find('.field-int_val'), + datetimeVal = row.find('.field-datetime_val'); + + var showRow = function(selector) { + selector.show(); + selector.find('label').addClass('required') + }; + var hideRow = function(selector) { + selector.hide(); + selector.find(':input').val(''); + }; + + var toggleVerified = function(value) { + value === 'str_val' ? showRow(strVal) : hideRow(strVal); + value === 'int_val' ? showRow(intVal) : hideRow(intVal); + value === 'datetime_val' ? showRow(datetimeVal) : hideRow(datetimeVal); + }; + toggleVerified(selectField.val()); + selectField.change(function() { + toggleVerified($(this).val()); + }); + }; + + $(document).on('formset:added', function (event, row, options) { + updateDisplayedFields(row); + }); + + + $('.dynamic-scheduler-jobkwarg-content_type-object_id, ' + + '.dynamic-scheduler-jobarg-content_type-object_id').each(function() { + updateDisplayedFields($(this)); + }); + }); +})(django.jQuery); diff --git a/scheduler/tests.py b/scheduler/tests.py index 279482c..ffcc8ca 100644 --- a/scheduler/tests.py +++ b/scheduler/tests.py @@ -10,7 +10,8 @@ import factory import pytz from django_rq import job -from scheduler.models import CronJob, RepeatableJob, ScheduledJob +from scheduler.models import CronJob, RepeatableJob, ScheduledJob, \ + JobArg, JobKwarg class ScheduledJobFactory(factory.Factory): @@ -212,29 +213,6 @@ def test_delete_and_unschedule(self): is_scheduled = job_id in scheduler self.assertFalse(is_scheduled) - def test_kwarg_string_parsing(self): - job = self.JobClass() - job.callable_kwargs = '{"kwarg1": "a", "kwarg2": "b"}' - func = job.parse_kwargs() - expected = dict(kwarg1="a", kwarg2="b") - self.assertEqual(expected, func) - - def test_arg_string_parsing(self): - job = self.JobClass() - job.callable_args = 'arg1, arg2' - func = job.parse_args() - expected = ["arg1", "arg2"] - self.assertEqual(expected, func) - - def test_function_string(self): - job = self.JobClass() - job.callable = 'fname' - job.callable_args = 'arg1, arg2' - job.callable_kwargs = '{"kwarg1": "a", "kwarg2": "b"}' - func = job.function_string() - expected = "fname(\u200b'arg1', 'arg2', kwarg1='a', kwarg2='b')" - self.assertEqual(expected, func) - class TestRepeatableJob(TestScheduledJob):