clean up new map form handling and improve error page

documentation/overview.adoc

@@ -0,0 +1,46 @@
+# MapOSMacti Overview
+
+## High level architecture
+
+The MapOSMatic project consists of two major component:
+
+* The OCitysMap render library
+* The MapOSMatic web frontend and render daemon
+
+These two are in the `ocitysmap` and `maposmatic` git repositories,
+respectively.
+
+Thre is also a third github repository `maposmatic-vagrant` providing
+a VM test setup for development and for runing self contained local
+instances.
+
+### OCitysMap
+
+The OcitysMap repo takes care of the actual map rendering, creating the
+map content itself, and decoration like a map title bar, indexes,
+annotation and copyright footer etc.
+
+OCitysMap is implemented as a Python library, and also comes with a
+command line rendering toll as an example implementation of a library
+client.
+
+OCitysMap relies on the Mapnik and CairoGraphics bindings for Python
+to render actual map content, and additiona decorations.
+
+By using Mapnik to render map content it can use the same Mapnik XML
+and CartoCSS stylesheets as are usually used for map tile rendering,
+and as Cairo Graphics not only supports bitmap output, but also vector
+formats like PDF and SVG maps can be created using these alternative
+file formats.
+
+### MapOSMatic
+
+The MapOSMatic repository actually provides two sub projects:
+
+* the actual web frontend implemented using the Django framework,
+  providing both a user frontend and a web API
+
+* a render daemon that performs the actual rendering of map
+  requests filed via the interactive web frontend or the API
+  in the background
+

scripts/render.py

@@ -480,22 +480,9 @@ class JobRenderer(threading.Thread):
                     config.overlays.append(renderer.get_overlay_by_name(overlay))
 
             config.import_files = []
-            # legacy files, eventually remove these
-            config.gpx_file     = False
-            config.umap_file    = False
-            config.poi_file     = False
-
             for file in self.job.uploads.all():
                 config.import_files.append((file.file_type, os.path.join(MEDIA_ROOT, file.uploaded_file.name)))
 
-                # legacy files, eventually remove these
-                if file.file_type == 'gpx':
-                    config.gpx_file  = os.path.join(MEDIA_ROOT, file.uploaded_file.name)
-                if file.file_type == 'umap':
-                    config.umap_file = os.path.join(MEDIA_ROOT, file.uploaded_file.name)
-                if file.file_type == 'poi':
-                    config.poi_file  = os.path.join(MEDIA_ROOT, file.uploaded_file.name)
-
             config.paper_width_mm = self.job.paper_width_mm
             config.paper_height_mm = self.job.paper_height_mm
 

www/maposmatic/apis/jobs.py

@@ -187,6 +187,7 @@ def _jobs_post(request):
     if 'extra_logo' in input:
         job.extra_logo = input['extra_logo']
 
+    # TODO why hardcoded?
     job.paper_width_mm  = 210
     job.paper_height_mm = 297
 

www/maposmatic/forms.py

