sails-disk - npm Package Compare versions

Comparing version 0.10.0-rc4 to 0.10.0-rc5

lib/database.js

@@ -7,2 +7,3 @@ /**
     fs = require('fs-extra'),
+    util = require('util'),
     async = require('async'),
@@ -333,9 +334,19 @@ waterlineCriteria = require('waterline-criteria'),
 
-  function addRecord(record) {
+  var originalValues = _.clone(values);
+  if(!Array.isArray(values)) values = [values];
 
+  // To hold any uniqueness constraint violations we encounter:
+  var constraintViolations = [];
+
+  // Iterate over each record being inserted, deal w/ auto-incrementing
+  // and checking the uniquness constraints.
+  for (var i in values) {
+    var record = values[i];
+
     // Check Uniqueness Constraints
-    var errors = self.uniqueConstraint(collectionName, record);
-    if(errors && errors.length > 0) return cb(errors);
+    // (stop at the first failure)
+    constraintViolations = constraintViolations.concat(self.enforceUniqueness(collectionName, record));
+    if (constraintViolations.length) break;
 
-    // Auto-Increment any values
+    // Auto-Increment any values that need it
     record = self.autoIncrement(collectionName, record);
@@ -346,9 +357,9 @@ record = self.serializeValues(collectionName, record);
     self.data[collectionName].push(record);
+  }
 
+  // If uniqueness constraints were violated, send back a validation error.
+  if (constraintViolations.length) {
+    return cb(new UniquenessError(constraintViolations));
   }
 
-  var originalValues = _.clone(values);
-  if(!Array.isArray(values)) values = [values];
-  values.forEach(addRecord);
-
   this.write(collectionName, function() {
@@ -374,3 +385,2 @@ cb(null, Array.isArray(originalValues) ? values : values[0]);
   if(!this.filePath) return cb(new Error('No filePath was configured for this collection'));
-
   // Filter Data based on Options criteria
@@ -382,42 +392,66 @@ var resultSet = waterlineCriteria(collectionName, this.data, options);
   // ii) have uniqueness constraints
-  var uniqueAttrs = _.where(_.keys(values), function(attrName) {
-    var attributeDef = self.schema[collectionName][attrName];
-    return attributeDef && attributeDef.unique;
-  });
+  // var uniqueAttrs = _.where(_.keys(values), function(attrName) {
+  //   var attributeDef = self.schema[collectionName][attrName];
+  //   return attributeDef && attributeDef.unique;
+  // });
 
   // If we're updating any attributes that are supposed to be unique, do some additional checks
-  if (uniqueAttrs.length && resultSet.indices.length) {
+  // if (uniqueAttrs.length && resultSet.indices.length) {
 
-    // If we would be updating more than one record, then uniqueness constraint automatically fails
-    if (resultSet.indices.length > 1) {
-      var error = new Error('Uniqueness check failed on attributes: ' + uniqueAttrs.join(','));
-      return cb(error);
-    }
+  //   // If we would be updating more than one record, then uniqueness constraint automatically fails
+  //   if (resultSet.indices.length > 1) {
+  //     var error = new Error('Uniqueness check failed on attributes: ' + uniqueAttrs.join(','));
+  //     return cb(error);
+  //   }
 
-    // Otherwise for each unique attribute, ensure that the matching result already has the value
-    // we're updating it to, so that there wouldn't be more than one record with the same value.
-    else {
-      var result = self.data[collectionName][resultSet.indices[0]];
-      var records = _.reject(self.data[collectionName], result);
-      var errors = [];
+  //   // Otherwise for each unique attribute, ensure that the matching result already has the value
+  //   // we're updating it to, so that there wouldn't be more than one record with the same value.
+  //   else {
+  //     var result = self.data[collectionName][resultSet.indices[0]];
+  //     var records = _.reject(self.data[collectionName], result);
+      
+  //     // Build an array of uniqueness errors
+  //     var uniquenessErrors = [];
+  //     uniquenessErrors.code = 'E_UNIQUE';
 
-      _.each(uniqueAttrs, function(uniqueAttr) {
-        // look for matching values in all records. note: this is case sensitive.
-        if (!!~_.pluck(records, uniqueAttr).indexOf(values[uniqueAttr])) {
-          errors.push(new Error('Uniqueness check failed on attribute: ' + uniqueAttr + ' with value: ' + values[uniqueAttr]));
-        }
-      });
-      if (errors.length) {
-        return cb(errors);
-      }
-    }
+  //     _.each(uniqueAttrs, function eachAttribute (uniqueAttrName) {
+
+  //       // look for matching values in all records. note: this is case sensitive.
+  //       if (!!~_.pluck(records, uniqueAttrName).indexOf(values[uniqueAttrName])) {
+          
+  //         uniquenessErrors.push({
+  //           attribute: uniqueAttrName,
+  //           value: values[uniqueAttrName]
+  //         });
+  //         // errors.push(new Error('Uniqueness check failed on attribute: ' + uniqueAttrName + ' with value: ' + values[uniqueAttrName]));
+  //       }
+  //     });
+
+  //     // Finally, send the uniqueness errors back to Waterline.
+  //     if (uniquenessErrors.length) {
+  //       return cb(uniquenessErrors);
+  //     }
+  //   }
+  // }
+
+
+  // Enforce uniquness constraints
+  // If uniqueness constraints were violated, send back a validation error.
+  var violations = self.enforceUniqueness(collectionName, values);
+  if (violations.length) {
+    return cb(new UniquenessError(violations));
   }
 
+  // Otherwise, success!
+  // Build up final set of results.
   var results = [];
+  for (var i in resultSet.indices) {
+    var matchIndex = resultSet.indices[i];
+    var _values = self.data[collectionName][matchIndex];
 
-  resultSet.indices.forEach(function(matchIndex) {
-    var _values = self.data[collectionName][matchIndex];
+    // Clone the data to avoid providing raw access to the underlying
+    // in-memory data, lest a user makes inadvertent changes in her app.
     self.data[collectionName][matchIndex] = _.extend(_values, values);
     results.push(_.cloneDeep(self.data[collectionName][matchIndex]));
-  });
+  }
 
@@ -526,15 +560,29 @@ self.write(collectionName, function() {
 
+
+
+
+
 /**
- * Unique Constraint
+ * enforceUniqueness
+ * 
+ * Enforces uniqueness constraint.
  *
+ * PERFORMANCE NOTE:
+ * This is O(N^2) - could be easily optimized with a logarithmic algorithm,
+ * but this is a development-only database adapter, so this is fine for now.
+ *
  * @param {String} collectionName
- * @param {Object} values
- * @return {Array}
+ * @param {Object} values           - attribute values for a single record
+ * @return {Object}
  * @api private
  */
 
