Access api subscription endpoints #58
Conversation
WalkthroughThe recent updates significantly enhance the Flow SDK, focusing on asynchronous programming and improved data access. Key changes include the introduction of new subscription methods for execution data and events based on block ID and height. The integration of Kotlin coroutines streamlines concurrent processing, boosting the SDK's efficiency and usability while enabling real-time data handling. 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 Configuration File (
|
Integration Test Results2 files - 3 2 suites - 3 12s ⏱️ - 1m 39s Results for commit 10f1de8. ± Comparison against base commit 2feb1f7. This pull request removes 31 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
lealobanov
changed the base branch from
access-api-remove-null-returns
to
access-api-methods-v2
lealobanov
changed the base branch from
access-api-methods-v2
to
access-api-methods-v3
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: 0
Outside diff range, codebase verification and nitpick comments (1)
src/main/kotlin/org/onflow/flow/sdk/Flow.kt (1)
51-52: Function calls tonewAccessApineed to be updated to handle the new parameter.The following files have calls to
newAccessApithat need to be updated to include the new parameter forExecutionDataAPIGrpc.newBlockingStub(channel):
src/testFixtures/kotlin/org/onflow/flow/sdk/test/FlowTestUtil.ktsrc/testFixtures/kotlin/org/onflow/flow/sdk/test/AbstractFlowEmulatorExtension.ktsrc/test/kotlin/org/onflow/flow/sdk/TestUtils.ktsrc/intTest/org/onflow/flow/sdk/IntegrationTestUtils.ktPlease update these calls to match the new function signature.
Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
newAccessApiare updated to handle the new parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `newAccessApi` handle the new parameter. # Test: Search for the function usage. Expect: Only occurances of the updated function signature. rg --type kotlin -A 5 $'newAccessApi'Length of output: 3577
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- build.gradle.kts (2 hunks)
- src/main/kotlin/org/onflow/flow/sdk/Flow.kt (2 hunks)
- src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2 hunks)
- src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- build.gradle.kts
- src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt
Additional comments not posted (14)
src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (14)
47-48: LGTM!The addition of the new mock for
ExecutionDataAPIGrpc.ExecutionDataAPIBlockingStubis appropriate and correctly implemented.
334-343: LGTM!The test function
getTransactionsByBlockIdcorrectly validates the retrieval of transactions by block ID.
346-361: LGTM!The test function
getTransactionsByBlockId with multiple resultscorrectly validates the retrieval of multiple transactions by block ID.
364-373: LGTM!The test function
getTransactionResultsByBlockIdcorrectly validates the retrieval of transaction results by block ID.
376-391: LGTM!The test function
getTransactionResultsByBlockId with multiple resultscorrectly validates the retrieval of multiple transaction results by block ID.
394-410: LGTM!The test function
getExecutionResultByBlockIdcorrectly validates the retrieval of execution results by block ID.
420-448: LGTM!The test function
subscribeExecutionDataByBlockHeight success casecorrectly validates the subscription to execution data by block height in a successful scenario.
451-479: LGTM!The test function
subscribeExecutionDataByBlockHeight error casecorrectly validates the subscription to execution data by block height in an error scenario.
482-510: LGTM!The test function
subscribeEventsByBlockId success casecorrectly validates the subscription to events by block ID in a successful scenario.
513-541: LGTM!The test function
subscribeEventsByBlockId error casecorrectly validates the subscription to events by block ID in an error scenario.
544-572: LGTM!The test function
subscribeEventsByBlockHeight success casecorrectly validates the subscription to events by block height in a successful scenario.
575-603: LGTM!The test function
subscribeEventsByBlockHeight error casecorrectly validates the subscription to events by block height in an error scenario.
606-634: LGTM!The test function
subscribeExecutionDataByBlockId success casecorrectly validates the subscription to execution data by block ID in a successful scenario.
637-665: LGTM!The test function
subscribeExecutionDataByBlockId error casecorrectly validates the subscription to execution data by block ID in an error scenario.
|
|
||
| when (val result = flowAccessApiImpl.subscribeExecutionDataByBlockHeight(blockHeight)) { | ||
| is FlowAccessApi.AccessApiCallResponse.Success -> { | ||
| fail("Expected error but got success") | ||
| val (_, errorChannel) = result.data |
There was a problem hiding this comment.
@jribbink after switching the implementation to be fully non-blocking the result of the error channel is fed through FlowAccessApi.AccessApiCallResponse.Success which is not ideal.
I was playing around with a few different options of how to handle this, but I think we would be forced to add a blocking step to the function, e.g. something along the lines of
var receivedException: Throwable? = null
runBlocking {
// Try to receive from the error channel
try {
receivedException = errorChannel.receive()
} catch (e: ClosedReceiveChannelException) {
// No errors
}
}
if (receivedException != null) {
FlowAccessApi.AccessApiCallResponse.Error("Failed to subscribe execution data by block height", receivedException)
} else {
FlowAccessApi.AccessApiCallResponse.Success(responseChannel to errorChannel)
}
which is not what we want to have. Let me know what you think - thanks!
There was a problem hiding this comment.
I think that wrapping this response in a FlowAccessApi.AccessApiCallResponse is probably just redundant here since for these streams we have the error channel anyways. It implies a success/failure state when it will always return "Success" anyways with this implementation.
SDK Examples Unit Test Results2 files 2 suites 34s ⏱️ Results for commit 10f1de8. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- sdk/build.gradle.kts (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (13 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (2 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2 hunks)
Files skipped from review due to trivial changes (1)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt
Additional comments not posted (33)
sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (4)
76-78: LGTM! But verify the function implementation.The code changes are approved.
However, ensure that the function is implemented correctly and that error handling is in place.
Verification successful
The previous search did not yield any results. Let's adjust our search pattern to ensure we capture the correct function implementation and search again.
LGTM! The function
subscribeEventsByBlockHeightis implemented correctly with proper error handling.
- The implementation in
sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktcreates response and error channels.- It uses a coroutine to handle asynchronous event subscription.
- Exceptions are caught, and channels are closed in the
finallyblock to ensure resource cleanup.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `subscribeEventsByBlockHeight`. # Test: Search for the function implementation. Expect: Proper implementation with error handling. rg --type kotlin -A 10 $'fun subscribeEventsByBlockHeight(height: Long)'Length of output: 73
Script:
#!/bin/bash # Description: Verify the implementation of `subscribeEventsByBlockHeight`. # Test: Search for the function implementation. Expect: Proper implementation with error handling. rg --type kotlin -A 20 'fun subscribeEventsByBlockHeight'Length of output: 2768
68-71: LGTM! But verify the function implementation.The code changes are approved.
However, ensure that the function is implemented correctly and that error handling is in place.
Verification successful
LGTM! The function implementation is correct and includes error handling.
The function
subscribeExecutionDataByBlockHeightis implemented insdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktwith proper error handling using atry-catchblock.
- Implementation location:
sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `subscribeExecutionDataByBlockHeight`. # Test: Search for the function implementation. Expect: Proper implementation with error handling. rg --type kotlin -A 10 $'fun subscribeExecutionDataByBlockHeight(height: Long)'Length of output: 80
Script:
#!/bin/bash # Description: Verify the implementation of `subscribeExecutionDataByBlockHeight`. # Test: Search for the function implementation using ast-grep. Expect: Proper implementation with error handling. ast-grep --lang kotlin --pattern $'fun subscribeExecutionDataByBlockHeight(height: Long): AccessApiCallResponse<Pair<ReceiveChannel<FlowBlockExecutionData>, ReceiveChannel<Throwable>>> { $$$ }'Length of output: 196
Script:
#!/bin/bash # Description: Verify the implementation of `subscribeExecutionDataByBlockHeight`. # Test: Perform a broader search for the function name. rg --type kotlin -A 10 'fun subscribeExecutionDataByBlockHeight'Length of output: 2474
64-67: LGTM! But verify the function implementation.The code changes are approved.
However, ensure that the function is implemented correctly and that error handling is in place.
72-75: LGTM! But verify the function implementation.The code changes are approved.
However, ensure that the function is implemented correctly and that error handling is in place.
sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (1)
51-52: LGTM! But verify the function implementation.The code changes are approved.
However, ensure that the function is implemented correctly and that the coroutine scope is properly managed.
Verification successful
LGTM! But verify the function implementation.
The function
newAccessApiis correctly implemented and the coroutine scope is properly managed.
sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `newAccessApi`. # Test: Search for the function implementation. Expect: Proper implementation with coroutine scope management. rg --type kotlin -A 10 $'fun newAccessApi(host: String, port: Int = 9000, secure: Boolean = false, userAgent: String = DEFAULT_USER_AGENT, maxMessageSize: Int = DEFAULT_MAX_MESSAGE_SIZE): FlowAccessApi'Length of output: 203
Script:
#!/bin/bash # Description: Verify the implementation of `newAccessApi`. # Test: Search for the function implementation with a more general pattern. rg --type kotlin -A 20 'fun newAccessApi'Length of output: 2281
sdk/build.gradle.kts (3)
37-37: Approved: Added dependency for Kotlin coroutines core.The addition of
kotlinx-coroutines-core:1.5.2is necessary for coroutine support in Kotlin.
38-38: Approved: Added dependency for Kotlin coroutines core JVM.The addition of
kotlinx-coroutines-core-jvm:1.5.2is necessary for coroutine support in JVM-based Kotlin projects.
39-39: Approved: Added dependency for Kotlin coroutines Guava.The addition of
kotlinx-coroutines-guava:1.5.2is necessary for coroutine support with Guava in Kotlin.sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (5)
17-19: Constructor changes approved.The addition of
executionDataApiandcoroutineScopeparameters is necessary for the new subscription methods.
390-415: Method implementation approved.The
subscribeExecutionDataByBlockIdmethod correctly handles errors, closes channels, and uses coroutines efficiently.However, ensure that all function calls to
subscribeExecutionDataByBlockIdmatch the new implementation.Verification successful
Method implementation approved.
The
subscribeExecutionDataByBlockIdmethod correctly handles errors, closes channels, and uses coroutines efficiently. The usage of the method in the test filesFlowAccessApiTest.ktandFlowAccessApiImplTest.ktmatches the new implementation.
sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.ktsdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktThese test cases cover both success and error scenarios, ensuring that the new implementation is thoroughly tested.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeExecutionDataByBlockId` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type kotlin -A 5 $'subscribeExecutionDataByBlockId'Length of output: 6872
472-497: Method implementation approved.The
subscribeEventsByBlockHeightmethod correctly handles errors, closes channels, and uses coroutines efficiently.However, ensure that all function calls to
subscribeEventsByBlockHeightmatch the new implementation.Verification successful
Method implementation approved.
The
subscribeEventsByBlockHeightmethod correctly handles errors, closes channels, and uses coroutines efficiently. The function calls in the test filesFlowAccessApiTest.ktandFlowAccessApiImplTest.ktmatch the new implementation and comprehensively test various scenarios.
sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.ktsdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktsdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.ktsdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeEventsByBlockHeight` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type kotlin -A 5 $'subscribeEventsByBlockHeight'Length of output: 6501
445-470: Method implementation approved.The
subscribeEventsByBlockIdmethod correctly handles errors, closes channels, and uses coroutines efficiently.However, ensure that all function calls to
subscribeEventsByBlockIdmatch the new implementation.Verification successful
Method implementation approved.
The
subscribeEventsByBlockIdmethod correctly handles errors, closes channels, and uses coroutines efficiently. The function calls tosubscribeEventsByBlockIdmatch the new implementation, and the tests cover both success and error cases.
FlowAccessApi.kt: Definition ofsubscribeEventsByBlockId.FlowAccessApiImpl.kt: Implementation ofsubscribeEventsByBlockId.FlowAccessApiTest.kt: Tests forsubscribeEventsByBlockId.FlowAccessApiImplTest.kt: Additional tests forsubscribeEventsByBlockId.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeEventsByBlockId` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type kotlin -A 5 $'subscribeEventsByBlockId'Length of output: 6628
417-443: Method implementation approved.The
subscribeExecutionDataByBlockHeightmethod correctly handles errors, closes channels, and uses coroutines efficiently.However, ensure that all function calls to
subscribeExecutionDataByBlockHeightmatch the new implementation.Verification successful
Method implementation approved.
The
subscribeExecutionDataByBlockHeightmethod correctly handles errors, closes channels, and uses coroutines efficiently.The function calls to
subscribeExecutionDataByBlockHeightare consistent with the new implementation across the codebase. The method is being used and tested appropriately.
- Instances found in
sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.ktandsdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt.- Tests found in
sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.ktandsdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeExecutionDataByBlockHeight` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type kotlin -A 5 $'subscribeExecutionDataByBlockHeight'Length of output: 6939
sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (4)
332-372: Test case approved.The
Test subscribeExecutionDataByBlockIdtest case covers both success and error scenarios and uses coroutines correctly.
374-414: Test case approved.The
Test subscribeExecutionDataByBlockHeighttest case covers both success and error scenarios and uses coroutines correctly.
417-456: Test case approved.The
Test subscribeEventsByBlockIdtest case covers both success and error scenarios and uses coroutines correctly.
458-498: Test case approved.The
Test subscribeEventsByBlockHeighttest case covers both success and error scenarios and uses coroutines correctly.sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (8)
419-448: Test case approved.The
Test subscribeExecutionDataByBlockHeight success casetest case covers the success scenario and uses coroutines correctly.
450-479: Test case approved.The
Test subscribeExecutionDataByBlockHeight error casetest case covers the error scenario and uses coroutines correctly.
481-510: Test case approved.The
Test subscribeEventsByBlockId success casetest case covers the success scenario and uses coroutines correctly.
512-541: Test case approved.The
Test subscribeEventsByBlockId error casetest case covers the error scenario and uses coroutines correctly.
543-572: Test case approved.The
Test subscribeEventsByBlockHeight success casetest case covers the success scenario and uses coroutines correctly.
575-603: Test case approved.The
Test subscribeEventsByBlockHeight error casetest case covers the error scenario and uses coroutines correctly.
605-634: Test case approved.The
Test subscribeExecutionDataByBlockId success casetest case covers the success scenario and uses coroutines correctly.
636-665: Test case approved.The
Test subscribeExecutionDataByBlockId error casetest case covers the error scenario and uses coroutines correctly.sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (8)
784-798: Ensure proper handling ofgrpcExecutionResultinFlowChunkExecutionData.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
800-830: Ensure proper handling ofgrpcExecutionResultinFlowTrieUpdate.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
832-858: Ensure proper handling ofgrpcExecutionResultinFlowPayload.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
860-886: Ensure proper handling ofgrpcExecutionResultinFlowKeyPart.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
888-901: Ensure proper handling ofgrpcExecutionResultinFlowExecutionDataTransactionResult.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
903-911: Ensure proper handling ofgrpcExecutionResultinFlowExecutionDataCollection.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
913-925: Ensure proper handling ofgrpcExecutionResultinFlowBlockExecutionData.The
offunction in the companion object should handle potential null values and errors fromgrpcExecutionResult.Ensure that
grpcExecutionResultis always non-null and valid before calling this function. If not, consider adding null checks or error handling.
1087-1089: Ensure proper error handling indecodeToAny.The
decodeToAnyfunction should handle potential errors during decoding.Ensure that the
decodeToAnyfunction properly handles errors during decoding. If not, consider adding error handling.
| val signerList: List<FlowAddress> | ||
| get() { | ||
| val ret = mutableListOf<FlowAddress>() | ||
| val seen = mutableSetOf<FlowAddress>() | ||
| val addSigner = fun(address: FlowAddress) { | ||
| if (address in seen) { | ||
| return | ||
| } | ||
| ret.add(address) | ||
| seen.add(address) | ||
| } | ||
| ret.add(address) | ||
| seen.add(address) | ||
| addSigner(proposalKey.address) | ||
| addSigner(payerAddress) | ||
| authorizers.forEach(addSigner) | ||
| return ret |
There was a problem hiding this comment.
Improve the performance of the signerList property.
The current implementation of signerList uses a mutable list and a set to track seen addresses, which can be optimized. Consider using a LinkedHashSet to maintain insertion order and eliminate duplicates in a single data structure.
val signerList: List<FlowAddress>
get() {
val seen = LinkedHashSet<FlowAddress>()
seen.add(proposalKey.address)
seen.add(payerAddress)
seen.addAll(authorizers)
return seen.toList()
}| val signerMap: Map<FlowAddress, Int> | ||
| get() { | ||
| return signerList.withIndex() | ||
| .map { it.value to it.index } | ||
| .toMap() | ||
| } |
There was a problem hiding this comment.
Optimize the signerMap property.
The current implementation of signerMap can be simplified by using the associateWith function.
val signerMap: Map<FlowAddress, Int>
get() = signerList.withIndex().associate { it.value to it.index }| val ret = this.events.filter { | ||
| if (exact) { | ||
| it.type == type | ||
| } else { | ||
| it.type.endsWith(type) | ||
| } | ||
| }.map { it.event } |
There was a problem hiding this comment.
Optimize the getEventsOfType function.
The current implementation of getEventsOfType can be simplified by using the filter function directly.
val ret = this.events.filter {
if (exact) it.type == type else it.type.endsWith(type)
}.map { it.event }
| val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) | ||
| return FlowAccessApiImpl(AccessAPIGrpc.newBlockingStub(channel), ExecutionDataAPIGrpc.newBlockingStub(channel), coroutineScope) |
There was a problem hiding this comment.
I believe that we should be passing the coroutineScope in on each individual subscription call instead so that the caller has control over the lifecycle of the subscription (i.e. can terminate it).
| override fun subscribeExecutionDataByBlockHeight( | ||
| height: Long | ||
| ): FlowAccessApi.AccessApiCallResponse<Pair<ReceiveChannel<FlowBlockExecutionData>, ReceiveChannel<Throwable>>> { |
There was a problem hiding this comment.
e.g.
| override fun subscribeExecutionDataByBlockHeight( | |
| height: Long | |
| ): FlowAccessApi.AccessApiCallResponse<Pair<ReceiveChannel<FlowBlockExecutionData>, ReceiveChannel<Throwable>>> { | |
| override fun subscribeExecutionDataByBlockHeight( | |
| scope: CoroutineScope | |
| height: Long, | |
| ): FlowAccessApi.AccessApiCallResponse<Pair<ReceiveChannel<FlowBlockExecutionData>, ReceiveChannel<Throwable>>> { |
There was a problem hiding this comment.
Thank you for addressing the previous feedback @lealobanov 🎉 . I just left a couple other comments.
|
Thank you, will follow up on these shortly! |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- sdk/build.gradle.kts (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (14 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (2 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2 hunks)
Files skipped from review due to trivial changes (1)
- sdk/build.gradle.kts
Files skipped from review as they are similar to previous changes (1)
- sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt
Additional comments not posted (25)
sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (4)
65-69: LGTM!The method
subscribeExecutionDataByBlockIdis correctly defined with appropriate parameters and return type.
70-74: LGTM!The method
subscribeExecutionDataByBlockHeightis correctly defined with appropriate parameters and return type.
75-79: LGTM!The method
subscribeEventsByBlockIdis correctly defined with appropriate parameters and return type.
80-83: LGTM!The method
subscribeEventsByBlockHeightis correctly defined with appropriate parameters and return type.sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (2)
14-14: LGTM!The import statement for
ExecutionDataAPIGrpcis necessary for the new functionality.
48-48: LGTM!The method
newAccessApiis correctly updated to include the new parameter forExecutionDataAPIGrpc.sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (4)
332-373: LGTM!The test case
Test subscribeExecutionDataByBlockIdis correctly defined and validates the subscription to execution data using a block ID.
375-416: LGTM!The test case
Test subscribeEventsByBlockIdis correctly defined and validates the subscription to events using a block ID.
418-459: LGTM!The test case
Test subscribeEventsByBlockHeightis correctly defined and validates the subscription to events using a block height.
419-459: LGTM!The test case
Test subscribeExecutionDataByBlockHeightis correctly defined and validates the subscription to execution data using a block height.sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (5)
17-18: Constructor update approved.The constructor now correctly accepts an additional parameter
executionDataApi.
417-444: MethodsubscribeExecutionDataByBlockHeightlooks good.The method correctly handles data and error channels using coroutines.
Verify the method usage in the codebase to ensure proper integration.
Verification successful
Method
subscribeExecutionDataByBlockHeightis correctly integrated and used in the codebase.
- The method's definition is consistent with the new signature in
FlowAccessApi.kt.- The implementation in
FlowAccessApiImpl.ktmatches the expected behavior.- Test cases in
FlowAccessApiImplTest.ktcover both success and error scenarios adequately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeExecutionDataByBlockHeight` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type kotlin -A 5 $'subscribeExecutionDataByBlockHeight'Length of output: 4588
446-472: MethodsubscribeEventsByBlockIdlooks good.The method correctly handles data and error channels using coroutines.
Verify the method usage in the codebase to ensure proper integration.
Verification successful
Method
subscribeEventsByBlockIdusage is consistent.The method
subscribeEventsByBlockIdis correctly used in the codebase with the new signature.
sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.ktsdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.ktsdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeEventsByBlockId` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type kotlin -A 5 $'subscribeEventsByBlockId'Length of output: 6738
389-415: MethodsubscribeExecutionDataByBlockIdlooks good.The method correctly handles data and error channels using coroutines.
Verify the method usage in the codebase to ensure proper integration.
Verification successful
Method
subscribeExecutionDataByBlockIdis correctly integrated.The method signature is consistent across the codebase, including in the main implementation files and test cases.
sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.ktsdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.ktsdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktsdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeExecutionDataByBlockId` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type kotlin -A 5 $'subscribeExecutionDataByBlockId'Length of output: 6982
474-500: MethodsubscribeEventsByBlockHeightlooks good.The method correctly handles data and error channels using coroutines.
Verify the method usage in the codebase to ensure proper integration.
Verification successful
Method
subscribeEventsByBlockHeightis correctly integrated and tested.The method is properly used across the codebase, and the tests cover both success and error scenarios.
sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.ktsdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.ktsdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.ktsdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.ktScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `subscribeEventsByBlockHeight` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type kotlin -A 5 $'subscribeEventsByBlockHeight'Length of output: 6689
sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (10)
4-9: Imports approved.The new imports for testing coroutines and mock dependencies are necessary and correctly used.
Also applies to: 20-25
47-48: Setup method update approved.The setup method correctly initializes the new mock dependency
mockExecutionDataApi.
420-448: TestsubscribeExecutionDataByBlockHeight success caseapproved.The test correctly mocks the responses and validates the expected behavior.
451-479: TestsubscribeExecutionDataByBlockHeight error caseapproved.The test correctly mocks the responses and validates the expected error handling.
482-510: TestsubscribeEventsByBlockId success caseapproved.The test correctly mocks the responses and validates the expected behavior.
513-541: TestsubscribeEventsByBlockId error caseapproved.The test correctly mocks the responses and validates the expected error handling.
544-572: TestsubscribeEventsByBlockHeight success caseapproved.The test correctly mocks the responses and validates the expected behavior.
575-603: TestsubscribeEventsByBlockHeight error caseapproved.The test correctly mocks the responses and validates the expected error handling.
606-634: TestsubscribeExecutionDataByBlockId success caseapproved.The test correctly mocks the responses and validates the expected behavior.
637-665: TestsubscribeExecutionDataByBlockId error caseapproved.The test correctly mocks the responses and validates the expected error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (13 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (2 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt
Additional comments not posted (16)
sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (8)
412-434: LGTM!The test method successfully verifies the subscription to execution data by block height using mocks and coroutines.
436-457: LGTM!The test method successfully verifies the error handling for subscription to execution data by block height using mocks and coroutines.
459-481: LGTM!The test method successfully verifies the subscription to events by block ID using mocks and coroutines.
483-503: LGTM!The test method successfully verifies the error handling for subscription to events by block ID using mocks and coroutines.
505-527: LGTM!The test method successfully verifies the subscription to events by block height using mocks and coroutines.
529-549: LGTM!The test method successfully verifies the error handling for subscription to events by block height using mocks and coroutines.
551-573: LGTM!The test method successfully verifies the subscription to execution data by block ID using mocks and coroutines.
575-595: LGTM!The test method successfully verifies the error handling for subscription to execution data by block ID using mocks and coroutines.
sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (8)
420-434: LGTM!The refactoring of the
signerListproperty improves readability without altering functionality.
436-438: LGTM!The update to the
signerMapproperty aligns with the new structure ofsignerListand maintains the existing functionality.
761-775: LGTM!The new
FlowChunkExecutionDatadata class enhances the SDK's capabilities for handling execution data.
777-807: LGTM!The new
FlowTrieUpdatedata class enhances the SDK's capabilities for handling trie updates.
809-835: LGTM!The new
FlowPayloaddata class enhances the SDK's capabilities for handling payloads.
837-863: LGTM!The new
FlowKeyPartdata class enhances the SDK's capabilities for handling key parts.
865-878: LGTM!The new
FlowExecutionDataTransactionResultdata class enhances the SDK's capabilities for handling execution data transaction results.
880-902: LGTM!The new
FlowExecutionDataCollectionandFlowBlockExecutionDatadata classes enhance the SDK's capabilities for handling execution data collections and block execution data.
Description
Access api subscription endpoints
ExecutionData fields: https://github.com/onflow/flow/blob/3d7cfcfd52ac2532ebe244f9f66ecc8d76bd9fed/protobuf/flow/entities/block_execution_data.proto
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
New Features
Bug Fixes
Tests