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

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

Open
AdrianKBL opened this issue Aug 29, 2024 · 4 comments
Open

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

AdrianKBL opened this issue Aug 29, 2024 · 4 comments

Comments

@AdrianKBL
Copy link

AdrianKBL commented Aug 29, 2024

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
}
  1. 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.

  2. 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
Copy link

pat-rg commented Sep 3, 2024


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:

  • Improve consistency in how changes are handled.
  • Optimize the efficiency of data return to avoid unnecessary overload.
  • Ensure that identifiers used in patches are meaningful to users to enhance system usability.

@tmctl
Copy link
Member

tmctl commented Sep 10, 2024

@AdrianKBL @pat-rg

We are using the JSON patch specification. Here is the library and tools we are importing -

import { createPatch } from 'rfc6902'

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
Copy link

pat-rg commented Sep 10, 2024

@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
Copy link
Member

tmctl commented Sep 13, 2024

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?

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

3 participants