@@ -174,17 +174,17 @@ class MapRenderingJobForm(forms.ModelForm):
         Returns the cleaned_data array.
         """
 
-        cleaned_data = self.cleaned_data
+        data = self.cleaned_data
 
-        mode = cleaned_data.get("mode")
-        city = cleaned_data.get("administrative_city")
-        title = cleaned_data.get("maptitle")
-        layout = cleaned_data.get("layout")
-        indexer = cleaned_data.get("indexer")
-        stylesheet = cleaned_data.get("stylesheet")
+        mode = data.get("mode")
+        city = data.get("administrative_city")
+        title = data.get("maptitle")
+        layout = data.get("layout")
+        indexer = data.get("indexer")
+        stylesheet = data.get("stylesheet")
         overlay_array = []
         try:
-            for overlay in cleaned_data.get("overlay"):
+            for overlay in data.get("overlay"):
                 overlay_array.append(overlay)
         except:
             pass
@@ -193,17 +193,14 @@ class MapRenderingJobForm(forms.ModelForm):
         if layout == '':
             msg = _(u"Layout required")
             self._errors["layout"] = ErrorList([msg])
-            del cleaned_data["layout"]
 
         if indexer == '':
             msg = _(u"Indexer required")
             self._errors["indexer"] = ErrorList([msg])
-            del cleaned_data["indexer"]
 
         if stylesheet == '':
             msg = _(u"Stylesheet required")
             self._errors["stylesheet"] = ErrorList([msg])
-            del cleaned_data["stylesheet"]
 
         if mode == 'admin':
             # TODO as bounding box override now exists (Issue #24)
@@ -212,64 +209,56 @@ class MapRenderingJobForm(forms.ModelForm):
             if city == "":
                 msg = _(u"Administrative city required")
                 self._errors["administrative_city"] = ErrorList([msg])
-                del cleaned_data["administrative_city"]
 
             try:
-                self._check_osm_id(cleaned_data.get("administrative_osmid"))
+                self._check_osm_id(data.get("administrative_osmid"))
             except Exception as ex:
                 msg = _(u"Error with osm city: %s" % ex)
                 self._errors['administrative_osmid'] \
                     = ErrorList([msg])
 
         elif mode == 'bbox':
+            msgs = []
+
             # Check bounding box corners are provided
             for f in [ "lat_upper_left", "lon_upper_left",
-                       "lat_bottom_right", "lon_bottom_right" ]:
-                val = cleaned_data.get(f)
-                if val is None:
-                    msg = _(u"Required")
-                    self._errors['bbox'] = ErrorList([msg])
-                    if f in cleaned_data:
-                        del cleaned_data[f]
+                       "lat_bottom_right", "lon_bottom_right"]:
+                if not f in data:
+                    msgs.append(_(u"Required field '%s' missing") % f)
 
             # Check latitude and longitude are different
-            if (cleaned_data.get("lat_upper_left")
-                == cleaned_data.get("lat_bottom_right")):
-                msg = _(u"Same latitude")
-                self._errors['bbox'] = ErrorList([msg])
-                del cleaned_data["lat_upper_left"]
-                del cleaned_data["lat_bottom_right"]
+            # TODO: relax this as auto-extend can deal with zero width OR height?
+            #       and even both being zero could be handled by having a min. width/height?
+            if (data.get("lat_upper_left") == data.get("lat_bottom_right")):
+                msgs.append(_(u"Same latitude"))
 
-            if (cleaned_data.get("lon_upper_left")
-                == cleaned_data.get("lon_bottom_right")):
-                msg = _(u"Same longitude")
-                self._errors['bbox'] = ErrorList([msg])
-                del cleaned_data["lon_upper_left"]
-                del cleaned_data["lon_bottom_right"]
+            if (data.get("lon_upper_left") == data.get("lon_bottom_right")):
+                msgs.append(_(u"Same longitude"))
 
             # Make sure that bbox and admin modes are exclusive
-            cleaned_data["administrative_city"] = ''
-            cleaned_data["administrative_osmid"] = None
+            data["administrative_city"] = ''
+            data["administrative_osmid"] = None
 
             # Don't try to instanciate a bounding box with empty coordinates
-            if self._errors:
-                return cleaned_data
+            if len(msgs):
+                self._errors['bbox'] = ErrorList(msgs)
+                return data
 
-            lat_upper_left = cleaned_data.get("lat_upper_left")
-            lon_upper_left = cleaned_data.get("lon_upper_left")
-            lat_bottom_right = cleaned_data.get("lat_bottom_right")
-            lon_bottom_right = cleaned_data.get("lon_bottom_right")
+            lat_upper_left = data.get("lat_upper_left")
+            lon_upper_left = data.get("lon_upper_left")
+            lat_bottom_right = data.get("lat_bottom_right")
+            lon_bottom_right = data.get("lon_bottom_right")
 
             boundingbox = ocitysmap.coords.BoundingBox(
                 lat_upper_left, lon_upper_left,
                 lat_bottom_right, lon_bottom_right)
+
             (metric_size_lat, metric_size_long) = boundingbox.spheric_sizes()
             if (metric_size_lat > www.settings.BBOX_MAXIMUM_LENGTH_IN_METERS
                 or metric_size_long > www.settings.BBOX_MAXIMUM_LENGTH_IN_METERS):
-                msg = _(u"Bounding Box too large")
-                self._errors['bbox'] = ErrorList([msg])
+                self._errors['bbox'] = ErrorList([_(u"Bounding Box too large")])
 
-        return cleaned_data
+        return data
 
     def _check_osm_id(self, osm_id):
         """Make sure that the supplied OSM Id is valid and can be accepted for
