Create API #220

savchenkoarts · 2025-01-16T00:47:47Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! 🌟 While your code meets many of the task requirements, there are a few critical issues that need to be addressed to ensure the functionality and robustness of your application. Here are the key points to focus on:

HTTP Response Handling: Ensure that you check the HTTP response status code before parsing the response body to handle unsuccessful responses appropriately.
Mapper Configuration: Correct the componentModel attribute to a valid option like 'spring' to ensure proper MapStruct configuration.
Repository Type Mismatch: Update the generic type for JpaSpecificationExecutor to CharacterModel to match the entity type.
Random ID Generation: Improve the random character selection logic to avoid assumptions about sequential IDs.
Liquibase Configuration: Correct the specification of the unique constraint in the YAML file to ensure it is applied correctly.
Exception Handling: Pass the cause to the superclass constructor to preserve the original exception stack trace.
Parameter Mapping: Annotate searchParameter with @RequestParam for proper request parameter mapping.
Method Naming: Consider renaming overloaded methods for clarity.
Validation Check: Add a validation check for nextPageUrl to prevent potential infinite loops or exceptions.
Custom Exception Handling: Use the status parameter in CustomGlobalExceptionHandler for consistency.

Once these issues are resolved, your code will be in great shape! Keep up the excellent work and don't hesitate to reach out if you have any questions. 😊

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

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

mateacademy-ai-mentor · 2025-01-16T00:51:56Z

src/main/java/mate/academy/rickandmorty/component/CharacterClient.java

+            HttpResponse<String> response = client
+                    .send(request, HttpResponse.BodyHandlers.ofString());
+            return objectMapper.readValue(response.body(), CharacterResponseDataDto.class);


Consider checking the HTTP response status code before attempting to parse the response body. If the response is not successful (e.g., status code is not 200), you should handle it appropriately, possibly by throwing an exception or returning a default value.

mateacademy-ai-mentor · 2025-01-16T00:51:56Z

src/main/java/mate/academy/rickandmorty/config/MapperConfig.java

+import org.mapstruct.NullValueCheckStrategy;
+
+@org.mapstruct.MapperConfig(
+        componentModel = "string",


The componentModel attribute is set to 'string', which is not a valid option. Consider changing it to a valid option such as 'spring' if you are using Spring framework for dependency injection.

mateacademy-ai-mentor · 2025-01-16T00:51:56Z

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

+
+    @GetMapping("/search")
+    @Operation(summary = "Search characters", description = "Search characters")
+    public List<CharacterConversionDto> searchCharacters(String searchParameter) {


Consider annotating searchParameter with @RequestParam to ensure it is correctly mapped from the request parameters. This will allow Spring to bind the query parameter from the HTTP request to this method parameter.

mateacademy-ai-mentor · 2025-01-16T00:51:56Z

src/main/java/mate/academy/rickandmorty/exeption/CharacterClientException.java

+
+public class CharacterClientException extends RuntimeException {
+    public CharacterClientException(String message, Exception e) {
+        super(message);


Consider passing the cause e to the superclass constructor by using super(message, e); to preserve the original exception stack trace.

mateacademy-ai-mentor · 2025-01-16T00:51:56Z

src/main/java/mate/academy/rickandmorty/exeption/CustomGlobalExceptionHandler.java

+    ) {
+        Map<String, Object> body = new LinkedHashMap<>();
+        body.put("timestamp", LocalDateTime.now());
+        body.put("status", HttpStatus.BAD_REQUEST);


Consider using the status parameter instead of hardcoding HttpStatus.BAD_REQUEST to ensure consistency with the provided status parameter.

mateacademy-ai-mentor · 2025-01-16T00:51:57Z

src/main/java/mate/academy/rickandmorty/repository/CharacterRepository.java

+
+public interface CharacterRepository
+        extends JpaRepository<CharacterModel, Long>,
+        JpaSpecificationExecutor<Character> {


The generic type for JpaSpecificationExecutor should be CharacterModel instead of Character. Correct this to JpaSpecificationExecutor<CharacterModel> to match the entity type.

mateacademy-ai-mentor · 2025-01-16T00:51:57Z

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

+            nextPageUrl = responseData.getInfo().next();
+        } while (nextPageUrl != null);


Consider adding a validation check for nextPageUrl to ensure it is not empty or malformed before proceeding to the next iteration. This will help prevent potential infinite loops or exceptions.

mateacademy-ai-mentor · 2025-01-16T00:51:57Z

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

+        CharacterModel model = repository.findById((long) (Math.random() * count))
+                .orElseThrow(() -> new EntityNotFoundException("Character not found"));


The random ID generation logic assumes that IDs are sequential and start from zero, which may not be the case. Consider using a more reliable method to select a random character, such as fetching a random element from a list of IDs.

mateacademy-ai-mentor · 2025-01-16T00:51:57Z

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

+    @Override
+    public List<CharacterConversionDto> searchCharacters(String searchParameter) {
+        if (searchParameter.isBlank()) {
+            throw new EntityNotFoundException("Search parameter is empty");


The exception message 'Search parameter is empty' could be more descriptive. Consider providing more context, such as 'Search parameter cannot be empty'.

mateacademy-ai-mentor · 2025-01-16T00:51:57Z

src/main/resources/db/changelog/changes/01-create-rick-and-morty-table.yaml

+                  type: VARCHAR(255)
+                  constraints:
+                    nullable: false
+                    uniqueConstraintName: uq_characters_external_id


The uniqueConstraintName should be specified under a unique attribute. Correct this by using unique: true and then specifying the uniqueConstraintName.

mateacademy-ai-mentor

Great job on implementing the solution! 🎉 The code meets the task requirements and constraints effectively. Here are some key points from the review:

HTTP Response Handling: You've correctly handled HTTP responses by checking the status code before parsing the response body in CharacterClient.java.
Mapper Configuration: The componentModel is set to "spring" in CharacterMapper.java, aligning with best practices.
Repository Type Mismatch: The use of CharacterModel in CharacterRepository.java is appropriate.
Random ID Generation: The logic for selecting a random character is well-implemented, avoiding assumptions about sequential IDs.
Liquibase Configuration: The unique constraint in the YAML file is correctly specified.
Exception Handling: Proper exception handling is in place, with causes passed to superclass constructors.
Parameter Mapping: The searchParameter is correctly annotated in CharacterController.java.
Validation Check: You've added a validation check for nextPageUrl to prevent infinite loops.

While the code is approved, remember to address any minor issues mentioned in the review summary to enhance your code quality further. Keep up the great work and continue learning from each iteration! 🚀

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

Create API

2ed17a7

mateacademy-ai-mentor suggested changes Jan 16, 2025

View reviewed changes

Fixed after first review

7d7defe

savchenkoarts requested a review from mateacademy-ai-mentor January 16, 2025 12:09

mateacademy-ai-mentor approved these changes Jan 16, 2025

View reviewed changes

Fixed after second review

65e98ba

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Create API #220

Create API #220

Uh oh!

savchenkoarts commented Jan 16, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor Jan 16, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

Uh oh!

		nextPageUrl = responseData.getInfo().next();
		} while (nextPageUrl != null);

		CharacterModel model = repository.findById((long) (Math.random() * count))
		.orElseThrow(() -> new EntityNotFoundException("Character not found"));

Create API #220

Are you sure you want to change the base?

Create API #220

Uh oh!

Conversation

savchenkoarts commented Jan 16, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!