Skip to content

Add test with graphql and sqlite storage#6786

Closed
mapineaultvooban wants to merge 1 commit intopubkey:masterfrom
mapineaultvooban:graphql_replication_attachment_v16
Closed

Add test with graphql and sqlite storage#6786
mapineaultvooban wants to merge 1 commit intopubkey:masterfrom
mapineaultvooban:graphql_replication_attachment_v16

Conversation

@mapineaultvooban
Copy link

this is a copy of pull request (#6782). This pull request uses RxDB 16 instead of 15.39. If possible, we would like the fix to be in 15.39.

This PR contains:

A unit test that executes a migration on a database with a sqlite storage. Then a graphql server sends an update with leaked property (assumedMasterState contains the _attachments property). I wrote some comments in the test, because I don't have access to the premium package (RxStorageSQLite) in this repository.

When we use a different storage (e.g in memory or another default storage in the public package), the test passed.

Describe the problem you have without this PR

In our project, we use a RxDatabase with

  • a Sqlite storage (RxStorageSQLite from the premium package)
  • a graphQL server to push updates to our backend

In one of our schemas, we add a new migration. when executed, it seems that the _attachments property is followed. When a user updates a property in an entity, the graphQL server push the update to our backend, causing an error in the serialization (input -> classes).

image

Here is a payload of one of our shemas with the problem. In the entity's masterState, we can see that the _attachments property is leaked.

{
  "query": "mutation PushContractAgreements($pushRows: [PushRowContractAgreementInput!]!) {
    pushContractAgreements(contractAgreements: $pushRows) {
        id
        status
        date
        number
        lunchBreaks {
          from
          durationInMinutes
          startsOnNextDay
        }
        signature
        signedAt
        updatedAt
        isDeleted
    }
  }",
  "operationName": "PushContractAgreements",
  "variables": {
    "pushRows": [
      {
        "assumedMasterState": {
          "id": "c3fdbc58-4588-4fe4-a3b4-e0e5467e1d82",
          "status": "created",
          "date": "2025-01-23T12:00:00.000Z",
          "number": null,
          "lunchBreaks": [],
          "signature": null,
          "signedAt": null,
          "updatedAt": 1737640220258,
          "_attachments": {},
          "isDeleted": false
        },
        "newDocumentState": {
          "id": "c3fdbc58-4588-4fe4-a3b4-e0e5467e1d82",
          "status": "ongoing",
          "date": "2025-01-23T12:00:00.000Z",
          "number": null,
          "lunchBreaks": [],
          "signature": null,
          "signedAt": null,
          "updatedAt": 1737647944354,
          "isDeleted": false
        }
      }
    ]
  }
}

Here is our database setup

db = await createRxDatabase<RxGuayCollections>({
     eventReduce: true,
     multiInstance: false,
     name: dbName,
     password: env.DATABASE_PASSWORD,
     storage: getRxStorageSQLite({
       sqliteBasics: getSQLiteBasicsExpoSQLiteAsync(ExpoSqliteStorageNext.openDatabaseAsync),
       // log: console.log.bind(console),
     }),
     ignoreDuplicate: isTest,
     cleanupPolicy: {
       minimumDeletedTime: 1000 * 60 * 60 * 24 * 31, // one month,
       minimumCollectionAge: 1000 * 60, // 60 seconds
       runEach: 1000 * 60 * 5, // 5 minutes
       awaitReplicationsInSync: true,
       waitForLeadership: false,
     },
     allowSlowCount: true,
   });

db.addCollections({
     contract_agreement: {
       autoMigrate: true,
       migrationStrategies: {
         1: (doc: never): never => doc,
       },
       schema: {
         description: 'describes a contract agreement',
         keyCompression: true,
         primaryKey: 'id',
         indexes: ['updatedAt'],
         properties: {
           id: {
             maxLength: 100,
             type: 'string', // <- the primary key must have set maxLength
           },
           isDeleted: {
             type: 'boolean',
           },
           lunchBreaks: {
             items: {
               description: 'describes a time range',
               properties: {
                 from: {
                   type: 'string',
                 },
                 durationInMinutes: {
                   type: 'number',
                 },
                 startsOnNextDay: {
                   type: 'boolean',
                 },
               },
               type: 'object',
             },
             type: 'array',
           },
           number: {
             type: ['null', 'number'],
           },
           signature: {
             type: ['string', 'null'],
           },
           signedAt: {
             type: ['string', 'null'],
           },
           status: {
             type: 'string',
           },
           updatedAt: {
             type: 'number',
             minimum: 0,
             maximum: 99999999999999,
             multipleOf: 1,
           },
         },
         required: ['id', 'status'],
         title: 'Contract agreement schema',
         type: 'object',
         version: 1,
       },
     },
   });

Todos

  • Tests
  • Documentation
  • Typings
  • Changelog

