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

DBCore middlewares not working as expected #1180

Closed
LFFATE opened this issue Nov 28, 2020 · 9 comments
Closed

DBCore middlewares not working as expected #1180

LFFATE opened this issue Nov 28, 2020 · 9 comments
Labels

Comments

@LFFATE
Copy link

LFFATE commented Nov 28, 2020

DBCore is a middleware-approach for Dexie that is superior to the hooks API
https://dexie.org/docs/Dexie/Dexie.use()#example

I can't use hooks API because of async functions to be applied for dexie results

DBCore has several middlewares: get, getMany, query... So I think I can use it instead of reading hook.
But I can't make these middlewares works.
For example, collection.toArray someway prevents these middlewares from firing:

const categories = await Database.categories.filter(...).toArray()
const products = await Database.products.where('id').anyOf(productsIds).toArray()

No one of this middlewares works.

But this works well:

Database.products.get(key) // will fire DBCoreTable.get 
Database.products.get({ slug: key }) // will fire DBCoreTable.query 

Is it known issue? Or may be I'm doing wrong? Should I make a repro?

General purpose is map dexie results to class factory function, which is async

"dexie": "^3.0.3",
"fake-indexeddb": "^3.1.2",
"jest": "^26.6.3",
@dfahlander
Copy link
Collaborator

dfahlander commented Nov 28, 2020

To catch all read-queries you must implement all functons in the DBCoreTable interface except mutate:

get(req: IDBCoreGetRequest),
getMany(req: DBCoreGetManyRequest),
query(req: DBCoreQueryRequest),
openCursor(req: DBCoreOpenCursorRequest),
count(req: DBCoreCountRequest)

openCursor() returns a DBCoreCursor.
You can create a proxy cursor from your overridden openCursor() as is being done in virtual-index-middleware.ts. Notice some things here:

  1. The async result of openCursor() will be null if no items were found. So your middleware must also return null if so.
  2. cursor.value will be undefined if cursor is opened with {values: false} so make sure your mapper handles that and returns undefined if so. Will happen when using Collection.eachKey(), Collection.keys() and maybe some more.
  3. Methods and properties in the cursor are bound so it is safe to return a prototype-derived instance of the cursor and override properties and methods using Object.create() and just override the methods and properties that are nescessary. It is not very obvious which these are, but generally, an implementor may override key, primaryKey, value, continue(), continuePrimaryKey() and start().
  4. Your async mapper must not call non-indexedDB async APIs because transaction would be lost. If you need to do that anyway (for example if you need to call crypto APIs), you might go for Dexie.waitFor() but consider if there could be another solution, like using a sync crypto library for example, as Dexie.waitFor() can hurt performance.

If your aim is to map value results you would only need to override get, getMany, query and openCursor. As your mapping function is async, your proxy cursor may need to resolve the initial value prior to resolving the promise, as the getter of value is not async. It will also need to override start() to prefetch value using your async mapper:

const myDBCoreTable = {
  ...table,

  get: (req) => table.get(req).then(myAsyncMapper),

  getMany: (req) =>
    table.getMany(req).then((res) => Promise.all(res.map(myAsyncMapper))),

  query: (req) =>
    table.query(req).then((res) => req.values // Check if request wants values
      ? Promise.all(res.result.map(myAsyncMapper)).then((result) => ({
           result,
         }))
      : res // Caller only want primary keys. res is only the keys. Don't map.
  ),

  openCursor: (req) =>
    table.openCursor(req).then((cursor) => {
      if (!cursor) return cursor; // cursor is null
      if (!req.values) return cursor; // caller only want to enumerate keys.
      return createCursor(cursor);
    }),
};

function createCursor(cursor) {
  return myAsyncMapper(cursor.value).then((value) => {
    return Object.create(cursor, {
      key: { get: () => cursor.key }, // Added 2020-12-14: Needed to get a proper this-pointer.
      primaryKey: { get: () => cursor.primaryKey }, //Added 2020-12-14: Needed to get a proper this-pointer.
      value: {
        get: () => value, // value is not just argument - it's changed by code within `start()`.
      },
      start: {
        value: (onNext) => cursor.start(() =>
          myAsyncMapper(cursor.value).then((val) => {
            value = val; // Updating `value` to make cursor.value return new value.
            onNext();
          }).catch((error) => {
            cursor.fail(error);
          })
        )
      },
    });
  });
}

I've just dry-coded this so it may contain syntax errors or bugs, but please try it and tell me if this works for you. I will need to update the docs with such a sample and it would be nice to have one that has been tested so your feedback is valuable!

