Spring Boot - Working with third-party API #11

sshestaka · 2023-10-20T15:44:33Z

Created API has two methods:

The request randomly generates a wiki about one character in the universe the animated series Rick & Morty (according the task)
The request takes a string as an argument, and returns a list of all characters whose name contains the search string.

All data is loaded from the source: "https://rickandmortyapi.com/api/character/"
Data loading occurs one-time during the launch of the application.
Liquibase with changelog files have been added.
MySQL DB is the main data base.
H2 DB used for tests.
All requests were documented using Swagger. Swagger is available by path: http://localhost:8080/swagger-ui/index.html#/

- Created API has two methods: 1. The request randomly generates a wiki about one character in the universe the animated series Rick & Morty (according the task) 2. The request takes a string as an argument, and returns a list of all characters whose name contains the search string. - All data is loaded from the source: "https://rickandmortyapi.com/api/character/" - Data loading occurs one-time during the launch of the application. - Liquibase with changelog files have been added. - MySQL DB is the main data base. - H2 DB used for tests. - All requests were documented using Swagger. Swagger is available by path: http://localhost:8080/swagger-ui/index.html#/

olekskov · 2023-10-21T16:11:00Z

src/main/java/mate/academy/rickandmorty/Application.java

+    //private static final CharacterService characterService =
+    //        new CharacterServiceImpl();


Please remove commented code

olekskov · 2023-10-21T16:11:04Z

src/main/java/mate/academy/rickandmorty/Application.java


    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
+        //characterService.saveAll();


olekskov · 2023-10-21T16:13:22Z

src/main/java/mate/academy/rickandmorty/controler/CharacterController.java

+    @GetMapping("/download/all")
+    @Operation(summary = "Download all characters",
+            description = "Download a list of all available characters from "
+                    + "https://rickandmortyapi.com/api/character/")
+    public List<Character> downloadAll() {
+        return characterService.downloadAllCharacters();
+    }


Anyone who finds this endpoint may spam external api on your behalf. Please do not leave such obvious vulnerabilities in code

OK, thanks!

olekskov · 2023-10-21T16:15:02Z

src/main/java/mate/academy/rickandmorty/service/CharacterServiceImpl.java

+    @PostConstruct
+    public void init() {
+        downloadAllCharacters();
+    }
+
+    @Override
+    public List<Character> downloadAllCharacters() {
+        HttpClient httpClient = HttpClient.newHttpClient();
+        HttpRequest httpRequest = HttpRequest.newBuilder()
+                .GET()
+                .uri(URI.create(GET_ALL_CHARACTERS_URL))
+                .build();
+        CharacterResponseDataDto dataDto;
+        try {
+            HttpResponse<String> response = httpClient.send(httpRequest,
+                    HttpResponse.BodyHandlers.ofString());
+            dataDto = objectMapper.readValue(response.body(),
+                    CharacterResponseDataDto.class);
+        } catch (IOException | InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+        List<CharacterResults> characterResults = dataDto.getResults();
+        List<CharacterDto> characterDtos = characterResults.stream()
+                .map(characterMapper::fromApiToDto)
+                .toList();
+        List<Character> characterList = characterDtos.stream()
+                .map(characterMapper::toModel)
+                .toList();
+        deleteAllCharacters();
+        return characterRepository.saveAll(characterList);
+    }


Please extract this part to separate component, it's should not be part of character service

OK, I have moved this logic to the separate class...

olekskov · 2023-10-21T16:16:18Z

src/main/java/mate/academy/rickandmorty/service/CharacterServiceImpl.java

+        List<Character> allCharactersFromDb = characterRepository.findAll();
+        int maxIdInDb = allCharactersFromDb.size();


Please do not fetch more data from DB than required.
Ok that this data has few hundreds of rows, what would you do if there were few millions, also make list with all of them? :)

Suggested change

List<Character> allCharactersFromDb = characterRepository.findAll();

int maxIdInDb = allCharactersFromDb.size();