this is from commit 9f85889 (rxdb 15.39 -> 16)
@mapineaultvooban mapineaultvooban changed the title Cherry pick commit Add test with graphql and sqlite storage Jan 27, 2025
@pubkey
Copy link
Owner

pubkey commented Jan 28, 2025

Hi @mapineaultvooban
I think your test is not testing correctly. It fails for memory storage and everything non-dexie. When I add logs, it seems not to fail because of leaking the _attachments:

console.log('in the _attachments assert:');
console.dir(firstRow);

Shouldnt this test be green for the memory storage?

@pubkey
Copy link
Owner

pubkey commented Jan 28, 2025

Ok fixed this by adding await replicationState.awaitInSync(); after the update.

pubkey added a commit that referenced this pull request Jan 28, 2025
@pubkey
Copy link
Owner

pubkey commented Jan 28, 2025

@mapineaultvooban I ran the exact same test with the SQLite storage and it does not fail.
Not sure how to proceed.

@jwallet
Copy link
Contributor

jwallet commented Jan 29, 2025

Hello Daniel,

We are running our react-native app with expo using the newest sdk50 and soon 52 which is using now expo/sqlite/next in 52 sqlite/legacy is removed. Do you have a storage CI test on expo sqlite package? maybe the issue comes from that lib.

EDIT: I will try to debug the sqlite wrapper on my spare time and find where the _attachments is added, cause the issue never happens if we do not run a migration, and it does not happen with a schema with flatten properties (no reference to another schema)

@pubkey
Copy link
Owner

pubkey commented Jan 31, 2025

Does it only fail with Expo SQLite or also with other SQLite libs like in node.js?

@jwallet
Copy link
Contributor

jwallet commented Feb 5, 2025

we upgraded to rxdb v16 and it still happens, but yes, it happens on expo-sqlite v15 using the expo-sqlite async refact, on previous version it was named expo-sqlite/next and the expo-sqlite/legacy was working fine so it was working with that sqlite storage.

edit : btw on expo, rxdb v16 is crashing even for a memory storage with a database using keyCompression, until you attach to the database creator

import * as ExpoCrypto from 'expo-crypto';
await createRxDatabase({
  // ...
  hashFunction: (input: string) => ExpoCrypto.digestStringAsync(ExpoCrypto.CryptoDigestAlgorithm.SHA256, input)
});

@pubkey
Copy link
Owner

pubkey commented Feb 5, 2025

Are you using the latest expo version?

@jwallet
Copy link
Contributor

jwallet commented Feb 6, 2025

libs version

"expo": "52.0.28" // 52.0.30 latest,
"expo-crypto": "14.0.2", // for hashFunction, 14.0.2 latest
"expo-sqlite": "15.1.0", // 15.1.2 latest

tried to update to v16

we tried rxdb v16, and the assumed master state does not update anymore when the server responds with the body {data: { pullRep: { documents : [], checkpoint: { id: uuid, updatedAt: long } } }

when the second push occurs on the same collection, the server will return conflicted entities because the assumedMasterState is not accurate, it was not updated with the payload that this client sent just a minute before. we even try ios which does not use the same native sqlite, we tried the memory storage and even remove all conflict handlers, and this occurs in dev build or production build. we are also using hooks, preSave, preInsert, just to bump the updatedAt: Date.now()

const preSave = <T extends { updatedAt: number }>(_db: RxDatabase<RxMyCollections>) => {
  return (docData: T): T => {
    docData.updatedAt = new Date().getTime();
    return docData;
  };
};

anyway we went back to v15... we will try to open an expo snack example with rxdb 16 classic to show what happen maybe using for the server the msw local server.

retesting the issue on rxdb v15

so for the issue on rxdb 15, I tried the ios expo-sqlite version since it uses a different native sqlite (still wrapped with expo-sqlite), and the issue still happens, with or without our conflict handlers, and with or without the hooks (preSave, preInsert). I don't know what to do next for that issue, changing for another sqlite package will require to many changes

@pubkey
Copy link
Owner

pubkey commented Feb 6, 2025

Is this a differerent problem? Can you make test case for the replication problem so we can fix that first?
Notice that there will be no more v15 releases.

@jwallet
Copy link
Contributor

jwallet commented Feb 6, 2025

Yes it's a different problem. We tried to upgrade to see if this error was fixed, but we got more issues by upgrading. Right now, we will stay on v15, v15 is working fine if we filter out all rxdb private keys, meta, rev and attachments.

So you can close that issue. We will probably work again on it in the summer.

We would love to see a LTS version of rxdb in the future.

@pubkey
Copy link
Owner

pubkey commented Feb 9, 2025

Ok, closing this. I would still love to fix this for you, but I need a reproduction.

@pubkey pubkey closed this Feb 9, 2025
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 this pull request may close these issues.

3 participants