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

Keystone Postgres data corruption #3119

Closed
pahaz opened this issue Jun 5, 2020 · 3 comments
Closed

Keystone Postgres data corruption #3119

pahaz opened this issue Jun 5, 2020 · 3 comments

Comments

@pahaz
Copy link
Contributor

pahaz commented Jun 5, 2020

Bug report

I have lists like so:

...
keystone.createList('Organization', {
    fields: {
        name: {
            factory: () => faker.company.companyName(),
            type: Text,
        },
        userLinks: {
            type: Relationship, ref: 'OrganizationToUserLink.organization', many: true,
        },
    },
    plugins: [byTracking(), atTracking()],
})

keystone.createList('OrganizationToUserLink', {
    fields: {
        organization: {
            type: Relationship,
            ref: 'Organization.userLinks',
            isRequired: true,
            knexOptions: { isNotNullable: true },
        },
        user: {
            type: Relationship,
            ref: 'User',
            isRequired: true,
            knexOptions: { isNotNullable: true },
        },
    },
    plugins: [byTracking(), atTracking()],
})

And CI TEST which does something like:

const DELETE_ORGANIZATION_TO_USER_LINK_MUTATION = gql`
    mutation delObj($id: ID!) {
        deleteOrganizationToUserLink(id: $id) { id }
    }
`
...
const link3Id = 54
const client = await makeLoggedInClient(...)
const { data, errors } = await client.mutate(DELETE_ORGANIZATION_TO_USER_LINK_MUTATION, { id: link3Id })

When I run this code with a DEBUG=knex:query,knex:tx env I can see these queries:

...
  knex:query update "public"."OrganizationToUserLink" set "organization" = ? where "organization" = ? undefined +18ms
  knex:query update "public"."OrganizationToUserLink" set "user" = ? where "user" = ? undefined +1ms
  knex:query update "public"."OrganizationToUserLink" set "updatedBy" = ? where "updatedBy" = ? undefined +0ms
  knex:query update "public"."OrganizationToUserLink" set "createdBy" = ? where "createdBy" = ? undefined +0ms
  knex:query delete from "public"."OrganizationToUserLink" where "id" = ? undefined +32ms
...

This queries is triggered by the code:
image

It looks strange, why do I need to update something before deleting it.

Then, I raned the code in debug mode. On a screenshot below, I'm trying to delete OrganizationToUserLink.id == 54 . But, as you can see, it's also trying to set null user column where OrganizationToUserLink.user.id == 54 😲

image

image

The Keystone tries to update the value of neighboring records for all primary keys that refer to other tables. Probably there is missing a test, this code should only update the table references itself.

additional

I have a CI test here: https://github.com/8iq/nodejs-hackathon-boilerplate-starter-kit/runs/740039167
There is a reason why my tests fail on Postgres and work well on Mongo.
Because there is a different way to generate object ID.
On the Postgres, the mutation also updates other table rows and corrupt the table data.

@jesstelford
Copy link
Contributor

This may have been fixed in a very recent version. Can you confirm you're on the latest versions of all the Keystone packages?

@pahaz
Copy link
Contributor Author

pahaz commented Jun 5, 2020

Yes. It's looks fixed but there is another problem with the latest version. I need some time to research it.

@pahaz
Copy link
Contributor Author

pahaz commented Jun 5, 2020

I've closed this. This is fixed after the version update. But there is another one problem accrued.

Here is a fix and explanation:
#3120
#3121

Thanks for all!

@pahaz pahaz closed this as completed Jun 5, 2020
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

2 participants