Bump java-stellar-sdk to v1.0.0 #1575
Conversation
|
Wait for the wallet SDK update before continuing this PR. stellar/kotlin-wallet-sdk#155 |
overcat
changed the title
gradle/libs.versions.toml
Outdated
| @@ -27,7 +28,7 @@ jakarta-annotation-api = "3.0.0" | |||
| jakarta-transaction-api = "2.0.1" | |||
| jakarta-validation-api = "3.1.0" | |||
| jakarta-xml-bind-api = "4.0.2" | |||
| java-stellar-sdk = "0.44.1" | |||
| java-stellar-sdk = "1.0.0-beta1" | |||
There was a problem hiding this comment.
This should be 1.0.0-rc0
There was a problem hiding this comment.
I have already updated it to rc0. I expect to release the stable version of the stellar sdk next week. At that time, we can release the new wallet sdk and then update this repo.
| @@ -148,8 +148,8 @@ SSEStream<OperationResponse> startSSEStream() { | |||
| new EventListener<>() { | |||
| @Override | |||
| public void onEvent(OperationResponse operationResponse) { | |||
| if (operationResponse.getTransaction().isPresent()) { | |||
| metricLatestBlockRead.set(operationResponse.getTransaction().get().getLedger()); | |||
| if (operationResponse.getTransaction() != null) { | |||
There was a problem hiding this comment.
getTransaction() returns Optional.
| if (operationResponse.getTransaction() != null) { | |
| if (operationResponse.getTransaction().isPresent()) { |
There was a problem hiding this comment.
Previously, some fields in the response were checked using null, while others were checked using Optional. Now we uniformly check for existence using null.
There was a problem hiding this comment.
I think this introduces further inconsistency. The getTransaction() returns Optional. If we want to check the existence using null, the getTransaction() should return null / value instead of Optional.
However, I don't think it is right to make the return value uniform. Whether a function returns null or Optional should depend on each function's definition.
I think for functions returning Optional should be checked by isPresent(). Otherwise, !=null.
There was a problem hiding this comment.
Hi @lijamie98, I apologize for not mentioning earlier that in the latest version, getTransaction() no longer returns Optional. You can check the current code here: https://github.com/lightsail-network/java-stellar-sdk/blob/master/src/main/java/org/stellar/sdk/responses/operations/OperationResponse.java#L55
Let me explain the rationale behind removing Optional:
- Since we're using
lombok, there's no need to manually write get functions anymore. Not usingOptionalhelps keep things simpler. - The existence of response fields is heavily dependent on the Horizon version. For instance, a field might exist in Horizon 1.0 but be removed in Horizon 2.0. We want users to have the flexibility to determine field existence based on their specific Horizon version. This way, we avoid tight coupling between the SDK version and Horizon versions.
|
Thanks for submitting this PR. |
…o-v1 # Conflicts: # core/src/test/kotlin/org/stellar/anchor/sep10/Sep10ServiceTest.kt
Description
Bump java-stellar-sdk to v1.0.0
Context
Due to the upcoming 1.0.0 containing many breaking changes, I created this PR to make the process a bit easier.
Testing
./gradlew testDocumentation
https://github.com/lightsail-network/java-stellar-sdk/blob/master/CHANGELOG.md
Known limitations
N/A