Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make constructures public to allow creating objects without gson. #390

Merged
merged 1 commit into from
Dec 20, 2021
Merged

Make constructures public to allow creating objects without gson. #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps.
### Changes
* Fix missing Liquidity Pool ID in AccountResponse Balance ([#379](https://github.com/stellar/java-stellar-sdk/pull/379)).
* Fix null pointer when calling ChangeTrustOperationResponse.getAsset() for LiquidityPool trust line ([#378](https://github.com/stellar/java-stellar-sdk/pull/378)).

* Changed the access modifiers of the inner static classes of `AccountResponse` to public.
### Breaking changes
* Changed offer ids to be represented in requests and response models as long data type. ([#386](https://github.com/stellar/java-stellar-sdk/pull/386)).
* Changed all *MuxedId* attributes to be of data type `java.math.BigInteger` in request and response models ([#388](https://github.com/stellar/java-stellar-sdk/pull/388)).
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/stellar/sdk/responses/AccountResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public static class Thresholds {
@SerializedName("high_threshold")
private final int highThreshold;

Thresholds(int lowThreshold, int medThreshold, int highThreshold) {
public Thresholds(int lowThreshold, int medThreshold, int highThreshold) {
this.lowThreshold = lowThreshold;
this.medThreshold = medThreshold;
this.highThreshold = highThreshold;
Expand Down Expand Up @@ -175,7 +175,7 @@ public static class Flags {
@SerializedName("auth_immutable")
private final boolean authImmutable;

Flags(boolean authRequired, boolean authRevocable, boolean authImmutable) {
public Flags(boolean authRequired, boolean authRevocable, boolean authImmutable) {
this.authRequired = authRequired;
this.authRevocable = authRevocable;
this.authImmutable = authImmutable;
Expand Down Expand Up @@ -223,7 +223,7 @@ public static class Balance {
@SerializedName("sponsor")
private String sponsor;

Balance(String assetType, String assetCode, String assetIssuer, LiquidityPoolID liquidityPoolID, String balance, String limit, String buyingLiabilities, String sellingLiabilities, Boolean isAuthorized, Boolean isAuthorizedToMaintainLiabilities, Integer lastModifiedLedger, String sponsor) {
public Balance(String assetType, String assetCode, String assetIssuer, LiquidityPoolID liquidityPoolID, String balance, String limit, String buyingLiabilities, String sellingLiabilities, Boolean isAuthorized, Boolean isAuthorizedToMaintainLiabilities, Integer lastModifiedLedger, String sponsor) {
this.assetType = checkNotNull(assetType, "assertType cannot be null");
this.balance = checkNotNull(balance, "balance cannot be null");
this.limit = limit;
Expand Down Expand Up @@ -312,7 +312,7 @@ public static class Signer {
@SerializedName("sponsor")
private String sponsor;

Signer(String key, String type, int weight, String sponsor) {
public Signer(String key, String type, int weight, String sponsor) {
this.key = checkNotNull(key, "key cannot be null");
this.type = checkNotNull(type, "type cannot be null");
this.weight = checkNotNull(weight, "weight cannot be null");
Expand Down Expand Up @@ -393,7 +393,7 @@ public static class Links {
@SerializedName("transactions")
private final Link transactions;

Links(Link effects, Link offers, Link operations, Link self, Link transactions) {
public Links(Link effects, Link offers, Link operations, Link self, Link transactions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lijamie98 , was just curious what is the use case driving the need to populate the model manually rather than from server api responses?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is where I use it
https://github.com/stellar/java-stellar-anchor-dev/pull/7/files#diff-bb19215491831165a984cf8e66a77832f8567401759bba44621d8a21ff46dbceR169

This is an work around. I had to create another class TestSigner to convert to Signer at:
https://github.com/stellar/java-stellar-anchor-dev/pull/7/files#diff-bb19215491831165a984cf8e66a77832f8567401759bba44621d8a21ff46dbceR42

Copy link
Contributor Author

@lijamie98 lijamie98 Dec 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually the Signer class that the tests need a public constructor. However, similar pattern may occur when we do tests in the future.

Builder pattern would work too if public constructors are not preferred. Changing to Builder pattern will also need to change the way we construct AccountResponse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it's for testing concerns. I noticed a few other response POJO's here in the SDK already had public constructors on their inner classes, so, hopefully this is contained to AccountResponse. I wanted to also mention, Spring's ReflectionTestUtils , as an optional brute force way for tests to access private members of classes under test. I didn't see Spring in the anchor project's pom.xml, but maybe could use that with test scope if this issue of access crops up more.

Yes, It might be nice to for us to consider adding Lombok here into the SDK on these POJOs, and bring in builders. I see you have that in the anchor project as well, nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the anchor dev project requirements is NOT to use Spring :-(. So, I used gson to bruteforce it. However, I think this can be better solved with either a builder or public constructor.

Lombok is a real nice way to reduce the verboseness of Java. :-) +1

I will change the CHANGELOG on Monday and update the PR.

this.effects = effects;
this.offers = offers;
this.operations = operations;
Expand Down