From f299b40fe63fcba28d64920562532fb3469fc95f Mon Sep 17 00:00:00 2001
From: Simon Lejeune <sle@openerp.com>
Date: Fri, 19 Sep 2014 11:13:55 +0200
Subject: [PATCH] [FIX] report: windows: lower level of file opening/closing,
which to find wkhtmltpdf
* Use of 'which' to find the wkhtmltopdf binary (allow the win32 service to find it)
* Use of mkstemp and manual close of the file descriptors
---
addons/report/models/report.py | 147 ++++++++++++++++++---------------
1 file changed, 81 insertions(+), 66 deletions(-)
diff --git a/addons/report/models/report.py b/addons/report/models/report.py
index c9b4490d34f7..eb2261a00498 100644
--- a/addons/report/models/report.py
+++ b/addons/report/models/report.py
@@ -21,7 +21,7 @@
from openerp import api
from openerp.osv import osv
-from openerp.tools import config
+from openerp.tools import config, which
from openerp.tools.translate import _
from openerp.addons.web.http import request
from openerp.tools.safe_eval import safe_eval as eval
@@ -32,31 +32,41 @@ import base64
import logging
import tempfile
import lxml.html
-import cStringIO
+import os
import subprocess
+from contextlib import closing
from distutils.version import LooseVersion
from functools import partial
-from os import name as OsName
from pyPdf import PdfFileWriter, PdfFileReader
-from shutil import rmtree
_logger = logging.getLogger(__name__)
-"""Check the presence of wkhtmltopdf and return its version at OpnerERP start-up."""
+#--------------------------------------------------------------------------
+# Helpers
+#--------------------------------------------------------------------------
+def _get_wkhtmltopdf_bin():
+ defpath = os.environ.get('PATH', os.defpath).split(os.pathsep)
+ return which('wkhtmltopdf', path=os.pathsep.join(defpath))
+
+
+#--------------------------------------------------------------------------
+# Check the presence of Wkhtmltopdf and return its version at Odoo start-up
+#--------------------------------------------------------------------------
wkhtmltopdf_state = 'install'
try:
process = subprocess.Popen(
- ['wkhtmltopdf', '--version'], stdout=subprocess.PIPE, stderr=subprocess.PIPE
+ [_get_wkhtmltopdf_bin(), '--version'], stdout=subprocess.PIPE, stderr=subprocess.PIPE
)
-except OSError:
- _logger.info('You need wkhtmltopdf to print a pdf version of the reports.')
+except (OSError, IOError):
+ _logger.info('You need Wkhtmltopdf to print a pdf version of the reports.')
else:
+ _logger.info('Will use the Wkhtmltopdf binary at %s' % _get_wkhtmltopdf_bin())
out, err = process.communicate()
version = re.search('([0-9.]+)', out).group(0)
if LooseVersion(version) < LooseVersion('0.12.0'):
- _logger.info('Upgrade wkhtmltopdf to (at least) 0.12.0')
+ _logger.info('Upgrade Wkhtmltopdf to (at least) 0.12.0')
wkhtmltopdf_state = 'upgrade'
else:
wkhtmltopdf_state = 'ok'
@@ -353,9 +363,7 @@ class Report(osv.Model):
:param save_in_attachment: dict of reports to save/load in/from the db
:returns: Content of the pdf as a string
"""
- command = ['wkhtmltopdf']
command_args = []
- tmp_dir = tempfile.mkdtemp(prefix='report.tmp.')
# Passing the cookie to wkhtmltopdf in order to resolve internal links.
try:
@@ -383,44 +391,45 @@ class Report(osv.Model):
# Execute WKhtmltopdf
pdfdocuments = []
-
- # Workaround the NamedTemporaryFile limitation on Windows, which requires the file to
- # be closed before accessing it by its name.
- delete_tempfile = True
- if OsName == 'nt':
- delete_tempfile = False
+ temporary_files = []
for index, reporthtml in enumerate(bodies):
local_command_args = []
- pdfreport = tempfile.NamedTemporaryFile(delete=delete_tempfile, suffix='.pdf', prefix='report.tmp.', dir=tmp_dir, mode='w+b')
+ pdfreport_fd, pdfreport_path = tempfile.mkstemp(suffix='.pdf', prefix='report.tmp.')
+ temporary_files.append(pdfreport_path)
# Directly load the document if we already have it
if save_in_attachment and save_in_attachment['loaded_documents'].get(reporthtml[0]):
- pdfreport.write(save_in_attachment['loaded_documents'].get(reporthtml[0]))
- pdfreport.seek(0)
- pdfdocuments.append(pdfreport)
+ with closing(os.fdopen(pdfreport_fd, 'w')) as pdfreport:
+ pdfreport.write(save_in_attachment['loaded_documents'][reporthtml[0]])
+ pdfdocuments.append(pdfreport_path)
continue
+ else:
+ os.close(pdfreport_fd)
# Wkhtmltopdf handles header/footer as separate pages. Create them if necessary.
if headers:
- head_file = tempfile.NamedTemporaryFile(delete=delete_tempfile, suffix='.html', prefix='report.header.tmp.', dir=tmp_dir, mode='w+')
- head_file.write(headers[index])
- head_file.seek(0)
- local_command_args.extend(['--header-html', head_file.name])
+ head_file_fd, head_file_path = tempfile.mkstemp(suffix='.html', prefix='report.header.tmp.')
+ temporary_files.append(head_file_path)
+ with closing(os.fdopen(head_file_fd, 'w')) as head_file:
+ head_file.write(headers[index])
+ local_command_args.extend(['--header-html', head_file_path])
if footers:
- foot_file = tempfile.NamedTemporaryFile(delete=delete_tempfile, suffix='.html', prefix='report.footer.tmp.', dir=tmp_dir, mode='w+')
- foot_file.write(footers[index])
- foot_file.seek(0)
- local_command_args.extend(['--footer-html', foot_file.name])
+ foot_file_fd, foot_file_path = tempfile.mkstemp(suffix='.html', prefix='report.footer.tmp.')
+ temporary_files.append(foot_file_path)
+ with closing(os.fdopen(foot_file_fd, 'w')) as foot_file:
+ foot_file.write(footers[index])
+ local_command_args.extend(['--footer-html', foot_file_path])
# Body stuff
- content_file = tempfile.NamedTemporaryFile(delete=delete_tempfile, suffix='.html', prefix='report.body.tmp.', dir=tmp_dir, mode='w+')
- content_file.write(reporthtml[1])
- content_file.seek(0)
+ content_file_fd, content_file_path = tempfile.mkstemp(suffix='.html', prefix='report.body.tmp.')
+ temporary_files.append(content_file_path)
+ with closing(os.fdopen(content_file_fd, 'w')) as content_file:
+ content_file.write(reporthtml[1])
try:
- wkhtmltopdf = command + command_args + local_command_args
- wkhtmltopdf += [content_file.name] + [pdfreport.name]
+ wkhtmltopdf = [_get_wkhtmltopdf_bin()] + command_args + local_command_args
+ wkhtmltopdf += [content_file_path] + [pdfreport_path]
process = subprocess.Popen(wkhtmltopdf, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
out, err = process.communicate()
@@ -432,37 +441,38 @@ class Report(osv.Model):
# Save the pdf in attachment if marked
if reporthtml[0] is not False and save_in_attachment.get(reporthtml[0]):
- attachment = {
- 'name': save_in_attachment.get(reporthtml[0]),
- 'datas': base64.encodestring(pdfreport.read()),
- 'datas_fname': save_in_attachment.get(reporthtml[0]),
- 'res_model': save_in_attachment.get('model'),
- 'res_id': reporthtml[0],
- }
- self.pool['ir.attachment'].create(cr, uid, attachment)
+ with open(pdfreport_path, 'rb') as pdfreport:
+ attachment = {
+ 'name': save_in_attachment.get(reporthtml[0]),
+ 'datas': base64.encodestring(pdfreport.read()),
+ 'datas_fname': save_in_attachment.get(reporthtml[0]),
+ 'res_model': save_in_attachment.get('model'),
+ 'res_id': reporthtml[0],
+ }
+ self.pool['ir.attachment'].create(cr, uid, attachment)
_logger.info('The PDF document %s is now saved in the '
'database' % attachment['name'])
- pdfreport.seek(0)
- pdfdocuments.append(pdfreport)
-
- content_file.close()
-
- if headers:
- head_file.close()
- if footers:
- foot_file.close()
+ pdfdocuments.append(pdfreport_path)
except:
raise
# Return the entire document
if len(pdfdocuments) == 1:
- content = pdfdocuments[0].read()
- pdfdocuments[0].close()
+ entire_report_path = pdfdocuments[0]
else:
- content = self._merge_pdf(pdfdocuments)
+ entire_report_path = self._merge_pdf(pdfdocuments)
+ temporary_files.append(entire_report_path)
- rmtree(tmp_dir)
+ with open(entire_report_path, 'rb') as pdfdocument:
+ content = pdfdocument.read()
+
+ # Manual cleanup of the temporary files
+ for temporary_file in temporary_files:
+ try:
+ os.unlink(temporary_file)
+ except (OSError, IOError):
+ _logger.error('Error when trying to remove file %s' % temporary_file)
return content
@@ -499,7 +509,7 @@ class Report(osv.Model):
if specific_paperformat_args and specific_paperformat_args.get('data-report-dpi'):
command_args.extend(['--dpi', str(specific_paperformat_args['data-report-dpi'])])
elif paperformat.dpi:
- if OsName == 'nt' and int(paperformat.dpi) <= 95:
+ if os.name == 'nt' and int(paperformat.dpi) <= 95:
_logger.info("Generating PDF on Windows platform require DPI >= 96. Using 96 instead.")
command_args.extend(['--dpi', '96'])
else:
@@ -526,18 +536,23 @@ class Report(osv.Model):
def _merge_pdf(self, documents):
"""Merge PDF files into one.
- :param documents: list of pdf files
- :returns: string containing the merged pdf
+ :param documents: list of path of pdf files
+ :returns: path of the merged pdf
"""
writer = PdfFileWriter()
+ streams = [] # We have to close the streams *after* PdfFilWriter's call to write()
for document in documents:
- reader = PdfFileReader(file(document.name, "rb"))
+ pdfreport = file(document, 'rb')
+ streams.append(pdfreport)
+ reader = PdfFileReader(pdfreport)
for page in range(0, reader.getNumPages()):
writer.addPage(reader.getPage(page))
- document.close()
- merged = cStringIO.StringIO()
- writer.write(merged)
- merged.seek(0)
- content = merged.read()
- merged.close()
- return content
+
+ merged_file_fd, merged_file_path = tempfile.mkstemp(suffix='.html', prefix='report.merged.tmp.')
+ with closing(os.fdopen(merged_file_fd, 'w')) as merged_file:
+ writer.write(merged_file)
+
+ for stream in streams:
+ stream.close()
+
+ return merged_file_path
--
GitLab