erp5_core&erp5_property_sheets&OOoDocument: improve document conversion

1. accept a list of converison server
2. retry in case of network error

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/SystemPreference_view.xml

@@ -83,7 +83,8 @@
                       <list>
                         <string>my_priority</string>
                         <string>my_preferred_time_zone</string>
-                        <string>my_preferred_document_conversion_server_url</string>
+                        <string>my_preferred_document_conversion_server_url_list</string>
+                        <string>my_preferred_document_conversion_server_retry</string>
                         <string>my_preferred_ooodoc_server_timeout</string>
                         <string>my_preferred_hide_rows_on_no_search_criterion</string>
                         <string>my_translated_preference_state_title</string>

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/SystemPreference_view/my_preferred_document_conversion_server_url.xml → product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/SystemPreference_view/my_preferred_document_conversion_server_url_list.xml

@@ -10,14 +10,13 @@
             <key> <string>delegated_list</string> </key>
             <value>
               <list>
-                <string>description</string>
                 <string>title</string>
               </list>
             </value>
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>my_preferred_document_conversion_server_url</string> </value>
+            <value> <string>my_preferred_document_conversion_server_url_list</string> </value>
         </item>
         <item>
             <key> <string>message_values</string> </key>
@@ -42,10 +41,6 @@
                     <key> <string>form_id</string> </key>
                     <value> <string></string> </value>
                 </item>
-                <item>
-                    <key> <string>target</string> </key>
-                    <value> <string></string> </value>
-                </item>
               </dictionary>
             </value>
         </item>
@@ -53,10 +48,6 @@
             <key> <string>tales</string> </key>
             <value>
               <dictionary>
-                <item>
-                    <key> <string>description</string> </key>
-                    <value> <string></string> </value>
-                </item>
                 <item>
                     <key> <string>field_id</string> </key>
                     <value> <string></string> </value>
@@ -65,14 +56,6 @@
                     <key> <string>form_id</string> </key>
                     <value> <string></string> </value>
                 </item>
-                <item>
-                    <key> <string>target</string> </key>
-                    <value> <string></string> </value>
-                </item>
-                <item>
-                    <key> <string>title</string> </key>
-                    <value> <string></string> </value>
-                </item>
               </dictionary>
             </value>
         </item>
@@ -80,25 +63,17 @@
             <key> <string>values</string> </key>
             <value>
               <dictionary>
-                <item>
-                    <key> <string>description</string> </key>
-                    <value> <string>URL of the document conversion server.</string> </value>
-                </item>
                 <item>
                     <key> <string>field_id</string> </key>
-                    <value> <string>my_string_field</string> </value>
+                    <value> <string>my_lines_field</string> </value>
                 </item>
                 <item>
                     <key> <string>form_id</string> </key>
                     <value> <string>Base_viewFieldLibrary</string> </value>
                 </item>
-                <item>
-                    <key> <string>target</string> </key>
-                    <value> <string>Click to edit the target</string> </value>
-                </item>
                 <item>
                     <key> <string>title</string> </key>
-                    <value> <string>Conversion Server URL</string> </value>
+                    <value> <string>Conversion Server URLs</string> </value>
                 </item>
               </dictionary>
             </value>

product/ERP5/bootstrap/erp5_property_sheets/PropertySheetTemplateItem/portal_property_sheets/DocumentPreference/preferred_document_conversion_server_retry_property.xml

+<?xml version="1.0"?>
+<ZopeData>
+  <record id="1" aka="AAAAAAAAAAE=">
+    <pickle>
+      <global name="Standard Property" module="erp5.portal_type"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>categories</string> </key>
+            <value>
+              <tuple>
+                <string>elementary_type/int</string>
+              </tuple>
+            </value>
+        </item>
+        <item>
+            <key> <string>description</string> </key>
+            <value>
+              <none/>
+            </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>preferred_document_conversion_server_retry_property</string> </value>
+        </item>
+        <item>
+            <key> <string>portal_type</string> </key>
+            <value> <string>Standard Property</string> </value>
+        </item>
+        <item>
+            <key> <string>preference</string> </key>
+            <value> <int>1</int> </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+</ZopeData>

product/ERP5/bootstrap/erp5_property_sheets/PropertySheetTemplateItem/portal_property_sheets/DocumentPreference/preferred_document_conversion_server_url_property.xml

@@ -10,7 +10,7 @@
             <key> <string>categories</string> </key>
             <value>
               <tuple>
-                <string>elementary_type/string</string>
+                <string>elementary_type/lines</string>
               </tuple>
             </value>
         </item>
@@ -30,6 +30,10 @@
             <key> <string>preference</string> </key>
             <value> <int>1</int> </value>
         </item>
