feat(ledger): add metadata at ledger level #1353
Conversation
WalkthroughThe recent modifications introduce the capability to manage ledger metadata, including updating and deleting specific metadata entries. This enhancement is reflected across various components of the system, from the backend logic and API controllers to integration with storage drivers and SDK updates. The changes facilitate more dynamic and flexible ledger configurations, allowing for a richer set of metadata operations. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
gfyrag
force-pushed
the
feat/ledger-add-metadata
branch
from
55fdfeb
to
4346f79
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
components/ledger/openapi.yamlis excluded by:!**/*.yamlcomponents/ledger/openapi/v2.yamlis excluded by:!**/*.yaml
Files selected for processing (31)
- components/ledger/internal/api/backend/backend.go (2 hunks)
- components/ledger/internal/api/backend/backend_generated.go (4 hunks)
- components/ledger/internal/api/v2/controllers_create_ledger_test.go (1 hunks)
- components/ledger/internal/api/v2/controllers_ledgers.go (2 hunks)
- components/ledger/internal/api/v2/controllers_ledgers_test.go (1 hunks)
- components/ledger/internal/api/v2/routes.go (1 hunks)
- components/ledger/internal/storage/driver/driver.go (3 hunks)
- components/ledger/internal/storage/systemstore/ledgers.go (5 hunks)
- components/ledger/internal/storage/systemstore/ledgers_test.go (3 hunks)
- components/ledger/internal/storage/systemstore/migrations.go (1 hunks)
- releases/sdks/go/.speakeasy/gen.lock (7 hunks)
- releases/sdks/go/README.md (2 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2deleteledgermetadatarequest.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2deleteledgermetadataresponse.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2getledgerresponse.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2updateledgermetadatarequest.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2updateledgermetadataresponse.md (1 hunks)
- releases/sdks/go/docs/pkg/models/shared/v2createledgerrequest.md (1 hunks)
- releases/sdks/go/docs/pkg/models/shared/v2getledgerresponse.md (1 hunks)
- releases/sdks/go/docs/pkg/models/shared/v2ledger.md (1 hunks)
- releases/sdks/go/docs/sdks/ledger/README.md (6 hunks)
- releases/sdks/go/ledger.go (3 hunks)
- releases/sdks/go/pkg/models/operations/v2deleteledgermetadata.go (1 hunks)
- releases/sdks/go/pkg/models/operations/v2getledger.go (2 hunks)
- releases/sdks/go/pkg/models/operations/v2updateledgermetadata.go (1 hunks)
- releases/sdks/go/pkg/models/shared/v2createledgerrequest.go (2 hunks)
- releases/sdks/go/pkg/models/shared/v2getledgerresponse.go (1 hunks)
- releases/sdks/go/pkg/models/shared/v2ledger.go (2 hunks)
- tests/integration/suite/ledger-create.go (2 hunks)
- tests/integration/suite/ledger-delete-metadata.go (1 hunks)
- tests/integration/suite/ledger-update-metadata.go (1 hunks)
Files skipped from review as they are similar to previous changes (21)
- components/ledger/internal/api/backend/backend.go
- components/ledger/internal/api/backend/backend_generated.go
- components/ledger/internal/api/v2/controllers_create_ledger_test.go
- components/ledger/internal/api/v2/controllers_ledgers.go
- components/ledger/internal/api/v2/controllers_ledgers_test.go
- components/ledger/internal/api/v2/routes.go
- components/ledger/internal/storage/driver/driver.go
- components/ledger/internal/storage/systemstore/ledgers_test.go
- components/ledger/internal/storage/systemstore/migrations.go
- releases/sdks/go/.speakeasy/gen.lock
- releases/sdks/go/docs/sdks/ledger/README.md
- releases/sdks/go/ledger.go
- releases/sdks/go/pkg/models/operations/v2deleteledgermetadata.go
- releases/sdks/go/pkg/models/operations/v2getledger.go
- releases/sdks/go/pkg/models/operations/v2updateledgermetadata.go
- releases/sdks/go/pkg/models/shared/v2createledgerrequest.go
- releases/sdks/go/pkg/models/shared/v2getledgerresponse.go
- releases/sdks/go/pkg/models/shared/v2ledger.go
- tests/integration/suite/ledger-create.go
- tests/integration/suite/ledger-delete-metadata.go
- tests/integration/suite/ledger-update-metadata.go
Additional comments: 17
releases/sdks/go/docs/pkg/models/shared/v2createledgerrequest.md (1)
- 9-9: The addition of the
Metadatafield toV2CreateLedgerRequestis well-documented and aligns with the PR's objectives. Ensure that the formatting, especially around theMetadatafield, adheres to the documentation standards for consistency and readability.releases/sdks/go/docs/pkg/models/operations/v2deleteledgermetadatarequest.md (1)
- 8-9: The introduction of
KeyandLedgerfields inV2DeleteLedgerMetadataRequestis well-documented and supports the PR's objectives for managing ledger metadata. Ensure that the formatting adheres to the documentation standards for consistency and readability.releases/sdks/go/docs/pkg/models/operations/v2updateledgermetadatarequest.md (1)
- 8-9: The introduction of
RequestBodyandLedgerfields inV2UpdateLedgerMetadataRequestis well-documented and supports the PR's objectives for updating ledger metadata. Ensure that the formatting adheres to the documentation standards for consistency and readability.releases/sdks/go/docs/pkg/models/shared/v2getledgerresponse.md (1)
- 1-8: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-12]
Ensure that the formatting in
V2GetLedgerResponseadheres to the documentation standards for consistency and readability. The structure appears to align with the PR's objectives, but a detailed review of any specific changes was not possible based on the provided context.releases/sdks/go/docs/pkg/models/shared/v2ledger.md (1)
- 10-10: The addition of the
Metadatafield toV2Ledgeris well-documented and aligns with the PR's objectives for managing ledger metadata. Ensure that the formatting, especially around theMetadatafield, adheres to the documentation standards for consistency and readability.releases/sdks/go/docs/pkg/models/operations/v2deleteledgermetadataresponse.md (1)
- 8-11: The documentation of the response structure in
V2DeleteLedgerMetadataResponseis clear and supports the PR's objectives for managing ledger metadata. Ensure that the formatting adheres to the documentation standards for consistency and readability.releases/sdks/go/docs/pkg/models/operations/v2updateledgermetadataresponse.md (1)
- 8-11: The documentation of the response structure in
V2UpdateLedgerMetadataResponseis clear and supports the PR's objectives for updating ledger metadata. Ensure that the formatting adheres to the documentation standards for consistency and readability.releases/sdks/go/docs/pkg/models/operations/v2getledgerresponse.md (1)
- 12-12: The renaming of the
V2Ledgerfield toV2GetLedgerResponseis well-documented and aligns with the PR's objectives for updating SDKs and internal APIs. Ensure that the formatting adheres to the documentation standards for consistency and readability.components/ledger/internal/storage/systemstore/ledgers.go (6)
- 22-22: Adding a
Metadatafield of typemap[string]stringto theLedgerstruct is a straightforward way to store key-value pairs. However, consider documenting the expected use of this field, especially regarding the size and type of data expected to be stored. Large or complex metadata could impact performance or storage.- 44-44: The inclusion of
"metadata"in the column list for theListLedgersquery ensures that metadata is fetched along with other ledger details. This is consistent with the objective of enhancing ledger functionality with metadata management. Ensure that the performance implications of fetching potentially large JSONB columns are considered, especially in listings with many ledgers.- 67-67: Similar to the
ListLedgersmethod, including"metadata"in theGetLedgerquery ensures consistency in how ledger details are fetched. This aligns with the goal of comprehensive metadata management. Again, be mindful of the potential performance impact when fetching large metadata objects.- 76-82: The
UpdateLedgerMetadatamethod correctly updates the metadata for a given ledger. It's important to ensure that themetadata.Metadatatype is compatible with themap[string]stringtype expected by theLedgerstruct. Additionally, consider handling potential conflicts or merges with existing metadata entries.- 85-91: The
DeleteLedgerMetadatamethod uses a PostgreSQL-specific operation (metadata = metadata - ?) to remove a key from the JSONBmetadatafield. This is an efficient way to handle the deletion of metadata entries. Ensure that this operation is supported by the database version in use and consider adding error handling for cases where the key does not exist.- 95-97: Initializing the
Metadatafield in theRegisterLedgermethod if it is nil is a good practice to ensure that the ledger object is always in a valid state. This preemptive initialization avoids potential null pointer exceptions when interacting with theMetadatafield.releases/sdks/go/README.md (3)
- 110-116: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-11]
The introduction section provides a good overview of the SDK's purpose and initial steps for customization and local development. However, it might be beneficial to clarify that while the SDK is generated based on the OpenAPI spec, users are encouraged to contribute or report issues for further improvements.
Consider adding a sentence encouraging users to contribute to the SDK or report issues for enhancements, making it clear that community feedback is valued.
- 110-116: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [113-127]
The addition of
V2DeleteLedgerMetadataandV2UpdateLedgerMetadatamethods to the SDK is clearly documented, providing users with the necessary functions to manage ledger metadata. This aligns with the PR objectives to enhance metadata management capabilities.Ensure that examples or further documentation on how to use these new methods effectively are also provided, either in this README or in linked detailed documentation.
- 110-116: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-500]
There are several grammatical and typographical errors identified by static analysis, such as possible spelling mistakes and repeated whitespaces. While some of these may be false positives, especially in code blocks or URLs, it's important to review and correct genuine errors in the documentation text.
Review the document for genuine grammatical and typographical errors and correct them to improve readability. For example, ensure consistent use of spaces after periods and correct any spelling mistakes in the non-code text.
| * [V2CreateLedger](docs/sdks/ledger/README.md#v2createledger) - Create a ledger | ||
| * [V2CreateTransaction](docs/sdks/ledger/README.md#v2createtransaction) - Create a new transaction to a ledger | ||
| * [V2DeleteAccountMetadata](docs/sdks/ledger/README.md#v2deleteaccountmetadata) - Delete metadata by key | ||
| * [V2DeleteLedgerMetadata](docs/sdks/ledger/README.md#v2deleteledgermetadata) - Delete ledger metadata by key | ||
| * [V2DeleteTransactionMetadata](docs/sdks/ledger/README.md#v2deletetransactionmetadata) - Delete metadata by key | ||
| * [V2GetAccount](docs/sdks/ledger/README.md#v2getaccount) - Get account by its address | ||
| * [V2GetBalancesAggregated](docs/sdks/ledger/README.md#v2getbalancesaggregated) - Get the aggregated balances from selected accounts |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-500]
Throughout the document, there are several instances of unpaired symbols, mostly related to markdown links. This could potentially break the formatting when viewed on platforms like GitHub or in markdown editors.
Ensure all markdown links are correctly formatted with no unpaired symbols. For example:
- [CreatePolicy](docs/sdks/reconciliation/README.md#cre
+ [CreatePolicy](docs/sdks/reconciliation/README.md#createpolicy)
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- components/ledger/internal/storage/systemstore/migrations.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- components/ledger/internal/storage/systemstore/migrations.go
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- components/fctl/cmd/ledger/accounts/delete_metadata.go (1 hunks)
- components/fctl/cmd/ledger/create.go (3 hunks)
- components/fctl/cmd/ledger/delete_metadata.go (1 hunks)
- components/fctl/cmd/ledger/list.go (2 hunks)
- components/fctl/cmd/ledger/root.go (1 hunks)
- components/fctl/cmd/ledger/set_metadata.go (1 hunks)
- components/fctl/cmd/ledger/transactions/delete_metadata.go (1 hunks)
- components/ledger/internal/storage/systemstore/ledgers.go (5 hunks)
Additional comments: 9
components/fctl/cmd/ledger/root.go (1)
- 22-23: The addition of
NewSetMetadataCommandandNewDeleteMetadataCommandto the ledger command suite is a logical extension for managing ledger metadata. Ensure that these commands are thoroughly tested and documented to align with the PR objectives.components/fctl/cmd/ledger/accounts/delete_metadata.go (1)
- 31-31: The change to
NewDeleteMetadataCommandto accept only a single<key>parameter simplifies the interface but limits batch deletion capabilities. Ensure that this behavior change is documented and communicated to users, especially if batch deletion was previously supported.components/fctl/cmd/ledger/list.go (1)
- 73-76: The inclusion of the creation timestamp and metadata in the ledger list enhances the information provided to users. Ensure that the metadata is presented in a concise and readable format, considering its potential complexity. It might be helpful to limit the amount of metadata displayed or provide a summarized view.
components/fctl/cmd/ledger/delete_metadata.go (1)
- 29-36: The implementation of
NewDeleteMetadataCommandfor ledgers aligns with the PR objectives of enhancing metadata management. Ensure that error handling is robust and user feedback is clear, especially in cases where the specified metadata key does not exist.components/fctl/cmd/ledger/transactions/delete_metadata.go (1)
- 31-31: The modification to
NewDeleteMetadataCommandto accept only a single<key>parameter for transaction metadata deletion simplifies the command interface. As with similar changes in other components, ensure that the implications of this limitation are well-documented and communicated to users.components/fctl/cmd/ledger/set_metadata.go (1)
- 31-38: The implementation of
NewSetMetadataCommandfor ledgers is a valuable addition, enhancing the ledger's metadata management capabilities. Ensure that the parsing of metadata key-value pairs is robust and that clear error messages are provided for invalid inputs.components/fctl/cmd/ledger/create.go (1)
- 45-45: The introduction of a
metadataflag in the ledger creation command is a significant enhancement, allowing users to specify metadata at creation time. Ensure that the metadata is correctly parsed and validated, providing clear error messages for invalid formats.components/ledger/internal/storage/systemstore/ledgers.go (2)
- 22-22: The addition of the
Metadatafield to theLedgerstruct is a key enhancement for supporting metadata management at the ledger level. Ensure that metadata is correctly validated and sanitized before storage to prevent potential security issues.- 76-92: The implementation of
UpdateLedgerMetadataandDeleteLedgerMetadatamethods provides the necessary functionality for managing ledger metadata. Ensure comprehensive testing of these methods, particularly edge cases such as attempting to delete non-existent metadata keys or updating metadata with invalid data types.
Add PUT /{ledger}/metadata urls to add metadata to ledgers.
Add DELETE /{ledger}/metadata/{key} to remove keys from metadata.
Fixes ENG-868
Summary by CodeRabbit