hgraph-io / sdk

SDK for developing with hgraph.io
1 stars 1 forks source link

patchedSubscription --> patch not enough information returned. #28

Open AdrianKBL opened 2 months ago

AdrianKBL commented 2 months ago

Good morning,

I'm currently testing the new patchedSubscriptions to implement them in our DApps to improve performance, and I'm encountering a few issues:

  1. Balance updated at patch: /data/entity_by_pk/token_account/39/token/total_supply - New value: 27282926759423626. What does the 39 after token_account refer to? How can I determine which token this new value belongs to? Given that we have numerous tokens, different balances, various public keys for each token, and different supplies for each token, I need each value to clearly identify a specific token. For instance, if a key changes, it should indicate what type of key it is.

    {
    "op": "replace",
    "path": "/data/entity_by_pk/balance",
    "value": 570062825537
    }
  2. Is there a way to subscribe to a specific field? For example, there are too many different burns and mints on Hedera concerning FTs. I don't want to keep receiving notifications for each burn that occurs for each token ID, but I do want to get notifications when the balance changes.

  3. Regarding NFTs, the value being returned includes the Token ID and Serial Number, which is great. However, for NFTs, the path should ideally be something like /data/tokenId/serialNumber instead of /data/nft/5, as I don't know what the 5 refers to.

{
    "op": "remove",
    "path": "/data/nft/5",
    "value": {
        "token_id": 3958582,
        "metadata": "\\x697066733a2f2f62616679626569666c77696935637965686c666f79357a6b37347a626e686d7433716e7073706f357a6a3764727733746b6f6f6e6c76626d6879692f6d657461646174615f3132322e6a736f6e",
        "serial_number": 122,
        "spender": null,
        "modified_timestamp": "1723910081709934947"
    }
}
  1. Regarding Nft_Aggregate, same problem, how do I determine which collection has been modified If TokenId is not being returned?
    {
    "op": "replace",
    "path": "/data/account/collections/167/nft_aggregate/aggregate/count",
    "value": 69
    }

    I didn't test other operations, but I guess we will have these problems in many different operations.

Example of a subscription:

Query

export function subFtsBalances(accountId: number, extraArgs?: string[]) {
  return `
  subscription AccountTokenBalances {
    entity_by_pk(id: ${accountId}) {
          balance
          token_account(
            where: {token: {type: {_eq: "FUNGIBLE_COMMON"}}}
          ) {
        balance
        associated
        }
      }
    }
  }`;
}

patchedSubscription

  patchedSubscribe(
    query: any,
    onNext: <T>(data: T) => void,
    onError?: (data: any) => void,
    onComplete?: () => void
  ): () => ObservableSubscription {
    const subscription = this.client.patchedSubscribe(query, {
      next: ({ data }, patches) => {
        console.log('data', data);
        onNext(data);
        patches.forEach((patch) => {
          console.log('patch', patch);
          switch (patch.op) {
            case 'add':
              console.log('New balance received:', patch.value);
              break;
            case 'remove':
              console.log('Balance removed:', patch.path);
              break;
            case 'replace':
              console.log(`Balance updated at patch: ${patch.path} - new value:  ${patch.value}`);
              break;
          }
        });
      },
      error: (error) => {
        if (onError) onError(error);
      },
      complete: () => {
        if (onComplete) onComplete();
      }
    });
    return () => subscription;
  }
pat-rg commented 2 months ago

Identified Cases and Issues

Case 1:

subscription {
  nft(
    where: {
      spender: { _eq: 3654319 }
      account_id: { _eq: 2665305 }
      token_id: { _eq: 3515204 }
    }
  ) {
    token_id
    serial_number
  }
}

.In subscriptions:
- All data received

- Modify spender

.In subscriptions:
- All data received
- 🚀 ~ patches.forEach ~ patch: {"op":"remove","path":"/data/nft/103","value":{"token_id":3515204,"serial_number":170}}

Case 2:

subscription {
  nft(where: { account_id: { _eq: 2665305 }, token_id: { _eq: 3515204 } }) {
    token_id
    serial_number
    spender
  }
}

.In subscriptions:
- All data received

- Modify spender

.In subscriptions:
- All data received
- patches.forEach ~ patch: {"op":"replace","path":"/data/nft/118/spender","value":null}

Case 3:

subscription {
  nft(where: { account_id: { _eq: 2665305 }, token_id: { _eq: 3515204 } }) {
    token_id
    serial_number
  }
}

.In subscriptions:
- All data received

- Modify spender

.In subscriptions:
- No patches received

Case 4:

subscription {
  nft(
    limit: 2
    where: {
      spender: { _eq: 3654319 }
      account_id: { _eq: 2665305 }
      token_id: { _eq: 3515204 }
    }
  ) {
    token_id
    serial_number
    spender
  }
}

.In subscriptions:
- All data received

- Modify spender

.In subscriptions:
- No patches received

Summary of Issues and Inconsistencies

  1. Inconsistencies in Handling Changes:

    • Issue: Subscriptions react inconsistently to the same data changes (e.g., modifying spender), leading to different operations (remove, replace, or no changes).
    • Impact: This affects the system's reliability and predictability.
  2. Inefficiency in Data Return:

    • Issue: The system returns all data results in full for every modification, which can cause performance issues and excessive memory usage.
    • Impact: This is inefficient and could overload the system, particularly with large data volumes.
  3. Poor Usability of External Identifiers:

    • Issue: The patches include internal identifiers that are not meaningful or useful to users.
    • Impact: This makes interpreting the data harder and reduces the usability of the responses.

Conclusion

To address these issues, it's crucial to:


tmctl commented 2 months ago

@AdrianKBL @pat-rg

We are using the JSON patch specification. Here is the library and tools we are importing - https://github.com/hgraph-io/sdk/blob/90b2759b5aaaf7a05d3daa1601569279344ab2d6/src/client/index.ts#L4

The number is the position in the in the array: https://datatracker.ietf.org/doc/html/rfc6902#appendix-A.2

You may consider writing subscriptions ONLY for the critical data you want to be notified on change vs. placing the whole query in a subscription. See https://www.apollographql.com/docs/react/data/subscriptions/#when-to-use-subscriptions

pat-rg commented 2 months ago

@tmctl JSON Patch specification does not require returning the full resource after applying a patch. Would it be possible to return only the patch operations instead of the full resource to avoid memory issues? That could indeed be a useful approach, especially for larger resources (ex: all NFTs of a spender).

tmctl commented 2 months ago

hi @pat-rg our API doesn't implement JSON Patch, so this code would need to be run on a server or on the client side.

we can expose a relay endpoint - https://relay.dev. Maybe some features of this might help your use cases?