Skip to content

Commit

Permalink
Require array or object as the root type in object metrics
Browse files Browse the repository at this point in the history
While this could be considered breaking, it's not in practice.
E.g. the FOG code implementing object metrics already requires arrays/objects at the root.
  • Loading branch information
badboy committed Jan 6, 2025
1 parent ac5e3a1 commit 604d07d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

- New lint: error when there are metrics whose names are too similar ([bug 1934099](https://bugzilla.mozilla.org/show_bug.cgi?id=1934099))
- Require `array` or `object` as the root type in object metrics ([#780](https://github.com/mozilla/glean_parser/pull/780))

## 16.1.0

Expand Down
14 changes: 14 additions & 0 deletions glean_parser/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,20 @@ def validate_structure(structure):
if None:
raise ValueError("`structure` needed for object metric.")

# Different from `ALLOWED_TYPES`:
# We _require_ the root type to be an object or array.
allowed_types = ["object", "array"]
if "type" not in structure:
raise ValueError(
f"missing `type` in object structure. Allowed: {allowed_types}"
)
if structure["type"] not in allowed_types:
raise ValueError(
"invalid `type` in object structure. found: {}, allowed: {}".format(
structure["type"], allowed_types
)
)

structure = Object._validate_substructure(structure)
return structure

Expand Down
8 changes: 8 additions & 0 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -1273,3 +1273,11 @@ def test_object_invalid():
errors = list(all_metrics)
assert len(errors) == 1
assert "`items` not allowed in object structure" in errors[0]

structure = {"type": "string"}
contents = [{"category": {"metric": {"type": "object", "structure": structure}}}]
contents = [util.add_required(x) for x in contents]
all_metrics = parser.parse_objects(contents)
errors = list(all_metrics)
assert len(errors) == 1
assert "invalid `type` in object structure." in errors[0]

0 comments on commit 604d07d

Please # to comment.