int maxIdInDb = characterRepository.count();

olekskov · 2023-10-21T16:20:06Z

src/main/java/mate/academy/rickandmorty/service/CharacterServiceImpl.java

+    @Override
+    public void deleteAllCharacters() {
+        characterRepository.deleteAll();
+    }


Looks strange as public method, it's used only internally. Please remove from interface and inline in implementation

2. Removed URL from API description 3. Data download logic moved to the separate class. 4. Refactored the logic which calculate first and last id in the DB in findByRandomId method. For this purpose I have been added a new method getMaxIdFromCharacters in the CharacterRepository. 5. Refactored access modifier for deleteAllCharacters - to private.

olekskov · 2023-10-23T17:26:37Z

src/main/java/mate/academy/rickandmorty/controler/CharacterController.java

+    @GetMapping("/download/all")
+    @Operation(summary = "Download all characters",
+            description = "Download a list of all available characters")
+    public List<Character> downloadAll() {
+        return characterService.saveAll();
+    }


Метод так і не видалено. Навіщо він тобі, якщо ти використовуєш postConstruct? Логіка і так запуститься без ендпоінту взагалі

Я не зрозумів що треба було його видалити... Я навмистно добавив цей метод, щоб коли аплікейшн вже запущено, можна було ще раз перезавантажити контент)) мені здалося що це буде корисно... але зараз видаляю)

olekskov · 2023-10-23T17:32:06Z

src/main/java/mate/academy/rickandmorty/service/CharacterServiceImpl.java

+    @Override
+    public List<Character> saveAll() {
+        List<CharacterResults> characterResults = charactersClient.downloadAllCharacters();
+        List<Character> characterList = characterResults.stream()
+                .map(characterMapper::fromApiToDto)
+                .map(characterMapper::toModel)
+                .toList();
+        deleteAllCharacters();
+        return characterRepository.saveAll(characterList);
+    }


Наявність методу saveAll() без аргументів дуже спантеличує. Загалом сервіс собі працює з базою і дтошками, а тут чогось йому треба йти кудись на стороннє апі і щось там витягувати

Suggested change

@Override

public List<Character> saveAll() {

List<CharacterResults> characterResults = charactersClient.downloadAllCharacters();

List<Character> characterList = characterResults.stream()

.map(characterMapper::fromApiToDto)

.map(characterMapper::toModel)

.toList();

deleteAllCharacters();

return characterRepository.saveAll(characterList);

}

olekskov · 2023-10-23T17:32:13Z

src/main/java/mate/academy/rickandmorty/service/CharacterService.java

