add new file #263

Volikk · 2025-09-05T18:05:30Z

No description provided.

mateacademy-ai-mentor

Review Summary

Decision: REQUEST_CHANGES — there are critical blockers that violate core requirements.

Why: The implementation currently risks duplicate data on startup and does not guarantee the required response schema for externalId. These issues break the idempotent loading requirement and the API contract.

Critical issues to fix (blockers):

Idempotent startup data loading
- Where: src/main/java/mate/academy/rickandmorty/service/impl/CharacterServiceImpl.java, method saveAllCharacters (around the loop where you save fetched characters).
- What’s wrong: It saves all fetched characters without checking if a record with the same externalId already exists. Re-running the app will create duplicates.
- Requirement reference: Data loading must be idempotent and avoid duplicates.
- How to fix (options):
  - Add a uniqueness check before save (e.g., repository method findByExternalId(...) and skip existing), or
  - Enforce uniqueness at DB level and use upsert/merge logic, or
  - Clear the table before loading (least preferred unless explicitly allowed).
API response schema: externalId must be a string
- Where: src/main/java/mate/academy/rickandmorty/dto/CharacterDto.java (field externalId), mapping in CharacterMapper.
- What’s wrong: externalId is Long in DTO, so it will serialize as a number, not a string.
- Requirement reference: The API must return externalId as a string.
- How to fix (options):
  - Change CharacterDto.externalId to String and update CharacterMapper to map Long↔String, or
  - Keep it as Long in the entity but annotate the DTO field with a serializer (e.g., Jackson) to output as string.

Strongly recommended (supports the blockers):

Add a uniqueness constraint on externalId
- Where: src/main/java/mate/academy/rickandmorty/model/RickAndMortyCharacter.java (field externalId).
- How: Use @Column(unique = true) or a table-level unique constraint to prevent duplicates and ensure idempotency at the DB level.

Minor (non-blockers):

Package typo: rename controler to controller.
Consider DB-native random selection for scalability.

You’ve done a solid job with controllers, mapping setup, and the external API client. Addressing the above points will bring your solution fully in line with the requirements. Keep it up!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-09-05T18:10:58Z

src/main/java/mate/academy/rickandmorty/dto/CharacterDto.java

+@Data
+public class CharacterDto {
+    private Long id;
+    private Long externalId;


The externalId field is of type Long, but the requirements specify that it should be a string in the API response. Ensure that this field is serialized as a string in the API response, either by changing its type to String or by configuring serialization appropriately.

mateacademy-ai-mentor · 2025-09-05T18:10:58Z

src/main/java/mate/academy/rickandmorty/model/RickAndMortyCharacter.java

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


Consider adding a uniqueness constraint to the externalId field to prevent duplicate records, as required for idempotent startup data loading.

mateacademy-ai-mentor · 2025-09-05T18:10:59Z

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

+        List<CharacterDto> characterDtos = characterClient.getCharacters();
+        List<RickAndMortyCharacter> rickAndMortyCharacters = characterDtos.stream()
+                .map(characterMapper::toModel).toList();
+        characterRepository.saveAll(rickAndMortyCharacters);


The saveAllCharacters method saves all fetched characters without checking for existing entries, which can lead to duplicate records on repeated startups. The requirements specify that data loading must be idempotent and avoid duplicates. Consider checking for existing characters by externalId before saving, or enforcing uniqueness at the database level.

mateacademy-ai-mentor

Review Summary

The implementation is solid overall, but there is one critical blocker that breaks the API contract.

Critical issue (must fix): In src/main/java/mate/academy/rickandmorty/mapper/CharacterMapper.java, the toDto method constructs CharacterDto with the wrong argument order. Currently it uses new CharacterDto(e.getId(), e.getName(), e.getExternalId(), e.getStatus(), e.getGender()). According to CharacterDto’s constructor and the API contract, the order must be: id, externalId, name, status, gender. Passing name as the externalId and externalId as the name will cause clients to receive swapped values and violates requirements for response schema correctness.

Why this is wrong:

The API requires externalId to contain the original external API ID as a string, and name to contain the character’s name. Swapping them returns incorrect data to consumers.

How to fix:

Update the constructor call in CharacterMapper.toDto to: new CharacterDto(e.getId(), e.getExternalId(), e.getName(), e.getStatus(), e.getGender()).

Minor (non-blocking) suggestions:

Consider removing unused DTOs (CharacterDataDto, CharacterInfoDto, client/RickAndMortyPage.java) if they aren’t used.
Optionally rename package controler to controller for convention.

You’ve done great work on pagination, idempotent loading, and Swagger docs. Fixing the mapping line will align the API responses with the contract. Keep it up!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-09-05T19:18:34Z

src/main/java/mate/academy/rickandmorty/mapper/CharacterMapper.java

+    }
+
+    public CharacterDto toDto(RickAndMortyCharacter e) {
+        return new CharacterDto(e.getId(), e.getName(),


The argument order in the CharacterDto constructor is incorrect. It should be: id, externalId, name, status, gender. Currently, you are passing name as externalId and externalId as name. This will break the API contract. Change the constructor call to: new CharacterDto(e.getId(), e.getExternalId(), e.getName(), e.getStatus(), e.getGender()).

mateacademy-ai-mentor

Review Summary

Educational approval: Great job—your solution meets the core requirements, and I’m approving it to support your progress while learning.

What you did well

Correctly import all Rick & Morty characters on startup (idempotent) via DataInitializer and CharacterServiceImpl.
Clean separation of layers (controller/service/repository/mapper) and robust mappings in CharacterMapper.
Random character and name substring search work from the local DB, and Swagger/OpenAPI is properly configured.

Minor improvements (maintainability):

Unused classes:
- src/main/java/mate/academy/rickandmorty/client/RickAndMortyPage.java
- src/main/java/mate/academy/rickandmorty/dto/CharacterDataDto.java
- src/main/java/mate/academy/rickandmorty/dto/CharacterInfoDto.java
Why: They are not referenced and add clutter.
How: Remove them or document their intended future use to keep the codebase lean.
Package typo:
- src/main/java/mate/academy/rickandmorty/controler/CharacterController.java
Why: Conventional naming improves clarity and tooling support.
How: Rename package to controller and update imports accordingly.

Keep up the excellent work—your implementation is clean, robust, and well-documented. Addressing the minor cleanups will make it even stronger.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

add new file

489bb93

mateacademy-ai-mentor suggested changes Sep 5, 2025

View reviewed changes

fix

ea23001

Volikk requested a review from mateacademy-ai-mentor September 5, 2025 19:12

mateacademy-ai-mentor suggested changes Sep 5, 2025

View reviewed changes

fix2.0

e20fa1d

Volikk requested a review from mateacademy-ai-mentor September 5, 2025 19:20

mateacademy-ai-mentor approved these changes Sep 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add new file #263

add new file #263

Uh oh!

Volikk commented Sep 5, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

Uh oh!

add new file #263

Are you sure you want to change the base?

add new file #263

Uh oh!

Conversation

Volikk commented Sep 5, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

mateacademy-ai-mentor Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Uh oh!