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

Inline keys and autoincrement not working #122

Closed
jorgemanrubia opened this issue Apr 28, 2014 · 14 comments
Closed

Inline keys and autoincrement not working #122

jorgemanrubia opened this issue Apr 28, 2014 · 14 comments

Comments

@jorgemanrubia
Copy link

Hi,

I am experiencing this issue.

When using inline keys with autoincrement, the generated key is not stored with the data. When you read them from the database, there is no reference to its original key, so you can't make updates.

This problem doesn't happen with the facebook polyfill.

I think this issue is pretty serious, as it impacts a very common usage pattern.

Regards

@DickvdBrink
Copy link
Collaborator

You are right, it is an issue. There is a workaround though (if I understand the issue correctly).
I believe the cursor.primaryKey property works correctly so it is still possible to update.

Note: You are right, this definitely needs to be fixed.

@IntranetFactory
Copy link

I'd run into the same issue today. I changed the .add logic to include the autogenerated key into the record:

1009c1002
<         function getNextAutoIncKey(props) {
---
>         function getNextAutoIncKey() {
1012d1004
<                     value[props.keyPath] = 0; 
1016d1007
<                     value[props.keyPath] = data.rows.item(0).seq; // save new key in record
1035,1036c1026,1027
<                         var primaryKey = value[props.keyPath];
<                         if (primaryKey == undefined) {
---
>                         var primaryKey = eval("value['" + props.keyPath + "']");
>                         if (!primaryKey) {
1038c1029
<                                 getNextAutoIncKey(props);
---
>                                 getNextAutoIncKey();
1110d1100
< 
1114,1117c1104,1106
< 
<         me.transaction.__pushToQueue(request, function (tx, args, success, error) {
<             me.__deriveKey(tx, value, key, function (primaryKey) {
<                 idbModules.Sca.encode(value, function (encoded) {
---
>         idbModules.Sca.encode(value, function (encoded) {
>             me.transaction.__pushToQueue(request, function (tx, args, success, error) {
>                 me.__deriveKey(tx, value, key, function (primaryKey) {
1124d1112
< 

@jorgemanrubia
Copy link
Author

Thanks a lot @IntranetFactory. Why not submitting that patch as a PR? Would that be ok @DickvdBrink?

@DickvdBrink
Copy link
Collaborator

Yeah I think so, I can't really see the difference with the above patch but I will definitely take a look at it when submitted as a PR!

@IntranetFactory
Copy link

The patch stores the auto generated key value inline in the records in the property defined by keyPath. I'm sorry, but I've no idea how to make a PR.

@jorgemanrubia
Copy link
Author

Hi @IntranetFactory,

I am having some problems interpreting the diff code too. Creating a Pull Request is very easy:

image

image

The difficult part was fixing this. Creating a PR is really easy :), so let me know if you need further help.

@XOlegator
Copy link

I updated IndexedDBShim.min.js and db.js to the latest version as of December 23, 2014. But still get the error with keys in my Phonegap app at Android emulator (in desktop browser Firefox 34 there is no such problem). Example:
Shema:
"settings": {
"key": {
"keyPath": "id",
"autoIncrement": true
},
"indexes": {
"units": {
"unique": true
},
"language": {
"unique": true
}
}
}
Code:
server.settings.add({
'units': 'Metric',
'language': 'English'
}).then(function(item){
console.log('Add new settings: ' + JSON.stringify(item));
});
In emulator: "Add new settings: [{"units":"Metric","language":"English","undefined":0}]"
In Firefox: "Add new settings: [{"units":"Metric","language":"English","id":1}]"
All 'keyPath' of my keys in my program are converted into "undefined" at emulator! How to fix it?

@JamesMessinger
Copy link
Collaborator

This issue has been fixed in the latest version. Give it a try and let us know if you find any other issues. Thanks!

@XOlegator
Copy link

Now the problem is gone (application is work). But the object in emulator (Android) was to look like this: [{"units":"Metric","language":"English","id":0,"undefined":0}] instead of [{"units":"Metric","language":"English","id":0}]

@JamesMessinger
Copy link
Collaborator

Just to clarify... are you saying that "undefined": 0 is still being added to your saved data, even with the latest version of IndexedDBShim? If so, then can you post some sample code so I can reproduce the problem?

@XOlegator
Copy link

I made some experiments and found that the problem occurs not always. For example this code (it is initialization of app) return additional "undefined" after "id":

server.settings.query()
        .all()
        .execute()
        .then(function(results) {
          if(results.length) {
            // some init operations
          } else {
            // need to add default settings to DB
            server.settings.add({
              'units': 'metric',
              'language': 'english'
            }).then(function(item) {
              // all done
              console.log(JSON.stringify(item));
            });
          }
        });

Output is: "[{"units":"metric","language":"english","id":1,"undefined":1}]"

@JamesMessinger
Copy link
Collaborator

Can you post a code sample using the raw IndexedDB api instead of IDBWrapper? That'll make it much easier to figure out what's causing the problem.

@XOlegator
Copy link

I made some new experiments with new indexeddbshim.js and it's working fine. Thanks!

@JamesMessinger
Copy link
Collaborator

Awesome! Great to hear.

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

Successfully merging a pull request may close this issue.

5 participants