Untangle template selection logic from template loading functionality.

Author: techtonik

CHANGES.txt

@@ -9,6 +9,10 @@ Features:
 
 - Renamed old Templates classes to Loader classes to clarify sources
   for alternative templating engines, updated docs (anatoly techtonik)
+- Template selection code is moved from Loader classes into cgi.client
+  limiting the responsibility of Loaders to compilation and rendering.
+  Internally, templating.find_template is replaced with
+  client.selectTemplate (anatoly techtonik)
 
 Fixed:
 

roundup/cgi/client.py

@@ -1063,31 +1063,58 @@ def renderFrontPage(self, message):
         self.error_message.append(message)
         self.write_html(self.renderContext())
 
-    def selectTemplate(self):
-        """ Template selection logic """
+    def selectTemplate(self, name, view):
+        """ Choose existing template for the given combination of
+            classname (name parameter) and template request variable
+            (view parameter) and return its name.
+
+            In most cases the name will be "classname.view", but
+            if "view" is None, then template name "classname" will
+            be returned.
+
+            If "classname.view" template doesn't exist, the
+            "_generic.view" is used as a fallback.
+
+            [ ] cover with tests
+        """
         loader = self.instance.templates
 
-        name = self.classname
-        view = self.template
-        
         # if classname is not set, use "home" template
         if name is None:
             name = 'home'
 
-        return name, view
+        tplname = name     
+        if view:
+            tplname = '%s.%s' % (name, view)
+
+        if loader.check(tplname):
+            return tplname
+
+        # rendering class/context with generic template for this view.
+        # with no view it's impossible to choose which generic template to use
+        if not view:
+            raise templating.NoTemplate('Template "%s" doesn\'t exist' % name)
+
+        generic = '_generic.%s' % view
+        if loader.check(generic):
+            return generic
+
+        raise templating.NoTemplate('No template file exists for templating '
+            '"%s" with template "%s" (neither "%s" nor "%s")' % (name, view,
+            tplname, generic))
 
     def renderContext(self):
         """ Return a PageTemplate for the named page
         """
-        name, view = self.selectTemplate()
+        tplname = self.selectTemplate(self.classname, self.template)
 
         # catch errors so we can handle PT rendering errors more nicely
         args = {
             'ok_message': self.ok_message,
             'error_message': self.error_message
         }
         try:
-            pt = self.instance.templates.load(name, view)
+            pt = self.instance.templates.load(tplname)
             # let the template render figure stuff out
             result = pt.render(self, None, None, **args)
             self.additional_headers['Content-Type'] = pt.content_type

roundup/cgi/engine_chameleon.py

@@ -5,15 +5,22 @@
 import os.path
 import chameleon
 
-from roundup.cgi.templating import StringIO, context, find_template, LoaderBase
+from roundup.cgi.templating import StringIO, context, LoaderBase
 
 class Loader(LoaderBase):
     def __init__(self, dir):
         self.dir = dir
         self.loader = chameleon.PageTemplateLoader(dir)
 
-    def load(self, name, view=None):
-        src, filename = find_template(self.dir, name, view)
+    def check(self, name):
+        for extension in ['', '.html', '.xml']:
+            f = name + extension
+            src = os.path.join(self.dir, f)
+            if os.path.exists(src):
+                return (src, f)
+
+    def load(self, tplname):
+        src, filename = self.check(tplname)
         return RoundupPageTemplate(self.loader.load(src))
 
 class RoundupPageTemplate(object):

roundup/cgi/engine_zopetal.py

@@ -8,7 +8,7 @@
 import os
 import os.path
 
-from roundup.cgi.templating import StringIO, context, translationService, find_template, LoaderBase
+from roundup.cgi.templating import StringIO, context, translationService, LoaderBase
 from roundup.cgi.PageTemplates import PageTemplate, GlobalTranslationService
 from roundup.cgi.PageTemplates.Expressions import getEngine
 from roundup.cgi.TAL import TALInterpreter
@@ -21,9 +21,16 @@ class Loader(LoaderBase):
     def __init__(self, dir):
         self.dir = dir
 
-    def load(self, name, view=None):
+    def check(self, name):
+        for extension in ['', '.html', '.xml']:
+            f = name + extension
+            src = os.path.join(self.dir, f)
+            if os.path.exists(src):
+                return (src, f)
+
+    def load(self, tplname):
         # find the source
-        src, filename = find_template(self.dir, name, view)
+        src, filename = self.check(tplname)
 
         # has it changed?
         try:

roundup/cgi/templating.py

@@ -77,49 +77,8 @@ def __str__(self):
             'items of class %(class)s') % {
             'action': self.action, 'class': self.klass}
 
