Access api methods - aggregate changes into main branch #63
Conversation
WalkthroughThe recent update to the Flow SDK introduces new methods for retrieving transactions, transaction results, and execution results by block ID. Both synchronous and asynchronous versions of these methods have been added to their respective interfaces and implementations. Additionally, new data models for handling execution results, service events, and data chunks have been introduced. Comprehensive tests have been included to validate these new functionalities, ensuring robustness and reliability. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AsyncFlowAccessApi
participant FlowAccessApiImpl
participant FlowBlockchain
Note over Client,AsyncFlowAccessApi: Asynchronous API call to get transactions by block ID
Client->>AsyncFlowAccessApi: getTransactionsByBlockId(blockId)
AsyncFlowAccessApi->>FlowAccessApiImpl: getTransactionsByBlockId(blockId)
FlowAccessApiImpl->>FlowBlockchain: fetchTransactions(blockId)
FlowBlockchain->>FlowAccessApiImpl: List<FlowTransaction>
FlowAccessApiImpl->>AsyncFlowAccessApi: List<FlowTransaction>
AsyncFlowAccessApi->>Client: CompletableFuture<List<FlowTransaction>>
Note over Client,FlowAccessApiImpl: Synchronous API call to get execution result by block ID
Client->>FlowAccessApiImpl: getExecutionResultByBlockId(blockId)
FlowAccessApiImpl->>FlowBlockchain: fetchExecutionResult(blockId)
FlowBlockchain->>FlowAccessApiImpl: FlowExecutionResult
FlowAccessApiImpl->>Client: FlowExecutionResult
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 (
|
lealobanov
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- src/main/kotlin/org/onflow/flow/sdk/AsyncFlowAccessApi.kt (1 hunks)
- src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (1 hunks)
- src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (1 hunks)
- src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (1 hunks)
- src/main/kotlin/org/onflow/flow/sdk/models.kt (1 hunks)
- src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (1 hunks)
- src/test/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImplTest.kt (2 hunks)
- src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2 hunks)
Additional context used
Learnings (6)
src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (1)
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.src/main/kotlin/org/onflow/flow/sdk/AsyncFlowAccessApi.kt (1)
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (2)
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:122-130 Timestamp: 2024-07-03T12:37:31.319Z Learning: In the `FlowAccessApiImpl` and `AsyncFlowAccessApiImpl` classes, the `transactionResultsList` is non-nullable, making the use of `.orEmpty()` redundant.src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2)
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:122-130 Timestamp: 2024-07-03T12:37:31.319Z Learning: In the `FlowAccessApiImpl` and `AsyncFlowAccessApiImpl` classes, the `transactionResultsList` is non-nullable, making the use of `.orEmpty()` redundant.src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2)
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:122-130 Timestamp: 2024-07-03T12:37:31.319Z Learning: In the `FlowAccessApiImpl` and `AsyncFlowAccessApiImpl` classes, the `transactionResultsList` is non-nullable, making the use of `.orEmpty()` redundant.src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (2)
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:122-130 Timestamp: 2024-07-03T12:37:31.319Z Learning: In the `FlowAccessApiImpl` and `AsyncFlowAccessApiImpl` classes, the `transactionResultsList` is non-nullable, making the use of `.orEmpty()` redundant.
Additional comments not posted (30)
src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (3)
57-57: New Method Added: getTransactionsByBlockIdThis method appears correctly defined and follows the existing pattern of method signatures within the interface. The return type and parameters are consistent with the interface's design.
59-59: New Method Added: getTransactionResultsByBlockIdThe method signature is consistent with the interface's design principles and correctly retrieves transaction results by block ID.
61-61: New Method Added: getExecutionResultByBlockIdThis method is properly added to fetch execution results by block ID. The method signature aligns well with the existing structure of the interface.
src/main/kotlin/org/onflow/flow/sdk/AsyncFlowAccessApi.kt (3)
53-53: New Asynchronous Method Added: getTransactionsByBlockIdThe method correctly returns a
CompletableFuturewith the expected response type, adhering to the asynchronous pattern of the interface.
55-55: New Asynchronous Method Added: getTransactionResultsByBlockIdThis method is appropriately defined to handle asynchronous fetching of transaction results by block ID.
57-57: New Asynchronous Method Added: getExecutionResultByBlockIdThe method signature is correct for asynchronous operations and returns a nullable
FlowExecutionResult, which is a thoughtful design choice considering that execution results might not always be available.src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (5)
247-256: New Test Method: Test getTransactionsByBlockIdThe test method is well-structured, mocking dependencies correctly and asserting the expected behavior. It ensures that the method returns the correct transactions for a given block ID.
259-278: New Test Method: Test getTransactionsByBlockId with multiple resultsThis test method effectively checks the scenario where multiple transactions are returned. It correctly asserts the size of the result and the equality of expected transactions.
281-290: New Test Method: Test getTransactionResultsByBlockIdThe test setup and assertions are correct, ensuring that the method behaves as expected when fetching transaction results by block ID.
293-312: New Test Method: Test getTransactionResultsByBlockId with multiple resultsThis test method is comprehensive, checking for multiple transaction results and asserting their correctness and order.
315-324: New Test Method: Test getExecutionResultByBlockIdThe test is well-implemented, covering the scenario of fetching an execution result by block ID and asserting the correct response.
src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2)
339-349: Implementation of getTransactionsByBlockIdThe method implementation correctly handles the API call and maps the response to the expected type. The error handling is also appropriately managed.
352-362: Implementation of getTransactionResultsByBlockIdThe method is implemented to fetch transaction results by block ID correctly. The mapping of the response and error handling are well executed.
src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (4)
13-13: Added import forExecutionResultOuterClassis appropriate.This import is necessary for the new test methods that deal with execution results.
374-391: Review of new test method for execution results retrieval by block ID.The test method correctly mocks the API response for execution results and asserts the expected outcome. The structure and logic are consistent with the existing test framework.
344-372: Review of new test methods for transaction results retrieval by block ID.These tests are well-structured and ensure that the transaction results are fetched and asserted correctly. Similar to the previous comment, verify the use of
.orEmpty()for transaction results.Verification successful
Verification successful: No occurrences of
.orEmpty()found in transaction results retrieval methods.The search confirmed that the
.orEmpty()method is not used in theFlowAccessApiImplclass or the associated test file. Thus, the concern about its usage is not applicable.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the use of `.orEmpty()` in transaction results retrieval methods. # Test: Search for the use of `.orEmpty()` in the transaction results retrieval methods. Expect: No occurrences. rg --type kotlin --context 5 $'.orEmpty()' src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktLength of output: 109
Script:
#!/bin/bash # Description: Verify the use of `.orEmpty()` in the FlowAccessApiImpl class. # Test: Search for the use of `.orEmpty()` in the FlowAccessApiImpl class. Expect: No occurrences. rg --type kotlin '.orEmpty()' src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktLength of output: 93
314-342: Review of new test methods for transaction retrieval by block ID.These tests appropriately mock the API responses and verify that the transactions are retrieved correctly. However, ensure that the use of
.orEmpty()is avoided if the transactions list is non-nullable.Verification successful
Verification successful: Non-nullable transactions list.
The
getTransactionsByBlockIdmethod inFlowAccessApiImpluses thetransactionsListdirectly without.orEmpty(), confirming it is treated as non-nullable.
src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt: getTransactionsByBlockIdScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the use of `.orEmpty()` in transaction retrieval methods. # Test: Search for the use of `.orEmpty()` in the transaction retrieval methods. Expect: No occurrences. rg --type kotlin --context 5 $'.orEmpty()' src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktLength of output: 109
Script:
#!/bin/bash # Description: Verify the non-nullability of the transactions list in the getTransactionsByBlockId method. # Step 1: Locate the implementation of the getTransactionsByBlockId method in the FlowAccessApiImpl class. ast-grep --lang kotlin --pattern 'fun getTransactionsByBlockId($_) { $$$ }' > method_implementation.txt # Step 2: Check the method implementation for the transactions list and its non-nullability. cat method_implementation.txt | rg 'transactionsList'Length of output: 131
Script:
#!/bin/bash # Description: Locate and inspect the implementation of the getTransactionsByBlockId method in the FlowAccessApiImpl class. # Step 1: Locate the implementation of the getTransactionsByBlockId method in the FlowAccessApiImpl class. ast-grep --lang kotlin --pattern 'fun getTransactionsByBlockId($_) { $$$ }' src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktLength of output: 139
Script:
#!/bin/bash # Description: Locate and inspect the implementation of the getTransactionsByBlockId method across the entire codebase. # Step 1: Locate the implementation of the getTransactionsByBlockId method across the entire codebase. ast-grep --lang kotlin --pattern 'fun getTransactionsByBlockId($_) { $$$ }'Length of output: 77
Script:
#!/bin/bash # Description: Locate and inspect the implementation of the getTransactionsByBlockId method across the entire codebase. # Step 1: Locate the implementation of the getTransactionsByBlockId method across the entire codebase. rg 'fun getTransactionsByBlockId' -A 20Length of output: 6353
src/main/kotlin/org/onflow/flow/sdk/models.kt (3)
678-736: Review ofFlowChunkclass: Properly implemented with necessary methods.The
FlowChunkclass is well-implemented with all necessary properties and methods for a Kotlin data class. Theofmethod correctly converts from gRPC types, and theequalsandhashCodemethods are properly overridden to handle all properties, ensuring correct behavior in collections and other contexts.
738-764: Review ofFlowServiceEventclass: Well-formed with essential methods.The
FlowServiceEventclass is correctly implemented and includes all necessary methods for a Kotlin data class. Theofmethod accurately converts from gRPC types, and theequalsandhashCodemethods are effectively overridden to handle the properties, ensuring accurate behavior in collections and other contexts.
766-780: Review ofFlowExecutionResultclass: Correctly integrates new features.The
FlowExecutionResultclass is well-integrated with the new features, correctly handling lists ofFlowChunkandFlowServiceEvent. Theofmethod effectively converts from gRPC types, ensuring that all properties are correctly initialized. This class ties together the individual components of the execution result, facilitating easier access and manipulation of the data.src/test/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImplTest.kt (8)
8-8: Approved new imports for enhanced testing capabilities.The imports for
assertThrows,ExecutionResultOuterClass,Executors,TimeUnit, andTimeoutExceptionare necessary for the new test functionalities, including exception handling and asynchronous operations.Also applies to: 15-15, 19-21
27-30: Approved addition of companion object for centralized test constants.The
BLOCK_ID_BYTESandPARENT_ID_BYTESconstants are well-placed in a companion object, enhancing readability and maintainability of tests by centralizing mock data.
319-330: Approved test forgetTransactionsByBlockId.The test method is well-constructed, appropriately using mocks and assertions to verify that the
getTransactionsByBlockIdmethod processes and returns the expected results correctly.
332-347: Approved test for handling multiple transactions ingetTransactionsByBlockId.This test method effectively verifies that the method can handle multiple transactions correctly, using mocks and assertions appropriately.
349-360: Approved test forgetTransactionResultsByBlockId.The test method correctly simulates the API response for transaction results and uses assertions to verify the correctness of the functionality.
362-377: Approved comprehensive test for multiple transaction results ingetTransactionResultsByBlockId.This test method effectively ensures that multiple transaction results are handled correctly, using a consistent testing approach with appropriate mocks and assertions.
379-427: Approved detailed test forgetExecutionResultByBlockId.This test method is comprehensive, covering all aspects of execution results, including chunks and service events. It effectively uses mocks and assertions to ensure the functionality is correctly implemented.
429-444: Approved test for handling timeout exceptions ingetTransactionsByBlockId.This test method correctly simulates a timeout scenario and uses
assertThrowsto verify that aTimeoutExceptionis appropriately thrown, demonstrating good practice in testing asynchronous operations.src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (2)
539-558: Review:getTransactionResultsByBlockIdmethod implementation.The structure and error handling in this method are well-implemented. The use of
CompletableFuturefor asynchronous operations is appropriate.Like the previous method, verify if
.orEmpty()is used unnecessarily withresponse.transactionResultsList, as indicated by previous learnings.Skipped due to learnings
Learnt from: lealobanov PR: onflow/flow-jvm-sdk#55 File: src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt:109-117 Timestamp: 2024-07-03T12:36:05.001Z Learning: `transactionsList` in the `getTransactionsByBlockID` method of `FlowAccessApiImpl` is non-nullable, making the use of `.orEmpty()` redundant.
561-582: Review:getExecutionResultByBlockIdmethod implementation.The method is well-structured for fetching execution results by block ID. The handling of the nullable
FlowExecutionResultis appropriate, ensuring that an error is returned when the execution result is not found.Consider verifying the handling of null values and exceptions to ensure robustness.
| override fun getTransactionsByBlockId(id: FlowId): CompletableFuture<FlowAccessApi.AccessApiCallResponse<List<FlowTransaction>>> { | ||
| return try { | ||
| completableFuture( | ||
| try { | ||
| api.getTransactionsByBlockID( | ||
| Access.GetTransactionsByBlockIDRequest.newBuilder().setBlockId(id.byteStringValue).build() | ||
| ) | ||
| } catch (e: Exception) { | ||
| return CompletableFuture.completedFuture(FlowAccessApi.AccessApiCallResponse.Error("Failed to get transactions by block ID", e)) | ||
| } | ||
| ).handle { response, ex -> | ||
| if (ex != null) { | ||
| FlowAccessApi.AccessApiCallResponse.Error("Failed to get transactions by block ID", ex) | ||
| } else { | ||
| FlowAccessApi.AccessApiCallResponse.Success(response.transactionsList.map { FlowTransaction.of(it) }) | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| CompletableFuture.completedFuture(FlowAccessApi.AccessApiCallResponse.Error("Failed to get transactions by block ID", e)) | ||
| } |
There was a problem hiding this comment.
Review: getTransactionsByBlockId method implementation.
This method correctly implements asynchronous fetching of transactions by block ID. The use of CompletableFuture and proper exception handling within the try-catch blocks is consistent with Java's asynchronous programming patterns.
However, ensure that .orEmpty() is not used unnecessarily when handling response.transactionsList, as per previous learnings indicating that the list is non-nullable.
Description
Pulls in changes from #61 to main
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
New Features
FlowChunk,FlowServiceEvent,FlowExecutionResult) for detailed execution result handling.Tests