QML API 1.2: Fix importModule() behavior. Fixes #3

This changes the behavior of an existing function, so we need to
bump the QML import version to still support code that depends on
the old (broken) behavior of the importModule() function.

docs/index.rst

@@ -28,14 +28,22 @@ This section describes the QML API exposed by the *PyOtherSide* QML Plugin.
 Import Versions
 ---------------
 
-The current QML API version of PyOtherSide is 1.0. When new features are
-introduced, the API version will be bumped and documented here.
+The current QML API version of PyOtherSide is 1.2. When new features are
+introduced, or behavior is changed, the API version will be bumped and
+documented here.
 
 io.thp.pyotherside 1.0
 ``````````````````````
 
 * Initial API release.
 
+io.thp.pyotherside 1.2
+``````````````````````
+
+* :func:`importModule` now behaves like the ``import`` statement in Python
+  for names with dots. This means that ``importModule('x.y.z', ...)`` now
+  works like ``import x.y.z`` in Python.
+
 QML ``Python`` Element
 ----------------------
 
@@ -88,6 +96,12 @@ path and then importing the module asynchronously:
 
     Import a Python module.
 
+.. versionchanged:: 1.2.0
+    Previously, this function didn't work correctly for importing
+    modules with dots in their name. Starting with the API version 1.2
+    (``import io.thp.pyotherside 1.2``), this behavior is now fixed,
+    and ``importModule('x.y.z, ...)`` behaves like ``import x.y.z``.
+
 Once modules are imported, Python function can be called on the
 imported modules using:
 
@@ -121,6 +135,12 @@ plugin and Python interpreter.
 
     Get the version of the PyOtherSide plugin that is currently used.
 
+.. note::
+    This is not necessarily the same as the QML API version currently in use.
+    The QML API version is decided by the QML import statement, so even if
+    :func:`pluginVersion`` returns 1.2.0, if the plugin has been imported as
+    ``import io.thp.pyotherside 1.0``, the API version used would be 1.0.
+
 .. versionadded:: 1.1.0
 
 .. function:: pythonVersion() -> string
@@ -673,6 +693,12 @@ BB10), the QML plugins folder can be deployed with the .bar file.
 ChangeLog
 =========
 
+Version 1.2.0dev (UNRELEASED)
+-----------------------------
+
+* Introduced versioned QML imports for API change.
+* QML API 1.2: Change :func:`importModule` behavior for imports with dots.
+
 Version 1.1.0 (2014-02-06)
 --------------------------
 

pyotherside.pri

@@ -1,2 +1,2 @@
 PROJECTNAME = pyotherside
-VERSION = 1.1.0
+VERSION = 1.2.0dev

src/pyotherside_plugin.cpp

@@ -57,5 +57,7 @@ void
 PyOtherSideExtensionPlugin::registerTypes(const char *uri)
 {
     Q_ASSERT(QString(PYOTHERSIDE_PLUGIN_ID) == uri);
-    qmlRegisterType<QPython>(uri, 1, 0, PYOTHERSIDE_QPYTHON_NAME);
+    qmlRegisterType<QPython10>(uri, 1, 0, PYOTHERSIDE_QPYTHON_NAME);
+    // There is no PyOtherSide 1.1 import, as it's the same as 1.0
+    qmlRegisterType<QPython12>(uri, 1, 2, PYOTHERSIDE_QPYTHON_NAME);
 }

src/qpython.cpp

@@ -30,11 +30,13 @@
 QPythonPriv *
 QPython::priv = NULL;
 
-QPython::QPython(QObject *parent)
+QPython::QPython(QObject *parent, int major, int minor)
     : QObject(parent)
     , worker(new QPythonWorker(this))
     , thread()
     , handlers()
+    , major(major)
+    , minor(minor)
 {
     if (priv == NULL) {
         priv = new QPythonPriv;
@@ -108,13 +110,37 @@ QPython::importModule_sync(QString name)
     const char *moduleName = utf8bytes.constData();
 
     priv->enter();
-    PyObject *module = PyImport_ImportModule(moduleName);
+
+    bool use_api_10 = (major == 1 && minor == 0);
+
+    PyObject *module = NULL;
+
+    if (use_api_10) {
+        // PyOtherSide API 1.0 behavior (star import)
+        module = PyImport_ImportModule(moduleName);
+    } else {
+        // PyOtherSide API 1.2 behavior: "import x.y.z"
+        PyObject *fromList = PyList_New(0);
+        module = PyImport_ImportModuleEx(moduleName, NULL, NULL, fromList);
+        Py_XDECREF(fromList);
+    }
+
     if (module == NULL) {
         emit error(QString("Cannot import module: %1 (%2)").arg(name).arg(priv->formatExc()));
         priv->leave();
         return false;
     }
 
+    if (!use_api_10) {
+        // PyOtherSide API 1.2 behavior: "import x.y.z"
+        // If "x.y.z" is imported, we need to set "x" in globals
+        if (name.indexOf('.') != -1) {
+            name = name.mid(0, name.indexOf('.'));
+            utf8bytes = name.toUtf8();
+            moduleName = utf8bytes.constData();
+        }
+    }
+
     PyDict_SetItemString(priv->globals, moduleName, module);
     priv->leave();
     return true;

src/qpython.h

@@ -50,8 +50,10 @@ class QPython : public QObject {
          * \endcode
          *
          * \arg parent The parent QObject
+         * \arg major Major API version (used internally)
+         * \arg minor Minor API version (used internally)
          **/
-        QPython(QObject *parent=NULL);
+        QPython(QObject *parent, int major, int minor);
 
         virtual ~QPython();
 
@@ -292,6 +294,27 @@ class QPython : public QObject {
         QPythonWorker *worker;
         QThread thread;
         QMap<QString,QJSValue> handlers;
+
+        int major;
+        int minor;
+};
+
+class QPython10 : public QPython {
+Q_OBJECT
+public:
+    QPython10(QObject *parent=0)
+        : QPython(parent, 1, 0)
+    {
+    }
+};
+
+class QPython12 : public QPython {
+Q_OBJECT
+public:
+    QPython12(QObject *parent=0)
+        : QPython(parent, 1, 2)
+    {
+    }
 };
 
 #endif /* PYOTHERSIDE_QPYTHON_H */

tests/test_nested_import/README

@@ -0,0 +1,5 @@
+Test for importModule() with dots in the module name.
+
+Tests the (broken) behavior of QML API import 1.0, and the fixed 1.2 behavior.
+
+See: https://github.com/thp/pyotherside/issues/3

tests/test_nested_import/example_api10.qml

@@ -0,0 +1,49 @@
+import QtQuick 2.0
+import io.thp.pyotherside 1.0
+
+Python {
+    Component.onCompleted: {
+        addImportPath(Qt.resolvedUrl('.'));
+
+        /**
+         * Here, we test the broken behavior of the PyOtherSide 1.0 API
+         * for imports with "." in the name:
+         *  1. It uses PyImport_ImportModule() which does a "*"_import
+         *  2. The variable in the globals dict that gets set is the full
+         *     module name (and not the module after the "." for
+         *     non-"*"-imports) including the dot, which is broken, anyway, as
+         *     there's not way to retrieve that name in normal Python syntax
+         *     (names cannot contain a "."), so for this test we use a dirty
+         *     way of accessing they key via the globals() dict (just for
+         *     testing - I hope nobody used that in old code, but we want to
+         *     have a stable API, so we will drag this behavior along with the
+         *     1.0 API support - new code should definitely use the 1.2 API)
+         **/
+
+        importModule('thp_io.pyotherside.nested', function () {
+            console.log('"nested" imported successfully');
+
+            // In API version 1.0, we expect the import to have done a "*"
+            // import, and to add insult to injury, we assign the module
+            // name with a ".", which basically makes the import unaccessible
+            // from normal Python code (the entry in the globals dict contains
+            // a ".", which isn't a valid name in Python), so we access the
+            // globals dictionary directly
+            console.log('repr of the module: ' + evaluate('repr(globals()["thp_io.pyotherside.nested"])'));
+            call('globals()["thp_io.pyotherside.nested"].info', [], function (result) {
+                console.log('from nested.info(): ' + result);
+            });
+
+            importModule('thp_io.pyotherside.nested.module', function () {
+                console.log('"nested.module" imported successfully');
+                // Globals hack - see above
+                call('globals()["thp_io.pyotherside.nested.module"].info', [], function (result) {
+                    console.log('from nested.module.info(): ' + result);
+                    // Globals hack again - see above
+                    console.log('nested.module.value: ' + evaluate('globals()["thp_io.pyotherside.nested.module"].value'));
+                    Qt.quit();
+                });
+            });
+        });
+    }
+}

tests/test_nested_import/example_api12.qml

@@ -0,0 +1,23 @@
+import QtQuick 2.0
+import io.thp.pyotherside 1.2
+
+Python {
+    Component.onCompleted: {
+        addImportPath(Qt.resolvedUrl('.'));
+        importModule('thp_io.pyotherside.nested', function () {
+            console.log('"nested" imported successfully');
+            console.log('repr of the module: ' + evaluate('repr(thp_io.pyotherside.nested)'));
+            call('thp_io.pyotherside.nested.info', [], function (result) {
+                console.log('from nested.info(): ' + result);
+            });
+            importModule('thp_io.pyotherside.nested.module', function () {
+                console.log('"nested.module" imported successfully');
+                call('thp_io.pyotherside.nested.module.info', [], function (result) {
+                    console.log('from nested.module.info(): ' + result);
+                    console.log('nested.module.value: ' + evaluate('thp_io.pyotherside.nested.module.value'));
+                    Qt.quit();
+                });
+            });
+        });
+    }
+}

tests/test_nested_import/thp_io/pyotherside/nested/__init__.py

@@ -0,0 +1,2 @@
+def info():
+    return 'This is the nested package'

tests/test_nested_import/thp_io/pyotherside/nested/module.py

@@ -0,0 +1,4 @@
+def info():
+    return 'This is the nested.module module'
+
+value = 123

tests/tests.cpp

@@ -124,11 +124,9 @@ TestPyOtherSide::testConvertToPythonAndBack()
     QVERIFY(v == v2);
 }
 
-void
-TestPyOtherSide::testEvaluate()
+static void testEvaluateWith(QPython *py)
 {
-    QPython py;
-    QVariant squares = py.evaluate("[x*x for x in range(10)]");
+    QVariant squares = py->evaluate("[x*x for x in range(10)]");
     QVERIFY(squares.canConvert(QMetaType::QVariantList));
 
     QVariantList squares_list = squares.toList();
@@ -144,3 +142,15 @@ TestPyOtherSide::testEvaluate()
     QVERIFY(squares_list[8] == 64);
     QVERIFY(squares_list[9] == 81);
 }
+
+void
+TestPyOtherSide::testEvaluate()
+{
+    // PyOtherSide API 1.0
+    QPython10 py10;
+    testEvaluateWith(&py10);
+
+    // PyOtherSide API 1.2
+    QPython12 py12;
+    testEvaluateWith(&py12);
+}