From f42dcce74b304a9e5e94439def4ffe17b27cedef Mon Sep 17 00:00:00 2001 From: Piotr Dec Date: Mon, 10 Jun 2024 23:52:47 +0200 Subject: [PATCH] fix: Double to BigDecimal --- pom.xml | 1 + readme.md | 2 +- .../wymiana/data/entity/CurrencyEntity.java | 4 +- .../java/eu/ztsh/wymiana/model/Currency.java | 4 +- .../ztsh/wymiana/service/CurrencyService.java | 36 +++++++------ .../eu/ztsh/wymiana/service/NbpService.java | 9 ++-- .../java/eu/ztsh/wymiana/util/UserMapper.java | 2 +- .../ValidExchangeRequestValidator.java | 4 +- .../web/model/CurrencyExchangeRequest.java | 6 ++- .../wymiana/web/model/UserCreateRequest.java | 4 +- .../java/eu/ztsh/wymiana/EntityCreator.java | 50 +++++++++++++------ .../wymiana/service/CurrencyServiceTest.java | 14 +++--- .../ztsh/wymiana/service/UserServiceTest.java | 3 +- .../ValidExchangeRequestValidatorTest.java | 6 ++- .../web/ApplicationIntegrationTests.java | 5 +- 15 files changed, 93 insertions(+), 57 deletions(-) diff --git a/pom.xml b/pom.xml index 69862f2..d716a75 100644 --- a/pom.xml +++ b/pom.xml @@ -110,6 +110,7 @@ ${basedir}/src/main/resources/schema eu.ztsh.wymiana.model + true diff --git a/readme.md b/readme.md index 92a57f9..75a4586 100644 --- a/readme.md +++ b/readme.md @@ -37,7 +37,7 @@ Prosty mikroserwis stworzony na potrzeby rekrutacji "name", "surname", "pesel", - "pln" + "initial" ] } ``` diff --git a/src/main/java/eu/ztsh/wymiana/data/entity/CurrencyEntity.java b/src/main/java/eu/ztsh/wymiana/data/entity/CurrencyEntity.java index 441b224..f468a67 100644 --- a/src/main/java/eu/ztsh/wymiana/data/entity/CurrencyEntity.java +++ b/src/main/java/eu/ztsh/wymiana/data/entity/CurrencyEntity.java @@ -7,6 +7,8 @@ import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import java.math.BigDecimal; + @Data @NoArgsConstructor @AllArgsConstructor @@ -18,6 +20,6 @@ public class CurrencyEntity { String pesel; @Id String symbol; - Double amount; + BigDecimal amount; } diff --git a/src/main/java/eu/ztsh/wymiana/model/Currency.java b/src/main/java/eu/ztsh/wymiana/model/Currency.java index 0cc6b16..e644aff 100644 --- a/src/main/java/eu/ztsh/wymiana/model/Currency.java +++ b/src/main/java/eu/ztsh/wymiana/model/Currency.java @@ -1,5 +1,7 @@ package eu.ztsh.wymiana.model; -public record Currency(String symbol, double amount) { +import java.math.BigDecimal; + +public record Currency(String symbol, BigDecimal amount) { } diff --git a/src/main/java/eu/ztsh/wymiana/service/CurrencyService.java b/src/main/java/eu/ztsh/wymiana/service/CurrencyService.java index 401626a..9480825 100644 --- a/src/main/java/eu/ztsh/wymiana/service/CurrencyService.java +++ b/src/main/java/eu/ztsh/wymiana/service/CurrencyService.java @@ -45,8 +45,8 @@ public class CurrencyService { } var exchanged = performExchange(from, Optional.ofNullable(user.currencies().get(request.to().toUpperCase())).orElse(create(request.to())), - Optional.ofNullable(request.toSell()).orElse(0D), - Optional.ofNullable(request.toBuy()).orElse(0D)); + Optional.ofNullable(request.toSell()).orElse(BigDecimal.ZERO), + Optional.ofNullable(request.toBuy()).orElse(BigDecimal.ZERO)); user.currencies().putAll(exchanged); return userService.update(user); }) @@ -55,32 +55,36 @@ public class CurrencyService { private Currency create(String symbol) { // TODO: check if supported - now limited to PLN <-> USD - return new Currency(symbol.toUpperCase(), 0D); + return new Currency(symbol.toUpperCase(), BigDecimal.ZERO); } - private Map performExchange(Currency from, Currency to, double toSell, double toBuy) { - double exchangeRate; - double neededFromAmount; - double requestedToAmount; + private Map performExchange(Currency from, Currency to, BigDecimal toSell, BigDecimal toBuy) { + BigDecimal exchangeRate; + BigDecimal neededFromAmount; + BigDecimal requestedToAmount; if (from.symbol().equalsIgnoreCase("PLN")) { exchangeRate = nbpService.getSellRate(to.symbol()); - neededFromAmount = round(toBuy != 0 ? toBuy * exchangeRate : toSell); - requestedToAmount = round(toBuy != 0 ? toBuy : toSell / exchangeRate); + neededFromAmount = round(toBuy.signum() != 0 ? toBuy.multiply(exchangeRate) : toSell); + requestedToAmount = round(toBuy.signum() != 0 ? toBuy : divide(toSell, exchangeRate)); } else { exchangeRate = nbpService.getBuyRate(from.symbol()); - neededFromAmount = round(toBuy != 0 ? toBuy / exchangeRate : toSell); - requestedToAmount = round(toBuy != 0 ? toBuy : toSell * exchangeRate); + neededFromAmount = round(toBuy.signum() != 0 ? divide(toBuy, exchangeRate) : toSell); + requestedToAmount = round(toBuy.signum() != 0 ? toBuy : toSell.multiply(exchangeRate)); } - if (neededFromAmount > from.amount()) { + if (neededFromAmount.compareTo(from.amount()) > 0) { throw new InsufficientFundsException(); } - var newFrom = new Currency(from.symbol(), from.amount() - neededFromAmount); - var newTo = new Currency(to.symbol(), to.amount() + requestedToAmount); + var newFrom = new Currency(from.symbol(), from.amount().subtract(neededFromAmount)); + var newTo = new Currency(to.symbol(), to.amount().add(requestedToAmount)); return Stream.of(newFrom, newTo).collect(Collectors.toMap(Currency::symbol, currency -> currency)); } - private double round(double input) { - return BigDecimal.valueOf(input).setScale(2, RoundingMode.HALF_UP).doubleValue(); + private BigDecimal round(BigDecimal input) { + return input.setScale(2, RoundingMode.HALF_UP); + } + + private BigDecimal divide(BigDecimal input, BigDecimal division) { + return input.setScale(2, RoundingMode.HALF_UP).divide(division, RoundingMode.HALF_UP); } } diff --git a/src/main/java/eu/ztsh/wymiana/service/NbpService.java b/src/main/java/eu/ztsh/wymiana/service/NbpService.java index e612e0a..3fca0d1 100644 --- a/src/main/java/eu/ztsh/wymiana/service/NbpService.java +++ b/src/main/java/eu/ztsh/wymiana/service/NbpService.java @@ -8,6 +8,7 @@ import org.springframework.http.HttpStatusCode; import org.springframework.stereotype.Service; import org.springframework.web.client.RestClient; +import java.math.BigDecimal; import java.time.Clock; import java.time.DayOfWeek; import java.time.LocalDate; @@ -30,11 +31,11 @@ public class NbpService { private final ConcurrentMap cache = new ConcurrentHashMap<>(1); - public double getSellRate(String currency) { + public BigDecimal getSellRate(String currency) { return getCurrency(currency.toUpperCase()).sell(); } - public double getBuyRate(String currency) { + public BigDecimal getBuyRate(String currency) { return getCurrency(currency.toUpperCase()).buy(); } @@ -43,7 +44,7 @@ public class NbpService { var cacheObject = cache.get(currency); if (cacheObject == null || cacheObject.date().isBefore(today)) { var fresh = fetchData(currency, dtf.format(today)); - var rate = fresh.getRates().get(0); + var rate = fresh.getRates().getFirst(); cacheObject = new RatesCache( LocalDate.parse(rate.getEffectiveDate(), dtf), rate.getBid(), @@ -82,7 +83,7 @@ public class NbpService { || today.getDayOfWeek() == DayOfWeek.SUNDAY; } - private record RatesCache(LocalDate date, double buy, double sell) { + private record RatesCache(LocalDate date, BigDecimal buy, BigDecimal sell) { } diff --git a/src/main/java/eu/ztsh/wymiana/util/UserMapper.java b/src/main/java/eu/ztsh/wymiana/util/UserMapper.java index 39985fb..15a9e5a 100644 --- a/src/main/java/eu/ztsh/wymiana/util/UserMapper.java +++ b/src/main/java/eu/ztsh/wymiana/util/UserMapper.java @@ -21,7 +21,7 @@ public class UserMapper { public static UserEntity requestToEntity(UserCreateRequest request) { return new UserEntity(request.pesel(), request.name(), request.surname(), - List.of(new CurrencyEntity(request.pesel(), "PLN", request.pln()))); + List.of(new CurrencyEntity(request.pesel(), "PLN", request.initial()))); } private UserMapper() { diff --git a/src/main/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidator.java b/src/main/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidator.java index 143876b..9a01a0c 100644 --- a/src/main/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidator.java +++ b/src/main/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidator.java @@ -18,8 +18,8 @@ public class ValidExchangeRequestValidator implements return (request.from() != null && !request.from().equals(request.to())) && !((request.toBuy() == null && request.toSell() == null) || (request.toBuy() != null && request.toSell() != null)) - && ((request.toBuy() != null && request.toBuy() >= 0) - || (request.toSell() != null && request.toSell() >= 0)); + && ((request.toBuy() != null && request.toBuy().signum() >= 0) + || (request.toSell() != null && request.toSell().signum() >= 0)); } } diff --git a/src/main/java/eu/ztsh/wymiana/web/model/CurrencyExchangeRequest.java b/src/main/java/eu/ztsh/wymiana/web/model/CurrencyExchangeRequest.java index 54f192e..c804720 100644 --- a/src/main/java/eu/ztsh/wymiana/web/model/CurrencyExchangeRequest.java +++ b/src/main/java/eu/ztsh/wymiana/web/model/CurrencyExchangeRequest.java @@ -5,14 +5,16 @@ import jakarta.validation.constraints.NotNull; import lombok.Builder; import org.hibernate.validator.constraints.pl.PESEL; +import java.math.BigDecimal; + @Builder @ValidExchangeRequest public record CurrencyExchangeRequest( @PESEL String pesel, @NotNull String from, @NotNull String to, - Double toBuy, - Double toSell + BigDecimal toBuy, + BigDecimal toSell ) { } diff --git a/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java b/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java index bd7b1b4..cb5de05 100644 --- a/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java +++ b/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java @@ -6,11 +6,13 @@ import jakarta.validation.constraints.NotNull; import lombok.Builder; import org.hibernate.validator.constraints.pl.PESEL; +import java.math.BigDecimal; + @Builder public record UserCreateRequest( @NotNull String name, @NotNull String surname, @PESEL @Adult String pesel, - @Min(0) double pln) { + @Min(0) BigDecimal initial) { } diff --git a/src/test/java/eu/ztsh/wymiana/EntityCreator.java b/src/test/java/eu/ztsh/wymiana/EntityCreator.java index 617cbfa..6535f11 100644 --- a/src/test/java/eu/ztsh/wymiana/EntityCreator.java +++ b/src/test/java/eu/ztsh/wymiana/EntityCreator.java @@ -9,6 +9,8 @@ import eu.ztsh.wymiana.model.User; import eu.ztsh.wymiana.web.model.CurrencyExchangeRequest; import eu.ztsh.wymiana.web.model.UserCreateRequest; +import java.math.BigDecimal; +import java.math.RoundingMode; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -24,13 +26,13 @@ public class EntityCreator { public static String ANOTHER_PESEL = "08280959342"; public static String NAME = "Janina"; public static String SURNAME = "Kowalska"; - public static double PLN = 20.10; - public static double USD_SELL = 5.18; - public static double USD_BUY = 5.08; + public static BigDecimal PLN = createScaled(20.10, 2); + public static BigDecimal USD_SELL = createScaled(5.18, 2); + public static BigDecimal USD_BUY = createScaled(5.08, 2); public static String PLN_SYMBOL = "PLN"; public static String USD_SYMBOL = "USD"; - public static double BUY_RATE = 3.8804; - public static double SELL_RATE = 3.9572; + public static BigDecimal BUY_RATE = createScaled(3.8804, 4); + public static BigDecimal SELL_RATE = createScaled(3.9572, 4); } @@ -39,25 +41,29 @@ public class EntityCreator { } public static User user() { - return user(Constants.PLN, 0); + return user(Constants.PLN, BigDecimal.ZERO); } - public static User user(double pln, double usd) { + public static User user(BigDecimal pln, BigDecimal usd) { Map currencies = new HashMap<>(); - if (pln > 0) { + if (pln.signum() > 0) { currencies.put("PLN", new Currency("PLN", pln)); } - if (usd > 0) { + if (usd.signum() > 0) { currencies.put("USD", new Currency("USD", usd)); } return new User(Constants.NAME, Constants.SURNAME, Constants.PESEL, currencies); } public static UserCreateRequest.UserCreateRequestBuilder userRequest() { + return userRequest(null); + } + + public static UserCreateRequest.UserCreateRequestBuilder userRequest(Double initial) { return UserCreateRequest.builder().name(Constants.NAME) .surname(Constants.SURNAME) .pesel(Constants.PESEL) - .pln(Constants.PLN); + .initial(initial != null ? createScaled(initial, 2) : Constants.PLN); } public static CurrencyExchangeRequest.CurrencyExchangeRequestBuilder exchangeRequest() { @@ -83,8 +89,8 @@ public class EntityCreator { String name; String surname; String pesel; - double pln = -1; - double usd = -1; + BigDecimal pln = BigDecimal.valueOf(-1); + BigDecimal usd = BigDecimal.valueOf(-1); public UserEntityBuilder name(String name) { this.name = name; @@ -102,22 +108,30 @@ public class EntityCreator { } public UserEntityBuilder pln(double pln) { - this.pln = pln; + return this.pln(BigDecimal.valueOf(pln)); + } + + public UserEntityBuilder pln(BigDecimal pln) { + this.pln = pln.setScale(2, RoundingMode.HALF_UP); return this; } public UserEntityBuilder usd(double usd) { - this.usd = usd; + return this.usd(BigDecimal.valueOf(usd)); + } + + public UserEntityBuilder usd(BigDecimal usd) { + this.usd = usd.setScale(2, RoundingMode.HALF_UP); return this; } public UserEntity build() { var nonnulPesel = Optional.ofNullable(pesel).orElse(Constants.PESEL); List currencies = new ArrayList<>(); - if (pln > -1) { + if (pln.signum() > -1) { currencies.add(new CurrencyEntity(nonnulPesel, "PLN", pln)); } - if (usd > -1) { + if (usd.signum() > -1) { currencies.add(new CurrencyEntity(nonnulPesel, "USD", usd)); } if (currencies.isEmpty()) { @@ -133,4 +147,8 @@ public class EntityCreator { } + private static BigDecimal createScaled(double input, int scale) { + return BigDecimal.valueOf(input).setScale(scale, RoundingMode.HALF_UP); + } + } diff --git a/src/test/java/eu/ztsh/wymiana/service/CurrencyServiceTest.java b/src/test/java/eu/ztsh/wymiana/service/CurrencyServiceTest.java index c44625e..474f11f 100644 --- a/src/test/java/eu/ztsh/wymiana/service/CurrencyServiceTest.java +++ b/src/test/java/eu/ztsh/wymiana/service/CurrencyServiceTest.java @@ -16,6 +16,8 @@ import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; +import java.math.BigDecimal; +import java.util.Objects; import java.util.stream.Stream; import static eu.ztsh.wymiana.EntityCreator.Constants.*; @@ -46,7 +48,7 @@ class CurrencyServiceTest extends RepositoryBasedTest { .toSell(PLN) .build()); assertThat(result.currencies()) - .matches(map -> map.get(PLN_SYMBOL).amount() == 0 && map.get(USD_SYMBOL).amount() == USD_BUY); + .matches(map -> map.get(PLN_SYMBOL).amount().signum() == 0 && Objects.equals(map.get(USD_SYMBOL).amount(), USD_BUY), "USD 5.08"); var expected = EntityCreator.userEntity().pln(0).usd(USD_BUY).build(); expect(expected); } @@ -62,7 +64,7 @@ class CurrencyServiceTest extends RepositoryBasedTest { .toBuy(USD_BUY) .build()); assertThat(result.currencies()) - .matches(map -> map.get(PLN_SYMBOL).amount() == 0 && map.get(USD_SYMBOL).amount() == USD_BUY); + .matches(map -> map.get(PLN_SYMBOL).amount().signum() == 0 && Objects.equals(map.get(USD_SYMBOL).amount(), USD_BUY)); var expected = EntityCreator.userEntity().pln(0).usd(USD_BUY).build(); expect(expected); } @@ -78,7 +80,7 @@ class CurrencyServiceTest extends RepositoryBasedTest { .toSell(USD_SELL) .build()); assertThat(result.currencies()) - .matches(map -> map.get(PLN_SYMBOL).amount() == PLN && map.get(USD_SYMBOL).amount() == 0); + .matches(map -> Objects.equals(map.get(PLN_SYMBOL).amount(), PLN) && map.get(USD_SYMBOL).amount().signum() == 0, "PLN 20.10"); var expected = EntityCreator.userEntity().pln(PLN).usd(0).build(); expect(expected); } @@ -94,7 +96,7 @@ class CurrencyServiceTest extends RepositoryBasedTest { .toBuy(PLN) .build()); assertThat(result.currencies()) - .matches(map -> map.get(PLN_SYMBOL).amount() == PLN && map.get(USD_SYMBOL).amount() == 0); + .matches(map -> Objects.equals(map.get(PLN_SYMBOL).amount(), PLN) && map.get(USD_SYMBOL).amount().signum() == 0, "PLN 20.10"); var expected = EntityCreator.userEntity().pln(PLN).usd(0).build(); expect(expected); } @@ -116,7 +118,7 @@ class CurrencyServiceTest extends RepositoryBasedTest { @Transactional @Test void doubleExchangeTest() { - var initialValue = 100; + var initialValue = BigDecimal.valueOf(100); var entity = EntityCreator.userEntity().pln(initialValue).build(); userRepository.save(entity); var result1 = currencyService.exchange(EntityCreator.exchangeRequest() @@ -136,7 +138,7 @@ class CurrencyServiceTest extends RepositoryBasedTest { assertThat(resultEntity.getCurrencies() .stream() .filter(c -> c.getSymbol().equalsIgnoreCase("PLN")) - .findFirst()).isNotEmpty().get().matches(currencyEntity -> currencyEntity.getAmount() < initialValue); + .findFirst()).isNotEmpty().get().matches(currencyEntity -> currencyEntity.getAmount().compareTo(initialValue) < 0); } @Transactional diff --git a/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java b/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java index 1ab7690..fef02b9 100644 --- a/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java +++ b/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java @@ -50,10 +50,9 @@ class UserServiceTest extends RepositoryBasedTest { @DisplayName("Try to create another user with same PESEL") void createDuplicatedPeselTest() { var first = EntityCreator.userRequest().build(); - var second = EntityCreator.userRequest() + var second = EntityCreator.userRequest(30.30) .name("Jan") .surname("Kowalski") - .pln(30.30) .build(); userService.create(first); assertThatThrownBy(() -> userService.create(second)) diff --git a/src/test/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidatorTest.java b/src/test/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidatorTest.java index 5226e14..e8ef550 100644 --- a/src/test/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidatorTest.java +++ b/src/test/java/eu/ztsh/wymiana/validation/ValidExchangeRequestValidatorTest.java @@ -5,6 +5,8 @@ import eu.ztsh.wymiana.web.model.CurrencyExchangeRequest; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import java.math.BigDecimal; + import static eu.ztsh.wymiana.EntityCreator.Constants.PLN_SYMBOL; import static eu.ztsh.wymiana.EntityCreator.Constants.USD_BUY; import static eu.ztsh.wymiana.EntityCreator.Constants.USD_SELL; @@ -72,7 +74,7 @@ class ValidExchangeRequestValidatorTest extends ValidatorTest