-Database.prototype.uniqueConstraint = function(collectionName, values) {
+Database.prototype.enforceUniqueness = function(collectionName, values) {
 
   var errors = [];
 
+  // Get the primary key attribute name, so as not to inadvertently check
+  // uniqueness on something that doesn't matter.
+  var pkAttrName = getPrimaryKey(this.schema[collectionName]);
+
   for (var attrName in this.schema[collectionName]) {
@@ -547,10 +595,22 @@ var attrDef = this.schema[collectionName][attrName];
 
-      // Ignore uniquness check on undefined values
+      // Ignore uniqueness check on undefined values
+      // (they shouldn't have been stored anyway)
       if (_.isUndefined(values[attrName])) continue;
 
+      // Does it look like a "uniqueness violation"?
       if (values[attrName] === this.data[collectionName][index][attrName]) {
-        var error = new Error('Uniqueness check failed on attribute: ' + attrName +
-          ' with value: ' + values[attrName]);
 
-        errors.push(error);
+        // It isn't actually a uniquness violation if the record
+        // we're checking is the same as the record we're updating/creating
+        if (values[pkAttrName] === this.data[collectionName][index][pkAttrName]) {
+          continue;
+        }
+
+        var uniquenessError = {
+          attribute: attrName,
+          value: values[attrName],
+          rule: 'unique'
+        };
+
+        errors.push(uniquenessError);
       }
@@ -562,1 +622,66 @@ }
 };
+
+
+/**
+ * Convenience method to grab the name of the primary key attribute,
+ * given the schema.
+ * 
+ * @param  {Object} schema
+ * @return {String}
+ * @api private
+ */
+function getPrimaryKey (schema) {
+  var pkAttrName;
+  _.each(schema, function (def, attrName) {
+    if (def.primaryKey) pkAttrName = attrName;
+  });
+  return pkAttrName;
+}
+
+
+/**
+ * Given an array of errors, create a WLValidationError-compatible
+ * error definition.
+ * 
+ * @param {Array} errors
+ * @constructor
+ * @api private
+ */
+function UniquenessError ( errors ) {
+
+  // If no uniqueness constraints were violated, return early-
+  // there are no uniqueness errors to worry about.
+  if ( !errors.length ) return false;
+
+  //
+  // But if constraints were violated, we need to build a validation error.
+  // 
+
+  // First, group errors into an object of single-item arrays of objects:
+  // e.g.
+  // {
+  //   username: [{
+  //     attribute: 'username',
+  //     value: 'homeboi432'
+  //   }]
+  // }
+  // 
+  errors = _.groupBy(errors, 'attribute');
+  //
+  // Then remove the `attribute` key.
+  // 
+  errors = _.mapValues(errors, function (err) {
+    delete err[0].attribute;
+    return err;
+  });
+  
+  // Finally, build a validation error:
+  var validationError = {
+    code: 'E_UNIQUE',
+    invalidAttributes: errors
+  };
+
+  // and return it:
+  return validationError;
+  
+}

package.json

 {
   "name": "sails-disk",
-  "version": "0.10.0-rc4",
+  "version": "0.10.0-rc5",
   "description": "Persistent local-disk adapter for Sails.js / Waterline",
@@ -5,0 +5,0 @@ "main": "lib/adapter.js",

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

28029

increased by14.84%

Lines of code

779

increased by15.07%

No dependency changes