Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
92 changes: 74 additions & 18 deletions pom.xml
Original file line number Diff line number Diff line change
@@ -1,58 +1,101 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.1.4</version>
<relativePath/>
<version>3.2.4</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>mate.academy</groupId>
<artifactId>jv-rick-and-morty</artifactId>
<artifactId>jv-spring-boot-mapstruct</artifactId>
<version>0.0.1-SNAPSHOT</version>
<name>jv-rick-and-morty</name>
<description>jv-rick-and-morty</description>
<name>jv-spring-boot-mapstruct</name>
<description>jv-spring-boot-mapstruct</description>
<properties>
<java.version>17</java.version>
<maven.checkstyle.plugin.version>3.1.1</maven.checkstyle.plugin.version>
<lombok.mapstruct.binding.version>0.2.0</lombok.mapstruct.binding.version>
<mapstruct.version>1.5.5.Final</mapstruct.version>
<maven.checkstyle.plugin.configLocation>
https://raw.githubusercontent.com/mate-academy/style-guides/master/java/checkstyle.xml
</maven.checkstyle.plugin.configLocation>
</properties>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter</artifactId>
<artifactId>spring-boot-starter-web</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-jpa</artifactId>
</dependency>
<dependency>
<groupId>com.h2database</groupId>
<artifactId>h2</artifactId>
</dependency>
<dependency>
<groupId>com.mysql</groupId>
<artifactId>mysql-connector-j</artifactId>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<optional>true</optional>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-jpa</artifactId>
<artifactId>spring-boot-starter-validation</artifactId>
</dependency>

<dependency>
<groupId>com.h2database</groupId>
<artifactId>h2</artifactId>
<groupId>org.mapstruct</groupId>
<artifactId>mapstruct</artifactId>
<version>${mapstruct.version}</version>
</dependency>
<dependency>
<groupId>org.springdoc</groupId>
<artifactId>springdoc-openapi-starter-webmvc-ui</artifactId>
<version>2.5.0</version>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>${java.version}</source>
<target>${java.version}</target>
<annotationProcessorPaths>
<path>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>
</path>
<path>
<groupId>org.projectlombok</groupId>
<artifactId>lombok-mapstruct-binding</artifactId>
<version>${lombok.mapstruct.binding.version}</version>
</path>
<path>
<groupId>org.mapstruct</groupId>
<artifactId>mapstruct-processor</artifactId>
<version>${mapstruct.version}</version>
</path>
</annotationProcessorPaths>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>3.3.0</version>
<version>3.3.1</version>
<executions>
<execution>
<phase>compile</phase>
Expand All @@ -63,12 +106,25 @@
</executions>
<configuration>
<configLocation>${maven.checkstyle.plugin.configLocation}</configLocation>
<includeResources>false</includeResources>
<excludeGeneratedSources>true</excludeGeneratedSources>
<consoleOutput>true</consoleOutput>
<failsOnError>true</failsOnError>
<linkXRef>false</linkXRef>
</configuration>
</plugin>
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<configuration>
<excludes>
<exclude>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
</exclude>
</excludes>
</configuration>
</plugin>
</plugins>
</build>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package mate.academy.rickandmorty.client;

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.util.ArrayList;
import java.util.List;
import mate.academy.rickandmorty.dto.external.ExternalCharacterDto;
import mate.academy.rickandmorty.dto.external.RickAndMortyResponseDto;
import org.springframework.stereotype.Component;

@Component
public class RickAndMortyClient {
private static final String BASE_URL = "https://rickandmortyapi.com/api/character";
private final ObjectMapper objectMapper;

public RickAndMortyClient(ObjectMapper objectMapper) {
this.objectMapper = objectMapper
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);

}

