Skip to content

Commit 3d475e6

Browse files
badboybadboy
committed
Add a new oneOf type to Glean's object metric type
1 parent 12bc731 commit 3d475e6

File tree

7 files changed

+161
-4
lines changed

7 files changed

+161
-4
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- New lint: `UNUSED_NO_LINT` when a lint is listed for a metric but doesn't actually trigger
77
- New lint: `UNKNOWN_LINT` when an unknown lint is named in the `no_lint` list
88
- Add Sendable conformance to Pings ([#810](https://github.com/mozilla/glean_parser/pull/810))
9+
- BREAKING CHANGE: Add `oneOf` type to object metric type structure. This requires changes in the generated code and is thus breaking, but shouldn't affect users ([#803](https://github.com/mozilla/glean_parser/pull/803))
910

1011
## 17.3.0
1112

‎glean_parser/metrics.py‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,10 @@ def __init__(self, *args, **kwargs):
462462
self._generate_structure = self.validate_structure(structure)
463463
super().__init__(*args, **kwargs)
464464

465-
ALLOWED_TOPLEVEL = {"type", "properties", "items", "description"}
465+
ALLOWED_TOPLEVEL = {"type", "properties", "items", "description", "oneOf"}
466466
ALLOWED_TYPES = ["object", "array", "number", "string", "boolean"]
467+
ALLOWED_SUBTYPES = ["number", "string", "boolean"]
468+
ALLOWED_ONEOF_FIELDS = {"description", "type"}
467469

468470
@staticmethod
469471
def _validate_substructure(structure):
@@ -475,6 +477,34 @@ def _validate_substructure(structure):
475477
f"Found additional fields: {extra}. Only allowed: {allowed}"
476478
)
477479

480+
if "oneOf" in structure:
481+
subtypes = structure.pop("oneOf")
482+
structure["type"] = "oneof"
483+
structure["subtypes"] = []
484+
if not subtypes:
485+
raise ValueError("List of types required.")
486+
487+
for typ in subtypes:
488+
extra = set(typ.keys()) - Object.ALLOWED_ONEOF_FIELDS
489+
if extra:
490+
extra = ", ".join(extra)
491+
allowed = ", ".join(Object.ALLOWED_ONEOF_FIELDS)
492+
raise ValueError(
493+
f"Found additional fields: {extra}. Only allowed: {allowed}"
494+
)
495+
496+
ty = typ.get("type")
497+
if not ty:
498+
raise ValueError("element of `oneOf` list must contain a type")
499+
500+
if ty not in Object.ALLOWED_SUBTYPES:
501+
raise ValueError(
502+
f"invalid `type` in `oneOf` list. found: {ty}, only allowed: {Object.ALLOWED_SUBTYPES}"
503+
)
504+
505+
structure["subtypes"].append(ty)
506+
return structure
507+
478508
if "type" not in structure:
479509
raise ValueError(
480510
f"missing `type` in object structure. Allowed: {Object.ALLOWED_TYPES}"

‎glean_parser/templates/kotlin.jinja2‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,17 @@ data class {{ obj.name|Camelize }}{{ suffix }}(
6767
{%- endmacro -%}
6868

6969
{%- macro generate_structure(name, struct) %}
70-
{%- if struct.type == "array" -%}
70+
{%- if struct.type == "oneof" -%}
71+
sealed class {{ name }}(): ObjectSerialize {
72+
{% for ty in struct.subtypes %}
73+
class {{ty|Camelize}}(val inner: kotlin.{{ty|structure_type_name}}? = null) : {{ name }}() {
74+
override fun intoSerializedObject(): kotlin.String {
75+
return this.inner?.intoSerializedObject() ?: "null"
76+
}
77+
}
78+
{% endfor %}
79+
}
80+
{%- elif struct.type == "array" -%}
7181
data class {{ name }}(var items: MutableList<{{ name }}Item> = mutableListOf()) : ObjectSerialize {
7282
fun add(elem: {{ name }}Item) = items.add(elem)
7383

@@ -107,6 +117,8 @@ data class {{ obj.name|Camelize }}{{ suffix }}(
107117
var {{itemname|camelize}}: {{ name ~ itemname|Camelize }} = {{ name ~ itemname|Camelize }}(),
108118
{% elif val.type == "object" %}
109119
var {{itemname|camelize}}: {{ name ~ "Item" ~ itemname|Camelize ~ "Object" }}? = null,
120+
{% elif val.type == "oneof" %}
121+
var {{itemname|camelize}}: {{ name ~ itemname|Camelize ~ "Enum" }}? = null,
110122
{% else %}
111123
var {{itemname|camelize}}: {{val.type|structure_type_name}}? = null,
112124
{% endif %}
@@ -138,6 +150,9 @@ data class {{ obj.name|Camelize }}{{ suffix }}(
138150
{% elif val.type == "object" %}
139151
{% set nested_name = name ~ "Item" ~ itemname|Camelize ~ "Object" %}
140152
{{ generate_structure(nested_name, val) }}
153+
{% elif val.type == "oneof" %}
154+
{% set nested_name = name ~ itemname|Camelize ~ "Enum" %}
155+
{{ generate_structure(nested_name, val) }}
141156
{% endif %}
142157
{% endfor %}
143158

‎glean_parser/templates/rust.jinja2‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,16 @@ Jinja2 template is not. Please file bugs! #}
99
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
1010

1111
{%- macro generate_structure(name, struct) %}
12-
{% if struct.type == "array" %}
12+
{%- if struct.type == "oneof" -%}
13+
#[derive(Debug, Hash, Eq, PartialEq, Clone, ::glean::traits::__serde::Serialize, ::glean::traits::__serde::Deserialize)]
14+
#[serde(crate = "::glean::traits::__serde")]
15+
#[serde(untagged)]
16+
pub enum {{ name }} {
17+
{% for ty in struct.subtypes %}
18+
{{ty|Camelize}}({{ty|structure_type_name}}),
19+
{% endfor %}
20+
}
21+
{% elif struct.type == "array" %}
1322
pub type {{ name }} = Vec<{{ name }}Item>;
1423

1524
{{ generate_structure(name ~ "Item", struct["items"]) }}
@@ -26,6 +35,9 @@ Jinja2 template is not. Please file bugs! #}
2635
{% elif val.type == "object" %}
2736
#[serde(skip_serializing_if = "Option::is_none")]
2837
pub {{itemname|snake_case}}: Option<{{ name ~ "Item" ~ itemname|Camelize ~ "Object" }}>,
38+
{% elif val.type == "oneof" %}
39+
#[serde(skip_serializing_if = "Option::is_none")]
40+
pub {{itemname|snake_case}}: Option<{{ name ~ itemname|Camelize ~ "Enum" }}>,
2941
{% else %}
3042
#[serde(skip_serializing_if = "Option::is_none")]
3143
pub {{itemname|snake_case}}: Option<{{val.type|structure_type_name}}>,
@@ -40,6 +52,9 @@ Jinja2 template is not. Please file bugs! #}
4052
{% elif val.type == "object" %}
4153
{% set nested_name = name ~ "Item" ~ itemname|Camelize ~ "Object" %}
4254
{{ generate_structure(nested_name, val) }}
55+
{% elif val.type == "oneof" %}
56+
{% set nested_name = name ~ itemname|Camelize ~ "Enum" %}
57+
{{ generate_structure(nested_name, val) }}
4358
{% endif %}
4459
{% endfor %}
4560

‎glean_parser/templates/swift.jinja2‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,23 @@ struct {{ obj.name|Camelize }}{{ suffix }}: EventExtras {
4545
{% endmacro %}
4646

4747
{%- macro generate_structure(name, struct) %}
48-
{%- if struct.type == "array" -%}
48+
{%- if struct.type == "oneof" -%}
49+
enum {{ name }}: Codable, Equatable, ObjectSerialize {
50+
{% for ty in struct.subtypes %}
51+
case {{ty}}({{ty|structure_type_name}})
52+
{% endfor %}
53+
54+
func intoSerializedObject() -> String {
55+
switch self {
56+
{% for ty in struct.subtypes %}
57+
case .{{ty}}(let val):
58+
return val.intoSerializedObject()
59+
{% endfor %}
60+
}
61+
}
62+
}
63+
64+
{%- elif struct.type == "array" -%}
4965
typealias {{ name }} = [{{ name }}Item]
5066

5167
{{ generate_structure(name ~ "Item", struct["items"]) }}
@@ -57,6 +73,8 @@ struct {{ obj.name|Camelize }}{{ suffix }}: EventExtras {
5773
var {{itemname|camelize|variable_name}}: {{ name ~ itemname|Camelize }} = []
5874
{% elif val.type == "object" %}
5975
var {{itemname|camelize|variable_name}}: {{ name ~ "Item" ~ itemname|Camelize ~ "Object" }}?
76+
{% elif val.type == "oneof" %}
77+
var {{itemname|camelize|variable_name}}: {{ name ~ itemname|Camelize ~ "Enum" }}?
6078
{% else %}
6179
var {{itemname|camelize|variable_name}}: {{val.type|structure_type_name}}?
6280
{% endif %}
@@ -90,6 +108,9 @@ struct {{ obj.name|Camelize }}{{ suffix }}: EventExtras {
90108
{% elif val.type == "object" %}
91109
{% set nested_name = name ~ "Item" ~ itemname|Camelize ~ "Object" %}
92110
{{ generate_structure(nested_name, val) }}
111+
{% elif val.type == "oneof" %}
112+
{% set nested_name = name ~ itemname|Camelize ~ "Enum" %}
113+
{{ generate_structure(nested_name, val) }}
93114
{% endif %}
94115
{% endfor %}
95116

‎tests/data/object.yaml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,30 @@ complex.types:
3939
items:
4040
type: boolean
4141

42+
oneof:
43+
type: object
44+
description: |
45+
Array of key-value elements
46+
bugs:
47+
- https://bugzilla.mozilla.org/111
48+
data_reviews:
49+
- http://example.com/reviews
50+
notification_emails:
51+
- [email protected]
52+
expires: never
53+
structure:
54+
type: array
55+
items:
56+
type: object
57+
properties:
58+
key:
59+
type: string
60+
value:
61+
oneOf:
62+
- type: string
63+
- type: number
64+
- type: boolean
65+
4266
# yamllint disable rule:line-length
4367
activity.stream:
4468
tiles:

‎tests/test_parser.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,57 @@ def test_object_invalid():
12861286
assert "invalid `type` in object structure." in errors[0]
12871287

12881288

1289+
@pytest.mark.parametrize(
1290+
"structure,num_errors",
1291+
[
1292+
(
1293+
{
1294+
"type": "object",
1295+
"properties": {"key": {"type": "string"}, "value": {"oneOf": []}},
1296+
},
1297+
1,
1298+
),
1299+
(
1300+
{
1301+
"type": "object",
1302+
"properties": {"key": {"type": "string"}, "value": {"oneOf": [ { "type": "object" } ]}},
1303+
},
1304+
1,
1305+
),
1306+
(
1307+
{
1308+
"type": "object",
1309+
"properties": {"key": {"type": "string"}, "value": {"oneOf": [ { "type": "number", "other": "unsupported" } ]}},
1310+
},
1311+
1,
1312+
),
1313+
(
1314+
{
1315+
"type": "object",
1316+
"properties": {"key": {"type": "string"}, "value": {"oneOf": [ { "type": "number" } ]}},
1317+
},
1318+
0,
1319+
),
1320+
(
1321+
{
1322+
"type": "object",
1323+
"properties": {"key": {"type": "string"}, "value": {"oneOf": [ { "type": "number", "description": "desc" } ]}},
1324+
},
1325+
0,
1326+
),
1327+
],
1328+
)
1329+
def test_object_oneof(structure, num_errors):
1330+
contents = [{"category": {"metric": {"type": "object", "structure": structure}}}]
1331+
1332+
contents = [util.add_required(x) for x in contents]
1333+
all_metrics = parser.parse_objects(contents)
1334+
errors = list(all_metrics)
1335+
assert len(errors) == num_errors, errors
1336+
assert len(all_metrics.value) == 1
1337+
# assert all_metrics.value["category"]["metric"]._generate_structure == structure
1338+
1339+
12891340
def test_dual_labeled_counter():
12901341
"""
12911342
Ensure that `dual_labeled_counter` metrics parse properly.

0 commit comments

Comments
 (0)