Edited 2020-12-14: properties Cursor.key and Cursor.primaryKey needs to be declared as well in order to get a proper this pointer

@dfahlander
Copy link
Collaborator

Note: I've updated the code snippet since first reply. Still dry-coded so please verify.

@LFFATE
Copy link
Author

LFFATE commented Nov 29, 2020

@dfahlander great! All tests are passed now. Thank you. It was not obvious for me to use a custom cursor, and at query I had wrong implementation, so your example was really helpful. Tested also at browser and there is no problems at this moment

@dfahlander
Copy link
Collaborator

Ok great! Can I use my code snippet as it is in the docs or was it anything that you had to write differently?

@LFFATE
Copy link
Author

LFFATE commented Nov 30, 2020

Yes, but I combined your example with the one from docs. From here https://dexie.org/docs/Dexie/Dexie.use()#example
So I don't think it may cause any problems.

@LFFATE
Copy link
Author

LFFATE commented Dec 10, 2020

@dfahlander Hi again! Found a problem with IDBCursorWithValue
Here is a fast repro https://codesandbox.io/s/mystifying-sun-xwjhi?file=/src/App.js

Jest and fake-indexeddb doesn't show this error, so I missed it last time

I think there is problem with cloning IDBCursor, maybe types of cursor.start doesn't met. I'm trying to find out.

@LFFATE
Copy link
Author

LFFATE commented Dec 14, 2020

I see two options:

add key property to new cursor (copy from original cursor):

      key: {
        value: cursor.value,
      },

so it will be:

function createCursor(cursor) {
  return myAsyncMapper(cursor.value).then((value) => {
    return Object.create(cursor, {
      value: {
        get: () => value,
      },
      key: {
        value: cursor.value,
      },
      start: {
        value: (onNext) => cursor.start(() =>
          myAsyncMapper(cursor.value).then((val) => {
            value = val;
            onNext();
          }).catch((error) => {
            cursor.fail(error);
          })
        )
      },
    });
  });
}

or copy all data from original cursor:

function createCursor(cursor) {
  return myAsyncMapper(cursor.value).then((value) => {
   const newCursorData = Object.create(cursor, {
      value: {
        get: () => value,
      },
      start: {
        value: (onNext) => cursor.start(() =>
          myAsyncMapper(cursor.value).then((val) => {
            value = val;
            onNext();
          }).catch((error) => {
            cursor.fail(error);
          })
        )
      },
    });

    return Object.assign(cursor, newCursorData)
  });
}

@dfahlander
Copy link
Collaborator

In my comment on november 29 i missed to define the key and primaryKey properties. They are needed to apply the correct this-pointer for the cursor. I just now updated that comment to correct this.

It's not very obvious nor documented (will fix that), but the methods on the cursor like start(), stop(), continue(), continuePrimaryKey() etc are already bound on the lowest-level implementation of the cursor, while the readonly properties key, primaryKey and value are not.

It's not possible to clone the cursor as it is mutable object so the simplest way of creating a proxy cursor is still by using Object.create() and override props and methods accordingly. Just keep in mind that the three readonly properties key, primaryKey and value, will always have to be overridden.

@LFFATE
Copy link
Author

LFFATE commented Dec 13, 2021

Hello again!
I found a new problem due to my app has grown.
So just for readers may be helpful:

The error:
Unhandled rejection: TransactionInactiveError: Failed to execute 'continue' on 'IDBCursor': The transaction is not active.

So it looks like onNext(); calls after transaction is closed. I think it is not critical when a promise not takes so long time. Therefore the best way I found to handle it - is use Dexie.waitFor (https://dexie.org/docs/Dexie/Dexie.waitFor())

My quick code:

async function createCursor(cursor: DBCoreCursor, tableName: string, hooks: DatabaseHooks): Promise<DBCoreCursor> {

  const hookFunction = <T = any>(data: T) => hooks.transform(tableName, data)
  let cursorValue = await Dexie.waitFor(hookFunction(cursor.value))

  return Object.create(cursor, {
    value: {
      get: () => cursorValue,
    },
    key: { get: () => cursor.key },
    primaryKey: { get: () => cursor.primaryKey },
    start: {
      value: (onNext: () => void) => {

        return cursor.start(async () => {
          try {
            const localCursorValue = await Dexie.waitFor(hookFunction(cursor.value))
            cursorValue = localCursorValue
            onNext()
          } catch(error) {
            console.log('Fails here')
            cursor.fail(error)
          }
        })
      },
    },
  })
}

May be I should also rewrite all hookFunction calls (inside toArray, get, getMany and etc.), but It still works as is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants