Skip to content

Commit

Permalink
Merge pull request #258 from Meteor-Community-Packages/eslint
Browse files Browse the repository at this point in the history
Bugs in upsert hooks (and fixed code formatting)
  • Loading branch information
StorytellerCZ authored Nov 22, 2019
2 parents 69a95ab + 22de33c commit fced7c9
Show file tree
Hide file tree
Showing 15 changed files with 130 additions and 136 deletions.
1 change: 0 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"standard"
],
"parserOptions": {
"ecmaVersion": 6,
"sourceType": "module",
"ecmaFeatures": {
"modules": true
Expand Down
82 changes: 22 additions & 60 deletions collection-hooks.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { Meteor } from 'meteor/meteor';
import { Mongo } from 'meteor/mongo';
import { EJSON } from 'meteor/ejson';
import { LocalCollection } from 'meteor/minimongo';

/* global Package Meteor Mongo LocalCollection CollectionHooks _ EJSON */
/* eslint-disable no-proto, no-native-reassign, no-global-assign */
import { Meteor } from 'meteor/meteor'
import { Mongo } from 'meteor/mongo'
import { EJSON } from 'meteor/ejson'
import { LocalCollection } from 'meteor/minimongo'

// Relevant AOP terminology:
// Aspect: User code that runs before/after (hook)
Expand All @@ -19,44 +16,15 @@ export const CollectionHooks = {
all: { insert: {}, update: {}, remove: {}, find: {}, findOne: {}, all: {} }
},
directEnv: new Meteor.EnvironmentVariable(),
directOp(func) {
directOp (func) {
return this.directEnv.withValue(true, func)
},
hookedOp(func) {
hookedOp (func) {
return this.directEnv.withValue(false, func)
}
}

CollectionHooks.getUserId = function getUserId() {
let userId

if (Meteor.isClient) {
Tracker.nonreactive(function () {
userId = Meteor.userId && Meteor.userId()
})
}

if (Meteor.isServer) {
try {
// Will throw an error unless within method call.
// Attempt to recover gracefully by catching:
userId = Meteor.userId && Meteor.userId()
} catch (e) { }

if (userId == null) {
// Get the userId if we are in a publish function.
userId = publishUserId.get()
}
}

if (userId == null) {
userId = CollectionHooks.defaultUserId
}

return userId
}

CollectionHooks.extendCollectionInstance = function extendCollectionInstance(self, constructor) {
CollectionHooks.extendCollectionInstance = function extendCollectionInstance (self, constructor) {
// Offer a public API to allow the user to define aspects
// Example: collection.before.insert(func);
['before', 'after'].forEach(function (pointcut) {
Expand All @@ -74,13 +42,13 @@ CollectionHooks.extendCollectionInstance = function extendCollectionInstance(sel
})

return {
replace(aspect, options) {
replace (aspect, options) {
self._hookAspects[method][pointcut].splice(len - 1, 1, {
aspect: aspect,
aspect,
options: CollectionHooks.initOptions(options, pointcut, method)
})
},
remove() {
remove () {
self._hookAspects[method][pointcut].splice(len - 1, 1)
}
}
Expand Down Expand Up @@ -134,8 +102,8 @@ CollectionHooks.extendCollectionInstance = function extendCollectionInstance(sel
function (doc) {
return (
typeof self._transform === 'function'
? function (d) { return self._transform(d || doc) }
: function (d) { return d || doc }
? function (d) { return self._transform(d || doc) }
: function (d) { return d || doc }
)
},
args,
Expand All @@ -149,16 +117,16 @@ CollectionHooks.defineAdvice = (method, advice) => {
advices[method] = advice
}

CollectionHooks.getAdvice = method => advices[method];
CollectionHooks.getAdvice = method => advices[method]

CollectionHooks.initOptions = (options, pointcut, method) =>
CollectionHooks.extendOptions(CollectionHooks.defaults, options, pointcut, method);
CollectionHooks.extendOptions(CollectionHooks.defaults, options, pointcut, method)

CollectionHooks.extendOptions = (source, options, pointcut, method) =>
({...options, ...source.all.all, ...source[pointcut].all, ...source.all[method], ...source[pointcut][method]});
({ ...options, ...source.all.all, ...source[pointcut].all, ...source.all[method], ...source[pointcut][method] })

CollectionHooks.getDocs = function getDocs (collection, selector, options) {
const findOptions = {transform: null, reactive: false} // added reactive: false
const findOptions = { transform: null, reactive: false } // added reactive: false

/*
// No "fetch" support at this time.
Expand All @@ -181,7 +149,7 @@ CollectionHooks.getDocs = function getDocs (collection, selector, options) {
findOptions.limit = 1
}
const { multi, upsert, ...rest } = options
Object.assign(findOptions, rest);
Object.assign(findOptions, rest)
}

// Unlike validators, we iterate over multiple docs, so use
Expand All @@ -204,7 +172,7 @@ CollectionHooks.normalizeSelector = function (selector) {
// ~/.meteor/packages/mongo-livedata/collection.js
// It's contained in these utility functions to make updates easier for us in
// case this code changes.
CollectionHooks.getFields = function getFields(mutator) {
CollectionHooks.getFields = function getFields (mutator) {
// compute modified fields
const fields = []
// ====ADDED START=======================
Expand Down Expand Up @@ -259,12 +227,12 @@ CollectionHooks.reassignPrototype = function reassignPrototype (instance, constr
// Note: Assigning a prototype dynamically has performance implications
if (hasSetPrototypeOf) {
Object.setPrototypeOf(instance, constr.prototype)
} else if (instance.__proto__) {
instance.__proto__ = constr.prototype
} else if (instance.__proto__) { // eslint-disable-line no-proto
instance.__proto__ = constr.prototype // eslint-disable-line no-proto
}
}

CollectionHooks.wrapCollection = function wrapCollection(ns, as) {
CollectionHooks.wrapCollection = function wrapCollection (ns, as) {
if (!as._CollectionConstructor) as._CollectionConstructor = as.Collection
if (!as._CollectionPrototype) as._CollectionPrototype = new as.Collection(null)

Expand All @@ -282,7 +250,7 @@ CollectionHooks.wrapCollection = function wrapCollection(ns, as) {
ns.Collection.prototype = proto
ns.Collection.prototype.constructor = ns.Collection

for (let prop of Object.keys(constructor)) {
for (const prop of Object.keys(constructor)) {
ns.Collection[prop] = constructor[prop]
}

Expand All @@ -299,9 +267,3 @@ if (typeof Mongo !== 'undefined') {
} else {
CollectionHooks.wrapCollection(Meteor, Meteor)
}

// Make the above available for packages with hooks that want to determine
// whether they are running inside a publish function or not.
CollectionHooks.isWithinPublish = function isWithinPublish() {
return publishUserId.get() !== undefined
}
3 changes: 0 additions & 3 deletions findone.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { CollectionHooks } from './collection-hooks'

CollectionHooks.defineAdvice('findOne', function (userId, _super, instance, aspects, getTransform, args, suppressAspects) {
// args[0] : selector
// args[1] : options

const ctx = { context: this, _super, args }
const selector = CollectionHooks.normalizeSelector(instance._getFindSelector(args))
const options = instance._getFindOptions(args)
Expand Down
13 changes: 7 additions & 6 deletions insert.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { EJSON } from 'meteor/ejson';
import { CollectionHooks } from './collection-hooks';
import { EJSON } from 'meteor/ejson'
import { Mongo } from 'meteor/mongo'
import { CollectionHooks } from './collection-hooks'

CollectionHooks.defineAdvice('insert', function (userId, _super, instance, aspects, getTransform, args, suppressAspects) {
const ctx = {context: this, _super, args}
let [doc, callback] = args;
const ctx = { context: this, _super, args }
let [doc, callback] = args
const async = typeof callback === 'function'
let abort
let ret
Expand All @@ -12,7 +13,7 @@ CollectionHooks.defineAdvice('insert', function (userId, _super, instance, aspec
if (!suppressAspects) {
try {
aspects.before.forEach((o) => {
const r = o.aspect.call({transform: getTransform(doc), ...ctx}, userId, doc)
const r = o.aspect.call({ transform: getTransform(doc), ...ctx }, userId, doc)
if (r === false) abort = true
})

Expand All @@ -39,7 +40,7 @@ CollectionHooks.defineAdvice('insert', function (userId, _super, instance, aspec
doc._id = id
}
if (!suppressAspects) {
const lctx = {transform: getTransform(doc), _id: id, err, ...ctx}
const lctx = { transform: getTransform(doc), _id: id, err, ...ctx }
aspects.after.forEach((o) => {
o.aspect.call(lctx, userId, doc)
})
Expand Down
16 changes: 8 additions & 8 deletions remove.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { EJSON } from 'meteor/ejson';
import { CollectionHooks } from './collection-hooks';
import { EJSON } from 'meteor/ejson'
import { CollectionHooks } from './collection-hooks'

const isEmpty = a => !Array.isArray(a) || !a.length;
const isEmpty = a => !Array.isArray(a) || !a.length

CollectionHooks.defineAdvice('remove', function (userId, _super, instance, aspects, getTransform, args, suppressAspects) {
const ctx = {context: this, _super, args};
const [ selector, callback ] = args;
const ctx = { context: this, _super, args }
const [selector, callback] = args
const async = typeof callback === 'function'
let docs
let abort
let prev = []
const prev = []

if (!suppressAspects) {
try {
Expand All @@ -25,7 +25,7 @@ CollectionHooks.defineAdvice('remove', function (userId, _super, instance, aspec
// before
aspects.before.forEach((o) => {
docs.forEach((doc) => {
const r = o.aspect.call({transform: getTransform(doc), ...ctx}, userId, doc)
const r = o.aspect.call({ transform: getTransform(doc), ...ctx }, userId, doc)
if (r === false) abort = true
})
})
Expand All @@ -41,7 +41,7 @@ CollectionHooks.defineAdvice('remove', function (userId, _super, instance, aspec
if (!suppressAspects) {
aspects.after.forEach((o) => {
prev.forEach((doc) => {
o.aspect.call({transform: getTransform(doc), err, ...ctx}, userId, doc)
o.aspect.call({ transform: getTransform(doc), err, ...ctx }, userId, doc)
})
})
}
Expand Down
2 changes: 1 addition & 1 deletion server.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Meteor.publish = function (name, handler, options) {

// Make the above available for packages with hooks that want to determine
// whether they are running inside a publish function or not.
CollectionHooks.isWithinPublish = () => publishUserId.get() !== void 0
CollectionHooks.isWithinPublish = () => publishUserId.get() !== undefined

export {
CollectionHooks
Expand Down
4 changes: 2 additions & 2 deletions tests/find_findone_userid.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ if (Meteor.isServer) {
}

if (Meteor.isClient) {
function cleanup () {
const cleanup = () => {
beforeFindUserId = null
afterFindUserId = null
beforeFindOneUserId = null
afterFindOneUserId = null
}

function withLogin (testFunc) {
const withLogin = (testFunc) => {
return function (...args) {
const wrapper = (cb) => {
InsecureLogin.ready(() => {
Expand Down
18 changes: 9 additions & 9 deletions tests/find_users.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Meteor } from 'meteor/meteor';
import { Tinytest } from 'meteor/tinytest';
import { InsecureLogin } from './insecure_login';
import { Meteor } from 'meteor/meteor'
import { Tinytest } from 'meteor/tinytest'
import { InsecureLogin } from './insecure_login'

Tinytest.addAsync('users - find hooks should be capable of being used on special Meteor.users collection', function (test, next) {
const aspect1 = Meteor.users.before.find(function (userId, selector, options) {
Expand All @@ -16,10 +16,10 @@ Tinytest.addAsync('users - find hooks should be capable of being used on special
})

InsecureLogin.ready(function () {
const selector = {test: 1}
const selector = { test: 1 }
Meteor.users.find(selector)
test.equal(selector.hasOwnProperty('a'), true)
test.equal(selector.hasOwnProperty('b'), true)
test.equal(Object.prototype.hasOwnProperty.call(selector, 'a'), true)
test.equal(Object.prototype.hasOwnProperty.call(selector, 'b'), true)
aspect1.remove()
aspect2.remove()

Expand All @@ -34,7 +34,7 @@ Tinytest.addAsync('users - find hooks should be capable of being used on wrapped
return Object.assign(this, doc)
}

Meteor.users.__transform = doc => new TestUser(doc);
Meteor.users.__transform = doc => new TestUser(doc)

const MeteorUsersFind = Meteor.users.find

Expand All @@ -57,8 +57,8 @@ Tinytest.addAsync('users - find hooks should be capable of being used on wrapped
InsecureLogin.ready(function () {
const selector = { test: 1 }
Meteor.users.find(selector)
test.equal(selector.hasOwnProperty('a'), true)
test.equal(selector.hasOwnProperty('b'), true)
test.equal(Object.prototype.hasOwnProperty.call(selector, 'a'), true)
test.equal(Object.prototype.hasOwnProperty.call(selector, 'b'), true)
aspect1.remove()
aspect2.remove()

Expand Down
7 changes: 4 additions & 3 deletions tests/server/fetch.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Mongo } from 'meteor/mongo';
import { Tinytest } from 'meteor/tinytest';
import { Mongo } from 'meteor/mongo'
import { Tinytest } from 'meteor/tinytest'
import { InsecureLogin } from '../insecure_login'

Tinytest.addAsync('general - local collection documents should only have fetched fields', function (test, next) {
const collection = new Mongo.Collection(null)
Expand All @@ -12,7 +13,7 @@ Tinytest.addAsync('general - local collection documents should only have fetched
const fields = ['fetch_value1', 'fetch_value2']

collection.after.update(function (userId, doc, fieldNames, modifier) {
let { _id, ...docKeys } = Object.keys(doc);
const { _id, ...docKeys } = Object.keys(doc)
test.equal(same(docKeys, fields), true)
next()
}, {
Expand Down
20 changes: 10 additions & 10 deletions tests/transform.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import { Mongo } from 'meteor/mongo';
import { Tinytest } from 'meteor/tinytest';
import { InsecureLogin } from './insecure_login';
import { Mongo } from 'meteor/mongo'
import { Tinytest } from 'meteor/tinytest'
import { InsecureLogin } from './insecure_login'

const isFunction = (fn) => typeof fn === 'function';
const isFunction = (fn) => typeof fn === 'function'

Tinytest.addAsync('general - hook callbacks should have this.transform function that works', function (test, next) {
const collection = new Mongo.Collection(null, {
transform: doc => ({ ...doc, isTransformed: true })
})

collection.allow({
insert() { return true },
update() { return true },
remove() { return true }
insert () { return true },
update () { return true },
remove () { return true }
})

const counts = {
Expand Down Expand Up @@ -41,11 +41,11 @@ Tinytest.addAsync('general - hook callbacks should have this.transform function
// to pass? Probably not. Think more on this -- it could be that we simply
// shouldn't be running a .transform() in a before.insert -- how will we
// know the _id? And that's what transform is complaining about.
collection.insert({_id: '1', start_value: true}, function (err, id) {
collection.insert({ _id: '1', start_value: true }, function (err, id) {
if (err) throw err
collection.update({_id: id}, {$set: {update_value: true}}, function (err) {
collection.update({ _id: id }, { $set: { update_value: true } }, function (err) {
if (err) throw err
collection.remove({_id: id}, function (nil) {
collection.remove({ _id: id }, function (nil) {
test.equal(counts.before.insert, 1, 'before insert should have 1 count')
test.equal(counts.before.update, 1, 'before update should have 1 count')
test.equal(counts.before.remove, 1, 'before remove should have 1 count')
Expand Down
2 changes: 1 addition & 1 deletion tests/update_both.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ if (Meteor.isClient) {

collection2.after.update(function (userId, doc, fieldNames, modifier) {
test.equal(doc.update_value, true)
test.equal(this.previous.hasOwnProperty('update_value'), false)
test.equal(Object.prototype.hasOwnProperty.call(this.previous, 'update_value'), false)
n()
})

Expand Down
Loading

0 comments on commit fced7c9

Please sign in to comment.