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

Datastore Key type error when using cache #243

Closed
superwoou opened this issue Feb 16, 2021 · 7 comments
Closed

Datastore Key type error when using cache #243

superwoou opened this issue Feb 16, 2021 · 7 comments

Comments

@superwoou
Copy link

superwoou commented Feb 16, 2021

I'm using gstore-node 7.2.5 with cache-manager-redis-store.
When I using findOne method, it returns error with cached object.

The error message is : ERROR Entity Key must be a Datastore Key - Error: Entity Key must be a Datastore Key

The problem is that the key with cached object is not Key class, just object.
Without cache :

    [Symbol(KEY)]: Key {
      namespace: undefined,
      id: '7042',
      kind: 'User',
      path: [Getter]
    }

With cache:

[Symbol(KEY)]: { id: '7042', kind: 'User', path: [Array] }

I think gstore-node should convert key property to Key class.

@carnun
Copy link
Collaborator

carnun commented Feb 22, 2021

Thank you for the feedback @superwoou can you please add a code snippet so that we can recreate the issue? This will ensure that we properly resolve the issue and we can update our unit tests to ensure that the issue is properly addressed.

Ideally something like:

const { Gstore } = require('gstore-node');
const gstore = new Gstore({
    cache: {
        stores: [ ... ],
        config: { ... }
    },
});
// model setup
// steps of gstore calls which fail

What we would want to do is update the integration tests with a test to recreate this - https://github.com/sebelga/gstore-node/blob/master/__tests__/integration/cache.ts.

@superwoou
Copy link
Author

superwoou commented Jun 16, 2021

Sorry for late reply.

Oh, I added a code snippet to reproduce.
You can reproduce it with running findOne with same params multiple times.

datastore.js

const { Datastore } = require('@google-cloud/datastore');
const { Gstore } = require('gstore-node');
const env = require('./env');
const redisStore = require('cache-manager-redis-store');

export const datastore = new Datastore({
  projectId: env.gcp.projectId,
  namespace: env.datastore.namespace,
});

const cacheStores = [
  {
    store: redisStore,
    host: env.redis.redisHost,
    port: env.redis.redisPort,
  },
];

const cacheConfig = {
  ttl: {
    keys: 6 * 60 * 60, // 6 hour
    queries: 10 * 60, // 10 minute
  },
};

export const gstore = new Gstore({
  cache: {
    stores: cacheStores,
    config: cacheConfig,
  },
});

gstore.connect(datastore);

model.js

import { datastore } from './datastore';

const { gstore } = datastore;
const { Schema } = gstore;

const userSchema = new Schema(
  {
    username: { joi: Joi.string().required() },
  },
  {
    joi: true,
  },
};

export const User = gstore.model('User', userSchema);  

test.js

import { User } from './model';

(async() => {
  await User.findOne({ username: 'test' }); // get from datastore - return entity
  await User.findOne({ username: 'test' }); // get from cache - throw Error: Entity Key must be a Datastore Key
})();

(async() => {
  await User.get(1111); // get from datastore - return entity
  await User.get(1111); // get from cache - return entity
})();

I add logging on responseHandler

const [e] = entities;

I found the difference is type of key. (I said in original issue post).
console.log(e[this.Model.gstore.ds.KEY]);

When the data is from datastore - Key { namespace: undefined, id: '37232', kind: 'User', path: [Getter] }
When the data is from cache - { id: '37232', kind: 'User', path: [ 'User', '37232' ] }

I think there are some problem on load result of query from cache.

@SimonNord
Copy link

I noticed the same issue. Did you find out any resolution/workaround for this problem? @superwoou

@superwoou
Copy link
Author

I noticed the same issue. Did you find out any resolution/workaround for this problem? @superwoou

There are two simple workarounds.

  1. Disable cache on findOne
  2. Make function by list
await User.findOne({ username: 'test' }, null, null, { cache: false });

function findOne(model, filters) {
  const {entities} = await model.list({ filters, limit: 1 });
  return entities[0];
}

I think best way to fix this issue is casting to datastore key after retrieve from cache.

@carnun
Copy link
Collaborator

carnun commented Jul 29, 2021 via email

@superwoou
Copy link
Author

When i try to get entity by model.list, there is same error on convert entity from cache (Error: Entity Key must be a Datastore Key)

@carnun
Copy link
Collaborator

carnun commented Apr 7, 2022

Sorry for the long wait, thank you for all the feedback and suggestions. I just released the v7.2.8 which includes an integration test to verify the issue.

@carnun carnun closed this as completed Apr 7, 2022
carnun added a commit that referenced this issue Apr 7, 2022
…aded from cache

All entities which have id based keys now return those ids and their paths as numbers. If you rely
on the id being a string you will need to update your code.

BREAKING CHANGE: Entity keys return ids as Number

fix #243
carnun added a commit that referenced this issue Apr 7, 2022
…aded from cache

All entities which have id based keys now return those ids and their paths as numbers. If you rely
on the id being a string you will need to update your code.

BREAKING CHANGE: Entity keys return ids as Number

fix #243
carnun added a commit that referenced this issue Apr 7, 2022
* fix(datastore.ts): ensure entities with id based keys are correcly loaded from cache

All entities which have id based keys now return those ids and their paths as numbers. If you rely
on the id being a string you will need to update your code.

BREAKING CHANGE: Entity keys return ids as Number

fix #243

* "chore(release): 8.0.0"

* docs(changelog): add known Date caching issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants