Bug 1225715: Part 3 - Allow extending existing schema types. r?billm draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 20 Jan 2016 22:46:37 -0800
changeset 324463 4905fa3a603e1cd4c4e9ca98d2dfcffc7fa103f3
parent 324462 7b6fcdd90fa71cd6aee8137bf7a97847685dc2ca
child 324464 bdb1e9619946b9458251e4cd7eeffcceda394804
child 324469 bda062baae5de13fd0fb6cd608580c95ddc2c991
push id9920
push usermaglione.k@gmail.com
push dateSat, 23 Jan 2016 01:12:39 +0000
reviewersbillm
bugs1225715
milestone46.0a1
Bug 1225715: Part 3 - Allow extending existing schema types. r?billm This one's a bit weird. I was trying to avoid it for a while, but when we start to support different sets of APIs on different apps, it's going make it complicated to maintain a single, centralized manifest schema without some way for them to directly extend it.
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -163,16 +163,22 @@ class AnyType extends Type {
 
 // An untagged union type.
 class ChoiceType extends Type {
   constructor(choices) {
     super();
     this.choices = choices;
   }
 
+  extend(type) {
+    this.choices.push(...type.choices);
+
+    return this;
+  }
+
   normalize(value, context) {
     for (let choice of this.choices) {
       let r = choice.normalize(value, context);
       if (!r.error) {
         return r;
       }
     }
 
@@ -189,32 +195,31 @@ class RefType extends Type {
   // For a reference to a type named T declared in namespace NS,
   // namespaceName will be NS and reference will be T.
   constructor(namespaceName, reference) {
     super();
     this.namespaceName = namespaceName;
     this.reference = reference;
   }
 
-  normalize(value, context) {
+  get targetType() {
     let ns = Schemas.namespaces.get(this.namespaceName);
     let type = ns.get(this.reference);
     if (!type) {
       throw new Error(`Internal error: Type ${this.reference} not found`);
     }
-    return type.normalize(value, context);
+    return type;
+  }
+
+  normalize(value, context) {
+    return this.targetType.normalize(value, context);
   }
 
   checkBaseType(baseType) {
-    let ns = Schemas.namespaces.get(this.namespaceName);
-    let type = ns.get(this.reference);
-    if (!type) {
-      throw new Error(`Internal error: Type ${this.reference} not found`);
-    }
-    return type.checkBaseType(baseType);
+    return this.targetType.checkBaseType(baseType);
   }
 }
 
 class StringType extends Type {
   constructor(enumeration, minLength, maxLength, pattern, format) {
     super();
     this.enumeration = enumeration;
     this.minLength = minLength;
@@ -277,16 +282,29 @@ class ObjectType extends Type {
   constructor(properties, additionalProperties, patternProperties, isInstanceOf) {
     super();
     this.properties = properties;
     this.additionalProperties = additionalProperties;
     this.patternProperties = patternProperties;
     this.isInstanceOf = isInstanceOf;
   }
 
+  extend(type) {
+    for (let key of Object.keys(type.properties)) {
+      if (key in this.properties) {
+        throw new Error(`InternalError: Attempt to extend an object with conflicting property "${key}"`);
+      }
+      this.properties[key] = type.properties[key];
+    }
+
+    this.patternProperties.push(...type.patternProperties);
+
+    return this;
+  }
+
   checkBaseType(baseType) {
     return baseType == "object";
   }
 
   normalize(value, context) {
     let v = this.normalizeBase("object", value);
     if (v.error) {
       return v;
@@ -810,17 +828,22 @@ this.Schemas = {
         });
       }
 
       let additionalProperties = null;
       if ("additionalProperties" in type) {
         additionalProperties = this.parseType(namespaceName, type.additionalProperties);
       }
 
-      checkTypeProperties("properties", "additionalProperties", "patternProperties", "isInstanceOf");
+      if ("$extend" in type) {
+        // Only allow extending "properties" and "patternProperties".
+        checkTypeProperties("properties", "patternProperties");
+      } else {
+        checkTypeProperties("properties", "additionalProperties", "patternProperties", "isInstanceOf");
+      }
       return new ObjectType(properties, additionalProperties, patternProperties, type.isInstanceOf || null);
     } else if (type.type == "array") {
       checkTypeProperties("items", "minItems", "maxItems");
       return new ArrayType(this.parseType(namespaceName, type.items),
                            type.minItems || 0, type.maxItems || Infinity);
     } else if (type.type == "number") {
       checkTypeProperties();
       return new NumberType();
@@ -850,17 +873,42 @@ this.Schemas = {
       checkTypeProperties("minimum", "maximum");
       return new AnyType();
     } else {
       throw new Error(`Unexpected type ${type.type}`);
     }
   },
 
   loadType(namespaceName, type) {
-    this.register(namespaceName, type.id, this.parseType(namespaceName, type, ["id"]));
+    if ("$extend" in type) {
+      this.extendType(namespaceName, type);
+    } else {
+      this.register(namespaceName, type.id, this.parseType(namespaceName, type, ["id"]));
+    }
+  },
+
+  extendType(namespaceName, type) {
+    let ns = Schemas.namespaces.get(namespaceName);
+    let targetType = ns && ns.get(type.$extend);
+
+    // Only allow extending object and choices types for now.
+    if (targetType instanceof ObjectType) {
+      type.type = "object";
+    } else if (!targetType) {
+      throw new Error(`Internal error: Attempt to extend a nonexistant type ${type.$extend}`);
+    } else if (!(targetType instanceof ChoiceType)) {
+      throw new Error(`Internal error: Attempt to extend a non-extensible type ${type.$extend}`);
+    }
+
+    let parsed = this.parseType(namespaceName, type, ["$extend"]);
+    if (parsed.constructor !== targetType.constructor) {
+      throw new Error(`Internal error: Bad attempt to extend ${type.$extend}`);
+    }
+
+    targetType.extend(parsed);
   },
 
   loadProperty(namespaceName, name, prop) {
     if ("value" in prop) {
       this.register(namespaceName, name, new ValueProperty(name, prop.value));
     } else {
       // We ignore the "optional" attribute on properties since we
       // don't inject anything here anyway.
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
@@ -21,16 +21,45 @@ let json = [
      {
        id: "type2",
        type: "object",
        properties: {
          prop1: {type: "integer"},
          prop2: {type: "array", items: {"$ref": "type1"}},
        },
      },
+
+     {
+       id: "basetype1",
+       type: "object",
+       properties: {
+         prop1: {type: "string"},
+       },
+     },
+
+     {
+       id: "basetype2",
+       choices: [
+         {type: "integer"},
+       ],
+     },
+
+     {
+       $extend: "basetype1",
+       properties: {
+         prop2: {type: "string"},
+       },
+     },
+
+     {
+       $extend: "basetype2",
+       choices: [
+         {type: "string"},
+       ],
+     },
    ],
 
    functions: [
      {
        name: "foo",
        type: "function",
        parameters: [
          {name: "arg1", type: "integer", optional: true},
@@ -161,16 +190,32 @@ let json = [
            properties: {
              url: {type: "string", "format": "url", "optional": true},
              relativeUrl: {type: "string", "format": "relativeUrl", "optional": true},
              strictRelativeUrl: {type: "string", "format": "strictRelativeUrl", "optional": true},
            },
          },
        ],
      },
+
+     {
+       name: "extended1",
+       type: "function",
+       parameters: [
+         {name: "val", $ref: "basetype1"},
+       ],
+     },
+
+     {
+       name: "extended2",
+       type: "function",
+       parameters: [
+         {name: "val", $ref: "basetype2"},
+       ],
+     },
    ],
 
    events: [
      {
        name: "onFoo",
        type: "function",
      },
 
@@ -443,9 +488,39 @@ add_task(function* () {
   if (Symbol.toStringTag) {
     let target = {prop1: 12, prop2: ["value1", "value3"]};
     target[Symbol.toStringTag] = () => "[object Object]";
     let proxy = new Proxy(target, {});
     Assert.throws(() => root.testing.quack(proxy),
                   /Expected a plain JavaScript object, got a Proxy/,
                   "should throw when passing a Proxy");
   }
+
+
+  root.testing.extended1({prop1: "foo", prop2: "bar"});
+  verify("call", "testing", "extended1", [{prop1: "foo", prop2: "bar"}]);
+  tallied = null;
+
+  Assert.throws(() => root.testing.extended1({prop1: "foo", prop2: 12}),
+                /Expected string instead of 12/,
+                "should throw for wrong property type");
+
+  Assert.throws(() => root.testing.extended1({prop1: "foo"}),
+                /Property "prop2" is required/,
+                "should throw for missing property");
+
+  Assert.throws(() => root.testing.extended1({prop1: "foo", prop2: "bar", prop3: "xxx"}),
+                /Unexpected property "prop3"/,
+                "should throw for extra property");
+
+
+  root.testing.extended2("foo");
+  verify("call", "testing", "extended2", ["foo"]);
+  tallied = null;
+
+  root.testing.extended2(12);
+  verify("call", "testing", "extended2", [12]);
+  tallied = null;
+
+  Assert.throws(() => root.testing.extended2(true),
+                /Incorrect argument types/,
+                "should throw for wrong argument type");
 });