Repository: washingtonpost/ans-schema (js)

Issue:
Proposal: Formalize de-facto schema for label, source
## Problem

Data is already ingested in the `source` and `label` fields but the spec says very little about them.  

## Proposal

Let's make existing usage explicit to avoid confusion.  `label` is a dictionary-style (like `headlines`) set of text strings (similar to a WaPo kicker.)  They may have a `url` field attached.   `source` contains information about how and where the content originated.

## Examples

```
"label": {
  "basic": {
    "text": "Sports",
    "url": "/sports"
    "display": true
  }
  ...
}
```

```
"source": {
  "source_id": "123",  // Legacy CMS Id
  "name": "Reuters",  // Organization name (of content originator)
  "source_type": "Wires",       // "Wires" for ingested content; "Original" for CMS-entered 
  "system": "Arc I/O" // Name of CMS or app
}
```
  



Pull request description:
Formalizes schema for label, source; adds upverter
Fixes #93 

Patch:
diff --git a/lib/transform_utils.js b/lib/transform_utils.js
index 1244af1..492c472 100644
--- a/lib/transform_utils.js
+++ b/lib/transform_utils.js
@@ -47,6 +47,65 @@ var version_incrementer = function(new_version) {
   };
 };
 
+
+// Move all but allowed fields into additional_properties
+var limitFields = function limitFields(allowed_fields, original) {
+  var additional_properties = _.has(original, 'additional_properties') && _.isObject(original.additional_properties) ?
+      _.cloneDeep(original.additional_properties) : {};
+
+  var updated = _.transform(original, function(result, value, key, object) {
+    if (_.includes(allowed_fields, key)) {
+      result[key] = value;
+    }
+    else {
+      additional_properties[key] = value;
+    }
+  }, {});
+
+  // If previous additional_properties existed, merge with new additional_properties
+  if (_.size(additional_properties) > 0) {
+    if (_.has(updated.additional_properties) && _.isObject(updated.additional_properties)) {
+      _.merge(updated.additional_properties, additional_properties, updated.additional_properties);
+    }
+    else {
+      updated.additional_properties = additional_properties;
+    }
+  }
+
+  // If we don't have any additional properties, delete additional_properties
+  if (_.size(updated.additional_properties) === 0) {
+    delete updated.additional_properties;
+  }
+
+
+  return updated;
+};
+
+
+var field_name_valid = /^[a-zA-Z0-9_]*$/;
+var field_name_invalid_char = /[^a-zA-Z0-9_]/g
+
+// Rename fields
+var cleanFieldNames = function cleanFieldNames(original) {
+
+  var invalid_field_names = [];
+  var result = _.transform(original, function(result, value, key, object) {
+    if (field_name_valid.test(key) === false) {
+      var cleaned_name = '__renamed_' + key.replace(field_name_invalid_char, '_');
+      var final_name = cleaned_name;
+      var i = 0;
+      while (_.has(object, final_name)) {
+        final_name = cleaned_name + '_' + ++i;
+      }
+      result[final_name] = value;
+      delete result[key];
+    }
+  }, original);
+
+  return result;
+};
+
+
 var accumulatorFor = function(object) {
   if (_.isArray(object)) {
     return [];
@@ -58,5 +117,7 @@ module.exports = {
   'top_level_types': top_level_types,
   'version_incrementer': version_incrementer,
   'accumulatorFor': accumulatorFor,
-  'recursion_keys': recursion_keys
+  'recursion_keys': recursion_keys,
+  'limitFields': limitFields,
+  'cleanFieldNames': cleanFieldNames
 };
diff --git a/lib/upvert/0.5.7.js b/lib/upvert/0.5.7.js
index e37a6f2..7b4d6f6 100644
--- a/lib/upvert/0.5.7.js
+++ b/lib/upvert/0.5.7.js
@@ -8,6 +8,8 @@ var version_incrementer = transform_utils.version_incrementer;
 var top_level_types = transform_utils.top_level_types;
 var recursion_keys = transform_utils.recursion_keys;
 var accumulatorFor = transform_utils.accumulatorFor;
+var limitFields = transform_utils.limitFields;
+var cleanFieldNames = transform_utils.cleanFieldNames;
 
 // Tags in 0.5.7 were mostly undefined object. In 0.5.8 they have specified properties. Fields which
 // match the new properties should be preserved, those that do not should be moved into additional_properties.
@@ -40,6 +42,46 @@ var convertTag = function convertTag(old_tag) {
   return tag;
 };
 
+
+// Move label from undefined object to new format
+var convertLabel = function convertLabel(old_label) {
+
+  // Force each label to have correct keys.
+  var label = _.transform(old_label, function(result, value, key, object) {
+
+    if (key !== 'additional_properties') {
+      var label_item = limitFields(['text', 'url', 'display', 'additional_properties'], value);
+      if (!_.has(label_item, 'text')) {
+        label_item.text = '';
+      }
+
+      result[key] = label_item;
+    }
+    else {
+      result[key] = value;
+    }
+  }, {});
+
+  if (!_.has(label, 'basic')) {
+    label.basic = {
+      'text': ''
+    };
+  }
+
+  // Rename invalid keys to valid names.
+  return cleanFieldNames(label);
+
+};
+
+
+// Move source from undefined object to new format
+var convertSource = function convertSource(old_source) {
+  var source = limitFields(['source_id', 'source_type', 'name', 'system', 'additional_properties'], old_source);
+
+  return source;
+};
+
+
 // Convert a top-level ANS document and recur at known points
 var convertDocument = function convertDocument(doc) {
   return _.transform(doc, function(result, value, key, object) {
@@ -48,6 +90,12 @@ var convertDocument = function convertDocument(doc) {
       _.assign(taxonomy, value, { 'tags': _.map(value.tags, convertTag) });
       result[key] = taxonomy;
     }
+    else if (key === 'label') {
+      result[key] = convertLabel(value);
+    }
+    else if (key === 'source') {
+      result[key] = convertSource(value);
+    }
     else if (_.includes(recursion_keys, key)) {
       result[key] = convert(value);
     }
diff --git a/package.json b/package.json
index f3877bc..2efb4cc 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "ans-schema",
   "description": "The Washington Post's Arc Native Specification",
-  "version": "1.2.8",
+  "version": "1.2.9",
   "homepage": "https://github.com/washingtonpost/ans-schema",
   "repository": {
     "type": "git",
diff --git a/src/main/resources/schema/ans/0.5.8/traits/trait_label.json b/src/main/resources/schema/ans/0.5.8/traits/trait_label.json
index 47aa8f3..4e20717 100644
--- a/src/main/resources/schema/ans/0.5.8/traits/trait_label.json
+++ b/src/main/resources/schema/ans/0.5.8/traits/trait_label.json
@@ -4,9 +4,55 @@
   "title": "Label",
   "description": "What the Washington Post calls a Kicker",
   "type": "object",
+  "additionalProperties": false,
+  "properties": {
+    "basic": {
+      "description": "The default label object for this piece of content.",
+      "type": "object",
+      "additionalProperties": false,
+      "properties": {
+        "text": {
+          "type": "string",
+          "description": "The text of this label."
+        },
+        "url": {
+          "type": "string",
+          "description": "An optional destination url of this label."
+        },
+        "display": {
+          "type": "boolean",
+          "description": "If false, this label should be hidden."
+        },
+        "additional_properties": {
+          "$ref": "https://raw.githubusercontent.com/washingtonpost/ans-schema/master/src/main/resources/schema/ans/0.5.8/traits/trait_additional_properties.json"
+        }
+      },
+      "required": [ "text" ]
+    }
+  },
   "patternProperties": {
-    ".*": {
-      "type": "object"
+    "^[a-zA-Z0-9_]*$": {
+      "description": "Additional user-defined keyed label objects.",
+      "type": "object",
+      "additionalProperties": false,
+      "properties": {
+        "text": {
+          "type": "string",
+          "description": "The text of this label."
+        },
+        "url": {
+          "type": "string",
+          "description": "An optional destination url of this label."
+        },
+        "display": {
+          "type": "boolean",
+          "description": "If false, this label should be hidden."
+        },
+        "additional_properties": {
+          "$ref": "https://raw.githubusercontent.com/washingtonpost/ans-schema/master/src/main/resources/schema/ans/0.5.8/traits/trait_additional_properties.json"
+        }
+      },
+      "required": [ "text" ]
     }
   }
 }
diff --git a/src/main/resources/schema/ans/0.5.8/traits/trait_source.json b/src/main/resources/schema/ans/0.5.8/traits/trait_source.json
index 821b340..5bf5c69 100644
--- a/src/main/resources/schema/ans/0.5.8/traits/trait_source.json
+++ b/src/main/resources/schema/ans/0.5.8/traits/trait_source.json
@@ -4,11 +4,27 @@
   "title": "Source",
   "description": "Information about the original source and/or owner of this content",
   "type": "object",
-  "additionalProperties": {},
+  "additionalProperties": false,
   "properties": {
     "source_id": {
       "description": "Original source CMS id",
       "type":"string"
+    },
+    "source_type": {
+      "type": "string",
+      "description": "The method used to enter this content. E.g. 'Staff', 'Wires'"
+    },
+    "name": {
+      "type": "string",
+      "description": "The organization who originated this content."
+    },
+    "system": {
+      "type": "string",
+      "description": "The software (e.g., CMS) that was used to enter this content."
+    },
+
+    "additional_properties": {
+      "$ref": "https://raw.githubusercontent.com/washingtonpost/ans-schema/master/src/main/resources/schema/ans/0.5.8/traits/trait_additional_properties.json"
     }
   }
 }


Test patch:
diff --git a/tests/fixtures/schema/0.5.8/label-bad-arbitrary-properties.json b/tests/fixtures/schema/0.5.8/label-bad-arbitrary-properties.json
new file mode 100644
index 0000000..299ddd8
--- /dev/null
+++ b/tests/fixtures/schema/0.5.8/label-bad-arbitrary-properties.json
@@ -0,0 +1,6 @@
+{
+  "basic": {
+    "text": "This label is invalid because it has extra properties defined below.",
+    "a": "b"
+  }
+}
diff --git a/tests/fixtures/schema/0.5.8/label-bad-field-name-with-dots.json b/tests/fixtures/schema/0.5.8/label-bad-field-name-with-dots.json
new file mode 100644
index 0000000..5c3a22f
--- /dev/null
+++ b/tests/fixtures/schema/0.5.8/label-bad-field-name-with-dots.json
@@ -0,0 +1,5 @@
+{
+  "basic.label": {
+    "text": "This is not valid because there is a dot in the field name."
+  }
+}
diff --git a/tests/fixtures/schema/0.5.8/label-good.json b/tests/fixtures/schema/0.5.8/label-good.json
new file mode 100644
index 0000000..92051c7
--- /dev/null
+++ b/tests/fixtures/schema/0.5.8/label-good.json
@@ -0,0 +1,13 @@
+{
+  "basic": {
+    "text": "Breaking"
+  },
+  "secondary": {
+    "text": "Recent",
+    "url": "/recent",
+    "display": true,
+    "additional_properties": {
+      "bar": "baz"
+    }
+  }
+}
diff --git a/tests/fixtures/schema/0.5.8/source-bad-extra-properties.json b/tests/fixtures/schema/0.5.8/source-bad-extra-properties.json
new file mode 100644
index 0000000..af388f1
--- /dev/null
+++ b/tests/fixtures/schema/0.5.8/source-bad-extra-properties.json
@@ -0,0 +1,5 @@
+{
+  "type": "Wires",
+  "name": "Reuters",
+  "foo": "bar"
+}
diff --git a/tests/fixtures/schema/0.5.8/source-good.json b/tests/fixtures/schema/0.5.8/source-good.json
new file mode 100644
index 0000000..8adf6ee
--- /dev/null
+++ b/tests/fixtures/schema/0.5.8/source-good.json
@@ -0,0 +1,9 @@
+{
+  "source_type": "Wires",
+  "name": "Reuters",
+  "system": "Arc I/O",
+  "source_id": "ABC-DEF-45G",
+  "additional_properties": {
+    "foo": "bar"
+  }
+}
diff --git a/tests/fixtures/schema/0.5.8/story-fixture-good.json b/tests/fixtures/schema/0.5.8/story-fixture-good.json
index d513f8f..99b23dd 100644
--- a/tests/fixtures/schema/0.5.8/story-fixture-good.json
+++ b/tests/fixtures/schema/0.5.8/story-fixture-good.json
@@ -164,6 +164,19 @@
     ],
     "stock_symbols": [ "IBM", "INTL", "AMZN", "MRSFT" ]
   },
+  "label": {
+    "basic": {
+      "text": "The Kicker",
+      "url": "https://www.washingtonpost.com/kicker",
+      "display": true,
+      "additional_properties": {
+        "bar": "foo"
+      }
+    },
+    "another_one": {
+      "text": "Another One"
+    }
+  },
   "publish_date": "2015-06-24T09:49:00.10Z",
   "first_publish_date": "2015-06-24T09:49:00.10Z",
   "status": "published",
@@ -393,7 +406,13 @@
   },
 
   "source": {
-    "source_id": "washpost"
+    "source_id": "1234-5678",
+    "source_type": "Original",
+    "name": "The Washington Post",
+    "system": "Methode",
+    "additional_properties": {
+      "foo": "bar"
+    }
   },
 
   "syndication": {
diff --git a/tests/fixtures/transforms/0.5.7/story-fixture-good-old-labels-recursive.json b/tests/fixtures/transforms/0.5.7/story-fixture-good-old-labels-recursive.json
new file mode 100644
index 0000000..d74ea89
--- /dev/null
+++ b/tests/fixtures/transforms/0.5.7/story-fixture-good-old-labels-recursive.json
@@ -0,0 +1,18 @@
+{
+  "type": "story",
+  "version": "0.5.7",
+  "content_elements": [{
+    "type": "story",
+    "version": "0.5.7",
+    "label": {
+      "invalid.name": {
+        "text": "Foo"
+      }
+    }
+  }],
+  "label": {
+    "basic": {
+      "key": "value"
+    }
+  }
+}
diff --git a/tests/fixtures/transforms/0.5.7/story-fixture-good-old-style-label-and-source.json b/tests/fixtures/transforms/0.5.7/story-fixture-good-old-style-label-and-source.json
new file mode 100644
index 0000000..34c7813
--- /dev/null
+++ b/tests/fixtures/transforms/0.5.7/story-fixture-good-old-style-label-and-source.json
@@ -0,0 +1,20 @@
+{
+  "type": "story",
+  "version": "0.5.7",
+  "source": {
+    "source_type": "Wires",
+    "name": "Reuters",
+    "system": "Arc I/O",
+    "source_id": "ABC-DEF-45G",
+    "more": "data"
+  },
+  "label": {
+    "basic": {
+      "field": "value",
+      "why": "is there no text here"
+    },
+    "invalid.name": {
+      "text": "Invalid name for a label."
+    }
+  }
+}
diff --git a/tests/fixtures/transforms/0.5.7/story-fixture-good.json b/tests/fixtures/transforms/0.5.7/story-fixture-good.json
index 6044355..dc73d41 100644
--- a/tests/fixtures/transforms/0.5.7/story-fixture-good.json
+++ b/tests/fixtures/transforms/0.5.7/story-fixture-good.json
@@ -135,6 +135,29 @@
       }
     ]
   },
