Bug 1707896 - Transform generated folder into QML Module when building for Qt (#342)

Author: Beatriz Rizental

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+- Transform generated folder into QML Module when building Javascript templates for the Qt platform. ([bug 1707896](https://bugzilla.mozilla.org/show_bug.cgi?id=1707896)
+    - Import the Glean QML module from inside each generated file, removing the requirement to import Glean before importing any of the generated files;
+    - Prodive a `qmldir` file exposing all generated files;
+    - Drop the `namespace` option for Javascript templates;
+    - Add a new `version` option for Javascript templates, required when building for Qt, which expected the Glean QML module version.
+
 ## 3.4.0 (2021-05-28)
 
 - Add missing import for Kotlin code ([#339](https://github.com/mozilla/glean_parser/pull/341))

glean_parser/javascript.py

@@ -41,7 +41,7 @@ def iterencode(self, value):
     return "".join(JavascriptEncoder().iterencode(value))
 
 
-def class_name_factory(platform: str, namespace: str) -> Callable[[str], str]:
+def class_name_factory(platform: str) -> Callable[[str], str]:
     """
     Returns a function that receives an obj_type and
     returns the correct class name for that time in the current platform.
@@ -56,7 +56,7 @@ def class_name(obj_type: str) -> str:
             class_name = util.Camelize(obj_type) + "MetricType"
 
         if platform == "qt":
-            return namespace + ".Glean.default._private." + class_name
+            return "Glean.Glean._private." + class_name
 
         return class_name
 
@@ -101,24 +101,32 @@ def output(
         `parser.parse_objects`.
     :param output_dir: Path to an output directory to write to.
     :param options: options dictionary, with the following optional keys:
-
-        - `namespace`: The identifier of the global variable Glean was assigned to.
-                       This will only have and effect for Qt and static web sites.
-                       Default is `Glean`.
         - `platform`: Which platform are we building for. Options are `webext` and `qt`.
                       Default is `webext`.
+        - `version`: The version of the Glean.js Qt library being used.
+                     This option is mandatory when targeting Qt. Note that the version
+                     string must only contain the major and minor version i.e. 0.14.
+
     """
 
     if options is None:
         options = {}
 
-    namespace = options.get("namespace", "Glean")
     platform = options.get("platform", "webext")
+    if platform not in ["qt", "webext"]:
+        raise ValueError(
+            f"Unknown platform: {platform}. Accepted platforms are qt and webext."
+        )
+    version = options.get("version")
+    if platform == "qt" and version is None:
+        raise ValueError(
+            "'version' option is required when building for the 'qt' platform."
+        )
 
     template = util.get_jinja2_template(
         "javascript.jinja2",
         filters=(
-            ("class_name", class_name_factory(platform, namespace)),
+            ("class_name", class_name_factory(platform)),
             ("import_path", import_path),
             ("js", javascript_datatypes_filter),
             ("args", args),
@@ -151,8 +159,8 @@ def output(
                     category_name=category_key,
                     objs=category_val,
                     extra_args=util.extra_args,
-                    namespace=namespace,
                     platform=platform,
+                    version=version,
                     has_labeled_metrics=has_labeled_metrics,
                     types=types,
                     lang=lang,
@@ -161,6 +169,16 @@ def output(
             # Jinja2 squashes the final newline, so we explicitly add it
             fd.write("\n")
 
+    if platform == "qt":
+        # Explicitly create a qmldir file when building for Qt
+        template = util.get_jinja2_template("qmldir.jinja2")
+        filepath = output_dir / "qmldir"
+
+        with filepath.open("w", encoding="utf-8") as fd:
+            fd.write(template.render(categories=objs.keys(), version=version))
+            # Jinja2 squashes the final newline, so we explicitly add it
+            fd.write("\n")
+
 
 def output_javascript(
     objs: metrics.ObjectTree, output_dir: Path, options: Optional[Dict[str, Any]] = None

glean_parser/templates/javascript.jinja2

@@ -12,12 +12,6 @@ new {{ "labeled"|class_name }}({
     {{ arg_name|camelize }}: {{ obj[arg_name]|js }},
     {% endfor %}
 }, {{ obj.type|class_name }}{% if obj.labels is not none %}, {{ obj.labels|js }}{% endif %}){% endmacro %}
-{% if lang == "javascript" %}
-"use strict";
-
-{% endif %}
-/* eslint-disable */
-
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -31,6 +25,8 @@ import LabeledMetricType from "@mozilla/glean/{{ platform }}/private/metrics/lab
 {% for type in types %}
 import {{ type|class_name }} from "@mozilla/glean/{{ platform }}/private/{{ type|import_path }}";
 {% endfor %}
+{% else %}
+.import org.mozilla.Glean {{ version }} as Glean
 {% endif %}
 {% for obj in objs.values() %}
 /**

glean_parser/templates/qmldir.jinja2

@@ -0,0 +1,4 @@
+{% for category in categories|sort %}
+{{ category|Camelize }} {{ version }} {{ category|camelize }}.js
+{% endfor %}
+depends org.mozilla.Glean {{ version }}

tests/test_javascript.py

@@ -39,17 +39,14 @@ def test_parser_js(tmpdir):
     # Make sure descriptions made it in
     with (tmpdir / "corePing.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
-        assert "use strict" in content
         assert "True if the user has set Firefox as the default browser." in content
 
     with (tmpdir / "telemetry.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
-        assert "use strict" in content
         assert "جمع 搜集" in content
 
     with (tmpdir / "gleanInternalMetrics.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
-        assert "use strict" in content
         assert 'category: ""' in content
 
 
@@ -129,11 +126,11 @@ def test_metric_class_name():
         extra_keys={"my_extra": {"description": "an extra"}},
     )
 
-    webext_class_name = javascript.class_name_factory("webext", "Glean")
-    qt_class_name = javascript.class_name_factory("qt", "Glean")
+    webext_class_name = javascript.class_name_factory("webext")
+    qt_class_name = javascript.class_name_factory("qt")
 
     assert webext_class_name(event.type) == "EventMetricType"
-    assert qt_class_name(event.type) == "Glean.Glean.default._private.EventMetricType"
+    assert qt_class_name(event.type) == "Glean.Glean._private.EventMetricType"
 
     boolean = metrics.Boolean(
         type="boolean",
@@ -145,9 +142,7 @@ def test_metric_class_name():
         expires="never",
     )
     assert webext_class_name(boolean.type) == "BooleanMetricType"
-    assert (
-        qt_class_name(boolean.type) == "Glean.Glean.default._private.BooleanMetricType"
-    )
+    assert qt_class_name(boolean.type) == "Glean.Glean._private.BooleanMetricType"
 
     ping = pings.Ping(
         name="custom",
@@ -157,7 +152,7 @@ def test_metric_class_name():
         notification_emails=["nobody@nowhere.com"],
     )
     assert webext_class_name(ping.type) == "PingType"
-    assert qt_class_name(ping.type) == "Glean.Glean.default._private.PingType"
+    assert qt_class_name(ping.type) == "Glean.Glean._private.PingType"
 
 
 def test_import_path():
@@ -300,21 +295,61 @@ def test_arguments_are_generated_in_deterministic_order(tmpdir):
         assert expected in content
 
 
-def test_qt_platform_template_does_not_include_import_export_statements(tmpdir):
+def test_qt_platform_template_includes_expected_imports(tmpdir):
     """
-    Test when the platform is Qt, the template does not contain
+    Assert that when the platform is Qt, the template does not contain
     import/export statements.
     """
 
     tmpdir = Path(str(tmpdir))
 
     translate.translate(
-        ROOT / "data" / "single_labeled.yaml", "javascript", tmpdir, {"platform": "qt"}
+        ROOT / "data" / "single_labeled.yaml",
+        "javascript",
+        tmpdir,
+        {"platform": "qt", "version": "0.14"},
     )
 
-    assert set(x.name for x in tmpdir.iterdir()) == set(["category.js"])
+    assert set(x.name for x in tmpdir.iterdir()) == set(["category.js", "qmldir"])
 
     with (tmpdir / "category.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
-        assert content.count("import") == 0
+        assert content.count(".import org.mozilla.Glean 0.14") == 1
         assert content.count("export") == 0
+
+
+def test_qt_platform_generated_correct_qmldir_file(tmpdir):
+    """
+    Assert that when the platform is Qt, a qmldir is also generated
+    with the expected files listed in it.
+    """
+
+    tmpdir = Path(str(tmpdir))
+
+    translate.translate(
+        ROOT / "data" / "core.yaml",
+        "javascript",
+        tmpdir,
+        {"platform": "qt", "version": "0.14"},
+        {"allow_reserved": True},
+    )
+
+    assert set(x.name for x in tmpdir.iterdir()) == set(
+        [
+            "corePing.js",
+            "telemetry.js",
+            "environment.js",
+            "dottedCategory.js",
+            "gleanInternalMetrics.js",
+            "qmldir",
+        ]
+    )
+
+    with (tmpdir / "qmldir").open("r", encoding="utf-8") as fd:
+        content = fd.read()
+        assert content.count("CorePing 0.14 corePing.js") == 1
+        assert content.count("Telemetry 0.14 telemetry.js") == 1
+        assert content.count("Environment 0.14 environment.js") == 1
+        assert content.count("DottedCategory 0.14 dottedCategory.js") == 1
+        assert content.count("GleanInternalMetrics 0.14 gleanInternalMetrics.js") == 1
+        assert content.count("depends org.mozilla.Glean 0.14") == 1