From 4c13a291d74a812bcbdf95e554fea2cf81b9f311 Mon Sep 17 00:00:00 2001 From: akasakariko <148060537+akasakariko@users.noreply.github.com> Date: Tue, 3 Nov 2026 09:06:22 +0900 Subject: [PATCH 1/2] Bug 1929903 - Preserve object property names in Rust JSON Add conditional Serde renames when schema properties differ from their generated Rust fields in the Rust and Rust Sym templates Extend the existing object test to cover original JSON keys for dotted, hyphenated, uppercase, nested, array, and union properties --- CHANGELOG.md | 1 + glean_parser/templates/rust.jinja2 | 3 +++ glean_parser/templates/rust_sym.jinja2 | 3 +++ tests/data/object.yaml | 14 +++++++------- tests/test_rust.py | 25 ++++++++++++++++++++----- 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83123f8e..4d1337d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Rust and Rust Sym: Preserve object metric property names containing dots, hyphens, or uppercase letters when serializing and deserializing JSON ([bug 1929903](https://bugzilla.mozilla.org/show_bug.cgi?id=1929903)). - Support `flows.yaml` files (`moz://mozilla.org/schemas/glean/flows/1-0-0`), declaring multi-step tasks measured as a unit over existing event metrics. Flows are validated against the metrics passed to the parser, with warnings for events that aren't defined in any of them (`UNRESOLVED_FLOW_EVENT`) and for flows with no exit events (`EMPTY_ENDS_ON`). Kotlin, Swift, and Rust output gains the flow definitions to pass to Glean at initialization (`GleanFlows`), and Markdown output documents flows and the flows each event belongs to. Flows are passed to external translators as the `flows` option ([bug 2077133](https://bugzilla.mozilla.org/show_bug.cgi?id=2077133)) - Fix tests that failed when `SOURCE_DATE_EPOCH` is set in the environment - Add a `skip_schema_validation` option to `parse_objects` to skip schema validation of input files ([bug 2076540](https://bugzilla.mozilla.org/show_bug.cgi?id=2076540)) diff --git a/glean_parser/templates/rust.jinja2 b/glean_parser/templates/rust.jinja2 index 178aac94..345f1fd6 100644 --- a/glean_parser/templates/rust.jinja2 +++ b/glean_parser/templates/rust.jinja2 @@ -29,6 +29,9 @@ Jinja2 template is not. Please file bugs! #} #[serde(deny_unknown_fields)] pub struct {{ name }} { {% for itemname, val in struct.properties.items() %} + {% if itemname != itemname|snake_case %} + #[serde(rename = "{{ itemname }}")] + {% endif %} {% if val.type == "array" %} #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new", deserialize_with = "::glean::traits::__serde_helper::vec_null")] pub {{itemname|snake_case}}: {{ name ~ itemname|Camelize }}, diff --git a/glean_parser/templates/rust_sym.jinja2 b/glean_parser/templates/rust_sym.jinja2 index 44676c0b..4a46e891 100644 --- a/glean_parser/templates/rust_sym.jinja2 +++ b/glean_parser/templates/rust_sym.jinja2 @@ -29,6 +29,9 @@ Jinja2 template is not. Please file bugs! #} #[serde(deny_unknown_fields)] pub struct {{ name }} { {% for itemname, val in struct.properties.items() %} + {% if itemname != itemname|snake_case %} + #[serde(rename = "{{ itemname }}")] + {% endif %} {% if val.type == "array" %} #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new", deserialize_with = "::glean_sym::__serde_helper::vec_null")] pub {{itemname|snake_case}}: {{ name ~ itemname|Camelize }}, diff --git a/tests/data/object.yaml b/tests/data/object.yaml index cca52089..c517be0c 100644 --- a/tests/data/object.yaml +++ b/tests/data/object.yaml @@ -52,11 +52,11 @@ complex.types: structure: type: object properties: - key1: + foo.bar-baz: type: string - another_value: + CamelCase: type: number - sub_array: + sub-array: type: array items: type: number @@ -79,7 +79,7 @@ complex.types: properties: key: type: string - value: + choice-value: oneOf: - type: string - type: number @@ -91,7 +91,7 @@ activity.stream: type: object description: | The id, position, and shim for each newtab tile and any extra nested data, - e.g. `[{"id": 1, "pos": 2, "shim": "long-id", "nested": { "count": 17, "info": "more info"}}]` + e.g. `[{"id": 1, "pos": 2, "shim": "long-id", "nested-object": { "count": 17, "nested.info": "more info"}}]` bugs: - https://bugzilla.mozilla.org/11137353 data_reviews: @@ -110,12 +110,12 @@ activity.stream: type: number shim: type: string - nested: + nested-object: type: object properties: count: type: number - info: + nested.info: type: string crash.stack: diff --git a/tests/test_rust.py b/tests/test_rust.py index dc224ce0..3d7cf71f 100644 --- a/tests/test_rust.py +++ b/tests/test_rust.py @@ -8,6 +8,8 @@ import subprocess import re +import pytest + from glean_parser import rust from glean_parser import metrics from glean_parser import pings @@ -257,13 +259,12 @@ def test_event_extra_keys_with_types(tmp_path): ) -def test_object_metric(tmp_path): - """ - Assert that an object metric is created. - """ +@pytest.mark.parametrize("output_format", ["rust", "rust_sym"]) +def test_object_metric(tmp_path, output_format): + """Assert that object metrics preserve their property names in JSON.""" translate.translate( ROOT / "data" / "object.yaml", - "rust", + output_format, tmp_path, {"namespace": "Foo"}, ) @@ -288,6 +289,20 @@ def test_object_metric(tmp_path): assert "trust: Option, " in content assert "}" in content + for original, field in [ + ("foo.bar-baz", "foo_bar_baz"), + ("CamelCase", "camelcase"), + ("sub-array", "sub_array"), + ("nested-object", "nested_object"), + ("nested.info", "nested_info"), + ("choice-value", "choice_value"), + ]: + assert re.search( + rf'#\[serde\(rename = "{re.escape(original)}"\)\] ' + + rf"#\[serde\([^\]]*\)\] pub {field}:", + content, + ) + def test_in_session_output(tmp_path): """Assert that in_session: true produces in_session: true in Rust output.""" From 16f1b1b97a880b781e6267c5b30aeea9e1e931b9 Mon Sep 17 00:00:00 2001 From: akasakariko <148060537+akasakariko@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:16:04 +0900 Subject: [PATCH 2/2] Bug 1929903 - Always rename Rust object fields in Serde --- glean_parser/templates/rust.jinja2 | 2 -- glean_parser/templates/rust_sym.jinja2 | 2 -- tests/test_rust.py | 1 + 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/glean_parser/templates/rust.jinja2 b/glean_parser/templates/rust.jinja2 index 345f1fd6..45dd9d5a 100644 --- a/glean_parser/templates/rust.jinja2 +++ b/glean_parser/templates/rust.jinja2 @@ -29,9 +29,7 @@ Jinja2 template is not. Please file bugs! #} #[serde(deny_unknown_fields)] pub struct {{ name }} { {% for itemname, val in struct.properties.items() %} - {% if itemname != itemname|snake_case %} #[serde(rename = "{{ itemname }}")] - {% endif %} {% if val.type == "array" %} #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new", deserialize_with = "::glean::traits::__serde_helper::vec_null")] pub {{itemname|snake_case}}: {{ name ~ itemname|Camelize }}, diff --git a/glean_parser/templates/rust_sym.jinja2 b/glean_parser/templates/rust_sym.jinja2 index 4a46e891..a448feac 100644 --- a/glean_parser/templates/rust_sym.jinja2 +++ b/glean_parser/templates/rust_sym.jinja2 @@ -29,9 +29,7 @@ Jinja2 template is not. Please file bugs! #} #[serde(deny_unknown_fields)] pub struct {{ name }} { {% for itemname, val in struct.properties.items() %} - {% if itemname != itemname|snake_case %} #[serde(rename = "{{ itemname }}")] - {% endif %} {% if val.type == "array" %} #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new", deserialize_with = "::glean_sym::__serde_helper::vec_null")] pub {{itemname|snake_case}}: {{ name ~ itemname|Camelize }}, diff --git a/tests/test_rust.py b/tests/test_rust.py index 3d7cf71f..2ab618f1 100644 --- a/tests/test_rust.py +++ b/tests/test_rust.py @@ -296,6 +296,7 @@ def test_object_metric(tmp_path, output_format): ("nested-object", "nested_object"), ("nested.info", "nested_info"), ("choice-value", "choice_value"), + ("module_index", "module_index"), ]: assert re.search( rf'#\[serde\(rename = "{re.escape(original)}"\)\] '