+  "label": {
+    "basic": {
+      "text": "The Kicker",
+      "url": "https://www.washingtonpost.com/kicker",
+      "display": true,
+      "additional_properties": {
+        "bar": "foo"
+      }
+    },
+    "another_one": {
+      "text": "Another One"
+    }
+  },
+  "source": {
+    "source_id": "1234-5678",
+    "type": "Original",
+    "name": "The Washington Post",
+    "system": "Methode",
+    "additional_properties": {
+      "foo": "bar"
+    }
+  },
+
   "publish_date": "2015-06-24T09:49:00.10Z",
   "status": "published",
   "display_date": "2015-06-25T09:50:50.52Z",
diff --git a/tests/schema-tests-05.js b/tests/schema-tests-05.js
index 1f8146f..fb16fb9 100644
--- a/tests/schema-tests-05.js
+++ b/tests/schema-tests-05.js
@@ -709,6 +709,32 @@ describe("Schema: ", function() {
 
         });
 
+        describe("Label (0.5.8+)", function() {
+          it("should validate a valid label", function() {
+            validateIfFixtureExists(version, '/traits/trait_label.json', 'label-good');
+          });
+
+          it("should not validate a label with extra properties", function() {
+            validateIfFixtureExists(version, '/traits/trait_label.json', 'label-bad-arbitrary-properties', false);
+          });
+
+          it("should not validate a valid label with fields with extra characters", function() {
+            validateIfFixtureExists(version, '/traits/trait_label.json', 'label-bad-field-name-with-dots', false);
+          });
+        });
+
+
+        describe("Source (0.5.8+)", function() {
+          it("should validate a valid source", function() {
+            validateIfFixtureExists(version, '/traits/trait_source.json', 'source-good');
+          });
+
+          it("should not validate an invalid source", function() {
+            validateIfFixtureExists(version, '/traits/trait_source.json', 'source-bad-extra-properties', false);
+          });
+        });
+
+
         describe("Story Elements ", function() {
           var type_prefix = "/story_elements";
 
@@ -823,13 +849,13 @@ describe("Schema: ", function() {
             });
           });
 