+        <item>
+            <key> <string>property_default</string> </key>
+            <value> <string>python:[]</string> </value>
+        </item>
         <item>
             <key> <string>write_permission</string> </key>
             <value> <string>Manage properties</string> </value>

product/ERP5OOo/Document/OOoDocument.py

@@ -44,7 +44,7 @@ from Products.ERP5.Document.Document import Document, \
 from Products.ERP5.Document.Image import getDefaultImageQuality
 from Products.ERP5Type.Utils import fill_args_from_request
 from zLOG import LOG, WARNING, ERROR
-
+from functools import partial
 # Mixin Import
 from Products.ERP5.mixin.base_convertable import BaseConvertableFileMixin
 from Products.ERP5.mixin.text_convertable import TextConvertableMixin
@@ -58,36 +58,18 @@ dec=base64.decodestring
 
 EMBEDDED_FORMAT = '_embedded'
 OOO_SERVER_PROXY_TIMEOUT = 360
+OOO_SERVER_RETRY = 0
 
-class _ProtocolErrorCatcher(object):
-  def __init__(self, orig_callable):
-    self.__callable = orig_callable
-  def __call__(self, *args, **kw):
-    """
-    Catch Protocol Errors (transport layer) and specifically
-    identify them as OOo server network/communication error
-
-    xml-rpc application level errors still go through: if a wrong method
-    is called, or with wrong parameters, xmlrpclib.Fault will be raised.
-    """
-    try:
-      return self.__callable(*args, **kw)
-    except ProtocolError, e:
-      message = "%s %s" % (e.errcode, e.errmsg)
-      if e.errcode == -1:
-        message = "Connection refused"
-      raise ConversionError("Protocol error while contacting OOo conversion"
-                            " server: %s" % (message))
-
-class OOoServerProxy(ServerProxy):
+class OOoServerProxy():
   """
   xmlrpc-like ServerProxy object adapted for OOo conversion server
   """
   def __init__(self, context):
+    self._serverproxy_list = []
     preference_tool = getToolByName(context, 'portal_preferences')
-
-    uri = preference_tool.getPreferredDocumentConversionServerUrl()
-    if not uri:
+    self._ooo_server_retry = preference_tool.getPreferredDocumentConversionServerRetry() or OOO_SERVER_RETRY
+    uri_list = preference_tool.getPreferredDocumentConversionServerUrlList()
+    if not uri_list:
       address = preference_tool.getPreferredOoodocServerAddress()
       port = preference_tool.getPreferredOoodocServerPortNumber()
       if not (address and port):
@@ -96,9 +78,12 @@ class OOoServerProxy(ServerProxy):
 
       LOG('OOoDocument', WARNING, 'PreferredOoodocServer{Address,PortNumber}' + \
           ' are DEPRECATED please use PreferredDocumentServerUrl instead', error=True)
-      scheme = "http"
-      uri = '%s://%s:%s' % (scheme, address, port)
-    else:
+       
+      uri_list =  ['%s://%s:%s' % ('http', address, port)]
+
+    timeout = preference_tool.getPreferredOoodocServerTimeout() \
+                    or OOO_SERVER_PROXY_TIMEOUT
+    for uri in uri_list:
       if uri.startswith("http://"):
         scheme = "http"
       elif uri.startswith("https://"):
@@ -107,17 +92,67 @@ class OOoServerProxy(ServerProxy):
         raise ConversionError('OOoDocument: cannot proceed with conversion:'
               ' preferred conversion server url is invalid')
 
-    timeout = preference_tool.getPreferredOoodocServerTimeout() \
-                    or OOO_SERVER_PROXY_TIMEOUT
       transport = TimeoutTransport(timeout=timeout, scheme=scheme)
       
-    ServerProxy.__init__(self, uri, allow_none=True, transport=transport)
+      self._serverproxy_list.append(ServerProxy(uri, allow_none=True, transport=transport))
+  def _proxy_function(self, func_name, *args, **kw):
+    result_error_set_list = []
+    protocol_error_list = []
+    fault_error_list = []
+    count = 0
+    serverproxy_list = self._serverproxy_list
+    while True:
+      retry_server_list = []
+      for server_proxy in serverproxy_list:
+        func = getattr(server_proxy, func_name)
+        try:
+          # Cloudooo return result in (200 or 402, dict(), '') format or just based type
+          # 402 for error and 200 for ok
+          result_set =  func(*args, **kw)
+        except ProtocolError, e:
+          # Network issue
+          message = "%s: %s %s" % (e.url, e.errcode, e.errmsg)
+          if e.errcode == -1:
+            message = "%s: Connection refused" % (e.url)
+          protocol_error_list.append(message)
+          retry_server_list.append(server_proxy)
+          continue
+        except Fault, e:
+          # Return not supported data types
+          fault_error_list.append(e)
+          continue
+
+        try:
+          response_code, response_dict, response_message = result_set
+        except ValueError:
+          # Compatibility for old oood, result is based type, like string
+           response_code = 200
+
+        if response_code == 200:
+          return result_set
+        else:
+          # If error, try next one
+          result_error_set_list.append(result_set)
+
+      # All servers are failed
+      if count == self._ooo_server_retry or len(retry_server_list) == 0:
+        break
+      count += 1
+      serverproxy_list = retry_server_list
+
+    # Check error type
+    # Return only one error result for compability
+    if len(result_error_set_list):
+      return result_error_set_list[0]
+
+    if len(protocol_error_list):
+      raise ConversionError("Protocol error while contacting OOo conversion: "
+                          "%s" % (','.join(protocol_error_list)))
+    if len(fault_error_list):
+      raise fault_error_list[0]
    
   def __getattr__(self, attr):
