Rick and Morty solution #152
Conversation
| <property name="validateComments" value="true"/> | ||
| </module> | ||
| </module> | ||
| </module> |
There was a problem hiding this comment.
| </module> | |
| </module> | |
| <version>3.2.4</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-validation</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-web</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.projectlombok</groupId> | ||
| <artifactId>lombok</artifactId> | ||
| <optional>true</optional> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>mysql</groupId> | ||
| <artifactId>mysql-connector-java</artifactId> | ||
| <version>8.0.33</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.hibernate</groupId> | ||
| <artifactId>hibernate-core</artifactId> | ||
| <version>6.4.4.Final</version> |
There was a problem hiding this comment.
Please be consistent and use properties to define versions
| @RequiredArgsConstructor | ||
| @Component | ||
| public class CharacterClient { | ||
| private static final String URL = "https://rickandmortyapi.com/api/character"; |
There was a problem hiding this comment.
Better to make this value configurable via properties
| @Component | ||
| public class CharacterClient { | ||
| private static final String URL = "https://rickandmortyapi.com/api/character"; | ||
| @Qualifier("characterMapperImpl") |
There was a problem hiding this comment.
What is the reason to add Qualifier?
|
|
||
| public List<Character> getAll() throws IOException { | ||
| HttpClient httpClient = HttpClient.newHttpClient(); | ||
| ObjectMapper objectMapper = new ObjectMapper(); |
There was a problem hiding this comment.
No reason to create objectMapper, bean of this object exists in context
| public Character() { | ||
| } |
| @Override | ||
| public List<Character> getAll() { | ||
| return List.of(); | ||
| } |
There was a problem hiding this comment.
Is it expected to have empty list here?
| try { | ||
| HttpRequest request = HttpRequest.newBuilder() | ||
| .GET() | ||
| .uri(URI.create(URL)) | ||
| .build(); | ||
| HttpResponse<String> response = httpClient.send(request, | ||
| HttpResponse.BodyHandlers.ofString()); | ||
| JsonNode rootNode = objectMapper.readTree(response.body()); | ||
| resultsNode = rootNode.get("results"); |
There was a problem hiding this comment.
I suggest to map response object as is, and then on service level extract necessary information and convert to Character object and save to db. This approach will be more flexible and code will be more clear
|
|
||
| @Mappings({ | ||
| @Mapping(target = "externalId", source = "id"), | ||
| @Mapping(target = "id", ignore = true) // DB auto-generated ID |
| throw new LoadExternalDataException( | ||
| "Failed to load characters from external DB", e); | ||
| } |
There was a problem hiding this comment.
I believe you already catch case when characters is failed to load in CharacterClient
No description provided.