Merge lp:~camptocamp/ocb-addons/7.0-fix-1319095 into lp:ocb-addons

[Yannick Vaucher @ Camptocamp]

Use NamedTemporaryFile instead of file and of deprecated mktemp. That way we ensure 2 files created at the exact same time will have a unique name

Another hot fix from Florent was made here https://code.launchpad.net/~florent.x/openobject-addons/trunk-1290820-report_webkit/+merge/210387

I believe this one is a bit cleaner as it refactor a bit the code. However it doesn't keep timestamp in file name. But do we need timestamp in file name ?

If so I can still add it in prefix or suffix.

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Thanks for the fix.

LGTM

This patch will not be backportable as is to version 6.1, 6.0 as it is not Python 2.5 compliant

Regards

Nicolas

review: Approve (no test, code review)

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

This fix will not work on Windows.

out_filename is opened on line 11 of the file, and then the pdf converter will try to open a file with the same name while the file is still open in OpenERP which will fail on windows (https://docs.python.org/2.7/library/tempfile.html?highlight=namedtemporaryfile#tempfile.NamedTemporaryFile)

review: Needs Fixing (code review, no test)

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Good catch

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Thanks for the review and the explanations.

This has been fixed.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

LGTM

review: Approve (code review, no test)

[Pedro Manuel Baeza (pedro.baeza)]

LGTM.

Regards.

review: Approve (code review)

=== modified file 'report_webkit/webkit_report.py'
--- report_webkit/webkit_report.py	2014-04-16 07:43:09 +0000
+++ report_webkit/webkit_report.py	2014-05-14 11:21:39 +0000
@@ -105,8 +105,8 @@
         """Call webkit in order to generate pdf"""
         if not webkit_header:
             webkit_header = report_xml.webkit_header
-        tmp_dir = tempfile.gettempdir()
-        out_filename = tempfile.mktemp(suffix=".pdf", prefix="webkit.tmp.")
+        fd, out_filename = tempfile.mkstemp(suffix=".pdf",
+                                            prefix="webkit.tmp.")
         file_to_del = [out_filename]
         if comm_path:
             command = [comm_path]
@@ -117,25 +117,15 @@
         # default to UTF-8 encoding.  Use <meta charset="latin-1"> to override.
         command.extend(['--encoding', 'utf-8'])
         if header :
-            head_file = file( os.path.join(
-                                  tmp_dir,
-                                  str(time.time()) + '.head.html'
-                                 ),
-                                'w'
-                            )
-            head_file.write(self._sanitize_html(header))
-            head_file.close()
+            with tempfile.NamedTemporaryFile(suffix=".head.html",
+                                             delete=False) as head_file:
+                head_file.write(self._sanitize_html(header))
             file_to_del.append(head_file.name)
             command.extend(['--header-html', head_file.name])
         if footer :
-            foot_file = file(  os.path.join(
-                                  tmp_dir,
-                                  str(time.time()) + '.foot.html'
-                                 ),
-                                'w'
-                            )
-            foot_file.write(self._sanitize_html(footer))
-            foot_file.close()
+            with tempfile.NamedTemporaryFile(suffix=".foot.html",
+                                             delete=False) as foot_file:
+                foot_file.write(self._sanitize_html(footer))
             file_to_del.append(foot_file.name)
             command.extend(['--footer-html', foot_file.name])
 
@@ -153,10 +143,10 @@
             command.extend(['--page-size', str(webkit_header.format).replace(',', '.')])
         count = 0
         for html in html_list :
-            html_file = file(os.path.join(tmp_dir, str(time.time()) + str(count) +'.body.html'), 'w')
-            count += 1
-            html_file.write(self._sanitize_html(html))
-            html_file.close()
+            with tempfile.NamedTemporaryFile(suffix="%d.body.html" %count,
+                                             delete=False) as html_file:
+                count += 1
+                html_file.write(self._sanitize_html(html))
             file_to_del.append(html_file.name)
             command.append(html_file.name)
         command.append(out_filename)
@@ -176,9 +166,9 @@
             if status :
                 raise except_osv(_('Webkit error' ),
                                  _("The command 'wkhtmltopdf' failed with error code = %s. Message: %s") % (status, error_message))
-            pdf_file = open(out_filename, 'rb')
-            pdf = pdf_file.read()
-            pdf_file.close()
+            with open(out_filename, 'rb') as pdf_file:
+                pdf = pdf_file.read()
+            os.close(fd)
         finally:
             if stderr_fd is not None:
                 os.close(stderr_fd)