Remove addl properties

mozilla_schema_generator/config.py

@@ -64,6 +64,9 @@ def _get_splits(self) -> Dict[str, Dict[Tuple[str], Matcher]]:
 
         return splits
 
+    def get_match_keys(self) -> List[Tuple[str]]:
+        return [prepend_properties(key) for key in self.matchers.keys()]
+
     def split(self) -> List[Config]:
         """
         Split this config into multiple configs.

mozilla_schema_generator/generic_ping.py

@@ -94,12 +94,15 @@ def make_schemas(env: Schema, probes: List[Probe], config: Config,
             final_schema.set_schema_elem(
                 schema_key + ("properties", probe.name),
                 probe_schema.schema)
-            final_schema.set_schema_elem(
-                schema_key + ("additionalProperties",),
-                False)
-            final_schema.delete_group_from_schema(schema_key + ("propertyNames",))
 
-        return schemas + [final_schema]
+        # Remove all additionalProperties (#22)
+        schemas.append(final_schema)
+        for s in schemas:
+            for key in config.get_match_keys():
+                s.delete_group_from_schema(key + ("propertyNames",), propagate=False)
+                s.delete_group_from_schema(key + ("additionalProperties",), propagate=True)
+
+        return schemas
 
     @staticmethod
     def make_extra_schema(schema: Schema, probes: List[Probe],
@@ -127,7 +130,7 @@ def make_extra_schema(schema: Schema, probes: List[Probe],
         return [schema]
 
     @staticmethod
-    def _get_json_str(url: str) -> dict:
+    def _get_json_str(url: str) -> str:
         r = requests.get(url, stream=True)
         r.raise_for_status()
 

mozilla_schema_generator/schema.py

@@ -34,11 +34,21 @@ class Schema(object):
     def __init__(self, schema: dict):
         self.schema = schema
 
-    def set_schema_elem(self, key: Tuple[str], elem: Any) -> dict:
+    def set_schema_elem(self, key: Tuple[str], elem: Any, *, propagate=True) -> dict:
+        """
+        @param key: The key set
+        @param elem: The value to set the key to
+        @param propagate: If True, creates objects until it reaches the full key.
+                          If False, and the parent of the key is not in the
+                          schema, then the key will not be added.
+        """
         new_elem = self.schema
 
         for k in key[:-1]:
             if k not in new_elem:
+                if not propagate:
+                    return
+
                 new_elem[k] = {}
                 if k == "properties":
                     new_elem["type"] = "object"
@@ -62,19 +72,29 @@ def _delete_key(self, key: Tuple[str]):
         except KeyError:
             return
 
-    def delete_group_from_schema(self, key: Tuple[str]):
+    def delete_group_from_schema(self, key: Tuple[str], *, propagate=True):
+        """
+        @param key: The key to remove
+        @param propagate: If True, then removes any parents of the deleted key
+                          if they are now empty, i.e. there are no other
+                          `properties`.
+        """
         self._delete_key(key)
 
         # Now check, moving backwards, if that was the only available property
         # If it was, and there are no additionalProperties, delete the parent
-        for subkey in reversed([key[:i] for i in range(len(key))]):
-            if not subkey or subkey[-1] == "properties":
-                # we only want to check the actual entry
-                continue
-
-            elem = _get(self.schema, subkey)
-            if not elem["properties"] and not elem.get("additionalProperties", False):
-                self._delete_key(subkey)
+        if propagate:
+            for subkey in reversed([key[:i] for i in range(len(key))]):
+                if not subkey or subkey[-1] == "properties":
+                    # we only want to check the actual entry
+                    continue
+
+                try:
+                    elem = _get(self.schema, subkey)
+                    if not elem.get("properties") and not elem.get("additionalProperties", False):
+                        self._delete_key(subkey)
+                except KeyError:
+                    break
 
     @staticmethod
     def _get_schema_size(schema: dict, key=None) -> int:

tests/test_glean.py

@@ -7,10 +7,12 @@
 import yaml
 import pytest
 from .test_utils import print_and_test
-from mozilla_schema_generator import glean_ping
+from mozilla_schema_generator import glean_ping, probes
 from mozilla_schema_generator.config import Config
 from mozilla_schema_generator.utils import _get, prepend_properties
 
+from typing import List
+
 
 @pytest.fixture
 def glean():
@@ -24,6 +26,11 @@ def config():
         return Config(yaml.load(f))
 
 
+class NoProbeGleanPing(glean_ping.GleanPing):
+    def get_probes(self) -> List[probes.GleanProbe]:
+        return []
+
+
 class TestGleanPing(object):
 
     def test_env_size(self, glean):
@@ -37,12 +44,8 @@ def test_single_schema(self, glean, config):
         final_schemas = {k: schemas[k][0].schema for k in schemas}
         for name, schema in final_schemas.items():
             # A few parts of the generic structure
-            assert "metrics" in schema["properties"]
             assert "ping_info" in schema["properties"]
-
-            # Client id should not be included, since it's in the ping_info
-            uuids = _get(schema, prepend_properties(("metrics", "uuid")))
-            assert "client_id" not in uuids.get("properties", {})
+            assert "client_info" in schema["properties"]
 
             if name == "baseline":
                 # Device should be included, since it's a standard metric
@@ -56,9 +59,22 @@ def test_get_repos(self):
     def test_generic_schema(self, glean, config):
         schemas = glean.generate_schema(config, split=False, generic_schema=True)
         generic_schema = glean.get_schema().schema
-
         assert schemas.keys() == {"baseline", "events", "metrics"}
 
         final_schemas = {k: schemas[k][0].schema for k in schemas}
         for name, schema in final_schemas.items():
             print_and_test(generic_schema, schema)
+
+    def test_removing_additional_properties(self, config):
+        # When there are no probes, previously all the addlProps
+        # fields remained; we now remove them
+        not_glean = NoProbeGleanPing("LeanGleanPingNoIding")
+        schemas = not_glean.generate_schema(config, split=False)
+
+        assert schemas.keys() == {"baseline", "events", "metrics"}
+
+        final_schemas = {k: schemas[k][0].schema for k in schemas}
+        for name, schema in final_schemas.items():
+            # The metrics key should have been deleted through
+            # propagation
+            assert "metrics" not in schema["properties"]

tests/test_integration.py

@@ -47,14 +47,12 @@ def test_full_representation(self, schema, env, probes):  # noqa F811
                             "type": "object",
                             "properties": {
                                 "test_probe": MainProbe.histogram_schema
-                            },
-                            "additionalProperties": False
+                            }
                         },
                         "nested": {
                             "type": "object",
                             "properties": {
                                 "second_level": {
-                                    "additionalProperties": False,
                                     "type": "object",
                                     "properties": {
                                         "second_level_probe": MainProbe.histogram_schema
@@ -104,8 +102,7 @@ def test_split_representation(self, schema, env, probes):  # noqa F811
                             "type": "object",
                             "properties": {
                                 "test_probe": MainProbe.histogram_schema
-                            },
-                            "additionalProperties": False
+                            }
                         }
                     }
                 },
@@ -124,8 +121,7 @@ def test_split_representation(self, schema, env, probes):  # noqa F811
                                     "type": "object",
                                     "properties": {
                                         "second_level_probe": MainProbe.histogram_schema
-                                    },
-                                    "additionalProperties": False
+                                    }
                                 }
                             }
                         }
@@ -264,8 +260,7 @@ def test_max_size(self, schema, env, probes):  # noqa F811
                             "properties": {
                                 "test_probe": MainProbe.histogram_schema,
                                 "third_probe": MainProbe.histogram_schema
-                            },
-                            "additionalProperties": False
+                            }
                         }
                     }
                 },
@@ -279,7 +274,6 @@ def test_max_size(self, schema, env, probes):  # noqa F811
                             "type": "object",
                             "properties": {
                                 "second_level": {
-                                    "additionalProperties": False,
                                     "type": "object",
                                     "properties": {
                                         "second_level_probe": MainProbe.histogram_schema

tests/test_schema.py

@@ -5,6 +5,7 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from mozilla_schema_generator.schema import Schema
+from .test_utils import print_and_test
 
 
 class TestSchema(object):
@@ -47,3 +48,55 @@ def test_schema_size(self):
 
         _tuple = {"type": "array", "items": [{"type": "string"}, {"type": "string"}]}
         assert Schema._get_schema_size(_tuple) == 2
+
+    def test_set_elem_propagate(self):
+        schema = Schema({
+            "properties": {
+                "a": {
+                    "type": "int"
+                }
+            },
+            "type": "object"
+        })
+
+        res_schema = schema.clone()
+
+        # Shouldn't add it if not propogating
+        res_schema.set_schema_elem(
+                ("properties", "b", "properties", "hello"),
+                {"type": "string"},
+                propagate=False)
+
+        print_and_test(schema.schema, res_schema.schema)
+
+        # should add it if propogating
+        expected = {
+            "properties": {
+                "a": {
+                    "type": "int"
+                },
+                "b": {
+                    "properties": {
+                        "hello": {
+                            "type": "string"
+                        },
+                    },
+                    "type": "object"
+                }
+            },
+            "type": "object"
+        }
+
+        res_schema.set_schema_elem(
+                ("properties", "b", "properties", "hello"),
+                {"type": "string"},
+                propagate=True)
+
+        print_and_test(expected, res_schema.schema)
+
+        # Deleting the elem again should match our original schema
+        res_schema.delete_group_from_schema(
+                ("properties", "b", "properties", "hello"),
+                propagate=True)
+
+        print_and_test(schema.schema, res_schema.schema)