-
Notifications
You must be signed in to change notification settings - Fork 52
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
parseId in entity.js and model.js are different, leads to problems with keyType:name #168
Comments
Hello, |
@sebelga I worked on a patch on my fork yesterday and all I need to do is squash the commits, then make a PR, and you can see if that fix works for you. I couldn't run any of your tests though, because I'm on windows so your package.json tasks were throwing errors. Until you have a fix, or incorporate my fix, I can just keep running gstore-node from my fork. Cheers for the great library by the way! Docs are great and it's very user friendly. |
Maybe related issue: #172 |
It's also weird that the // When an entity key is constructed via the model, a string id is parsed into a number.
// It's kind of weird, because a datastore entity key id is a string.
expect(EntityModel.key(savedEntity.entityKey.id as string).id).to.equal(Number.parseInt(savedEntity.entityKey.id as string)) |
Hi! Sorry for the very late response. I finally can dedicate some time and look into this. Historically Google Datastore was making a difference between an integer being passed and a string. This would mean that, when using the auto-allocated id, it would be numeric (and returned in the EntityKey as numeric value). Then, when trying to GET the entity, if we didn't pass an integer it would not return it. So, when building an API and reading the id from the URL param, we had to parseInt it before trying to fetch it. So this is where the But now, after a few tests, I realized that saving
After this analysis, I will remove all the logic around the Cheers! 👍 [EDIT] Actually, when we run the local Datastore Emulator, the behaviour is different. The auto-allocated id is indeed an integer, but the Key returns it as a string id. Trying to fetch this string id does not return the entity and we need to convert it to integer before. |
…ity key id The live Datastore treats the same way User.get("123") than User.get(123). Both return the entity with the Key.id being "123". The convertion from string to integer has been removed inside gstore and let to the consumer to implement if needed. BREAKING CHANGE: The "keyType" Schema option has been removed as it is no longer needed. Also, as gstore does not parse the id anymore, running your project against the Datastore emulator locally might break as the emulator treats differently User.get(123) than User.get("123"). Auto-allocated ids are integer and need to be provided as integer for the Emulator. #168
…ity key id (#179) The live Datastore treats the same way User.get("123") than User.get(123). Both return the entity with the Key.id being "123". The conversion from string to an integer has been removed inside gstore and let to the consumer to implement if needed. BREAKING CHANGE: The "keyType" Schema option has been removed as it is no longer needed. Also, as gstore does not parse the id anymore, running your project against the Datastore emulator locally might break as the emulator treats differently User.get(123) than User.get("123"). Auto-allocated ids are integers and need to be provided as integers for the Emulator. #168
related to #86
I'm using gstore-node v6.0.2 and I'm having an issue updating an entity that uses a non-numeric id. The id is automatically parsed into a numeric one, which changes the value. I have set the keyType to "name" on my schema.
My datastore entity id/name is "200200000000000016353" and it gets turned into "200200000000000000000" because of rounding from parseInt and integer limits of js.
Why does gstore-node still use parseId, even when keyType is set on the schema? Is this expected?
Possible solutions?
Here is my schema:
In model.js of your library you have an update function:
What do you think?
The text was updated successfully, but these errors were encountered: