- introduce a new parameter "safe_substitute" to perform safe substitution,...

- introduce a new parameter "safe_substitute" to perform safe substitution, and enable it by default. This changes behaviour, but in previous case it was very hard for users to understand what's wrong in their notification message.
- add test for this new feature and for some substitution features that where not tested.


git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@33120 20353a03-c40f-0410-a6d1-a30d3c3de9de

product/ERP5/Document/TextDocument.py

@@ -33,7 +33,7 @@ from zLOG import LOG, WARNING
 from Products.ERP5Type.Base import WorkflowMethod
 from Products.CMFCore.utils import getToolByName
 from Products.CMFCore.utils import _setCacheHeaders, _ViewEmulator
-from Products.ERP5Type import Permissions, PropertySheet, Constraint, interfaces
+from Products.ERP5Type import Permissions, PropertySheet, Constraint
 from Products.ERP5.Document.Document import Document, ConversionError
 from Products.ERP5Type.WebDAVSupport import TextContent
 from Products.CMFDefault.utils import isHTMLSafe
@@ -153,7 +153,7 @@ class TextDocument(Document, TextContent):
       RESPONSE.setHeader('Accept-Ranges', 'bytes')
       return data
 
-    def _substituteTextContent(self, text, **kw):
+    def _substituteTextContent(self, text, safe_substitute=True, **kw):
       # If a method for string substitutions of the text content, perform it.
       # Decode everything into unicode before the substitutions, in order to
       # avoid encoding errors.
@@ -162,7 +162,9 @@ class TextDocument(Document, TextContent):
         try:
           mapping = guarded_getattr(self, method_id)(**kw)
         except AttributeError:
-          LOG('TextDocument', WARNING, 'could not get the substitution mapping method %s from %r, so the content will not be substituted.' % (method_id, self))
+          LOG('TextDocument', WARNING, 'could not get the substitution'
+              ' mapping method %s from %r, so the content will not be'
+              ' substituted.' % (method_id, self))
           return text
 
         is_str = isinstance(text, str)
@@ -177,7 +179,10 @@ class TextDocument(Document, TextContent):
             v = str(v).decode('utf-8')
           unicode_mapping[k] = v
 
-        text = Template(text).substitute(unicode_mapping)
+        if safe_substitute:
+          text = Template(text).safe_substitute(unicode_mapping)
+        else:
+          text = Template(text).substitute(unicode_mapping)
 
         # If the original was a str, convert it back to str.
         if is_str:
@@ -186,17 +191,18 @@ class TextDocument(Document, TextContent):
       return text
 
     security.declareProtected(Permissions.View, 'asSubjectText')
-    def asSubjectText(self, substitution_method_parameter_dict=None, **kw):
+    def asSubjectText(self, substitution_method_parameter_dict=None, safe_substitute=True, **kw):
       """
         Converts the subject of the document to a textual representation.
       """
       subject = TextDocument.inheritedAttribute('asSubjectText')(self, **kw)
       if substitution_method_parameter_dict is None:
         substitution_method_parameter_dict = {}
-      return self._substituteTextContent(subject, **substitution_method_parameter_dict)
+      return self._substituteTextContent(subject, safe_substitute=safe_substitute,
+                                         **substitution_method_parameter_dict)
 
     security.declareProtected(Permissions.AccessContentsInformation, 'convert')
-    def convert(self, format, substitution_method_parameter_dict=None, **kw):
+    def convert(self, format, substitution_method_parameter_dict=None, safe_substitute=True, **kw):
       """
         Convert text using portal_transforms or oood
       """
@@ -228,7 +234,8 @@ class TextDocument(Document, TextContent):
           mime_type, result = self.getConversion(format=format)
         if substitution_method_parameter_dict is None:
           substitution_method_parameter_dict = {}
-        result = self._substituteTextContent(result, **substitution_method_parameter_dict)
+        result = self._substituteTextContent(result, safe_substitute=safe_substitute,
+                                             **substitution_method_parameter_dict)
         return mime_type, result
       else:
         # text_content is not set, return empty string instead of None

product/ERP5/tests/testNotificationMessageModule.py

@@ -29,8 +29,8 @@
 import unittest
 
 import transaction
-from Testing import ZopeTestCase
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
+from Products.ERP5Type.tests.utils import createZODBPythonScript
 from AccessControl.SecurityManagement import newSecurityManager
 from AccessControl.SecurityManagement import getSecurityManager
 from zLOG import LOG
@@ -38,11 +38,8 @@ from Products.ERP5Type.tests.utils import DummyMailHost
 
 class TestNotificationMessageModule(ERP5TypeTestCase):
   """
-  Test notification tool
+  Test notification message module
   """
-  run_all_test = 1
-  quiet = 1
-
   def getBusinessTemplateList(self):
     return ('erp5_base',)
 
@@ -78,12 +75,7 @@ class TestNotificationMessageModule(ERP5TypeTestCase):
     transaction.commit()
     self.tic()
 
-  def test_01_get_document(self, quiet=quiet, run=run_all_test):
-    if not run: return
-    if not quiet:
-      message = 'Test type base Method'
-      ZopeTestCase._print('\n%s ' % message)
-      LOG('Testing... ', 0, message)
+  def test_01_get_document(self):
     module = self.getNotificationMessageModule()
     tool = self.getPortal().portal_notifications
     #Test Document A in english
@@ -117,6 +109,88 @@ class TestNotificationMessageModule(ERP5TypeTestCase):
     result = tool.getDocumentValue(reference='A', language='fr')
     self.assertEqual(result.getRelativeUrl(), n_m_fr_02.getRelativeUrl())
 
+
+  def test_substitution_content(self):
+    """Tests that content and subject have string.Template based substitutions
+    """
+    module = self.getNotificationMessageModule()
+    createZODBPythonScript(self.portal,
+                           'NotificationMessage_getDummySubstitionMapping',
+                           '**kw',
+                           '''return dict(a="b")''')
+    doc = module.newContent(portal_type='Notification Message',
+                            title='Test ${a}',
+                            text_content='substitution text: ${a}',
+                            text_content_substitution_mapping_method_id=
+                            'NotificationMessage_getDummySubstitionMapping')
+
+    mime, text = doc.convert('txt')
+    self.assertEqual('text/plain', mime)
+    self.assertEqual('substitution text: b', text)
+
+    self.assertEqual('Test b', doc.asSubjectText())
+
+
+  def test_substitution_content_parameters(self):
+    """Tests that we can pass parameters to convert to the substitution method,
+    by using substitution_method_parameter_dict """
+    module = self.getNotificationMessageModule()
+    createZODBPythonScript(self.portal,
+                           'NotificationMessage_getDummySubstitionMapping',
+                           '**kw',
+                           '''return kw''')
+    doc = module.newContent(portal_type='Notification Message',
+                            title='Test ${a}',
+                            text_content='substitution text: ${a}',
+                            text_content_substitution_mapping_method_id=
+                            'NotificationMessage_getDummySubstitionMapping')
+
+    mime, text = doc.convert('txt',
+                             substitution_method_parameter_dict=dict(a='b'))
+    self.assertEqual('substitution text: b', text)
+
+
+  def test_substitution_content_and_convert(self):
+    """Tests that substitution also works with different target format.
+    """
+    module = self.getNotificationMessageModule()
+    createZODBPythonScript(self.portal,
+                           'NotificationMessage_getDummySubstitionMapping',
+                           '**kw',
+                           '''return dict(a="b")''')
+    doc = module.newContent(portal_type='Notification Message',
+                            text_format='text/html',
+                            text_content='substitution text: <em>${a}</em>',
+                            text_content_substitution_mapping_method_id=
+                            'NotificationMessage_getDummySubstitionMapping')
+
+    mime, text = doc.convert('txt')
+    self.assertEqual('substitution text: b', text)
+
+  def test_safe_substitution_content(self):
+    """Tests that 'safe' substitution is performed, unless safe_substitute is
+    explicitly passed to False.
+    """
+    module = self.getNotificationMessageModule()
+    createZODBPythonScript(self.portal,
+                           'NotificationMessage_getDummySubstitionMapping',
+                           '**kw',
+                           '''return dict(a="b")''')
+    doc = module.newContent(portal_type='Notification Message',
+                            title='${b}',
+                            text_content='substitution text: ${b}',
+                            text_content_substitution_mapping_method_id=
+                            'NotificationMessage_getDummySubstitionMapping')
+
+    mime, text = doc.convert('txt')
+    self.assertEqual('substitution text: ${b}', text)
+    self.assertEqual('${b}', doc.asSubjectText())
+
+    self.assertRaises(KeyError, doc.convert, 'txt', safe_substitute=False)
+    self.assertRaises(KeyError, doc.convert, 'html', safe_substitute=False)
+    self.assertRaises(KeyError, doc.asSubjectText, safe_substitute=False)
+
+
 def test_suite():
   suite = unittest.TestSuite()
   suite.addTest(unittest.makeSuite(TestNotificationMessageModule))