+import org.springframework.data.domain.Pageable;
+
+public interface CharacterService {
+    List<Character> saveAll();


Suggested change

List<Character> saveAll();

olekskov · 2023-10-23T17:35:03Z

src/main/java/mate/academy/rickandmorty/service/CharactersClient.java

+@Component
+@RequiredArgsConstructor
+public class CharactersClient {
+    private final ObjectMapper objectMapper;
+
+    @Value("${mate.academy.get.all.characters.url}")
+    private String getAllCharactersUrl;
+
+    public List<CharacterResults> downloadAllCharacters() {
+        HttpClient httpClient = HttpClient.newHttpClient();
+        HttpRequest httpRequest = HttpRequest.newBuilder()
+                .GET()
+                .uri(URI.create(getAllCharactersUrl))
+                .build();
+        CharacterResponseDataDto dataDto;
+        try {
+            HttpResponse<String> response = httpClient.send(httpRequest,
+                    HttpResponse.BodyHandlers.ofString());
+            dataDto = objectMapper.readValue(response.body(), CharacterResponseDataDto.class);
+        } catch (IOException | InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+        return dataDto.getResults();
+    }
+}


Весь код наповнення бази логічніше виглядає тут

Suggested change

@Component

@RequiredArgsConstructor

public class CharactersClient {

private final ObjectMapper objectMapper;

@Value("${mate.academy.get.all.characters.url}")

private String getAllCharactersUrl;

public List<CharacterResults> downloadAllCharacters() {

HttpClient httpClient = HttpClient.newHttpClient();

HttpRequest httpRequest = HttpRequest.newBuilder()

.GET()

.uri(URI.create(getAllCharactersUrl))

.build();

CharacterResponseDataDto dataDto;

try {

HttpResponse<String> response = httpClient.send(httpRequest,

HttpResponse.BodyHandlers.ofString());

dataDto = objectMapper.readValue(response.body(), CharacterResponseDataDto.class);

} catch (IOException | InterruptedException e) {

throw new RuntimeException(e);

}

return dataDto.getResults();

}

}

@Component

@RequiredArgsConstructor

public class CharactersClient {

private final ObjectMapper objectMapper;

//add repository

@Value("${mate.academy.get.all.characters.url}")

private String getAllCharactersUrl;

@PostConstruct

private List<CharacterResults> downloadAllCharacters() {

HttpClient httpClient = HttpClient.newHttpClient();

HttpRequest httpRequest = HttpRequest.newBuilder()

.GET()

.uri(URI.create(getAllCharactersUrl))

.build();

CharacterResponseDataDto dataDto;

try {

HttpResponse<String> response = httpClient.send(httpRequest,

HttpResponse.BodyHandlers.ofString());

dataDto = objectMapper.readValue(response.body(), CharacterResponseDataDto.class);

} catch (IOException | InterruptedException e) {

throw new RuntimeException(e);

}

List<Character> characterList = dataDto.getResults().stream()

.map(characterMapper::fromApiToDto)

.map(characterMapper::toModel)

.toList();

characterRepository.deleteAll();

characterRepository.saveAll(characterList);

}

}

Я зрозумів) Взагалі видаляю метод save в CharacterService і переношу всю логіку по збереженню даних в CharactersClient!

2. Method saveAll and all logic connected with save a new data removed from CharacterService. 3. Class CharactersClient have been refactored: added logic for work with the DB and deleteAll / saveAll methods.

Ivan95kos

Nice

olekskov suggested changes Oct 21, 2023

View reviewed changes

sshestaka requested a review from olekskov October 23, 2023 14:22

olekskov suggested changes Oct 23, 2023

View reviewed changes

1. Method downloadAll removed from CharacterController.

e8709c3

2. Method saveAll and all logic connected with save a new data removed from CharacterService. 3. Class CharactersClient have been refactored: added logic for work with the DB and deleteAll / saveAll methods.

sshestaka requested a review from olekskov October 24, 2023 13:12

Ivan95kos approved these changes Oct 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spring Boot - Working with third-party API #11

Spring Boot - Working with third-party API #11

sshestaka commented Oct 20, 2023

olekskov Oct 21, 2023

olekskov Oct 21, 2023

olekskov Oct 21, 2023

sshestaka Oct 23, 2023

olekskov Oct 21, 2023

sshestaka Oct 23, 2023

olekskov Oct 21, 2023

sshestaka Oct 23, 2023

olekskov Oct 21, 2023

olekskov Oct 23, 2023

sshestaka Oct 24, 2023

olekskov Oct 23, 2023

olekskov Oct 23, 2023

olekskov Oct 23, 2023

sshestaka Oct 24, 2023

Ivan95kos left a comment

		//private static final CharacterService characterService =
		// new CharacterServiceImpl();

		List<Character> allCharactersFromDb = characterRepository.findAll();
		int maxIdInDb = allCharactersFromDb.size();

	List<Character> allCharactersFromDb = characterRepository.findAll();
	int maxIdInDb = allCharactersFromDb.size();
	int maxIdInDb = characterRepository.count();

Spring Boot - Working with third-party API #11

Are you sure you want to change the base?

Spring Boot - Working with third-party API #11

Conversation

sshestaka commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivan95kos left a comment

Choose a reason for hiding this comment