From c4ed283f8134d6214d9340e3ddb3c42e6a18c221 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Tue, 31 Jan 2017 14:26:53 +0100
Subject: [PATCH] Server: validate job settings before accepting POST

---
 .../flamenco/job_compilers/__init__.py        | 27 ++++++++++++++++---
 .../job_compilers/abstract_compiler.py        | 10 ++++++-
 packages/flamenco/flamenco/jobs/eve_hooks.py  | 14 +++++++++-
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/packages/flamenco/flamenco/job_compilers/__init__.py b/packages/flamenco/flamenco/job_compilers/__init__.py
index fbd22c9a..0c2dd263 100644
--- a/packages/flamenco/flamenco/job_compilers/__init__.py
+++ b/packages/flamenco/flamenco/job_compilers/__init__.py
@@ -23,7 +23,30 @@ from . import sleep, blender_render, blender_render_progressive
 def compile_job(job):
     """Creates tasks from the given job."""
 
+    compiler = construct_job_compiler(job)
+    compiler.compile(job)
+
+
+def validate_job(job):
+    """Validates job settings.
+
+    :raises flamenco.exceptions.JobSettingError if the settings are bad.
+    """
+
+    compiler = construct_job_compiler(job)
+    compiler.validate_job_settings(job)
+
+
+def construct_job_compiler(job):
     from flamenco import current_flamenco
+
+    compiler_class = find_job_compiler(job)
+    compiler = compiler_class(task_manager=current_flamenco.task_manager)
+
+    return compiler
+
+
+def find_job_compiler(job):
     from .abstract_compiler import AbstractJobCompiler
 
     # Get the compiler class for the job type.
@@ -35,6 +58,4 @@ def compile_job(job):
         raise KeyError('No compiler for job type %r' % job_type)
 
     assert issubclass(compiler_class, AbstractJobCompiler)
-
-    compiler = compiler_class(task_manager=current_flamenco.task_manager)
-    compiler.compile(job)
+    return compiler_class
diff --git a/packages/flamenco/flamenco/job_compilers/abstract_compiler.py b/packages/flamenco/flamenco/job_compilers/abstract_compiler.py
index 42326ccf..e8f9f9b6 100644
--- a/packages/flamenco/flamenco/job_compilers/abstract_compiler.py
+++ b/packages/flamenco/flamenco/job_compilers/abstract_compiler.py
@@ -34,5 +34,13 @@ class AbstractJobCompiler(object):
             return
 
         from flamenco import exceptions
+        job_id = job.get(u'_id', u'')
+        if job_id:
+            job_id = u' ' + job_id
+        if len(missing) == 1:
+            setting = u'setting'
+        else:
+            setting = u'settings'
+
         raise exceptions.JobSettingError(
-            u'Job %s is missing required settings: %s' % (job[u'_id'], u', '.join(missing)))
+            u'Job%s is missing required %s: %s' % (job_id, setting, u', '.join(missing)))
diff --git a/packages/flamenco/flamenco/jobs/eve_hooks.py b/packages/flamenco/flamenco/jobs/eve_hooks.py
index 01adbd79..69836844 100644
--- a/packages/flamenco/flamenco/jobs/eve_hooks.py
+++ b/packages/flamenco/flamenco/jobs/eve_hooks.py
@@ -10,6 +10,17 @@ from flamenco import current_flamenco, ROLES_REQUIRED_TO_VIEW_ITEMS
 log = logging.getLogger(__name__)
 
 
+def before_inserting_jobs(jobs):
+    from flamenco import job_compilers, exceptions
+
+    for job in jobs:
+        try:
+            job_compilers.validate_job(job)
+        except exceptions.JobSettingError as ex:
+            # We generally only submit one job at a time anyway.
+            raise wz_exceptions.BadRequest('Invalid job: %s' % ex)
+
+
 def after_inserting_jobs(jobs):
     from flamenco import job_compilers
 
@@ -107,7 +118,8 @@ def handle_job_status_update(job_doc, original_doc):
 
 
 def setup_app(app):
-    app.on_inserted_flamenco_jobs = after_inserting_jobs
+    app.on_insert_flamenco_jobs += before_inserting_jobs
+    app.on_inserted_flamenco_jobs += after_inserting_jobs
     app.on_fetched_item_flamenco_jobs += check_job_permission_fetch
     app.on_fetched_resource_flamenco_jobs += check_job_permission_fetch_resource
 
-- 
GitLab