Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: issue with collection hooks #404

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
acb4596
Allow checking for Subscription initialization
KoenLav Oct 6, 2021
46ae8d2
Merge remote-tracking branch 'cult-of-coders/master' into allow-check…
StorytellerCZ Oct 7, 2022
63aa86d
Merge branch 'master' into allow-checking-subscription-initialization
StorytellerCZ Oct 7, 2022
3a9685a
Merge branch 'master' into allow-checking-subscription-initialization
StorytellerCZ May 7, 2023
23d7c6d
Merge branch 'master' into allow-checking-subscription-initialization
StorytellerCZ May 30, 2023
a0ffa1e
Sync versionsFrom and tests
StorytellerCZ Aug 24, 2023
822d84d
Added docs and tests
KoenLav Feb 26, 2024
71bd6ce
Removed extra line
KoenLav Feb 26, 2024
19a03fa
Fixed tests
KoenLav Feb 26, 2024
9f3845d
Fixed test?
KoenLav Feb 26, 2024
a1c41ac
Update test dependencies
StorytellerCZ Feb 28, 2024
2207871
Reverse test deps upgrade to keep Meteor 1 compat
StorytellerCZ Feb 28, 2024
e5df7b2
Merge branch 'master' into allow-checking-subscription-initialization
jdgjsag67251 Mar 26, 2024
7403b15
Added missing wrap
jdgjsag67251 Mar 26, 2024
12a5853
Merge pull request #1 from jdgjsag67251/allow-checking-subscription-i…
jdgjsag67251 Mar 26, 2024
38340bc
Update test dependencies & revert redis action version
StorytellerCZ Mar 30, 2024
08e2cb8
More reverts
StorytellerCZ Mar 30, 2024
24b43a6
Use simpl-schema compatible with Meteor in testing
StorytellerCZ Mar 30, 2024
0a534cc
Merge branch 'Meteor-Community-Packages:master' into master
jdgjsag67251 Apr 2, 2024
471c239
Fixed issue with requery
jdgjsag67251 Apr 9, 2024
d4b220e
Don't swallow errors
jdgjsag67251 Apr 9, 2024
051591f
Removed 'SubscriptionInitialization'
jdgjsag67251 Apr 10, 2024
24abd09
Fix override
jdgjsag67251 Apr 10, 2024
af94119
Fixed test
jdgjsag67251 Aug 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
meteor: [1.12.2, 2.6.1, 2.7.3, 2.8.1, 2.12]
meteor: [1.12.2, 2.8.1, 2.12, 2.15]
redis-version: [4, 5, 6, 7]

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Start Redis
uses: supercharge/[email protected]
Expand All @@ -26,7 +26,7 @@ jobs:
run: |
meteor create --release ${{ matrix.meteor }} --bare test
cd test
meteor npm i --save [email protected] simpl-schema chai
meteor npm i --save [email protected] simpl-schema@1.13.1 chai
- name: Test
working-directory: ./test
run: METEOR_PACKAGE_DIRS="../" TEST_BROWSER_DRIVER=puppeteer meteor test-packages --raw-logs --once --driver-package meteortesting:mocha ../
2 changes: 1 addition & 1 deletion docs/finetuning.md
Original file line number Diff line number Diff line change
Expand Up @@ -278,4 +278,4 @@ SyntheticMutator.update(`company::{companyId}::threads`, threadId, {});
SyntheticMutator.update([`company::{companyId}::threads`, `threads::${threadId}`], threadId, {});

// If you pass-in the collection instance as argument, this will be automatically done.
```
```
6 changes: 3 additions & 3 deletions lib/cache/ObservableCollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default class ObservableCollection {
);
}

