From 3100bc72d1021e86faa33a72eceb187bd77172d8 Mon Sep 17 00:00:00 2001
From: charliebr0wn <cdelaorden@gmail.com>
Date: Thu, 11 Jan 2018 13:38:15 +0100
Subject: [PATCH 1/2] Add $joinEager relation eager loading with test

---
 src/index.js       | 191 +++++++++++++++++++++-------------
 test/company.js    |  14 ++-
 test/employee.js   |  28 +++++
 test/index.test.js | 254 +++++++++++++++++++++++++++++++--------------
 4 files changed, 333 insertions(+), 154 deletions(-)
 create mode 100644 test/employee.js

diff --git a/src/index.js b/src/index.js
index 9a7b280..a936716 100644
--- a/src/index.js
+++ b/src/index.js
@@ -58,7 +58,7 @@ class Service {
    * @param parentKey
    */
   objectify (query, params, parentKey) {
-    Object.keys(params || {}).forEach((key) => {
+    Object.keys(params || {}).forEach(key => {
       const value = params[key]
 
       if (isPlainObject(value)) {
@@ -89,23 +89,47 @@ class Service {
 
   createQuery (paramsQuery = {}) {
     const { filters, query } = filter(paramsQuery)
-    let q = this.Model.query().skipUndefined()
+    let q = this.Model.query()
+      .skipUndefined()
       .allowEager(this.allowedEager)
 
     // $eager for objection eager queries
     let $eager
+    let $joinEager
+
     if (query && query.$eager) {
       $eager = query.$eager
       delete query.$eager
       q.eager($eager, this.namedEagerFilters)
     }
 
+    if (query && query.$joinEager) {
+      $joinEager = query.$joinEager
+      delete query.$joinEager
+      q
+        .eagerAlgorithm(this.Model.JoinEagerAlgorithm)
+        .eager($joinEager, this.namedEagerFilters)
+    }
+
     // $select uses a specific find syntax, so it has to come first.
     if (filters.$select) {
-      q = this.Model.query().skipUndefined()
+      q = this.Model.query()
+        .skipUndefined()
         .allowEager(this.allowedEager)
-        .select(...filters.$select.concat(this.id))
-        .eager($eager, this.namedEagerFilters)
+        .select(
+          ...filters.$select.concat(
+            filters.$select.includes(this.id) ? [] : this.id
+          )
+        )
+      if ($eager) {
+        q.eager($eager, this.namedEagerFilters)
+      } else if ($joinEager) {
+        q
+          .eagerAlgorithm(this.Model.JoinEagerAlgorithm)
+          .eager($joinEager, this.namedEagerFilters)
+      }
+
+      // .joinEager($joinEager, this.namedEagerFilters)
     }
 
     // apply eager filters if specified
@@ -169,11 +193,15 @@ class Service {
     }
 
     if (count) {
-      let countQuery = this.Model.query().skipUndefined().count(`${this.id} as total`)
+      let countQuery = this.Model.query()
+        .skipUndefined()
+        .count(`${this.id} as total`)
 
       this.objectify(countQuery, query)
 
-      return countQuery.then(count => parseInt(count[0].total, 10)).then(executeQuery)
+      return countQuery
+        .then(count => parseInt(count[0].total, 10))
+        .then(executeQuery)
     }
 
     return executeQuery().catch(errorHandler)
@@ -184,9 +212,12 @@ class Service {
    * @param params
    */
   find (params) {
-    const paginate = (params && typeof params.paginate !== 'undefined') ? params.paginate : this.paginate
-    const result = this._find(params, !!paginate.default,
-      query => filter(query, paginate)
+    const paginate =
+      params && typeof params.paginate !== 'undefined'
+        ? params.paginate
+        : this.paginate
+    const result = this._find(params, !!paginate.default, query =>
+      filter(query, paginate)
     )
 
     if (!paginate.default) {
@@ -201,13 +232,14 @@ class Service {
     query[this.id] = id
 
     return this._find(Object.assign({}, params, { query }))
-    .then(page => {
-      if (page.data.length !== 1) {
-        throw new errors.NotFound(`No record found for id '${id}'`)
-      }
+      .then(page => {
+        if (page.data.length !== 1) {
+          throw new errors.NotFound(`No record found for id '${id}'`)
+        }
 
-      return page.data[0]
-    }).catch(errorHandler)
+        return page.data[0]
+      })
+      .catch(errorHandler)
   }
 
   /**
@@ -220,10 +252,14 @@ class Service {
   }
 
   _create (data, params) {
-    return this.Model.query().insert(data, this.id).then(row => {
-      const id = typeof data[this.id] !== 'undefined' ? data[this.id] : row[this.id]
-      return this._get(id, params)
-    }).catch(errorHandler)
+    return this.Model.query()
+      .insert(data, this.id)
+      .then(row => {
+        const id =
+          typeof data[this.id] !== 'undefined' ? data[this.id] : row[this.id]
+        return this._get(id, params)
+      })
+      .catch(errorHandler)
   }
 
   /**
@@ -247,32 +283,39 @@ class Service {
    */
   update (id, data, params) {
     if (Array.isArray(data)) {
-      return Promise.reject('Not replacing multiple records. Did you mean `patch`?')
+      return Promise.reject(
+        'Not replacing multiple records. Did you mean `patch`?'
+      )
     }
 
     // NOTE (EK): First fetch the old record so
     // that we can fill any existing keys that the
     // client isn't updating with null;
-    return this._get(id, params).then(oldData => {
-      let newObject = {}
-
-      for (var key of Object.keys(oldData)) {
-        if (data[key] === undefined) {
-          newObject[key] = null
-        } else {
-          newObject[key] = data[key]
+    return this._get(id, params)
+      .then(oldData => {
+        let newObject = {}
+
+        for (var key of Object.keys(oldData)) {
+          if (data[key] === undefined) {
+            newObject[key] = null
+          } else {
+            newObject[key] = data[key]
+          }
         }
-      }
 
-      // NOTE (EK): Delete id field so we don't update it
-      delete newObject[this.id]
+        // NOTE (EK): Delete id field so we don't update it
+        delete newObject[this.id]
 
-      return this.Model.query().where(this.id, id).update(newObject).then(() => {
-        // NOTE (EK): Restore the id field so we can return it to the client
-        newObject[this.id] = id
-        return newObject
+        return this.Model.query()
+          .where(this.id, id)
+          .update(newObject)
+          .then(() => {
+            // NOTE (EK): Restore the id field so we can return it to the client
+            newObject[this.id] = id
+            return newObject
+          })
       })
-    }).catch(errorHandler)
+      .catch(errorHandler)
   }
 
   /**
@@ -290,8 +333,8 @@ class Service {
     // By default we will just query for the one id. For multi patch
     // we create a list of the ids of all items that will be changed
     // to re-query them after the update
-    const ids = id === null ? this._find(params)
-      .then(mapIds) : Promise.resolve([ id ])
+    const ids =
+      id === null ? this._find(params).then(mapIds) : Promise.resolve([id])
 
     if (id !== null) {
       query[this.id] = id
@@ -303,32 +346,34 @@ class Service {
 
     delete data[this.id]
 
-    return ids.then(idList => {
-      // Create a new query that re-queries all ids that
-      // were originally changed
-      const findParams = Object.assign({}, params, {
-        query: {
-          [this.id]: { $in: idList },
-          $select: params.query && params.query.$select
-        }
-      })
+    return ids
+      .then(idList => {
+        // Create a new query that re-queries all ids that
+        // were originally changed
+        const findParams = Object.assign({}, params, {
+          query: {
+            [this.id]: { $in: idList },
+            $select: params.query && params.query.$select
+          }
+        })
 
-      return q.patch(data).then(() => {
-        return this._find(findParams).then(page => {
-          const items = page.data
+        return q.patch(data).then(() => {
+          return this._find(findParams).then(page => {
+            const items = page.data
 
-          if (id !== null) {
-            if (items.length === 1) {
-              return items[0]
-            } else {
-              throw new errors.NotFound(`No record found for id '${id}'`)
+            if (id !== null) {
+              if (items.length === 1) {
+                return items[0]
+              } else {
+                throw new errors.NotFound(`No record found for id '${id}'`)
+              }
             }
-          }
 
-          return items
+            return items
+          })
         })
       })
-    }).catch(errorHandler)
+      .catch(errorHandler)
   }
 
   /**
@@ -345,24 +390,26 @@ class Service {
       params.query[this.id] = id
     }
 
-    return this._find(params).then(page => {
-      const items = page.data
-      const query = this.Model.query()
+    return this._find(params)
+      .then(page => {
+        const items = page.data
+        const query = this.Model.query()
 
-      this.objectify(query, params.query)
+        this.objectify(query, params.query)
 
-      return query.delete().then(() => {
-        if (id !== null) {
-          if (items.length === 1) {
-            return items[0]
-          } else {
-            throw new errors.NotFound(`No record found for id '${id}'`)
+        return query.delete().then(() => {
+          if (id !== null) {
+            if (items.length === 1) {
+              return items[0]
+            } else {
+              throw new errors.NotFound(`No record found for id '${id}'`)
+            }
           }
-        }
 
-        return items
+          return items
+        })
       })
-    }).catch(errorHandler)
+      .catch(errorHandler)
   }
 }
 
diff --git a/test/company.js b/test/company.js
index 73a6dac..a87cf2d 100644
--- a/test/company.js
+++ b/test/company.js
@@ -9,9 +9,9 @@ export default class Company extends Model {
     required: ['name'],
 
     properties: {
-      id: {type: 'integer'},
-      name: {type: 'string'},
-      ceo: {type: ['integer', 'null']}
+      id: { type: 'integer' },
+      name: { type: 'string' },
+      ceo: { type: ['integer', 'null'] }
     }
   }
 
@@ -24,6 +24,14 @@ export default class Company extends Model {
         from: 'companies.ceo',
         to: 'people.id'
       }
+    },
+    employees: {
+      relation: Model.HasManyRelation,
+      modelClass: path.join(__dirname, '/employee'),
+      join: {
+        from: 'companies.id',
+        to: 'employees.companyId'
+      }
     }
   }
 }
diff --git a/test/employee.js b/test/employee.js
new file mode 100644
index 0000000..4721fbf
--- /dev/null
+++ b/test/employee.js
@@ -0,0 +1,28 @@
+import { Model } from 'objection'
+
+export default class Employee extends Model {
+  static tableName = 'employees'
+  static jsonSchema = {
+    type: 'object',
+    required: ['name'],
+
+    properties: {
+      id: { type: 'integer' },
+      companyId: { type: 'integer' },
+      name: { type: 'string' }
+    }
+  }
+
+  static get relationMappings () {
+    return {
+      company: {
+        relation: Model.BelongsToOneRelation,
+        modelClass: require('./company'),
+        join: {
+          from: 'employees.companyId',
+          to: 'companies.id'
+        }
+      }
+    }
+  }
+}
diff --git a/test/index.test.js b/test/index.test.js
index 832be41..35382ae 100644
--- a/test/index.test.js
+++ b/test/index.test.js
@@ -11,9 +11,11 @@ import People from './people'
 import PeopleCustomid from './people-customid'
 import Company from './company'
 import { Model } from 'objection'
+import Employee from './employee'
 
 const db = knex({
   client: 'sqlite3',
+  debug: false,
   connection: {
     filename: './db.sqlite'
   }
@@ -23,70 +25,99 @@ const db = knex({
 Model.knex(db)
 
 const app = feathers()
-  .use('/people', service({
-    model: People,
-    id: 'id',
-    events: ['testing']
-  }))
-  .use('/people-customid', service({
-    model: PeopleCustomid,
-    id: 'customid',
-    events: [ 'testing' ]
-  }))
-  .use('/companies', service({
-    model: Company,
-    id: 'id',
-    events: ['testing'],
-    allowedEager: 'ceos',
-    namedEagerFilters: {
-      notSnoop (builder) {
-        return builder.whereNot('name', 'Snoop')
-      }
-    },
-    eagerFilters: [
-      {
-        expression: 'ceos',
-        filter: function youngCEOs (builder) {
-          return builder.where('age', '<', '25')
+  .use(
+    '/people',
+    service({
+      model: People,
+      id: 'id',
+      events: ['testing']
+    })
+  )
+  .use(
+    '/people-customid',
+    service({
+      model: PeopleCustomid,
+      id: 'customid',
+      events: ['testing']
+    })
+  )
+  .use(
+    '/companies',
+    service({
+      model: Company,
+      id: 'id',
+      events: ['testing'],
+      allowedEager: 'ceos',
+      namedEagerFilters: {
+        notSnoop (builder) {
+          return builder.whereNot('name', 'Snoop')
+        }
+      },
+      eagerFilters: [
+        {
+          expression: 'ceos',
+          filter: function youngCEOs (builder) {
+            return builder.where('age', '<', '25')
+          }
         }
-      }
-    ]
-  }))
+      ]
+    })
+  )
+  .use(
+    '/employees',
+    service({
+      model: Employee,
+      allowedEager: 'company'
+    })
+  )
 
 let people = app.service('people')
 let companies = app.service('companies')
+let employees = app.service('employees')
 
 function clean (done) {
-  db.schema.dropTableIfExists('people').then(() => {
-    return db.schema.createTable('people', table => {
-      table.increments('id')
-      table.string('name')
-      table.integer('age')
-      table.integer('time')
-      table.boolean('created')
-    })
-  }).then(() => {
-    return db.schema.dropTableIfExists('people-customid').then(() => {
-      return db.schema.createTable('people-customid', table => {
-        table.increments('customid')
+  db.schema
+    .dropTableIfExists('people')
+    .then(() => {
+      return db.schema.createTable('people', table => {
+        table.increments('id')
         table.string('name')
         table.integer('age')
         table.integer('time')
         table.boolean('created')
       })
     })
-  }).then(() => {
-    return db.schema.dropTableIfExists('companies').then(() => {
-      db.schema.createTable('companies', table => {
-        table.increments('id')
-        table.string('name')
-        table.integer('ceo')
+    .then(() => {
+      return db.schema.dropTableIfExists('people-customid').then(() => {
+        return db.schema.createTable('people-customid', table => {
+          table.increments('customid')
+          table.string('name')
+          table.integer('age')
+          table.integer('time')
+          table.boolean('created')
+        })
       })
-        .then(() => {
-          done()
+    })
+    .then(() => {
+      return db.schema.dropTableIfExists('companies').then(() => {
+        return db.schema.createTable('companies', table => {
+          table.increments('id')
+          table.string('name')
+          table.integer('ceo')
         })
+      })
+    })
+    .then(() => {
+      return db.schema.dropTableIfExists('employees').then(() => {
+        return db.schema
+          .createTable('employees', table => {
+            table.increments('id')
+            table.integer('companyId').references('companies.id')
+            table.string('name')
+          })
+          .then(() => done())
+      })
     })
-  })
 }
 
 describe('Feathers Objection Service', () => {
@@ -96,13 +127,17 @@ describe('Feathers Objection Service', () => {
   describe('Initialization', () => {
     describe('when missing options', () => {
       it('throws an error', () => {
-        expect(service.bind(null)).to.throw('Objection options have to be provided')
+        expect(service.bind(null)).to.throw(
+          'Objection options have to be provided'
+        )
       })
     })
 
     describe('when missing a Model', () => {
       it('throws an error', () => {
-        expect(service.bind(null, {})).to.throw(/You must provide an Objection Model/)
+        expect(service.bind(null, {})).to.throw(
+          /You must provide an Objection Model/
+        )
       })
     })
 
@@ -139,8 +174,7 @@ describe('Feathers Objection Service', () => {
 
   describe('Common functionality', () => {
     it('is CommonJS compatible', () =>
-        assert.equal(typeof require('../lib'), 'function')
-    )
+      assert.equal(typeof require('../lib'), 'function'))
 
     base(app, errors, 'people')
     base(app, errors, 'people-customid', 'customid')
@@ -148,36 +182,44 @@ describe('Feathers Objection Service', () => {
 
   describe('Eager queries', () => {
     beforeEach(done => {
-      people.create({
-        name: 'Snoop',
-        age: 20
-      }).then(data => {
-        return companies.create([{
-          name: 'Google',
-          ceo: data.id
-        }, {
-          name: 'Apple',
-          ceo: data.id
-        }]).then(() => done())
-      }, done)
+      people
+        .create({
+          name: 'Snoop',
+          age: 20
+        })
+        .then(data => {
+          return companies
+            .create([
+              {
+                name: 'Google',
+                ceo: data.id
+              },
+              {
+                name: 'Apple',
+                ceo: data.id
+              }
+            ])
+            .then(() => done())
+        }, done)
     })
 
     it('allows eager queries', () => {
-      return companies.find({ query: { $eager: 'ceos' } })
-        .then(data => {
-          expect(data[0].ceos).to.be.ok
-        })
+      return companies.find({ query: { $eager: 'ceos' } }).then(data => {
+        expect(data[0].ceos).to.be.ok
+      })
     })
 
     it('allows eager queries with named filters', () => {
-      return companies.find({ query: { $eager: 'ceos(notSnoop)' } })
+      return companies
+        .find({ query: { $eager: 'ceos(notSnoop)' } })
         .then(data => {
           expect(data[0].ceos).to.be.null
         })
     })
 
     it('disallow eager queries', () => {
-      return companies.find({ query: { $eager: 'employees' } })
+      return companies
+        .find({ query: { $eager: 'employees' } })
         .then(data => {
           throw new Error('Should not reach here')
         })
@@ -190,19 +232,73 @@ describe('Feathers Objection Service', () => {
     })
   })
 
+  describe('Join Eager queries', () => {
+    before(done => {
+      companies
+        .create([
+          {
+            name: 'Google'
+          },
+          {
+            name: 'Apple'
+          }
+        ])
+        .then(data => {
+          const [google, apple] = data
+          return employees
+            .create([
+              {
+                name: 'Luke',
+                companyId: google.id
+              },
+              {
+                name: 'Yoda',
+                companyId: apple.id
+              }
+            ])
+            .then(() => done())
+        }, done)
+    })
+
+    it('allows joinEager queries', () => {
+      return employees.find({ query: { $joinEager: 'company' } }).then(data => {
+        expect(data[0].company).to.be.ok
+        expect(data[1].company).to.be.ok
+      })
+    })
+
+    it('allows filtering by relation field with joinEager quieres', () => {
+      return employees
+        .find({
+          query: {
+            $joinEager: 'company',
+            'company.name': {
+              $like: 'google'
+            }
+          }
+        })
+        .then(data => {
+          expect(data.length).to.equal(1)
+          expect(data[0].company.name).to.equal('Google')
+        })
+    })
+  })
+
   describe('$like method', () => {
     beforeEach(done => {
-      people.create({
-        name: 'Charlie Brown',
-        age: 10
-      }, done)
+      people.create(
+        {
+          name: 'Charlie Brown',
+          age: 10
+        },
+        done
+      )
     })
 
     it('$like in query', () => {
-      return people.find({ query: { name: { $like: '%lie%' } } })
-        .then(data => {
-          expect(data[0].name).to.be.equal('Charlie Brown')
-        })
+      return people.find({ query: { name: { $like: '%lie%' } } }).then(data => {
+        expect(data[0].name).to.be.equal('Charlie Brown')
+      })
     })
   })
 })

From 1a68e354fe3df80067d9de9a3740b75e4f9ce397 Mon Sep 17 00:00:00 2001
From: charliebr0wn <cdelaorden@gmail.com>
Date: Thu, 11 Jan 2018 13:52:06 +0100
Subject: [PATCH 2/2] Remove Array.includes from $select

---
 package.json | 17 +++++------------
 src/index.js |  6 +-----
 2 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/package.json b/package.json
index f7682cf..b1d1966 100644
--- a/package.json
+++ b/package.json
@@ -3,17 +3,12 @@
   "description": "A service plugin for ObjectionJS an ORM based on KnexJS",
   "version": "0.6.0",
   "homepage": "https://github.com/mcchrish/feathers-objection",
-  "keywords": [
-    "feathers",
-    "feathers-plugin",
-    "knex",
-    "objection",
-    "orm"
-  ],
+  "keywords": ["feathers", "feathers-plugin", "knex", "objection", "orm"],
   "licenses": [
     {
       "type": "MIT",
-      "url": "https://github.com/mcchrish/feathers-objection/blob/master/LICENSE"
+      "url":
+        "https://github.com/mcchrish/feathers-objection/blob/master/LICENSE"
     }
   ],
   "repository": {
@@ -40,7 +35,7 @@
     "release:major": "npm version major && npm publish",
     "compile": "rm -rf lib/ && babel -d lib/ src/",
     "watch": "babel --watch -d lib/ src/",
-    "lint": "standard src/**/*.js test/**/*.js --config",
+    "lint": "standard --fix src/**/*.js test/**/*.js --config",
     "mocha": "mocha test/ --compilers js:babel-core/register --timeout 5000",
     "test": "npm run compile && npm run lint && npm run mocha",
     "example": "babel-node example/app"
@@ -79,9 +74,7 @@
       "transform-class-properties",
       "add-module-exports"
     ],
-    "presets": [
-      "es2015"
-    ]
+    "presets": ["es2015"]
   },
   "standard": {
     "parser": "babel-eslint"
diff --git a/src/index.js b/src/index.js
index a936716..4eb62b1 100644
--- a/src/index.js
+++ b/src/index.js
@@ -116,11 +116,7 @@ class Service {
       q = this.Model.query()
         .skipUndefined()
         .allowEager(this.allowedEager)
-        .select(
-          ...filters.$select.concat(
-            filters.$select.includes(this.id) ? [] : this.id
-          )
-        )
+        .select(...filters.$select.concat(this.id))
       if ($eager) {
         q.eager($eager, this.namedEagerFilters)
       } else if ($joinEager) {