public List<ExternalCharacterDto> fetchAllCharacters() {

List<ExternalCharacterDto> allCharacters = new ArrayList<>();

Choose a reason for hiding this comment

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

  • Suggestion (memory/persistence): Collecting all characters into an in-memory list may be acceptable for this dataset, but coordinate with the service to ensure idempotency and uniqueness of externalId before saving (e.g., query existing externalIds and insert only new ones, or use unique constraint on the external_id column). This prevents duplicates on repeated startups. See CharacterServiceImpl and Character entity for where this will be used.


HttpClient httpClient = HttpClient.newHttpClient();

HttpRequest httpRequest = HttpRequest.newBuilder()
.GET()
.uri(URI.create(BASE_URL))
.build();

Choose a reason for hiding this comment

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

  • Issue: Only a single request to the base URL is built here which fetches only the first page of results.
  • Why: The task requires fetching "all data from the public API" once during startup. You must follow pagination (info.next) and loop until no next page remains.
  • Suggestion: Implement a loop that follows the next URL from the response info and accumulates results from every page; return the full list only after all pages are successfully fetched. See project description requirement about fetching all data once.

try {
HttpResponse<String> response = httpClient
.send(httpRequest, HttpResponse.BodyHandlers.ofString());

System.out.println(response.body());

Choose a reason for hiding this comment

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

  • Issue: Debug printing of entire response body.
  • Why: System.out.println(response.body()) can leak large payloads and is not appropriate for production/CI; tests and logs get noisy.
  • Suggestion: Remove the println and use a logger at DEBUG level to log minimal info (status, page number) if needed.


RickAndMortyResponseDto apiResponse = objectMapper
.readValue(response.body(), RickAndMortyResponseDto.class);

Choose a reason for hiding this comment

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

  • Issue: Deserialization target mismatch risk.
  • Why: The code deserializes into RickAndMortyResponseDto. Ensure that RickAndMortyResponseDto uses the correct InfoDto type that matches the external API (fields: count, pages, next, previous). Currently the project's wrapper imports a wrong Info type which will break Jackson mapping. Fix the wrapper DTO import/type so .info().next() (or equivalent) is available for pagination.


allCharacters.addAll(apiResponse.results());

Choose a reason for hiding this comment

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

  • Issue: Only adds first-page results to allCharacters.
  • Why: allCharacters.addAll(apiResponse.results()) is executed once. Without following apiResponse.info().next you will not gather subsequent pages and will save an incomplete dataset to DB.
  • Suggestion: Change the logic to add results for each page inside a loop that follows info.next until null. Accumulate results and return the full set. Also consider how to avoid partial DB writes on failure (persist only after full successful fetch or use transactional/atomic approach).


} catch (IOException | InterruptedException e) {
throw new RuntimeException("Failed to fetch information from Rick and Morty API ", e);

Choose a reason for hiding this comment

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

  • Issue: Exception handling lacks retries and may cause partial or unexpected failures.
  • Why: A single IO/InterruptedException is wrapped into RuntimeException without retry; transient network errors may be recoverable. The startup loader requirement expects robust handling (retries or clear failure) to avoid inconsistent DB state.
  • Suggestion: Add a small retry mechanism (e.g., 2–3 attempts with backoff) for transient failures. If full fetch cannot be completed, propagate a clear error to prevent persisting partial data.

}
return allCharacters;
}
}
18 changes: 18 additions & 0 deletions src/main/java/mate/academy/rickandmorty/config/DataLoader.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package mate.academy.rickandmorty.config;

import jakarta.annotation.PostConstruct;
import lombok.RequiredArgsConstructor;
import mate.academy.rickandmorty.service.CharacterService;
import org.springframework.stereotype.Component;

@Component

Choose a reason for hiding this comment

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

The loader runs unconditionally because the class is annotated with @component. To avoid network calls during tests and satisfy test stability requirements, make the loader conditional. For example add:

@ConditionalOnProperty(name = "app.initial-load.enabled", havingValue = "true", matchIfMissing = true)
@component

Then set app.initial-load.enabled=false in src/test/resources/application.properties and true (or omitted) in main properties. This addresses checklist item #2 (initial data fetch once on startup) while preventing test-time external calls and ensures property keys match between main and test configurations (checklist item #7).