this.cursor = this.collection.find(
this.cursor = this.collection._redisOplogCollectionMethods.find(
cursorDescription.selector,
cursorDescription.options
);
Expand Down Expand Up @@ -155,7 +155,7 @@ export default class ObservableCollection {
*/
isEligibleByDB(_id) {
if (this.matcher) {
return !!this.collection.findOne(
return !!this.collection._redisOplogCollectionMethods.findOne(
Object.assign({}, this.selector, { _id }),
{ fields: { _id: 1 } }
);
Expand Down Expand Up @@ -214,7 +214,7 @@ export default class ObservableCollection {
*/
addById(docId) {
const { limit, skip, ...cleanedOptions } = this.options;
const doc = this.collection.findOne({ _id: docId }, cleanedOptions);
const doc = this.collection._redisOplogCollectionMethods.findOne({ _id: docId }, cleanedOptions);

this.store.set(docId, doc);

Expand Down
13 changes: 11 additions & 2 deletions lib/mongo/mongoCollectionNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,22 @@ const constructor = Mongo.Collection;
const proto = Mongo.Collection.prototype;

const hook = function() {
let ret = constructor.apply(this, arguments);
const ret = constructor.apply(this, arguments);
map[arguments[0]] = this;
return ret;
};

hook.__getCollectionByName = function (name) {
return map[name];
if (!(name in map)) {
return null;
}

const collection = map[name];

// Use 'direct', if available, to skip all collection hooks
collection._redisOplogCollectionMethods = collection.direct ?? collection;

return collection;
};

hook.prototype = proto;
Expand Down
25 changes: 10 additions & 15 deletions lib/processors/actions/requery.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,35 @@ import { MongoIDMap } from '../../cache/mongoIdMap';

/**
* @param observableCollection
* @param newCommer
* @param newcomer
* @param event
* @param modifiedFields
*/
export default function (observableCollection, newCommer, event, modifiedFields) {
export default function (observableCollection, newcomer, event, modifiedFields) {
const { store, selector, options } = observableCollection;

const newStore = new MongoIDMap();
const freshIds = observableCollection.collection.find(
selector, { ...options, fields: { _id: 1 } }).fetch();
freshIds.forEach(doc => newStore.set(doc._id, doc));
const freshIds = observableCollection.collection._redisOplogCollectionMethods.find(selector, { ...options, fields: { _id: 1 } }).fetch();
freshIds.forEach((doc) => newStore.set(doc._id, doc));

let added = false;
store.compareWith(newStore, {
leftOnly(docId) {
observableCollection.remove(docId);
},
rightOnly(docId) {
if (newCommer && EJSON.equals(docId, newCommer._id)) {
if (newcomer && EJSON.equals(docId, newcomer._id)) {
added = true;
observableCollection.add(newCommer);
observableCollection.add(newcomer);
} else {
observableCollection.addById(docId);
}
}
},
});

// if we have an update, and we have a newcommer, that new commer may be inside the ids
// if we have an update, and we have a newcomer, that newcomer may be inside the ids
// TODO: maybe refactor this in a separate action (?)
if (newCommer
&& Events.UPDATE === event
&& modifiedFields
&& !added
&& store.has(newCommer._id)) {
observableCollection.change(newCommer, modifiedFields);
if (newcomer && Events.UPDATE === event && modifiedFields && !added && store.has(newcomer._id)) {
observableCollection.change(newcomer, modifiedFields);
}
}
10 changes: 8 additions & 2 deletions lib/redis/RedisSubscriptionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ class RedisSubscriptionManager {
debug(
`[RedisSubscriptionManager] Exception while processing event: ${e.toString()}`
);
Meteor._debug(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously these were just swallowed when debug was not turned on

`[RedisSubscriptionManager] Exception while processing event: ${e.toString()}`, e
);
}
});
} else {
Expand All @@ -179,6 +182,9 @@ class RedisSubscriptionManager {
debug(
`[RedisSubscriptionManager] Exception while processing synthetic event: ${e.toString()}`
);
Meteor._debug(
`[RedisSubscriptionManager] Exception while processing synthetic event: ${e.toString()}`, e
);
}
});
}
Expand All @@ -203,9 +209,9 @@ class RedisSubscriptionManager {
const fieldsOfInterest = getFieldsOfInterestFromAll(subscribers);

if (fieldsOfInterest === true) {
doc = collection.findOne(doc._id);
doc = collection._redisOplogCollectionMethods.findOne(doc._id);
} else {
doc = collection.findOne(doc._id, { fields: fieldsOfInterest });
doc = collection._redisOplogCollectionMethods.findOne(doc._id, { fields: fieldsOfInterest });
}

return doc;
Expand Down
4 changes: 2 additions & 2 deletions package.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package.describe({
name: 'cultofcoders:redis-oplog',
version: '2.2.1',
version: '2.2.2',
// Brief, one-line summary of the package.
summary: "Replacement for Meteor's MongoDB oplog implementation",
// URL to the Git repository containing the source code for this package.
Expand All @@ -17,7 +17,7 @@ Npm.depends({
});

Package.onUse(function(api) {
api.versionsFrom(['1.12.2', '2.8.1', '2.12']);
api.versionsFrom(['1.12.2', '2.8.1', '2.13', '2.15']);
api.use([
'underscore',
'ecmascript',
Expand Down
22 changes: 21 additions & 1 deletion testing/collection_hooks.server.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { assert } from 'chai';
import { _ } from 'meteor/underscore';
import { Random } from 'meteor/random';

describe('It should work with collection:hooks', function () {

Expand Down Expand Up @@ -53,5 +52,26 @@ describe('It should work with collection:hooks', function () {
assert.isTrue(value, key);
})
})

it('Should use direct methods when available: ' + JSON.stringify(options), function () {
let withDirect = false;

const id = Collection.insert({ someData: true });

Collection.before.find(function () {
withDirect = true;
});

const handle = Collection.find({
someData: true,
}).observeChanges({
added: function() {}
});

assert.isTrue(withDirect, 'Used direct');

handle.stop();
Collection.remove(id);
})
})
});
Loading