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.
There was a problem hiding this comment.
@overcat Thanks very much for your explanation.
|
Thanks for submitting this PR. |
…o-v1 # Conflicts: # core/src/test/kotlin/org/stellar/anchor/sep10/Sep10ServiceTest.kt
|
@overcat Is the PR still in progress? |
|
Hi @lijamie98, we need to wait for the wallet-sdk to release a new version. Maybe you can coordinate that? |
I will. I am also working on the wallet-sdk which is pending our ops to add GH secrets to automate the release process. ETA next week. |
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