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

[Bug]: MemoryDatabase/Store does not correctly handle entity relationships #1096

Open
mshakeg opened this issue Jan 3, 2025 · 8 comments
Open

Comments

@mshakeg
Copy link
Contributor

mshakeg commented Jan 3, 2025

Description

The MemoryDatabase/Store implementation currently does not properly handle entity relationships as defined in the GraphQL schema. This affects the ability to traverse relationships between entities, which is crucial for proper data modeling and querying.

Test Cases Added

I've added test cases to ./packages/sdk/src/store/tests/database.test.ts that demonstrate the expected behavior for relationship traversal. These tests currently fail, highlighting the gaps in the current implementation.

The test cases verify the following relationship patterns:

  • One-to-Many: Transaction -> TransactionReceipt[]
  • Many-to-One: TransactionReceipt -> Transaction
  • Interface Implementation: Transaction.sender (Owner interface)
  • Derived Relationships: User.organizations (via @derivedFrom)
  • Deep Relationship Traversal: Transaction -> Receipt -> Transaction -> Receipt

Expected Behavior

The database should allow traversal of relationships as defined in the GraphQL schema, maintaining referential integrity and proper type information. This includes:

  • Accessing required array relationships
  • Accessing optional entity relationships
  • Handling interface implementations
  • Supporting @derivedFrom relationships
  • Enabling multi-level relationship traversal

Current Behavior

Currently, attempting to traverse these relationships fails.

Alternatives

There are a few approaches to handle entity relationships:

  1. Current Workaround:
  • Instead of direct relationship traversal, developers can use stored entity IDs
  • E.g., Using receipt.transactionID to query the transaction separately via store.get(Transaction, receipt.transactionID)
  • While more verbose, this approach still enables access to related entities
  • Requires multiple separate queries instead of a single relationship traversal
  1. Potential Design Improvement:
  • Replace Promise properties with Promise-returning functions
  • E.g., receipt.transaction() instead of receipt.transaction
  • Benefits:
    • Makes database queries more explicit in code
    • Enables lazy loading - only fetch when explicitly requested
    • Better performance visibility and control
    • Clearer indication of async operations and potential costs
  • Example:
    // Current (implicit)
    const transaction = await receipt.transaction;
    
    // Proposed (explicit)
    const transaction = await receipt.transaction();
@zfy0701
Copy link
Contributor

zfy0701 commented Jan 8, 2025

@spacedragon

@mshakeg
Copy link
Contributor Author

mshakeg commented Jan 22, 2025

Hi @spacedragon and @zfy0701 I've updated to @sentio/[email protected] which seems to have the explicit async function fetching of an entity/s from an original entity. However, it doesn't seem to work. Below is a PR that demos some of the issues I'm experiencing:

Canopyxyz/sentio-processors#5

Could you please investigate?

@spacedragon
Copy link
Contributor

Image

comment out these lines will make these entity available.

From reading your code, basic you are initiating another MemoryDatabase in the processor code but using the one in the TestProcessorServer

@mshakeg
Copy link
Contributor Author

mshakeg commented Jan 24, 2025

Hey @spacedragon thanks, that makes sense. However, how am I supposed to access ctx.store in my tests? As I can't just create a new Store and use that in my tests. I just pushed a commit to my PR 5 and showing this is issue.

// This does NOT work
it('should ...', withStoreContext(storeContext, async () => {
    const store = new Store(storeContext)

    // rest of the test implementation that uses the above store will not work as it's different from ctx.store that is used in the event handlers
}))

@spacedragon
Copy link
Contributor

spacedragon commented Jan 24, 2025

access through TestProcessorServer.store

Take a look at this docs I recently updated.

https://docs.sentio.xyz/docs/write-test#/

@mshakeg
Copy link
Contributor Author

mshakeg commented Jan 25, 2025

Hey @spacedragon I'm attempting to get service.store however that fails with the following error. Also service.db is undefined. I pushed this attempt to my PR 5 in commit a5cb1fe

TypeError [Error]: Cannot read properties of undefined (reading 'store')
    at TestProcessorServer.get store (/Users/user/dev/canopyxyz/sentio-processors/node_modules/.pnpm/@[email protected][email protected][email protected][email protected][email protected]_utf-_6xei4vtmie2nrqt36mdczrm254/node_modules/@sentio/sdk/src/testing/test-processor-server.ts:141:20)
    at TestContext.<anonymous> (/Users/user/dev/canopyxyz/sentio-processors/src/tests/multi-rewards/tests/stake.test.ts:227:43)

@spacedragon
Copy link
Contributor

spacedragon commented Jan 26, 2025

I see you try to access the service.store before the db got initiated. Either move these after the testOnTransaction or try
sdk 2.57.0-rc.3

@mshakeg
Copy link
Contributor Author

mshakeg commented Jan 26, 2025

thanks @spacedragon upgrading to 2.57.0-rc.3 worked. I'll continue testing and let you know if I have any other issues.

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