Add subscription data to book details #31
Conversation
|
|
||
| public JsonNode generateBookDetailsDto(Object book, List<Object> rentalRecords, int availableBooksCount) { | ||
| public JsonNode generateBookDetailsDto(Object book, List<Object> rentalRecords, int availableBooksCount, boolean bookSubscription) { |
There was a problem hiding this comment.
Maybe it's good to introduce some kind of DTO when more then 3 parameters
| public Mono<Boolean> getBookSubscription(String bookId, String jwtToken) { | ||
| var subscriptionUrl = inventoryServiceUrl + "/api/inventory/subscriptions/" + bookId; | ||
|
|
||
| return webClient | ||
| .get() | ||
| .uri(subscriptionUrl) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + jwtToken) | ||
| .retrieve() | ||
| .bodyToMono(Boolean.class) | ||
| .onErrorReturn(RuntimeException.class, false); |
There was a problem hiding this comment.
If we create endpoint like this /api/inventory/subscriptions/{bookId}, by only reading it I would guess that it should return all book subscriptions, but I assume that you return if logged in user is subscribed or not
src/main/resources/application.yml
Outdated
| - id: inventory-route | ||
| uri: ${INVENTORY_SERVICE_URL} | ||
| predicates: | ||
| - Path=/api/inventory/subscriptions/** |
There was a problem hiding this comment.
We can route traffic just from /api/inventory/** part
| private Object bookDataDto; | ||
| private List<Object> rentalRecordsDto; |
There was a problem hiding this comment.
Is there a reason to have a *Dto suffix for bookData and rentalRecords?
BookDetailsDto is a fine name for the class, but I guess there is no need for DTO in the attribute names.
Can BookDetailsDto be a record instead of a class?
| var jwt = JWTParser.parse(jwtToken); | ||
| var userId = jwt.getJWTClaimsSet().getClaim(CLAIM_EMAIL).toString(); |
There was a problem hiding this comment.
This code is repeated in the method below, meaning it should be extracted in a separate method.
| var bookSubscriptionMono = inventoryClient.getBookSubscription(bookId, jwtToken, userId); | ||
|
|
||
| var bookDetailsDtoMono = Mono.zip(bookDtoMono, rentalRecordsDtoMono, availableBooksCountMono).flatMap(tuple -> { | ||
| var book = bookDetailsResponseCombiner.generateBookDetailsDto(tuple.getT1(), tuple.getT2(), tuple.getT3()); | ||
| var bookDetailsDtoMono = Mono.zip(bookDtoMono, rentalRecordsDtoMono, availableBooksCountMono, bookSubscriptionMono).flatMap(tuple -> { | ||
| var bookDetailsDto = new BookDetailsDto(); | ||
| bookDetailsDto.setBookDataDto(tuple.getT1()); | ||
| bookDetailsDto.setRentalRecordsDto(tuple.getT2()); | ||
| bookDetailsDto.setAvailableBookCount(tuple.getT3()); | ||
| bookDetailsDto.setBookSubscription(tuple.getT4()); | ||
| var book = bookDetailsResponseCombiner.generateBookDetailsDto(bookDetailsDto); |
There was a problem hiding this comment.
This part is repeated as well, getBookDetailsById and getBookDetailsByTitleAndAuthor can be optimized to avoid code duplication.
| public Mono<Integer> getAvailableBookCopiesCount(String bookId, String jwtToken){ | ||
| var inventoryBookUrl = inventoryServiceUrl + bookId; | ||
| public Mono<Integer> getAvailableBookCopiesCount(String bookId, String jwtToken) { | ||
| var inventoryBookUrl = inventoryServiceUrl + "/api/inventory/book/" + bookId; |
There was a problem hiding this comment.
This is an old URL, but it would be great to fix it and use the plural books instead of book.
Currently, we have /api/inventory/book/bookId and /api/inventory/books/bookId
| @SneakyThrows | ||
| public JsonNode getBookDetailsById(String bookId, String jwtToken) { | ||
| var userId = getClaimEmail(jwtToken); |
There was a problem hiding this comment.
Rename getClaimEmail toextractUserId or similar, because userId is what you get
| @@ -28,13 +29,16 @@ class BookServiceShould { | |||
| private static final String BOOK_ID = "1"; | |||
| private static final String BOOK_TITLE = "::title::"; | |||
| private static final String BOOK_AUTHOR = "::author::"; | |||
| private static final String JWT_TOKEN = ""; | |||
| private static final String JWT_TOKEN = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImVtYWlsIn0.9_82fPfiHdHoOkyx8WQY8FsgPivtguil3QL5a3bDE7g"; | |||
There was a problem hiding this comment.
It should be avoided to store any secrets in Git, even if it's used for testing.
Is it possible to create an Util class that will generate for you a token which can be used for the test?
Please investigate and let me know
| public class BookSubscriptionDto { | ||
| private String bookId; | ||
| private String userId; | ||
| private Date createdDate; | ||
| } |
There was a problem hiding this comment.
Do we need this DTO?
|
Subscription data included in response when requesting book details.