-          describe("Element Group", function() {
+          describe("Element Group (0.5.8+)", function() {
             it("should validate an element group", function() {
               validateIfFixtureExists(version, type_prefix + '/element_group.json', 'element-group-fixture-good');
             });
           });
 
-          describe("Quote", function() {
+          describe("Quote (0.5.8+)", function() {
             it("should not validate a quote with an invalid citation", function() {
               validateIfFixtureExists(version, type_prefix + '/quote.json', 'quote-fixture-bad-citation', false);
             });
@@ -838,6 +864,7 @@ describe("Schema: ", function() {
             });
           });
 
+
           describe("...all together now", function() {
             var valid_fixtures = [ "story-fixture-good", "story-fixture-references" ];
             valid_fixtures.forEach(function(fixtureName) {
diff --git a/tests/transform-tests.js b/tests/transform-tests.js
index 08958c5..6dc386d 100644
--- a/tests/transform-tests.js
+++ b/tests/transform-tests.js
@@ -544,6 +544,49 @@ describe("Transformations: ", function() {
 
         });
       });
+
+      describe("Labels", function() {
+        it("should leave valid labels alone", function() {
+          var result = transforms.upvert(fixtures['0.5.7']['story-fixture-good'], '0.5.8');
+          result.label.basic.text.should.eql("The Kicker");
+          result.label.basic.url.should.eql("https://www.washingtonpost.com/kicker");
+          result.label.basic.display.should.eql(true);
+          result.label.basic.additional_properties.bar.should.eql("foo");
+
+          result.label.another_one.text.should.eql("Another One");
+        });
+
+        it("should add 'text' where not present'", function() {
+          var result = transforms.upvert(fixtures['0.5.7']['story-fixture-good-old-style-label-and-source'], '0.5.8');
+          result.label.basic.text.should.eql('');
+        });
+
+        it("should move invalid properties to additional_properties'", function() {
+          var result = transforms.upvert(fixtures['0.5.7']['story-fixture-good-old-style-label-and-source'], '0.5.8');
+          result.label.basic.additional_properties.field.should.eql("value");
+          result.label.basic.additional_properties.why.should.eql("is there no text here");
+        });
+
+        it("should rename invalid keys'", function() {
+          var result = transforms.upvert(fixtures['0.5.7']['story-fixture-good-old-style-label-and-source'], '0.5.8');
+          result.label.should.have.property('__renamed_invalid_name');
+          result.label.__renamed_invalid_name.text.should.eql("Invalid name for a label.");
+        });
+      });
+
+      describe("Source", function() {
+        it("should leave valid source fields alone", function() {
+          var result = transforms.upvert(fixtures['0.5.7']['story-fixture-good-old-style-label-and-source'], '0.5.8');
+          result.source.source_type.should.eql("Wires");
+          result.source.name.should.eql("Reuters");
+        });
+
+        it("should move invalid source fields to additional_properties", function() {
+          var result = transforms.upvert(fixtures['0.5.7']['story-fixture-good-old-style-label-and-source'], '0.5.8');
+          result.source.additional_properties.more.should.eql("data");
+        });
+      });
+
     });
   });