-    obj = ServerProxy.__getattr__(self, attr)
-    if callable(obj):
-      obj.__call__ = _ProtocolErrorCatcher(obj.__call__)
-    return obj
+    return partial(self._proxy_function, attr)
 
 class OOoDocument(OOoDocumentExtensibleTraversableMixin, BaseConvertableFileMixin, File,
                   TextConvertableMixin, Document):

product/ERP5OOo/tests/testOOoConversionServerRetry.py

+##############################################################################
+#
+# Copyright (c) 2002-2018 Nexedi SA and Contributors. All Rights Reserved.
+#
+# WARNING: This program as such is intended to be used by professional
+# programmers who take the whole responsibility of assessing all potential
+# consequences resulting from its eventual inadequacies and bugs
+# End users who are looking for a ready-to-use solution with commercial
+# guarantees and support are strongly adviced to contract a Free Software
+# Service Company
+#
+# This program is Free Software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+#
+##############################################################################
+
+from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
+from Products.ERP5OOo.tests.testDms import makeFileUpload
+from Products.ERP5Form.PreferenceTool import Priority
+
+
+class TestOOoConversionServerRetry(ERP5TypeTestCase):
+  def getBusinessTemplateList(self):
+    business_template_list = ['erp5_core_proxy_field_legacy',
+                            'erp5_jquery',
+                            'erp5_full_text_mroonga_catalog',
+                            'erp5_base',
+                            'erp5_ingestion_mysql_innodb_catalog',
+                            'erp5_ingestion',
+                            'erp5_web',
+                            'erp5_dms']
+    return business_template_list
+  
+  def clearDocumentModule(self):
+    self.abort()
+    doc_module = self.portal.document_module
+    doc_module.manage_delObjects(list(doc_module.objectIds()))
+    self.tic()
+  
+  def beforeTearDown(self):
+    self.abort()
+    activity_tool = self.portal.portal_activities
+    activity_status = {m.processing_node < -1
+                       for m in activity_tool.getMessageList()}
+    if True in activity_status:
+      activity_tool.manageClearActivities()
+
+    self.clearDocumentModule()
+
+  def afterSetUp(self):
+    self.portal.portal_caches.clearAllCache()
+    self.retry_count = 2
+  
+  def getDefaultSystemPreference(self):
+    id = 'default_system_preference'
+    tool = self.getPreferenceTool()
+    try:
+      pref = tool[id]
+    except KeyError:
+      pref = tool.newContent(id, 'System Preference')
+      pref.setPriority(Priority.SITE)
+      pref.enable()
+    return pref
+
+
+  def test_01_no_retry_for_no_network_issue(self):
+    system_pref = self.getDefaultSystemPreference()
+    system_pref.setPreferredDocumentConversionServerRetry(self.retry_count)
+    self.tic()
+    
+    filename = 'monochrome_sample.tiff'
+    file = makeFileUpload(filename)
+    document = self.portal.document_module.newContent(portal_type='Text')
+    document.edit(file = file)
+    message = document.Document_tryToConvertToBaseFormat()
+    self.assertEqual(message.count('Error converting document to base format'), 1)
+
+    
+
+  def test_02_retry_for_network_issue(self):
+    system_pref = self.getDefaultSystemPreference()
+    saved_server_list = system_pref.getPreferredDocumentConversionServerUrlList()
+    system_pref.setPreferredDocumentConversionServerRetry(self.retry_count)
+    system_pref.setPreferredDocumentConversionServerUrlList(['https://broken.url'])
+    self.tic()
+    filename = 'TEST-en-002.doc'
+    file = makeFileUpload(filename)
+    document = self.portal.portal_contributions.newContent(file=file)
+    
+    message = document.Document_tryToConvertToBaseFormat()
+    self.assertEqual(message.count('broken.url: Connection refused'), self.retry_count + 1) 
+    system_pref.setPreferredDocumentConversionServerUrlList(saved_server_list)
+    self.commit()
+