Merge pull request #1365 from overlookmotel/order-by-associations-refactor

Order by associations refactor

lib/associations/belongs-to.js

@@ -17,8 +17,10 @@ module.exports = (function() {
       this.options.foreignKey = Utils._.underscoredIf(Utils.singularize(this.source.tableName, this.source.options.language) + "Id", this.source.options.underscored)
     }
 
-    if (!this.as) {
-      this.as = this.options.as = Utils.singularize(this.target.tableName, this.target.options.language)
+    if (this.as) {
+      this.isAliased = true
+    } else {
+      this.as = Utils.singularize(this.target.tableName, this.target.options.language)
     }
 
     this.associationAccessor = this.isSelfAssociation

lib/associations/has-many.js

@@ -21,9 +21,10 @@ module.exports = (function() {
     this.isMultiAssociation = true
     this.isSelfAssociation = this.source === this.target
     this.doubleLinked = false
+    this.as = this.options.as
     this.combinedTableName = Utils.combineTableNames(
       this.source.tableName,
-      this.isSelfAssociation ? (this.options.as || this.target.tableName) : this.target.tableName
+      this.isSelfAssociation ? (this.as || this.target.tableName) : this.target.tableName
     )
 
     /*
@@ -48,7 +49,7 @@ module.exports = (function() {
      * Determine associationAccessor, especially for include options to identify the correct model
      */
 
-    this.associationAccessor = this.options.as
+    this.associationAccessor = this.as
     if (!this.associationAccessor && (typeof this.through === "string" || Object(this.through) === this.through)) {
       this.associationAccessor = this.through.tableName || this.through
     }
@@ -112,16 +113,20 @@ module.exports = (function() {
 
     this.options.tableName = this.combinedName = (this.through === Object(this.through) ? this.through.tableName : this.through)
 
-    var as = (this.options.as || Utils.pluralize(this.target.tableName, this.target.options.language))
+    if (this.as) {
+      this.isAliased = true
+    } else {
+      this.as = Utils.pluralize(this.target.tableName, this.target.options.language)
+    }
     
     this.accessors = {
-      get: Utils._.camelize('get_' + as),
-      set: Utils._.camelize('set_' + as),
-      add: Utils._.camelize(Utils.singularize('add_' + as, this.target.options.language)),
-      create: Utils._.camelize(Utils.singularize('create_' + as, this.target.options.language)),
-      remove: Utils._.camelize(Utils.singularize('remove_' + as, this.target.options.language)),
-      hasSingle: Utils._.camelize(Utils.singularize('has_' + as, this.target.options.language)),
-      hasAll: Utils._.camelize('has_' + as)
+      get: Utils._.camelize('get_' + this.as),
+      set: Utils._.camelize('set_' + this.as),
+      add: Utils._.camelize(Utils.singularize('add_' + this.as, this.target.options.language)),
+      create: Utils._.camelize(Utils.singularize('create_' + this.as, this.target.options.language)),
+      remove: Utils._.camelize(Utils.singularize('remove_' + this.as, this.target.options.language)),
+      hasSingle: Utils._.camelize(Utils.singularize('has_' + this.as, this.target.options.language)),
+      hasAll: Utils._.camelize('has_' + this.as)
     }
   }
 

lib/associations/has-one.js

@@ -11,25 +11,28 @@ module.exports = (function() {
     this.options              = options
     this.isSingleAssociation  = true
     this.isSelfAssociation    = (this.source.tableName == this.target.tableName)
+    this.as = this.options.as
 
-    if (this.isSelfAssociation && !this.options.foreignKey && !!this.options.as) {
-      this.options.foreignKey = Utils._.underscoredIf(Utils.singularize(this.options.as, this.target.options.language) + "Id", this.options.underscored)
+    if (this.isSelfAssociation && !this.options.foreignKey && !!this.as) {
+      this.options.foreignKey = Utils._.underscoredIf(Utils.singularize(this.as, this.target.options.language) + "Id", this.options.underscored)
     }
 
-    if (!this.options.as) {
-      this.options.as = Utils.singularize(this.target.tableName, this.target.options.language)
+    if (this.as) {
+      this.isAliased = true
+    } else {
+      this.as = Utils.singularize(this.target.tableName, this.target.options.language)
     }
 
     this.associationAccessor = this.isSelfAssociation
-      ? Utils.combineTableNames(this.target.tableName, this.options.as)
-      : this.options.as
+      ? Utils.combineTableNames(this.target.tableName, this.as)
+      : this.as
 
     this.options.useHooks = options.useHooks
 
     this.accessors = {
-      get: Utils._.camelize('get_' + this.options.as),
-      set: Utils._.camelize('set_' + this.options.as),
-      create: Utils._.camelize('create_' + this.options.as)
+      get: Utils._.camelize('get_' + this.as),
+      set: Utils._.camelize('set_' + this.as),
+      create: Utils._.camelize('create_' + this.as)
     }
   }
 

lib/associations/mixin.js

@@ -59,36 +59,32 @@ Mixin.hasMany = function(associatedDAOFactory, options) {
   return this
 }
 
-Mixin.getAssociation = function(target) {
-  var result = null
-
+Mixin.getAssociation = function(target, alias) {
   for (var associationName in this.associations) {
     if (this.associations.hasOwnProperty(associationName)) {
       var association = this.associations[associationName]
 
-      if (!result && (association.target === target)) {
-        result = association
+      if (association.target === target && (alias === undefined ? !association.isAliased : association.as === alias)) {
+        return association
       }
     }
   }
 
-  return result
+  return null
 }
 
 Mixin.getAssociationByAlias = function(alias) {
-  var result = null
-
   for (var associationName in this.associations) {
     if (this.associations.hasOwnProperty(associationName)) {
       var association = this.associations[associationName]
 
-      if (!result && (association.options.as === alias)) {
-        result = association
+      if (association.as === alias) {
+        return association
       }
     }
   }
 
-  return result
+  return null
 }
 
 /* example for instance methods:

lib/dao-factory.js

@@ -1349,21 +1349,22 @@ module.exports = (function() {
 
   var validateIncludedElement = function(include, parent) {
     if (include instanceof DAOFactory) {
-      include = { daoFactory: include, as: include.tableName }
+      include = { daoFactory: include }
     }
 
     if (typeof parent === "undefined") {
       parent = this
     }
 
-    if (typeof include === 'object') {
+    if (typeof include !== 'object') {
+      throw new Error('Include unexpected. Element has to be either an instance of DAOFactory or an object.')
+    }
+
     if (include.hasOwnProperty('model')) {
       include.daoFactory = include.model
       delete include.model
-      }
-
-      if (!include.hasOwnProperty('as')) {
-        include.as = include.daoFactory.tableName
+    } else if (!include.hasOwnProperty('daoFactory')) {
+      throw new Error('Include malformed. Expected attributes: daoFactory, as!')
     }
 
     if (include.hasOwnProperty('attributes')) {
@@ -1384,18 +1385,11 @@ module.exports = (function() {
     // pseudo include just needed the attribute logic, return
     if (include._pseudo) return include
 
-      if (include.hasOwnProperty('daoFactory') && (include.hasOwnProperty('as'))) {
-        var usesAlias   = (include.as !== include.daoFactory.tableName)
-          , association = (usesAlias ? parent.getAssociationByAlias(include.as) : parent.getAssociation(include.daoFactory))
-
-        // If single (1:1) association, we singularize the alias, so it will match the automatically generated alias of belongsTo/HasOne
-        if (association && !usesAlias && association.isSingleAssociation) {
-          include.as = Utils.singularize(include.daoFactory.tableName, include.daoFactory.options.language)
-        }
-
     // check if the current daoFactory is actually associated with the passed daoFactory - or it's a pseudo include
-        if (association && (!association.options.as || (association.options.as === include.as))) {
+	var association = parent.getAssociation(include.daoFactory, include.as)
+    if (association) {
       include.association = association
+      include.as = association.as
 
       // If through, we create a pseudo child include, to ease our parsing later on
       if (Object(include.association.through) === include.association.through) {
@@ -1415,10 +1409,7 @@ module.exports = (function() {
       }
 
       if (include.required === undefined) {
-            include.required = false
-            if (include.where) {
-              include.required = true
-            }
+        include.required = !!include.where
       }
 
       // Validate child includes
@@ -1430,7 +1421,7 @@ module.exports = (function() {
     } else {
       var msg = include.daoFactory.name
 
-          if (usesAlias) {
+      if (include.as) {
         msg += " (" + include.as + ")"
       }
 
@@ -1438,12 +1429,6 @@ module.exports = (function() {
 
       throw new Error(msg)
     }
-      } else {
-        throw new Error('Include malformed. Expected attributes: daoFactory, as!')
-      }
-    } else {
-      throw new Error('Include unexpected. Element has to be either an instance of DAOFactory or an object.')
-    }
   }
 
   var replaceReferencesWithTableNames = function(attributes) {

lib/dialects/abstract/query-generator.js

@@ -356,58 +356,44 @@ module.exports = (function() {
       } else if (Array.isArray(obj)) {
         // loop through array, adding table names of models to quoted
         // (checking associations to see if names should be singularised or not)
-        var quoted = []
-          , i
+        var tableNames = []
+          , parentAssociation
           , len = obj.length
-        for (i = 0; i < len - 1; i++) {
+        for (var i = 0; i < len - 1; i++) {
           var item = obj[i]
           if (Utils._.isString(item) || item instanceof Utils.fn || item instanceof Utils.col || item instanceof Utils.literal || item instanceof Utils.cast || 'raw' in item) {
             break
           }
 
+          var model, as
           if (item instanceof daoFactory) {
-            item = {model: item}
-          }
-
-          // find applicable association for linking parent to this model
-          var model = item.model
-            , as
-            , associations = parent.associations
-            , association
-          if (item.hasOwnProperty('as')) {
-            as = item.as
-            association = Utils._.find(associations, function(association, associationName) {
-              return association.target === model && associationName === as
-            })
+            model = item
           } else {
-            association = Utils._.find(associations, function(association, associationName) {
-              return association.target === model ?
-                associationName === (
-                  association.doubleLinked ?
-                  association.combinedName:
-                  (
-                    association.isSingleAssociation ?
-                    Utils.singularize(model.tableName, model.options.language) :
-                    parent.tableName + model.tableName
-                  )
-                ) :
-                association.targetAssociation && association.targetAssociation.through === model
-            })
-            // NB association.target !== model clause below is to singularize names of through tables in hasMany-hasMany joins
-            as = (association && (association.isSingleAssociation || association.target !== model)) ? Utils.singularize(model.tableName, model.options.language) : model.tableName
+            model = item.model
+            as = item.as
           }
 
-          quoted[i] = as
-
-          if (!association) {
-            throw new Error('\'' + quoted.join('.') + '\' in order / group clause is not valid association')
+          // check if model provided is through table
+          var association
+          if (!as && parentAssociation && parentAssociation.through === model) {
+            association = {as: Utils.singularize(model.tableName, model.options.language)}
+          } else {
+            // find applicable association for linking parent to this model
+            association = parent.getAssociation(model, as)
           }
 
+          if (association) {
+            tableNames[i] = association.as
             parent = model
+            parentAssociation = association
+          } else {
+            tableNames[i] = model.tableName
+            throw new Error('\'' + tableNames.join('.') + '\' in order / group clause is not valid association')
+          }
         }
 
         // add 1st string as quoted, 2nd as unquoted raw
-        var sql = (i > 0 ? this.quoteIdentifier(quoted.join('.')) + '.' : '') + this.quote(obj[i], parent, force)
+        var sql = (i > 0 ? this.quoteIdentifier(tableNames.join('.')) + '.' : '') + this.quote(obj[i], parent, force)
         if (i < len - 1) {
           sql += ' ' + obj[i + 1]
         }

test/dao-factory/find.test.js

@@ -511,7 +511,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
                         'user_id',
                         'message'
                       ],
-                      include: [{ model: User, as: User.tableName, attributes: ['username'] }]
+                      include: [{ model: User, attributes: ['username'] }]
 
                     }).success(function(messages) {