-
Notifications
You must be signed in to change notification settings - Fork 8
Limit query depth and convert state change schema to interface #304
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
Conversation
Introduces shared resolver helper functions for common field resolution patterns in state change GraphQL resolvers, reducing duplication and improving maintainability. Updates all state change resolvers to use these helpers for nullable/required strings, JSONB fields, and arrays. Also updates account resolution logic to use accountID directly and makes minor import and type declaration cleanups.
Updated StateChangeEdge's node field to use the BaseStateChange interface instead of the StateChange type in GraphQL schema, models, and resolvers. Added type conversion logic to resolve the correct concrete type for BaseStateChange based on StateChangeCategory. This improves extensibility and supports polymorphic state change models in the API.
Introduces baseStateChangeWithCursor and refactors pagination logic to use converted state changes for account, operation, transaction, and query resolvers. Updates test assertions to use a new extractStateChangeIDs helper for cursor extraction, improving type safety and consistency across resolver and test code.
Introduces AccountModel.BatchGetByStateChangeIDs and supporting types to efficiently batch-load accounts associated with state changes. Updates GraphQL dataloaders and resolvers to use this method, reducing N+1 queries for state change account resolution. Includes tests for the new batch loading logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just dug into the schema to start and will follow up with a review of the implementation once we align on any schema changes. Super excited to see this!
| start := time.Now() | ||
| err := m.DB.SelectContext(ctx, &accountsWithStateChanges, query) | ||
| duration := time.Since(start).Seconds() | ||
| m.MetricsService.ObserveDBQueryDuration("SELECT", "accounts", duration) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metric collection can be pulled out into a helper/util
Refactors GraphQL schema, Go models, and resolvers to rename PaymentStateChange to BalanceStateChange and SignatureThresholdsStateChange to SignerThresholdsStateChange. Updates related field names for consistency (e.g., spenderAccountId to spenderAddress, signerAccountId to signerAddress, sponsorAccountId to sponsorAddress) and adjusts type mappings in gqlgen.yml and generated code accordingly.
Replaces 'stateChangeCategory' with 'type' and 'stateChangeReason' with 'reason' in GraphQL schema and resolvers for all StateChange types. Updates Go model methods and generated code to match new field names, ensuring consistency and explicit resolver usage for these fields.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Removed unused LiabilityStateChange and AllowanceStateChange models and related GraphQL types. Added AccountStateChange, TrustlineStateChange, and BalanceAuthorizationStateChange models and schema types. Updated enums and resolvers to match new state change categories and reasons, improving clarity and maintainability of state change handling.
Updated usage of StateChangeReason in test utilities to use variables instead of pointers to constants. Refactored resolver type definitions in statechange.resolvers.go to use a type group for improved readability. Fixed test cases to use StateChangeCategoryBalance instead of StateChangeCategoryCredit. Adjusted extractStateChangeIDs to handle correct model types.
Introduced StateChangeCursorGetter interface and implemented GetCursor method for StateChange. Updated extractStateChangeIDs to use the interface, reducing repetitive type assertions and improving code maintainability.
Renamed SponsorshipStateChange to ReservesStateChange across GraphQL schema, types, and resolvers for clarity. Updated state change category and reasons to use 'RESERVES', 'SPONSOR', and 'UNSPONSOR'. Refactored effect processor logic to create separate state changes for sponsoring and sponsored accounts, improving accuracy and maintainability.
Removed unused targetChange from processSponsorshipEffect and adjusted the return value to only include sponsorChanges. Updated tests to expect the correct number and type of state changes for sponsorship updates, reflecting the new logic.
Refactors GraphQL schema and resolvers to replace SponsorshipStateChange with ReservesStateChange, updating all related types, interfaces, and field resolvers for clarity and consistency. Also updates the effects processor to remove unused parameters from processSponsorshipEffect.
Replaces panic placeholders with actual resolver implementations for various state change models in statechange.resolvers.go. Removes unused resolveStringArray helper from resolver.go. This enables proper GraphQL responses for state change queries.
Renames various GraphQL types and resolvers related to state changes for improved clarity and consistency. For example, BalanceStateChange becomes StandardBalanceChange, AccountStateChange becomes AccountChange, and similar updates are made for other state change types across gqlgen.yml, Go models, resolvers, tests, and the GraphQL schema.
9f8176e to
f2421d1
Compare
What
Major GraphQL API improvements focusing on security, performance, and maintainability:
https://gqlgen.com/reference/complexity/)
StateChangetype to extensibleBaseStateChangeinterface with concrete implementations, enabling polymorphic state change models- Introduces
AccountModel.BatchGetByStateChangeIDsfor efficient batch loading to eliminate N+1 queries- Refactors pagination logic with baseStateChangeWithCursor for consistency across resolvers
The state change query is now converted to the following design:
Why
Issue that this PR addresses
#250 #305