Basic validation

lib/importer/attribute-types.js

@@ -22,89 +22,80 @@
 // Base types without being wrapped by requiredType or optionalType may then
 // handled undefined or "" as they see fit.
 
-class AttributeType {
-  constructor(validator, translator) {
-    this.v = validator;
-    this.t = translator;
+// An attribute type is a function from an input value to a result.
+
+class AttributeMappingResult {
+  constructor(value, warnings, errors) {
+    this.value = value;
+    this.warnings = warnings || [];
+    this.errors = errors;
   }
 
-  get validator() { return this.v; }
-  get translator() { return this.t; }
+  get valid() {
+    return !this.errors;
+  }
 
-  validate(inputValue) { return this.v(inputValue); }
-  translate(inputValue) { return this.t(inputValue); }
+  get empty() {
+    return this.value !== undefined;
+  }
 }
 
-exports.AttributeType = AttributeType;
+// These helpers define the three kinds of results:
+
+function emptyMapping(warnings) {
+  return new AttributeMappingResult(undefined, warnings);
+}
+
+function successfulMapping(outputValue, warnings) {
+  return new AttributeMappingResult(outputValue, warnings);
+}
+
+function failedMapping(warnings, errors) {
+  return new AttributeMappingResult(undefined, warnings, errors);
+}
 
 // Create an optional version of an existing type, that allows empty strings or
 // undefined values and maps then to "undefined", and converts any validation errors to warnings
 exports.optionalType = (baseType) => {
-  return new AttributeType(
-    (inputValue) => {
-      if(inputValue !== undefined && inputValue != "") {
-        const [warnings, errors] = baseType.validate(inputValue);
+  return (inputValue) => {
+    if(inputValue !== undefined && inputValue != "") {
+      const result = baseType(inputValue);
+      if(result.valid) {
+        return result;
+      } else {
         // Convert any errors into warnings, as this is an optional field
-        return [warnings.concat(errors), []];
+        return emptyMapping(result.warnings.concat(result.errors));
       }
-      else {
-        return [[],[]];
-      }
-    },
-    (inputValue) => {
-      if(inputValue !== undefined && inputValue != "") return baseType.translate(inputValue);
-      else return undefined;
     }
-  );
-}
-
-// Create a required version of an existing type, that 
-exports.optionalType = (baseType) => {
-  return new AttributeType(
-    (inputValue) => {
-      if(inputValue !== undefined && inputValue != "") {
-        const [warnings, errors] = baseType.validate(inputValue);
-        // Convert any errors into warnings, as this is an optional field
-        return [warnings.concat(errors), []];
-      }
-      else {
-        return [[],[]];
-      }
-    },
-    (inputValue) => {
-      if(inputValue !== undefined && inputValue != "") return baseType.translate(inputValue);
-      else return undefined;
+    else {
+      return emptyMapping();
     }
-  );
+  };
 }
 
-// The default string type. Does not allow empty strings!
-exports.basicStringType = new AttributeType(
-  (inputValue) => {
-    if(inputValue === undefined || inputValue == "") {
-      return [[],["A value must be provided"]];
+// Create a required version of an existing type, that reports an undefined or empty-string input value as an error
+exports.requiredType = (baseType) => {
+  return (inputValue) => {
+    if(inputValue !== undefined && inputValue != "") {
+      return baseType(inputValue);
     }
-    // Anything else can be converted into a string
-    return [[],[]];
-  },
-  (inputValue) => {
-    return String(inputValue);
-  }
-);
-
-exports.basicNumberType = new AttributeType(
-  (inputValue) => {
-    if(inputValue === undefined || inputValue == "") {
-      return [[],["A value must be provided"]];
-    }
-
-    if(parseFloat(inputValue) === NaN) {
-      return [[],[inputValue + " is not a valid number"]];
+    else {
+      return failedMapping([], ["A value must be provided"]);
     }
+  };
+}
 
-    return [[],[]];
-  },
-  (inputValue) => {
-    return parseFloat(inputValue);
+// The default string type
+exports.basicStringType = (inputValue) => {
+  return successfulMapping(String(inputValue));
+};
+
+// The default numeric type
+exports.basicNumberType = (inputValue) => {
+  const result = parseFloat(inputValue);
+  if(isNaN(result)) {
+    return failedMapping([], ["This is not a valid number"]);
+  } else {
+    return successfulMapping(result);
   }
-);
+};

lib/importer/attribute-types.test.js

@@ -1,54 +1,80 @@
 const attributeTypes = require('./attribute-types');
 
-function testType(t, invalidInputs, fixtures) {
-  for([input,warnings,errors] of invalidInputs) {
-    expect(t.validate(input)).toMatchObject([warnings, errors]);
-  }
-
-  for ([input, output] of fixtures) {
-    expect(t.validate(input)).toMatchObject([[],[]]);
-    expect(t.translate(input)).toEqual(output);
-  }
-};
-
-test('basic string', () => {
-  const invalidInputs = [
-    [undefined, [], ["A value must be provided"]],
-    ["", [], ["A value must be provided"]]
-  ];
-
-  const fixtures = [
-    ["foo", "foo"],
-    [123, "123"],
-  ];
-
-  testType(attributeTypes.basicStringType, invalidInputs, fixtures);
+test('required basic string', () => {
+  // Also tests the workings of the requiredType system
+  const t = attributeTypes.requiredType(attributeTypes.basicStringType);
+
+  let r = t(undefined);
+  expect(r.valid).toBeFalsy();
+  expect(r.warnings).toMatchObject([]);
+  expect(r.errors).toMatchObject(["A value must be provided"]);
+
+  r = t("");
+  expect(r.valid).toBeFalsy();
+  expect(r.warnings).toMatchObject([]);
+  expect(r.errors).toMatchObject(["A value must be provided"]);
+
+  r = t("foo");
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toEqual("foo");
+  expect(r.warnings).toMatchObject([]);
+
+  r = t(123);
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toEqual("123");
+  expect(r.warnings).toMatchObject([]);
 });
 
 test('optional basic string', () => {
-  const invalidInputs = [];
+  // Also tests the workings of the optionalType system
+  const t = attributeTypes.optionalType(attributeTypes.basicStringType);
+
+  let r = t(undefined);
+  expect(r.valid).toBeTruthy();
+  expect(r.warnings).toMatchObject([]);
+  expect(r.value).toEqual(undefined);
+
+  r = t("");
+  expect(r.valid).toBeTruthy();
+  expect(r.warnings).toMatchObject([]);
+  expect(r.value).toEqual(undefined);
+
+  r = t("foo");
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toEqual("foo");
+  expect(r.warnings).toMatchObject([]);
+
+  r = t(123);
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toEqual("123");
+  expect(r.warnings).toMatchObject([]);
+});
+
+test('required basic number', () => {
+  const t = attributeTypes.requiredType(attributeTypes.basicNumberType);
 
-  const fixtures = [
-    ["foo", "foo"],
-    [123, "123"],
-    ["", undefined],
-    [undefined, undefined]
-  ];
+  r = t("foo");
+  expect(r.valid).toBeFalsy();
+  expect(r.warnings).toMatchObject([]);
+  expect(r.errors).toMatchObject(["This is not a valid number"]);
 
-  testType(attributeTypes.optionalType(attributeTypes.basicStringType), invalidInputs, fixtures);
+  r = t(123);
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toEqual(123);
+  expect(r.warnings).toMatchObject([]);
 });
 
-test('basic number', () => {
-  const invalidInputs = [
-    [undefined, [], ["A value must be provided"]],
-    ["", [], ["A value must be provided"]],
-    ["foo", [], ["foo is not a valid number"]]
-  ];
+test('optional basic number', () => {
+  // Also tests that optionalType converts errors into warnings
+  const t = attributeTypes.optionalType(attributeTypes.basicNumberType);
 
-  const fixtures = [
-    [123, 123],
-    ["123", "123"],
-  ];
+  r = t("foo");
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toBeUndefined();
+  expect(r.warnings).toMatchObject(["This is not a valid number"]);
 
-  testType(attributeTypes.basicNumberType, invalidInputs, fixtures);
+  r = t(123);
+  expect(r.valid).toBeTruthy();
+  expect(r.value).toEqual(123);
+  expect(r.warnings).toMatchObject([]);
 });

lib/importer/backend.js

@@ -459,7 +459,7 @@ function validateMapping(range, mapping) {
   // Output types are optional, we fall back to basicStringType if not specified
   if(mapping.attributeTypes) {
     for(const [attribute, attrType] of Object.entries(mapping.attributeTypes)) {
-      assert(attrType instanceof AttributeType);
+      assert(attrType instanceof Function);
     }
   }
 }
@@ -499,13 +499,11 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => {
 
         // For now, attribute mappings are just integer column offsets
         const inputColumn = range.start.column + m;
-        if (inputColumn >= row.length) {
-          // If a row is missing values at the end, this may be
-          // represented as a "short" row array.
-          record[attr] = undefined;
-        } else {
-          const cell = row[range.start.column + m];
-          if(cell.v) {
+        // If a row is missing values at the end, this may be
+        // represented as a "short" row array.
+        if (inputColumn < row.length) {
+          const cell = row[inputColumn];
+          if(cell && cell.v) {
             record[attr] = mapCellValue(cell);
             foundSomeValues = true;
           }
@@ -521,21 +519,27 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => {
         const [attr, inputVal] = element;
         const attrType = attrTypes[attr] || attributeTypes.basicStringType;
 
-        const [warnings, errors] = attrType.validate(inputVal);
-        warnings.forEach((text) => {
+        const result = attrType(inputVal);
+
+        result.warnings.forEach((text) => {
           rowWarnings.push([attr,text]);
         });
-        errors.forEach((text) => {
-          rowErrors.push([attr,text]);
-        });
 
-        if(errors.length == 0) {
-          mappedRecord[attr] = attrType.translate(inputVal);
+        if(result.valid) {
+          // Succeeded, but maybe an empty result
+          if(result.value !== undefined) {
+            mappedRecord[attr] = result.value;
+          }
+        } else {
+          // Failed
+          result.errors.forEach((text) => {
+            rowErrors.push([attr,text]);
+          });
         }
       });
 
       if (rowErrors.length == 0) {
-        records.push(record);
+        records.push(mappedRecord);
       }
     }