@RequiredArgsConstructor
public class DataLoader {
private final CharacterService characterService;

@PostConstruct

Choose a reason for hiding this comment

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

@PostConstruct will run when the bean is created. This can execute before JPA/Hibernate schema or repositories are fully ready and will also run in test application contexts (causing external API calls during mvn test). Consider using ApplicationRunner/CommandLineRunner or @eventlistener(ApplicationReadyEvent.class) to ensure the load runs after the context is fully ready and make it easier to control execution timing. This addresses checklist item #3.14 (startup load triggers exactly once) and helps with #2.9 (mvn test using H2).

public void init() {
System.out.println("DataLoader запускається...");

Choose a reason for hiding this comment

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

Prefer using a logger (SLF4J) instead of System.out.println so messages are consistent with application logging and can be controlled by log configuration. This helps with production-quality diagnostics.

characterService.loadInitialData();

Choose a reason for hiding this comment

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

This line triggers the initial data load (external API call). Ensure you won't call external services during tests — either guard this call with a profile/property or move the trigger to a runner that can be disabled in test profile. Also confirm the service method is idempotent and handles failures/retries as required by checklist item #3.11 and duplicates prevention #3.3.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package mate.academy.rickandmorty.controller;import io.swagger.v3.oas.annotations.Operation;import io.swagger.v3.oas.annotations.tags.Tag;import java.util.List;import lombok.RequiredArgsConstructor;import mate.academy.rickandmorty.dto.CharacterResponseDto;import mate.academy.rickandmorty.service.CharacterService;import org.springframework.http.HttpStatus;import org.springframework.web.bind.annotation.GetMapping;import org.springframework.web.bind.annotation.RequestMapping;import org.springframework.web.bind.annotation.RequestParam;import org.springframework.web.bind.annotation.ResponseStatus;import org.springframework.web.bind.annotation.RestController;@RestController@RequestMapping("/characters")@RequiredArgsConstructor@Tag(name = "Rick and Morty API", description = "Endpoints for Rick and Morty characters")public class CharacterController { private final CharacterService characterService; @GetMapping("/random") @ResponseStatus(HttpStatus.OK) @Operation(summary = "Get random character", description = "Returns one of the character from local database") public CharacterResponseDto getRandomCharacter() { return characterService.getRandomCharacter(); } @GetMapping("/search") @ResponseStatus(HttpStatus.OK) @Operation(summary = "Search characters by name", description = "Finds all characters for the specific name") public List<CharacterResponseDto> searchCharacters(@RequestParam String name) { return characterService.searchByName(name); }}

Choose a reason for hiding this comment

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

Controller: make the search parameter optional and handle missing/blank values at the service layer. Change @RequestParam String name to @RequestParam(required = false) String name (or provide defaultValue) so requests without the parameter don't produce HTTP 400 and the service can return an empty list per requirements (#3.10). Also update Swagger docs to mark the parameter optional. See the service behavior in CharacterServiceImpl which already returns empty list for null/blank.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package mate.academy.rickandmorty.dto;

public record CharacterResponseDto(

Choose a reason for hiding this comment

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

This record already contains the exact required response fields. Optional suggestion: if OpenAPI generation doesn't pick up Java records in your setup, consider annotating the record/components with @Schema to ensure the response schema is visible in Swagger UI (helps guarantee requirement 3.6).

Long id,
String externalId,
String name,
String status,
String gender
) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package mate.academy.rickandmorty.dto.external;

public record ExternalCharacterDto(
Integer id,

Choose a reason for hiding this comment

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

Using Integer for id is appropriate since the external API supplies numeric IDs. Be mindful that dto.id() could be null in malformed responses; the client already configures the ObjectMapper to ignore unknown props but consider defensive checks if needed.

String name,
String status,
String species,
String type,
String gender
) {}
Comment on lines +3 to +10

Choose a reason for hiding this comment

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

This record correctly models the external API character shape and matches how the service maps fields (id -> externalId via dto.id().toString()). No change required.

Comment on lines +3 to +10

Choose a reason for hiding this comment

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

Optional improvement: add @JsonProperty annotations if you want explicit mapping or add a short javadoc explaining this DTO's purpose. Not required for functionality.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package mate.academy.rickandmorty.dto.external;

Choose a reason for hiding this comment

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

If you choose to use @JsonProperty("prev"), add the import for the annotation near the top of the file, e.g. import com.fasterxml.jackson.annotation.JsonProperty;. Place the annotation on the record component (or on the canonical accessor) so Jackson knows to map JSON prev -> previous. Example (conceptual):

@JsonProperty("prev")
String previous

Adding the import should be done on the blank/import area after the package declaration.

public record InfoDto(

Choose a reason for hiding this comment

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

Consider adding a short JavaDoc describing that this record maps the external API info object (count/pages/next/prev). It helps future maintainers understand usage when implementing pagination in the client. Also be mindful that next and prev may be null at runtime when there is no next/previous page.

Choose a reason for hiding this comment

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

Record declaration is otherwise fine. If you choose to annotate the component, add the import com.fasterxml.jackson.annotation.JsonProperty. Example: public record InfoDto(Integer count, Integer pages, String next, @JsonProperty("prev") String previous) {} — or rename to prev to keep it simple.

int count,
int pages,
String next,
String previous
Comment on lines +6 to +7

Choose a reason for hiding this comment

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

The public Rick & Morty API uses the key prev (not previous) in the info object. Your field previous will not be populated by Jackson. Rename this field to prev or annotate it with @JsonProperty("prev") so deserialization of pagination metadata works and the client can follow pages. This is critical for fetching all pages during startup.

Choose a reason for hiding this comment

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

The external API returns the previous page link under the prev key. Here the record component is named previous, so Jackson will not map prev to previous by default. Rename the component to prev or annotate it with @JsonProperty("prev") to ensure correct binding.

Choose a reason for hiding this comment

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

The external API uses prev as the field name for the previous page URL. Your record has previous so Jackson will not map the prev value into this component. This will break pagination when the client reads the API response and prevents correct handling of the next/prev links (violates requirement to handle pagination on import). Rename this component to prev or annotate it with @JsonProperty("prev") so Jackson can bind it.

) {}
Comment on lines 1 to 8

Choose a reason for hiding this comment

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

Ensure the response wrapper RickAndMortyResponseDto references this InfoDto (or the corrected type) instead of importing io.swagger.v3.oas.models.info.Info. The wrapper currently imports the wrong Info class which will break JSON deserialization. Update that file to use this InfoDto so pagination and results mapping work.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package mate.academy.rickandmorty.dto.external;

import io.swagger.v3.oas.models.info.Info;

Choose a reason for hiding this comment

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

This import is incorrect for deserializing the external API response. Replace with the internal Info DTO that matches the API (e.g., import mate.academy.rickandmorty.dto.external.InfoDto;). Using the swagger Info class will prevent Jackson from mapping the API's info object and break pagination handling, which is required by the task.

import java.util.List;

public record RickAndMortyResponseDto(
Info info,

Choose a reason for hiding this comment

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

The info field type must match your API-mapping DTO (InfoDto) not io.swagger's Info. Change this to InfoDto info (or the correct local type). After that, ensure InfoDto maps prev/next properly so the client can iterate pages and fetch all characters on startup.

List<ExternalCharacterDto> results
) {}
34 changes: 34 additions & 0 deletions src/main/java/mate/academy/rickandmorty/model/Character.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package mate.academy.rickandmorty.model;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Entity
@Table(name = "characters")
@Data
@NoArgsConstructor
@AllArgsConstructor
@Builder
public class Character {

Choose a reason for hiding this comment

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

The class name Character is the same as java.lang.Character. It's legal but can be confusing when importing types; consider renaming to CharacterEntity or explicitly qualifying imports if you run into ambiguous references. This is informational (not required by the checklist).

Choose a reason for hiding this comment

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

Class name Character collides in name with java.lang.Character. It's legal but can be confusing in code and imports. Consider renaming to CharacterEntity (or similar) to avoid ambiguity across the codebase. This is a recommendation, not a blocker.

Choose a reason for hiding this comment

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

Note: the class is named Character, which technically shadows java.lang.Character. That is not wrong because of your package, but it can cause confusion in code or imports. If you experience accidental imports of java.lang.Character, consider renaming the entity (e.g., CharacterEntity) or always using the fully qualified name where ambiguity might arise.

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(name = "external_id", nullable = false)
private String externalId;

Choose a reason for hiding this comment

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

The column for externalId is not marked unique. The project checklist requires externalId to be unique (requirement #3.12) to avoid duplicate entries and to support idempotent initial loads (requirement #3.3). Consider adding unique = true to the @column here or adding a table-level unique constraint so the DB enforces uniqueness.


@Column(nullable = false)
private String name;

private String status;

private String gender;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package mate.academy.rickandmorty.repository;

import java.util.List;
import mate.academy.rickandmorty.model.Character;

Choose a reason for hiding this comment

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

Minor note: keep in mind the entity is named Character. That is valid because of the package, but can sometimes be confused with java.lang.Character. No action needed unless you encounter accidental import issues; renaming to CharacterEntity is an optional clarity improvement.

import org.springframework.data.jpa.repository.JpaRepository;

public interface CharacterRepository extends JpaRepository<Character, Long> {

Choose a reason for hiding this comment

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

Consider adding Optional<Character> findByExternalId(String externalId); to the repository. The initial data loader should check for existing externalId values to avoid creating duplicates on repeated application context startups (checklist items #3.3 and #3.12).

Choose a reason for hiding this comment

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

To support efficient random selection (checklist item #3.15) you can use JpaRepository.count() together with findAll(PageRequest.of(randomIndex, 1)) from the service layer. If you prefer, add a repository method (or a @query) that returns a single row by offset. Either approach is fine; using count + pageable avoids loading all records into memory.

Choose a reason for hiding this comment

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

The interface correctly extends JpaRepository<Character, Long> and provides the CRUD/paging methods needed by the service (count(), findAll(PageRequest), etc.). This satisfies the technical needs for random selection and persistence operations.


List<Character> findByNameContainingIgnoreCase(String name);

Choose a reason for hiding this comment

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

Good: this method implements the contains search required by the task (SQL LIKE %query%) and is case-insensitive, satisfying checklist item #3.5. Keep this method as-is.

Choose a reason for hiding this comment

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

The findByNameContainingIgnoreCase method implements the substring search required by the search endpoint (checklist item 3.4). The signature is correct for Spring Data and will perform a contains-type, case-insensitive match.

Choose a reason for hiding this comment

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

Good: findByNameContainingIgnoreCase(String name) implements the required substring case-insensitive search used by the search endpoint (checklist item #1).

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package mate.academy.rickandmorty.service;

import java.util.List;
import mate.academy.rickandmorty.dto.CharacterResponseDto;

public interface CharacterService {
void loadInitialData();

Choose a reason for hiding this comment

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

Add JavaDoc to describe expected semantics of loadInitialData(): that it must fetch all pages from the external API once during application startup and be idempotent (no duplicate records based on externalId). This documents checklist requirements that the implementation must satisfy.


List<CharacterResponseDto> searchByName(String name);

Choose a reason for hiding this comment

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

Document searchByName behaviour: clarify that the method performs case-insensitive containment (SQL LIKE %query%) and should return an empty list for null/blank input instead of throwing. The implementation currently returns empty for blank; documenting it here makes the contract explicit.


CharacterResponseDto getRandomCharacter();

Choose a reason for hiding this comment

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

Document getRandomCharacter behaviour: specify expected behavior when the DB is empty and require efficient selection (count + offset) rather than loading all rows into memory. This aligns service-level expectations with checklist items about randomness and efficiency.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package mate.academy.rickandmorty.service.impl;

import java.util.List;
import java.util.Random;
import lombok.RequiredArgsConstructor;
import mate.academy.rickandmorty.client.RickAndMortyClient;
import mate.academy.rickandmorty.dto.CharacterResponseDto;
import mate.academy.rickandmorty.dto.external.ExternalCharacterDto;
import mate.academy.rickandmorty.model.Character;
import mate.academy.rickandmorty.repository.CharacterRepository;
import mate.academy.rickandmorty.service.CharacterService;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional

Choose a reason for hiding this comment

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

Transactionality: class-level @transactional is good, but to avoid partial DB state you must: 1) fetch all external pages first, 2) then begin a transaction and persist results (or upsert), and 3) fail the startup if the fetch is incomplete (fail-fast). This makes initial load atomic and consistent.

@RequiredArgsConstructor
public class CharacterServiceImpl implements CharacterService {

private final CharacterRepository characterRepository;
private final RickAndMortyClient rickAndMortyClient;
private final Random random = new Random();

@Override
public void loadInitialData() {
System.out.println("loadInitialData() виконується...");

Choose a reason for hiding this comment

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

Avoid System.out.println for production-ready apps; use an SLF4J Logger. Replace the prints at startup, after fetch, and after DB save with logger.debug/info/error as appropriate to provide structured and configurable logs.

List<ExternalCharacterDto> externalCharacterDto = rickAndMortyClient.fetchAllCharacters();

Choose a reason for hiding this comment

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

This call delegates to the HTTP client to fetch characters. The current RickAndMortyClient.fetchAllCharacters implementation only requests the base URL (first page) and does not iterate over pagination. That means you'll only persist the first page of characters — the task requires fetching all pages once at startup. Implement pagination in the client (follow info.next until null) and add retry/backoff for transient failures.

System.out.println("Отримано персонажів: " + externalCharacterDto.size());

List<Character> toSaveDataEntity = externalCharacterDto.stream()
.map(dto -> {
Character character = new Character();
character.setExternalId(dto.id().toString());
character.setName(dto.name());
character.setStatus(dto.status());
character.setGender(dto.gender());
return character;
})
.toList();

Choose a reason for hiding this comment

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

Mapping external DTOs to entities is correct in shape, but you must avoid inserting duplicates. There is no uniqueness constraint on externalId in the Character entity; and loadInitialData currently saves everything every startup. Add a unique constraint to external_id and make the loader idempotent (query existing externalIds and only save new ones or perform upsert). See Character entity for current mapping.


characterRepository.saveAll(toSaveDataEntity);

Choose a reason for hiding this comment

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

Calling saveAll right after mapping will persist all fetched items unconditionally. To avoid partial or duplicate data, first complete the full fetch (all pages) and then persist in a single transactional operation, filtering out existing items. Also consider batch size/flush behavior if dataset is large. Currently saveAll can create duplicates on restart.

System.out.println("Збережено у БД " + toSaveDataEntity.size() + " персонажів.");
}

@Override
public List<CharacterResponseDto> searchByName(String name) {
return (name == null || name.isBlank())
? List.of() :
characterRepository.findByNameContainingIgnoreCase(name).stream()
.map(this::toDto)
.toList();
Comment on lines 59 to 64

Choose a reason for hiding this comment

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

searchByName returns an empty list when name is null or blank — this is acceptable per requirements. Ensure the controller parameter is optional (@RequestParam(required = false)) so requests without the parameter don't produce 400. The controller currently requires the parameter by default; consider aligning controller and service behavior.

}

@Override
public CharacterResponseDto getRandomCharacter() {
List<Character> characters = characterRepository.findAll();
if (characters.isEmpty()) {
return null;
}
Character characterEntity = characters.get(random.nextInt(characters.size()));

Choose a reason for hiding this comment

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

The current random selection implementation loads all characters into memory (findAll) and then selects one. For large datasets this is inefficient. Use repository.count() to get total and then fetch a single row using pagination or a native query with LIMIT 1 OFFSET. Also handle the empty-database case explicitly (return Optional or throw a custom exception mapped to 404) rather than returning null. This addresses checklist item #3.15 and avoids loading entire table into memory.

return toDto(characterEntity);
}

private CharacterResponseDto toDto(Character character) {
return new CharacterResponseDto(character.getId(),
character.getExternalId(), character.getName(),
character.getStatus(), character.getGender());
}
}
Loading