-def find_template(dir, name, view):
-    """ Find a template in the nominated dir
-    """
-    # find the source
-    if view:
-        filename = '%s.%s'%(name, view)
-    else:
-        filename = name
-
-    # try old-style
-    src = os.path.join(dir, filename)
-    if os.path.exists(src):
-        return (src, filename)
-
-    # try with a .html or .xml extension (new-style)
-    for extension in '.html', '.xml':
-        f = filename + extension
-        src = os.path.join(dir, f)
-        if os.path.exists(src):
-            return (src, f)
-
-    # no view == no generic template is possible
-    if not view:
-        raise NoTemplate, 'Template file "%s" doesn\'t exist'%name
-
-    # try _generic template for the view
-    generic = '_generic.%s'%view
-    src = os.path.join(dir, generic)
-    if os.path.exists(src):
-        return (src, generic)
-
-    # finally, try _generic.html
-    generic = generic + '.html'
-    src = os.path.join(dir, generic)
-    if os.path.exists(src):
-        return (src, generic)
-
-    raise NoTemplate('No template file exists for templating "%s" '
-        'with template "%s" (neither "%s" nor "%s")'%(name, view,
-        filename, generic))
-
 class LoaderBase:
-    """Base for engine-specific template Loader class."""
+    """ Base for engine-specific template Loader class."""
     def precompileTemplates(self):
         """ Go through a directory and precompile all the templates therein
         """
@@ -137,37 +96,31 @@ def precompileTemplates(self):
 
             # remove extension
             filename = filename[:-len(extension)]
+            self.load(filename)
 
-            # load the template
-            if '.' in filename:
-                name, view = filename.split('.', 1)
-                self.load(name, view)
-            else:
-                self.load(filename, None)
-
-    def load(self, name, view=None):
+    def load(self, tplname):
         """ Load template and return template object with render() method.
 
-            "name" and "view" are used to select the template, which in
-            most cases will be "name.view". If "view" is None, then a
-            template called "name" will be selected.
+            "tplname" is a template name. For filesystem loaders it is a
+            filename without extensions, typically in the "classname.view"
+            format.
+        """
+        raise NotImplementedError
 
-            If the file "name.view" doesn't exist, we look for
-            "_generic.view" as a fallback.
+    def check(self, name):
+        """ Check if template with the given name exists. Return None or
+            a tuple (src, filename) that can be reused in load() method.
         """
-        # [ ] document default 'home' template and other special pages
         raise NotImplementedError
 
     def __getitem__(self, name):
         """Special method to access templates by loader['name']"""
-        view = None
-        if '.' in name:
-            name, view = name.split('.', 1)
         try:
-            return self.load(name, view)
+            return self.load(name)
         except NoTemplate, message:
             raise KeyError, message
 
+
 def get_loader(dir, engine_name):
     if engine_name == 'chameleon':
         import engine_chameleon as engine
@@ -700,7 +653,9 @@ def renderWith(self, name, **kwargs):
         req.update(kwargs)
 
         # new template, using the specified classname and request
-        pt = self._client.instance.templates.load(self.classname, name)
+        # [ ] this code is too similar to client.renderContext()
+        tplname = self._client.selectTemplate(self.classname, name)
+        pt = self._client.instance.templates.load(tplname)
 
         # use our fabricated request
         args = {
@@ -845,9 +800,8 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
                     isinstance(self._props[prop_n], hyperdb.Link)):
                 classname = self._props[prop_n].classname
                 try:
-                    template = find_template(self._db.config.TEMPLATES,
-                        classname, 'item')
-                    if template[1].startswith('_generic'):
+                    template = self._client.selectTemplate(classname, 'item')
+                    if template.startswith('_generic.'):
                         raise NoTemplate, 'not really...'
                 except NoTemplate:
                     pass
@@ -917,9 +871,9 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
                             ) % locals()
                         labelprop = linkcl.labelprop(1)
                         try:
-                            template = find_template(self._db.config.TEMPLATES,
-                                classname, 'item')
-                            if template[1].startswith('_generic'):
+                            template = self._client.selectTemplate(classname,
+                               'item')
+                            if template.startswith('_generic.'):
                                 raise NoTemplate, 'not really...'
                             hrefable = 1
                         except NoTemplate:
@@ -1087,7 +1041,10 @@ def renderQueryForm(self):
             '&@queryname=%s'%urllib.quote(name))
 
         # new template, using the specified classname and request
-        pt = self._client.instance.templates.load(req.classname, 'search')
+        # [ ] the custom logic for search page doesn't belong to
+        #     generic templating module (techtonik)
+        tplname = self._client.selectTemplate(req.classname, 'search')
+        pt = self._client.instance.templates.load(tplname)
         # The context for a search page should be the class, not any
         # node.
         self._client.nodeid = None

test/test_templating.py

@@ -336,7 +336,6 @@ class NoTemplate(Exception):
 class Unauthorised(Exception):
     def __init__(self, action, klass):
     def __str__(self):
-def find_template(dir, name, view):
 
 class Loader:
     def __init__(self, dir):