@@ -305,14 +294,14 @@ class MapRecreateForm(forms.Form):
     id = forms.IntegerField(widget=forms.HiddenInput, required=True)
 
     def clean(self):
-        cleaned_data = self.cleaned_data
+        data = self.cleaned_data
 
         try:
-            cleaned_data["id"] = int(cleaned_data.get("id", 0))
+            data["id"] = int(data.get("id", 0))
         except ValueError:
-            cleaned_data["id"] = 0
+            data["id"] = 0
 
-        return cleaned_data
+        return data
 
 class MapCancelForm(forms.Form):
     """
@@ -324,11 +313,11 @@ class MapCancelForm(forms.Form):
     nonce = forms.CharField(widget=forms.HiddenInput, required=True)
 
     def clean(self):
-        cleaned_data = self.cleaned_data
+        data = self.cleaned_data
 
         try:
-            cleaned_data["id"] = int(cleaned_data.get("id", 0))
+            data["id"] = int(data.get("id", 0))
         except ValueError:
-            cleaned_data["id"] = 0
+            data["id"] = 0
 
-        return cleaned_data
+        return data

www/maposmatic/views/new.py

@@ -30,7 +30,7 @@ import datetime
 from ipware import get_client_ip
 
 from django.core.paginator import Paginator, InvalidPage, EmptyPage
-from django.http import HttpResponseRedirect
+from django.http import HttpResponseRedirect, HttpResponseNotAllowed
 from django.shortcuts import render
 from django.utils.translation import gettext, gettext_lazy as _
 from django.utils.safestring import mark_safe
@@ -54,102 +54,109 @@ def _create_upload_file(job, file, keep_until = None):
     file_instance.save()
     file_instance.job.add(job)
 
+def _new_POST(request):
+    form = forms.MapRenderingJobForm(request.POST, request.FILES)
+
+    if not form.is_valid():
+        data = {'form': form }
+        return render(request, 'generic_error.html', data)
+
+    # remember some settings as future defaults
+    request.session['new_layout']          = form.cleaned_data.get('layout')
+    request.session['new_indexer']         = form.cleaned_data.get('indexer')
+    request.session['new_stylesheet']      = form.cleaned_data.get('stylesheet')
+    request.session['new_overlay']         = form.cleaned_data.get('overlay')
+    request.session['new_paper_width_mm']  = form.cleaned_data.get('paper_width_mm')
+    request.session['new_paper_height_mm'] = form.cleaned_data.get('paper_height_mm')
+
+    job = form.save(commit=False)
+    job.administrative_osmid = form.cleaned_data.get('administrative_osmid')
+    job.stylesheet = form.cleaned_data.get('stylesheet')
+    job.overlay = ",".join(form.cleaned_data.get('overlay'))
+    job.layout = form.cleaned_data.get('layout')
+    if job.layout.startswith('multi'):
+        job.queue = 'multipage'
+    job.indexer = form.cleaned_data.get('indexer')
+    job.paper_width_mm = form.cleaned_data.get('paper_width_mm')
+    job.paper_height_mm = form.cleaned_data.get('paper_height_mm')
+    job.status = 0 # Submitted
+    if www.settings.SUBMITTER_IP_LIFETIME != 0:
+        job.submitterip = request.META['REMOTE_ADDR']
+    else:
+        job.submitterip = None
+
+    job.submitteremail = form.cleaned_data.get('submitteremail')
+    job.map_language = form.cleaned_data.get('map_language')
+    job.index_queue_at_submission = (models.MapRenderingJob.objects
+                                     .queue_size(job.queue) + 1)
+    job.nonce = helpers.generate_nonce(models.MapRenderingJob.NONCE_SIZE)
+
+    client_ip, is_routable = get_client_ip(request)
+    if www.settings.EXTRA_IP is None or ( client_ip is not None and client_ip == www.settings.EXTRA_IP ):
+        job.extra_text = www.settings.EXTRA_FOOTER
+        job.logo = "bundled:osm-logo.svg"
+        job.extra_logo = www.settings.EXTRA_LOGO
+
+    job.save()
+
+    files = request.FILES.getlist('uploadfile')
+    if form.cleaned_data.get('delete_files_after_rendering'):
+        keep_until = None
+    else:
+        if www.settings.UPLOAD_FILE_LIFETIME > 0:
+            keep_until = datetime.datetime.now() + datetime.timedelta(days=www.settings.UPLOAD_FILE_LIFETIME)
+        else:
+            keep_until = '2999-12-30' # arbitrary 'max' value
+    for file in files:
+        _create_upload_file(job, file, keep_until)
+
+    return HttpResponseRedirect(reverse('map-by-id-and-nonce',
+                                        args=[job.id, job.nonce]))
+
+
+def _new_GET(request):
+    init_vals = request.GET.dict()
+    oc = ocitysmap.OCitySMap(www.settings.OCITYSMAP_CFG_PATH)
+
+    if not 'layout' in init_vals and 'new_layout' in request.session :
+        init_vals['layout'] = request.session['new_layout']
+    else:
+        request.session['new_layout'] = oc.get_all_renderer_names()[0]
+
+    if not 'indexer' in init_vals and 'new_indexer' in request.session :
+        init_vals['indexer'] = request.session['new_indexer']
+    else:
+        request.session['new_indexer'] = 'Street' # TODO make configurable
+
+    if not 'stylesheet' in init_vals and 'new_stylesheet' in request.session:
+        init_vals['stylesheet'] = request.session['new_stylesheet']
+    else:
+        request.session['new_stylesheet'] = oc.get_all_style_names()[0]
+
+    if not 'overlay' in init_vals and 'new_overlay' in request.session:
+        init_vals['overlay'] = request.session['new_overlay']
+
+    if not 'paper_width_mm' in init_vals and 'new_paper_width_mm' in request.session:
+        init_vals['paper_width_mm'] = request.session['new_paper_width_mm']
+
+    if not 'paper_height_mm' in init_vals and 'new_paper_width_mm' in request.session:
+        init_vals['paper_height_mm'] = request.session['new_paper_height_mm']
+
+    form = forms.MapRenderingJobForm(initial=init_vals)
+
+    papersize_buttons, multisize_buttons = views._papersize_buttons()
+
+    return render(request, 'maposmatic/new.html',
+                  { 'form' :                           form ,
+                    'papersize_suggestions':           mark_safe(papersize_buttons),
+                    'multipage_papersize_suggestions': mark_safe(multisize_buttons),
+                   })
+
 def new(request):
     """The map creation page and form."""
-
     if request.method == 'POST':
-        form = forms.MapRenderingJobForm(request.POST, request.FILES)
-        if form.is_valid():
-            # remember some settings as future defaults
-            request.session['new_layout']          = form.cleaned_data.get('layout')
-            request.session['new_indexer']         = form.cleaned_data.get('indexer')
-            request.session['new_stylesheet']      = form.cleaned_data.get('stylesheet')
-            request.session['new_overlay']         = form.cleaned_data.get('overlay')
-            request.session['new_paper_width_mm']  = form.cleaned_data.get('paper_width_mm')
-            request.session['new_paper_height_mm'] = form.cleaned_data.get('paper_height_mm')
-
-            job = form.save(commit=False)
-            job.administrative_osmid = form.cleaned_data.get('administrative_osmid')
-            job.stylesheet = form.cleaned_data.get('stylesheet')
-            job.overlay = ",".join(form.cleaned_data.get('overlay'))
-            job.layout = form.cleaned_data.get('layout')
-            if job.layout.startswith('multi'):
-                job.queue = 'multipage'
-            job.indexer = form.cleaned_data.get('indexer')
-            job.paper_width_mm = form.cleaned_data.get('paper_width_mm')
-            job.paper_height_mm = form.cleaned_data.get('paper_height_mm')
-            job.status = 0 # Submitted
-            if www.settings.SUBMITTER_IP_LIFETIME != 0:
-                job.submitterip = request.META['REMOTE_ADDR']
-            else:
-                job.submitterip = None
-
-            job.submitteremail = form.cleaned_data.get('submitteremail')
-            job.map_language = form.cleaned_data.get('map_language')
-            job.index_queue_at_submission = (models.MapRenderingJob.objects
-                                             .queue_size(job.queue) + 1)
-            job.nonce = helpers.generate_nonce(models.MapRenderingJob.NONCE_SIZE)
-
-            client_ip, is_routable = get_client_ip(request)
-            if www.settings.EXTRA_IP is None or ( client_ip is not None and client_ip == www.settings.EXTRA_IP ):
-                job.extra_text = www.settings.EXTRA_FOOTER
-                job.logo = "bundled:osm-logo.svg"
-                job.extra_logo = www.settings.EXTRA_LOGO
-            
-            job.save()
-
-            files = request.FILES.getlist('uploadfile')
-            if form.cleaned_data.get('delete_files_after_rendering'):
-                keep_until = None
-            else:
-                if www.settings.UPLOAD_FILE_LIFETIME > 0:
-                    keep_until = datetime.datetime.now() + datetime.timedelta(days=www.settings.UPLOAD_FILE_LIFETIME)
-                else:
-                    keep_until = '2999-12-30' # arbitrary 'max' value
-            for file in files:
-                _create_upload_file(job, file, keep_until)
-
-            return HttpResponseRedirect(reverse('map-by-id-and-nonce',
-                                                args=[job.id, job.nonce]))
-        else:
-            data = {'form': form }
-            return render(request, 'generic_error.html', data)
-
-            LOG.warning("FORM NOT VALID")
+        return _new_POST(request)
+    elif request.method == 'GET':
+        return _new_GET(request)
     else:
-        init_vals = request.GET.dict()
-        oc = ocitysmap.OCitySMap(www.settings.OCITYSMAP_CFG_PATH)
-
-        if not 'layout' in init_vals and 'new_layout' in request.session :
-            init_vals['layout'] = request.session['new_layout']
-        else:
-           request.session['new_layout'] = oc.get_all_renderer_names()[0]
-
-        if not 'indexer' in init_vals and 'new_indexer' in request.session :
-            init_vals['indexer'] = request.session['new_indexer']
-        else:
-           request.session['new_indexer'] = 'Street' # TODO make configurable
-
-        if not 'stylesheet' in init_vals and 'new_stylesheet' in request.session:
-            init_vals['stylesheet'] = request.session['new_stylesheet']
-        else:
-            request.session['new_stylesheet'] = oc.get_all_style_names()[0]
-
-        if not 'overlay' in init_vals and 'new_overlay' in request.session:
-            init_vals['overlay'] = request.session['new_overlay']
-
-        if not 'paper_width_mm' in init_vals and 'new_paper_width_mm' in request.session:
-            init_vals['paper_width_mm'] = request.session['new_paper_width_mm']
-
-        if not 'paper_height_mm' in init_vals and 'new_paper_width_mm' in request.session:
-            init_vals['paper_height_mm'] = request.session['new_paper_height_mm']
-
-        form = forms.MapRenderingJobForm(initial=init_vals)
-
-        papersize_buttons, multisize_buttons = views._papersize_buttons()
-
-        return render(request, 'maposmatic/new.html',
-                      { 'form' :                           form ,
-                        'papersize_suggestions':           mark_safe(papersize_buttons),
-                        'multipage_papersize_suggestions': mark_safe(multisize_buttons),
-                      })
+        return HttpResponseNotAllowed(request.method)

www/templates/generic_error.html

@@ -1,11 +1,40 @@
-<html>
-	<head>
-		<title>Error</title>
-	</head>
-	<body>
-		<h1>Error</h1>
-<pre>
-{{ form.errors }}
-</pre>
-	</body>
-</html>
+{% extends "maposmatic/base.html" %}
+
+{% comment %}
+ coding: utf-8
+
+ maposmatic, the web front-end of the MapOSMatic city map generation system
+ Copyright (C) 2018  Hartmut Holzgraefe
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU Affero General Public License as
+ published by the Free Software Foundation, either version 3 of the
+ License, or any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU Affero General Public License for more details.
+
+ You should have received a copy of the GNU Affero General Public License
+ along with this program.  If not, see <http://www.gnu.org/licenses/>.
+{% endcomment %}
+{% load i18n %}
+{% load l10n %}
+{% load extratags %}
+
+{% block title %}{% trans "Form validation error" %}{% endblock %}
+{% block page %}
+<div class="card">
+  <h1 class="card-header text-white bg-danger"><i class="fa fa-triangle-exclamation"></i> {% trans "Form validation error" %}</h1>
+
+  <div class="card-body">
+    <h5 class="card-title">{% trans "One or more form inputs turned out to be invalid:" %}</h5>
+    {{ form.errors }}
+    <hr/>
+    <pre>
+      {{ form.cleaned_data | pprint}}
+    </pre>
+  </div>
+</div>
+{% endblock %}