Skip to content

Commit

Permalink
CHORE fix lokijs bugs (#3496)
Browse files Browse the repository at this point in the history
* CHORE fix lokijs bugs

* FIX types

* FIX some tests

* FIX more lokijs tests

* FIX couchdb replication

* FIX some tests

* FIX browser tests

* FIX failing pouchdb test

* FIX randomly failing test

* REMOVE logs

* ADD logs for failing test

* ADD logs

* IMPROVE logs on failing tests

* FIX timing dependend test

* FIX ci

* ADD time out

* FIX sort order test

* FIX couchdb test

* FIX test config

* UPDATE docs

* FIX ci
  • Loading branch information
pubkey committed Nov 11, 2021
1 parent 579f2d4 commit 3390ba8
Show file tree
Hide file tree
Showing 44 changed files with 1,995 additions and 1,695 deletions.
10 changes: 7 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ jobs:
- name: build
run: npm run build

- name: npm run test:node
run: npm run test:node

- name: npm run test:node:pouchdb
run: npm run test:node:pouchdb

- name: npm run test:node:lokijs
run: npm run test:node:lokijs


- name: npm run test:fast
run: npm run test:fast

Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
Bugfixes:
- LokiJS: Ensure events emit exact the same data as with PouchDB.
- LokiJS: Queries with limit and skip where broken.
- LokiJS: Fix all bugs and run the whole test suite with LokiJS Storage

Other:
- Updated [event-reduce](https://github.com/pubkey/event-reduce) for more optimizations.

### 10.3.5 (8 November 2021)

Expand Down
2 changes: 0 additions & 2 deletions docs-src/backup.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# Backup

**(beta)**

With the backup plugin you can write the current database state and ongoing changes into folders on the filesystem.
The files are written in plain json together with their attachments.

Expand Down
2 changes: 1 addition & 1 deletion docs-src/replication-graphql.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ changeObservable.subscribe({

```

#### Helper Functions (beta)
#### Helper Functions

RxDB provides the helper functions `graphQLSchemaFromRxSchema()`, `pullQueryBuilderFromRxSchema()` and `pushQueryBuilderFromRxSchema()` that can be used to generate the GraphQL Schema from the `RxJsonSchema`. To learn how to use them, please inspect the [GraphQL Example](https://github.com/pubkey/rxdb/tree/master/examples/graphql).

Expand Down
2 changes: 1 addition & 1 deletion docs-src/replication.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Replication primitives (beta)
# Replication primitives

With the replication primitives plugin, you can build a realtime replication based on any transport layer like **REST**, **WebRTC** or **websockets**.

Expand Down
2 changes: 0 additions & 2 deletions docs-src/rx-collection.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ myCollection.findOne('foo')

### findByIds()

Notice: This method is in beta and might be changed without notice.

Find many documents by their id (primary value). This has a way better performance than running multiple `findOne()` or a `find()` with a big `$or` selector.

Returns a `Map` where the primary key of the document is mapped to the document. Documents that do not exist or are deleted, will not be inside of the returned Map.
Expand Down
2 changes: 1 addition & 1 deletion docs-src/rx-storage-lokijs.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# RxStorage LokiJS (beta)
# RxStorage LokiJS

Instead of using PouchDB as underlying storage engine, you can also use [LokiJS](https://github.com/techfort/LokiJS).
LokiJS has the main benefit of having a better performance. It can do this because it is an **in-memory** database that processes all data in memory and only saves to disc when the app is closed or an interval is reached.
Expand Down
5 changes: 5 additions & 0 deletions orga/before-next-major.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ Rename the paths in the `exports` field in the `package.json` so that users can

Atm we have duplicate code. Most of the graphql replication code can be switched out with the general replication plugin. Also we then could support bulk-push methods and replicate multiple changes from the local to the remote in the push replication.


## Ensure deterministic sorting.
The PouchDB RxStorage does not automatically add the primary key to a queries sort options.
But this must be done to ensure deterministic sorting and to ensure the event-reduce algorithm works exactly the same on each storage. Adding the sort field creates errors because we cannot sort over non-indexes stuff. So maybe we should fix this at the index creation.

# Maybe

## Use Proxy instead of getters/setter on RxDocument
Expand Down
16 changes: 10 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,26 @@
"pretest": "npm run transpile",
"test": "npm run test:node && npm run test:browser",
"// test:fast": "run tests in the fast-mode. Most of them will run in parrallel, skips tests that are known slow",
"test:fast": "npm run pretest && rimraf -rf pouch__all_dbs__ && cross-env NODE_ENV=fast mocha --config ./config/.mocharc.js ./test_tmp/unit.test.js",
"test:fast": "npm run test:fast:pouchdb && npm run test:fast:lokijs",
"test:fast:pouchdb": "npm run pretest && rimraf -rf pouch__all_dbs__ && cross-env DEFAULT_STORAGE=pouchdb NODE_ENV=fast mocha --config ./config/.mocharc.js ./test_tmp/unit.test.js",
"test:fast:lokijs": "npm run pretest && rimraf -rf pouch__all_dbs__ && cross-env DEFAULT_STORAGE=lokijs NODE_ENV=fast mocha --config ./config/.mocharc.js ./test_tmp/unit.test.js",
"// test:fast:loop": "runs tests in the fast-mode in a loop. Use this to debug tests that only fail sometimes",
"test:fast:loop": "npm run test:fast && npm run test:fast:loop",
"test:node": "npm run pretest && mocha --expose-gc --config ./config/.mocharc.js ./test_tmp/unit.test.js",
"test:node": "npm run test:node:pouchdb && npm run test:node:lokijs",
"test:node:pouchdb": "npm run pretest && cross-env DEFAULT_STORAGE=pouchdb mocha --expose-gc --config ./config/.mocharc.js ./test_tmp/unit.test.js",
"test:node:lokijs": "npm run pretest && cross-env DEFAULT_STORAGE=lokijs mocha --expose-gc --config ./config/.mocharc.js ./test_tmp/unit.test.js",
"// test:node:loop": "runs tests in node in a loop. Use this to debug tests that only fail sometimes",
"test:node:loop": "npm run test:node && npm run test:node:loop",
"test:browser": "npm run pretest && karma start ./config/karma.conf.js --single-run",
"test:browser": "npm run pretest && cross-env DEFAULT_STORAGE=pouchdb karma start ./config/karma.conf.js --single-run",
"test:core": "npm run pretest && mocha ./test_tmp/unit/core.node.js",
"test:typings": "npm run pretest && cross-env NODE_ENV=fast mocha --config ./config/.mocharc.js ./test_tmp/typings.test.js",
"test:typings": "npm run pretest && cross-env DEFAULT_STORAGE=pouchdb NODE_ENV=fast mocha --config ./config/.mocharc.js ./test_tmp/typings.test.js",
"test:typings:ci": "npm run pretest && mocha --config ./config/.mocharc.js ./test_tmp/typings.test.js",
"test:deps": "npm run build && dependency-check ./package.json ./dist/lib/plugins/replication-graphql/index.js ./dist/lib/plugins/server.js ./dist/lib/plugins/validate-z-schema.js ./dist/lib/plugins/lokijs/index.js --no-dev --ignore-module util --ignore-module url --ignore-module \"@types/*\"",
"test:circular": "npm run build && madge --circular ./dist/es/index.js",
"test:performance": "npm run pretest && cross-env NODE_ENV=fast mocha --config ./config/.mocharc.js ./test_tmp/performance.test.js --unhandled-rejections=strict --expose-gc",
"couch:start": "docker run -d -p 5984:5984 --rm --name rxdb-couchdb couchdb:2.1.1",
"couch:stop": "docker rm -f rxdb-couchdb",
"test:couchdb": "npm run pretest && mocha --config ./config/.mocharc.js ./test_tmp/couch-db-integration.test.js",
"test:couchdb": "npm run pretest && cross-env DEFAULT_STORAGE=pouchdb mocha --config ./config/.mocharc.js ./test_tmp/couch-db-integration.test.js",
"dockertest": "docker run -it -v $(pwd):/usr/src/app markadams/chromium-xvfb-js:latest-onbuild",
"profile": "npm run pretest && cross-env NODE_ENV=fast NODE_PROF=true mocha --config ./config/.mocharc.js ./test_tmp/unit.test.js --prof && node scripts/profile.js",
"clear": "rimraf -rf test_tmp/ && rimraf -rf dist/ && rimraf .transpile_state.json",
Expand Down Expand Up @@ -102,7 +106,7 @@
"custom-idle-queue": "3.0.1",
"deep-equal": "2.0.5",
"deep-freeze": "0.0.1",
"event-reduce-js": "1.4.1",
"event-reduce-js": "2.0.0",
"express": "4.17.1",
"get-graphql-from-jsonschema": "8.0.16",
"graphql-client": "2.0.1",
Expand Down
12 changes: 6 additions & 6 deletions src/event-reduce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import {
runAction,
QueryParams,
QueryMatcher,
SortComparator
DeterministicSortComparator
} from 'event-reduce-js';
import type { RxQuery, MangoQuery, RxChangeEvent } from './types';
import type { RxQuery, MangoQuery, RxChangeEvent, RxDocumentWriteData } from './types';
import { runPluginHooks } from './hooks';
import { rxChangeEventToEventReduceChangeEvent } from './rx-change-event';

Expand Down Expand Up @@ -40,7 +40,7 @@ export function getQueryParams<RxDocType>(
): QueryParams<RxDocType> {
if (!RXQUERY_QUERY_PARAMS_CACHE.has(rxQuery)) {
const collection = rxQuery.collection;
const queryJson: MangoQuery<RxDocType> = rxQuery.toJSON();
const queryJson: MangoQuery<RxDocType> = rxQuery.getPreparedQuery();
const primaryKey = collection.schema.primaryPath;


Expand All @@ -50,7 +50,7 @@ export function getQueryParams<RxDocType>(
* we send for example compressed documents to be sorted by compressed queries.
*/
const sortComparator = collection.storageInstance.getSortComparator(queryJson);
const useSortComparator: SortComparator<RxDocType> = (docA: RxDocType, docB: RxDocType) => {
const useSortComparator: DeterministicSortComparator<RxDocType> = (docA: RxDocType, docB: RxDocType) => {
const sortComparatorData = {
docA,
docB,
Expand All @@ -67,8 +67,7 @@ export function getQueryParams<RxDocType>(
* we send for example compressed documents to match compressed queries.
*/
const queryMatcher = collection.storageInstance.getQueryMatcher(queryJson);
const useQueryMatcher: QueryMatcher<RxDocType> = (doc: RxDocType) => {

const useQueryMatcher: QueryMatcher<RxDocumentWriteData<RxDocType>> = (doc: RxDocumentWriteData<RxDocType>) => {
const queryMatcherData = {
doc,
rxQuery
Expand Down Expand Up @@ -105,6 +104,7 @@ export function calculateNewResults<RxDocumentType>(
}
const queryParams = getQueryParams(rxQuery);
const previousResults: RxDocumentType[] = rxQuery._resultsData.slice();

const previousResultsMap: Map<string, RxDocumentType> = rxQuery._resultsDataMap;
let changed: boolean = false;

Expand Down
7 changes: 4 additions & 3 deletions src/plugins/backup/file-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
BackupOptions,
RxDatabase
} from '../../types';
import { now } from '../../util';

/**
* ensure that the given folder exists
Expand Down Expand Up @@ -39,10 +40,10 @@ export function prepareFolders(
const metaLoc = metaFileLocation(options);

if (!fs.existsSync(metaLoc)) {
const now = new Date().getTime();
const currentTime = now();
const metaData: BackupMetaFileContent = {
createdAt: now,
updatedAt: now,
createdAt: currentTime,
updatedAt: currentTime,
collectionStates: {}
};
fs.writeFileSync(metaLoc, JSON.stringify(metaData), 'utf-8');
Expand Down
53 changes: 33 additions & 20 deletions src/plugins/lokijs/rx-storage-instance-loki.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {
SortComparator,
DeterministicSortComparator,
QueryMatcher,
ChangeEvent
} from 'event-reduce-js';
Expand Down Expand Up @@ -43,7 +43,8 @@ import type {
LokiRemoteRequestBroadcastMessage,
LokiRemoteResponseBroadcastMessage,
LokiLocalState,
LokiDatabaseSettings
LokiDatabaseSettings,
RxDocumentWriteData
} from '../../types';
import type {
CompareFunction
Expand Down Expand Up @@ -277,13 +278,13 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
return mutateableQuery;
}

getSortComparator(query: MangoQuery<RxDocType>): SortComparator<RxDocType> {
getSortComparator(query: MangoQuery<RxDocType>): DeterministicSortComparator<RxDocType> {
// TODO if no sort is given, use sort by primary.
// This should be done inside of RxDB and not in the storage implementations.
const sortOptions: MangoQuerySortPart<RxDocType>[] = query.sort ? (query.sort as any) : [{
[this.primaryPath]: 'asc'
}];
const fun: CompareFunction<RxDocType> = (a: RxDocType, b: RxDocType) => {
const fun: DeterministicSortComparator<RxDocType> = (a: RxDocType, b: RxDocType) => {
let compareResult: number = 0; // 1 | -1
sortOptions.find(sortPart => {
const fieldName: string = Object.keys(sortPart)[0];
Expand Down Expand Up @@ -328,10 +329,13 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
* Instead we create a fake Resultset and apply the prototype method Resultset.prototype.find(),
* same with Collection.
*/
getQueryMatcher(query: MangoQuery<RxDocType>): QueryMatcher<RxDocType> {
const fun: QueryMatcher<RxDocType> = (doc: RxDocType) => {
getQueryMatcher(query: MangoQuery<RxDocType>): QueryMatcher<RxDocumentWriteData<RxDocType>> {
const fun: QueryMatcher<RxDocumentWriteData<RxDocType>> = (doc: RxDocumentWriteData<RxDocType>) => {
const docWithResetDeleted = flatClone(doc);
docWithResetDeleted._deleted = !!docWithResetDeleted._deleted;

const fakeCollection = {
data: [doc],
data: [docWithResetDeleted],
binaryIndices: {}
};
Object.setPrototypeOf(fakeCollection, (lokijs as any).Collection.prototype);
Expand All @@ -340,6 +344,7 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
};
Object.setPrototypeOf(fakeResultSet, (lokijs as any).Resultset.prototype);
fakeResultSet.find(query.selector, true);

const ret = fakeResultSet.filteredrows.length > 0;
return ret;
}
Expand Down Expand Up @@ -373,8 +378,8 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
error: new Map()
};

const startTime = now();
documentWrites.forEach(writeRow => {
const startTime = now();
const id: string = writeRow.document[this.primaryPath] as any;
const documentInDb = collection.by(this.primaryPath, id);

Expand Down Expand Up @@ -426,8 +431,14 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
}

if (
!writeRow.previous ||
revInDb !== writeRow.previous._rev
(
!writeRow.previous &&
!documentInDb._deleted
) ||
(
!!writeRow.previous &&
revInDb !== writeRow.previous._rev
)
) {
// conflict error
const err: RxStorageBulkWriteError<RxDocType> = {
Expand All @@ -440,37 +451,37 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
} else {
const newRevHeight = getHeightOfRevision(revInDb) + 1;
const newRevision = newRevHeight + '-' + createRevision(writeRow.document, true);

const writeDoc = Object.assign(
const isDeleted = !!writeRow.document._deleted;
const writeDoc: any = Object.assign(
{},
documentInDb,
writeRow.document,
{
$loki: documentInDb.$loki,
_rev: newRevision,
_deleted: isDeleted,
// TODO attachments are currently not working with lokijs
_attachments: {}
}
);
collection.update(writeDoc);
this.addChangeDocumentMeta(id);


let change: ChangeEvent<RxDocumentData<RxDocType>> | null = null;
if (writeRow.previous._deleted && !writeDoc._deleted) {
if (writeRow.previous && writeRow.previous._deleted && !writeDoc._deleted) {
change = {
id,
operation: 'INSERT',
previous: null,
doc: stripLokiKey(writeDoc)
};
} else if (!writeRow.previous._deleted && !writeDoc._deleted) {
} else if (writeRow.previous && !writeRow.previous._deleted && !writeDoc._deleted) {
change = {
id,
operation: 'UPDATE',
previous: writeRow.previous,
doc: stripLokiKey(writeDoc)
};
} else if (!writeRow.previous._deleted && writeDoc._deleted) {
} else if (writeRow.previous && !writeRow.previous._deleted && writeDoc._deleted) {
/**
* On delete, we send the 'new' rev in the previous property,
* to have the equal behavior as pouchdb.
Expand Down Expand Up @@ -520,10 +531,10 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
* to ensure all RxStorage implementations behave equal.
*/
await promiseWait(0);
const startTime = now();
const collection = localState.collection;

documents.forEach(docData => {
const startTime = now();
const id: string = docData[this.primaryPath] as any;
const documentInDb = collection.by(this.primaryPath, id);
if (!documentInDb) {
Expand Down Expand Up @@ -624,6 +635,7 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
if (!localState) {
return this.requestRemoteInstance('query', [preparedQuery]);
}

let query = localState.collection
.chain()
.find(preparedQuery.selector);
Expand All @@ -643,6 +655,7 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
if (preparedQuery.limit) {
query = query.limit(preparedQuery.limit);
}

const foundDocuments = query.data().map(lokiDoc => stripLokiKey(lokiDoc));
return {
documents: foundDocuments
Expand Down Expand Up @@ -674,7 +687,7 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
})
.simplesort(
'sequence',
!desc
desc
);
if (options.limit) {
query = query.limit(options.limit);
Expand All @@ -686,7 +699,7 @@ export class RxStorageInstanceLoki<RxDocType> implements RxStorageInstance<
sequence: result.sequence
}));

const useForLastSequence = desc ? lastOfArray(changedDocuments) : changedDocuments[0];
const useForLastSequence = !desc ? lastOfArray(changedDocuments) : changedDocuments[0];

const ret: {
changedDocuments: RxStorageChangedDocumentMeta[];
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/pouchdb/adapter-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from './pouch-db';
import {
adapterObject,
now,
PROMISE_RESOLVE_FALSE,
randomCouchString
} from '../../util';
Expand Down Expand Up @@ -45,7 +46,7 @@ export function checkAdapter(adapter: any): Promise<any> {
_id,
value: {
ok: true,
time: new Date().getTime()
time: now()
}
}))
// ensure read works
Expand Down
Loading

0 comments on commit 3390ba8

Please sign in to comment.