From e712448ca0c32878ec048b475312e2097e954bf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mika=C3=ABl=20Capelle?= <capelle.mikael@gmail.com>
Date: Mon, 8 Jun 2020 17:54:30 +0200
Subject: [PATCH] Fix genFilePreview issue with exceptions not being properly
 translated to C++.

---
 src/runner/proxypluginwrappers.cpp  | 23 ++-----------------
 src/runner/pythonwrapperutilities.h | 34 +++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/src/runner/proxypluginwrappers.cpp b/src/runner/proxypluginwrappers.cpp
index 32695eb..90ce3cd 100644
--- a/src/runner/proxypluginwrappers.cpp
+++ b/src/runner/proxypluginwrappers.cpp
@@ -5,7 +5,6 @@
 #include <QWidget>
 
 #include "pythonwrapperutilities.h"
-#include "sipApiAccess.h"
 
 #include <variant>
 #include <tuple>
@@ -405,26 +404,8 @@ std::set<QString> IPluginPreviewWrapper::supportedExtensions() const
 
 QWidget *IPluginPreviewWrapper::genFilePreview(const QString &fileName, const QSize &maxSize) const
 {
-  // This is complicated, so we can't use the basic implementation
-  try {
-    GILock lock;
-    boost::python::override implementation = this->get_override("genFilePreview");
-    if (!implementation)
-      throw pyexcept::MissingImplementation(this->className, "genFilePreview");
-    boost::python::object pyVersion = implementation(fileName, maxSize);
-    // We need responsibility for deleting the QWidget to be transferred to C++
-    sipAPIAccess::sipAPI()->api_transfer_to(pyVersion.ptr(), Py_None);
-    return boost::python::extract<QWidget *>(pyVersion)();
-  }
-  catch (const boost::python::error_already_set&) {
-    throw pyexcept::PythonError();
-  }
-  catch (pyexcept::MissingImplementation const& missingImplementation) {
-    throw missingImplementation;
-  }
-  catch (...) {
-    throw pyexcept::UnknownException();
-  }
+  // We need responsibility for deleting the QWidget to be transferred to C++:
+  return wrapperFunctionImplementationWithApiTransfer<QWidget*>(this, "genFilePreview", fileName, maxSize);
 }
 /// end IPluginPreview Wrapper
 /////////////////////////////
diff --git a/src/runner/pythonwrapperutilities.h b/src/runner/pythonwrapperutilities.h
index 55d6af1..9786474 100644
--- a/src/runner/pythonwrapperutilities.h
+++ b/src/runner/pythonwrapperutilities.h
@@ -8,6 +8,7 @@
 #include <log.h>
 #include <utility.h>
 
+#include "sipApiAccess.h"
 #include "error.h"
 #include "gilock.h"
 
@@ -17,7 +18,7 @@ namespace details {
    * @brief Common stuffs for all basicWrapperFunction methods.
    */
   template <class ReturnType, class WrapperTypePtr, class Fn, class... Args>
-  ReturnType wrapperFunctionImplementation(WrapperTypePtr wrapper, Fn fn, boost::python::object* objPtr, const char *methodName, Args... args) {
+  ReturnType wrapperFunctionImplementation(WrapperTypePtr wrapper, bool apiTransfer, Fn fn, boost::python::object* objPtr, const char *methodName, Args... args) {
     GILock lock;
     boost::python::override implementation = wrapper->get_override(methodName);
     if (!implementation) {
@@ -33,6 +34,9 @@ namespace details {
       if (objPtr) {
         *objPtr = result;
       }
+      else if (apiTransfer) {
+        sipAPIAccess::sipAPI()->api_transfer_to(result.ptr(), Py_None);
+      }
       if constexpr (!std::is_same_v<ReturnType, void>) {
         return boost::python::extract<ReturnType>(result)();
       }
@@ -65,7 +69,7 @@ namespace details {
 template <class ReturnType, class WrapperType, class... Args>
 ReturnType basicWrapperFunctionImplementation(const WrapperType *wrapper, const char *methodName, Args... args)
 {
-  return details::wrapperFunctionImplementation<ReturnType>(wrapper, nullptr, nullptr, methodName, args...);
+  return details::wrapperFunctionImplementation<ReturnType>(wrapper, false, nullptr, nullptr, methodName, args...);
 }
 
 /**
@@ -87,7 +91,29 @@ ReturnType basicWrapperFunctionImplementation(const WrapperType *wrapper, const
 template <class ReturnType, class WrapperType, class... Args>
 ReturnType basicWrapperFunctionImplementation(const WrapperType* wrapper, boost::python::object &ref, const char* methodName, Args... args)
 {
-  return details::wrapperFunctionImplementation<ReturnType>(wrapper, nullptr, &ref, methodName, args...);
+  return details::wrapperFunctionImplementation<ReturnType>(wrapper, false, nullptr, &ref, methodName, args...);
+}
+
+/**
+ * @brief Call the given method on the wrapper with the given arguments, with proper
+ *     exception handling, and transfer the responsibility of the returned object to
+ *     the C++ side.
+ *
+ * @param wrapper The wrapper object to use to retrieve the python method. Must have a publicly
+ *     available `className` attribute.
+ * @param methodName The name of the method.
+ * @param args... Arguments for the method.
+ *
+ * @return the result of calling the given Python method on the wrapper.
+ *
+ * @throw pyexcept::MissingImplementation if the method does not exist.
+ * @throw pyexcept::PythonError if an error occurs while executing the python method.
+ * @throw pyexecpt::UnknownException if an unknown error occurs.
+ */
+template <class ReturnType, class WrapperType, class... Args>
+ReturnType wrapperFunctionImplementationWithApiTransfer(const WrapperType* wrapper, const char* methodName, Args... args)
+{
+  return details::wrapperFunctionImplementation<ReturnType>(wrapper, true, nullptr, nullptr, methodName, args...);
 }
 
 /**
@@ -110,7 +136,7 @@ ReturnType basicWrapperFunctionImplementation(const WrapperType* wrapper, boost:
 template <class ReturnType, class WrapperTypePtr, class Fn, class... Args>
 ReturnType basicWrapperFunctionImplementationWithDefault(WrapperTypePtr wrapper, Fn fn, const char* methodName, Args... args)
 {
-  return details::wrapperFunctionImplementation<ReturnType>(wrapper, fn, nullptr, methodName, args...);
+  return details::wrapperFunctionImplementation<ReturnType>(wrapper, false, fn, nullptr, methodName, args...);
 }
 
 #endif // PYTHONWRAPPERUTILITIES_H