Consolidar e unificar controladores de API para itens alimentares e receitas, enriquecer configurações de serviço e segurança #35
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feat: Update CORS configuration in WebConfig to allow local development ports feat: Implement FoodController for managing food items and recipes refactor: Remove FoodItemController methods and consolidate into FoodController fix: Enhance GlobalExceptionHandler to handle validation exceptions with non-null parameters refactor: Rename recipe generation endpoint in RecipeController for clarity feat: Create UserController for user authentication and profile management refactor: Introduce BaseController to consolidate common functionality across controllers fix: Update UserRoles enum to use final for role field chore: Disable SameSiteCookieFilter and clean up unused imports fix: Improve error handling in RecipeMapper for JSON parsing feat: Add user association to Recipe model and update RecipeRepository fix: Update OAuth2AuthorizationRequestRepository to handle specific exceptions feat: Enhance OAuth2LoginSuccessHandler for better user management feat: Implement RateLimitingFilter with non-null parameters for request handling chore: Refactor SecurityConfig to streamline CORS and cookie settings feat: Revise FoodItemService to improve food item management and AI enhancement feat: Expand RecipeService to include user-specific recipe retrieval and deletion chore: Clean up DebugInfoExtractor by removing unused imports
Guia do RevisorEste PR reformula a superfície da API e a documentação, consolidando controladores separados em uma estrutura unificada com lógica base comum, refatora camadas de serviço para operações reativas impulsionadas por IA e suporte CRUD expandido, aprimora as configurações de segurança/CORS para ambientes de desenvolvimento, estende modelos de domínio para incluir associações de usuário, ajusta mappers e filtros reativos, e atualiza o ambiente de build/runtime para Java 21. Diagrama de sequência para o fluxo de criação de item alimentar aprimorado por IAsequenceDiagram
actor User
User->>Frontend: Submit food item (name, quantity, expiration)
Frontend->>FoodController: POST /api/food-items
FoodController->>FoodItemService: createWithAiEnhancement(createDto, user)
FoodItemService->>FoodAiService: determineNutritionalFacts(foodItem)
FoodAiService-->>FoodItemService: AI-enhanced food item
FoodItemService->>FoodItemRepository: save(foodItem)
FoodItemService-->>FoodController: saved food item
FoodController-->>Frontend: AI-enhanced food item response
Frontend-->>User: Display nutrition facts
Diagrama de classes para os novos FoodController e BaseController unificadosclassDiagram
class BaseController {
+isAuthenticated()
+getCurrentUser()
+extractOAuth2UserInfo(Authentication)
+success(data)
+response(status, data)
+error(status, message)
+isAuthorized(resourceOwnerId)
+notFound(resourceName, id)
+unauthorized()
+handleException(e)
}
class FoodController {
+listFoods()
+getFoodById(id)
+createFood(createDto)
+updateFood(id, createDto)
+deleteFood(id)
+generateRecipe()
+getRecipeById(id)
+listRecipes()
+deleteRecipe(id)
}
FoodController --|> BaseController
FoodController --> FoodItemService
FoodController --> FoodMapper
FoodController --> ChatService
FoodController --> RecipeService
class UserController {
+getCurrentUser(authentication)
+logout(request, response)
+getAuthStatus()
+getUserProfile()
}
UserController --|> BaseController
UserController --> UserRepository
Diagrama de classes para o modelo Recipe atualizado e associaçõesclassDiagram
class Recipe {
-id
-name
-description
-instructions
-nutritionalInfo
-ingredientsList
-user
}
class User {
-id
-email
-firstName
-lastName
-profilePicture
-role
}
class RecipeIngredient {
-id
-foodItem
-quantity
-unit
}
Recipe "1" -- "*" RecipeIngredient : ingredientsList
Recipe "*" -- "1" User : user
RecipeIngredient "*" -- "1" FoodItem : foodItem
Diagrama de classes para os aprimoramentos de FoodItemService e RecipeServiceclassDiagram
class FoodItemService {
+listAllForUser(user)
+getById(id)
+createWithAiEnhancement(createDto, user)
+update(id, createDto, user)
+delete(id)
+saveWithAIEnhancement(foodItem)
}
class RecipeService {
+getById(id)
+getAllRecipesForUser(user)
+delete(id)
+saveAndMapToDto(recipes)
}
FoodItemService --> FoodItemRepository
FoodItemService --> FoodAiService
FoodItemService --> User
RecipeService --> RecipeRepository
RecipeService --> RecipeMapper
RecipeService --> User
Alterações em Nível de Arquivo
Dicas e comandosInteragindo com Sourcery
Personalizando Sua ExperiênciaAcesse seu painel para:
Obtendo Ajuda
Original review guide in EnglishReviewer's GuideThis PR overhauls the API surface and documentation, consolidating separate controllers into a unified structure with common base logic, refactors service layers for reactive AI-powered operations and expanded CRUD support, enhances security/CORS configurations for development environments, extends domain models to include user associations, fine-tunes reactive mappers and filters, and updates the build/runtime environment to Java 21. Sequence diagram for AI-enhanced food item creation flowsequenceDiagram
actor User
User->>Frontend: Submit food item (name, quantity, expiration)
Frontend->>FoodController: POST /api/food-items
FoodController->>FoodItemService: createWithAiEnhancement(createDto, user)
FoodItemService->>FoodAiService: determineNutritionalFacts(foodItem)
FoodAiService-->>FoodItemService: AI-enhanced food item
FoodItemService->>FoodItemRepository: save(foodItem)
FoodItemService-->>FoodController: saved food item
FoodController-->>Frontend: AI-enhanced food item response
Frontend-->>User: Display nutrition facts
Class diagram for new unified FoodController and BaseControllerclassDiagram
class BaseController {
+isAuthenticated()
+getCurrentUser()
+extractOAuth2UserInfo(Authentication)
+success(data)
+response(status, data)
+error(status, message)
+isAuthorized(resourceOwnerId)
+notFound(resourceName, id)
+unauthorized()
+handleException(e)
}
class FoodController {
+listFoods()
+getFoodById(id)
+createFood(createDto)
+updateFood(id, createDto)
+deleteFood(id)
+generateRecipe()
+getRecipeById(id)
+listRecipes()
+deleteRecipe(id)
}
FoodController --|> BaseController
FoodController --> FoodItemService
FoodController --> FoodMapper
FoodController --> ChatService
FoodController --> RecipeService
class UserController {
+getCurrentUser(authentication)
+logout(request, response)
+getAuthStatus()
+getUserProfile()
}
UserController --|> BaseController
UserController --> UserRepository
Class diagram for updated Recipe model and associationsclassDiagram
class Recipe {
-id
-name
-description
-instructions
-nutritionalInfo
-ingredientsList
-user
}
class User {
-id
-email
-firstName
-lastName
-profilePicture
-role
}
class RecipeIngredient {
-id
-foodItem
-quantity
-unit
}
Recipe "1" -- "*" RecipeIngredient : ingredientsList
Recipe "*" -- "1" User : user
RecipeIngredient "*" -- "1" FoodItem : foodItem
Class diagram for FoodItemService and RecipeService enhancementsclassDiagram
class FoodItemService {
+listAllForUser(user)
+getById(id)
+createWithAiEnhancement(createDto, user)
+update(id, createDto, user)
+delete(id)
+saveWithAIEnhancement(foodItem)
}
class RecipeService {
+getById(id)
+getAllRecipesForUser(user)
+delete(id)
+saveAndMapToDto(recipes)
}
FoodItemService --> FoodItemRepository
FoodItemService --> FoodAiService
FoodItemService --> User
RecipeService --> RecipeRepository
RecipeService --> RecipeMapper
RecipeService --> User
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
sourcery-ai
bot
changed the title
qodo-code-review
bot
changed the title
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces significant refactoring and functionality enhancements including Java version upgrade, service method improvements, new controller architecture, and comprehensive API endpoint documentation.
- Upgrades Java runtime from version 17 to 21 with corresponding Docker image update
- Implements new unified controller architecture with BaseController for common functionality
- Adds comprehensive AI-enhanced food item management and recipe generation capabilities
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Upgrades Java version from 17 to 21 |
| Dockerfile | Updates base image to use Java 21 runtime |
| SecurityConfig.java | Adds development CORS origins and reorganizes imports |
| FoodItemService.java | Removes unused method and refactors AI enhancement logic |
| RecipeService.java | Adds new methods for recipe retrieval, user-specific queries, and deletion |
| BaseController.java | Introduces new base controller with common authentication and response utilities |
| UserController.java | New consolidated controller for user operations |
| FoodController.java | New unified controller combining food items and recipe management |
| RecipeController.java | Updates endpoint path from /gen to /generate |
| Various security classes | Adds @nonnull annotations and improves exception handling |
| Repository classes | Adds new query methods for user-specific data retrieval |
| Model classes | Adds user relationships and explicit import statements |
| ENDPOINTS_SUMMARY.md | Comprehensive API documentation with AI integration details |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+321
to
+328
| foodAiService.determineNutritionalFacts(foodItem) | ||
| .subscribe( | ||
| enhancedItem -> { | ||
| log.info("Successfully enhanced food item {} with AI", enhancedItem.getName()); | ||
| foodItemRepository.save(enhancedItem); | ||
| }, | ||
| error -> log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage()) | ||
| ); |
There was a problem hiding this comment.
This code has a potential race condition. The method saves the original foodItem immediately (line 330) while asynchronously processing the AI enhancement. This could result in the unenhanced version overwriting the enhanced version, or inconsistent data states. Consider using a synchronous approach or implementing proper async handling with callbacks.
Comment on lines
+320
to
+330
| // Enhance with AI (async) | ||
| foodAiService.determineNutritionalFacts(foodItem) | ||
| .subscribe( | ||
| enhancedItem -> { | ||
| log.info("Successfully enhanced food item {} with AI", enhancedItem.getName()); | ||
| foodItemRepository.save(enhancedItem); | ||
| }, | ||
| error -> log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage()) | ||
| ); | ||
|
|
||
| return foodItemRepository.save(foodItem); |
There was a problem hiding this comment.
This code has a potential race condition. The method saves the original foodItem immediately (line 330) while asynchronously processing the AI enhancement. This could result in the unenhanced version overwriting the enhanced version, or inconsistent data states. Consider using a synchronous approach or implementing proper async handling with callbacks.
Suggested change
| // Enhance with AI (async) | |
| foodAiService.determineNutritionalFacts(foodItem) | |
| .subscribe( | |
| enhancedItem -> { | |
| log.info("Successfully enhanced food item {} with AI", enhancedItem.getName()); | |
| foodItemRepository.save(enhancedItem); | |
| }, | |
| error -> log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage()) | |
| ); | |
| return foodItemRepository.save(foodItem); | |
| // Enhance with AI (synchronously) | |
| FoodItem enhancedItem = foodAiService.determineNutritionalFacts(foodItem).block(); | |
| if (enhancedItem == null) { | |
| log.error("Failed to enhance food item {} with AI: Enhancement returned null", foodItem.getName()); | |
| throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "AI enhancement failed"); | |
| } | |
| log.info("Successfully enhanced food item {} with AI", enhancedItem.getName()); | |
| return foodItemRepository.save(enhancedItem); |
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| import javax.validation.Valid; |
There was a problem hiding this comment.
The import uses javax.validation.Valid instead of the newer jakarta.validation.Valid. Since this is a Spring Boot 3.x application (evident from other Jakarta imports), you should use jakarta.validation.Valid for consistency and to avoid potential classpath issues.
Suggested change
| import javax.validation.Valid; | |
| import jakarta.validation.Valid; |
| } | ||
|
|
||
| // First check if recipe exists and user has access using entity | ||
| Optional<Recipe> recipeEntity = recipeService.findById(id); |
There was a problem hiding this comment.
The method findById is called on recipeService but this method is not defined in the RecipeService class shown in the diff. The service only has getById which returns Optional<RecipeDto>. This will cause a compilation error.
Comment on lines
+279
to
+287
| // First check if recipe exists and user has access using entity | ||
| Optional<Recipe> recipeEntity = recipeService.findById(id); | ||
| if (recipeEntity.isEmpty()) { | ||
| return notFound("Recipe", id); | ||
| } | ||
|
|
||
| if (!isAuthorized(recipeEntity.get().getUser().getId())) { | ||
| return unauthorized(); | ||
| } |
There was a problem hiding this comment.
Same issue as Comment 4 - the method findById is not defined in RecipeService. This will cause a compilation error.
Suggested change
| // First check if recipe exists and user has access using entity | |
| Optional<Recipe> recipeEntity = recipeService.findById(id); | |
| if (recipeEntity.isEmpty()) { | |
| return notFound("Recipe", id); | |
| } | |
| if (!isAuthorized(recipeEntity.get().getUser().getId())) { | |
| return unauthorized(); | |
| } | |
| // TODO: Implement recipe existence and authorization check when findById is available in RecipeService | |
| return ResponseEntity.status(HttpStatus.NOT_IMPLEMENTED) | |
| .body("Recipe existence check not implemented: findById not available in RecipeService."); |
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Olá - revisei suas alterações - aqui estão alguns comentários:
- FoodItemService.createWithAiEnhancement usa uma inscrição assíncrona e imediatamente retorna um salvamento básico, então o enriquecimento de IA pode acontecer após a resposta — escolha uma abordagem reativa ou de bloqueio consistente para garantir que as entidades retornadas sempre contenham dados enriquecidos.
- Existem inconsistências entre sua documentação e os mapeamentos do controlador (por exemplo,
/recipes/genvs/recipes/generatee chamandofindByIdquando apenasgetByIdexiste), causando potenciais erros de compilação/tempo de execução — por favor, alinhe os caminhos e implemente quaisquer métodos ausentes. - O FoodController unificado é bastante grande e repete a lógica de autenticação/autorização para alimentos e receitas — considere dividir as responsabilidades em controladores menores ou usar anotações do Spring Security como @PreAuthorize para simplificar o controle de acesso.
Prompt para Agentes de IA
Por favor, aborde os comentários desta revisão de código:
## Comentários Gerais
- FoodItemService.createWithAiEnhancement usa uma inscrição assíncrona e imediatamente retorna um salvamento básico, então o enriquecimento de IA pode acontecer após a resposta — escolha uma abordagem reativa ou de bloqueio consistente para garantir que as entidades retornadas sempre contenham dados enriquecidos.
- Existem inconsistências entre sua documentação e os mapeamentos do controlador (por exemplo, `/recipes/gen` vs `/recipes/generate` e chamando `findById` quando apenas `getById` existe), causando potenciais erros de compilação/tempo de execução — por favor, alinhe os caminhos e implemente quaisquer métodos ausentes.
- O FoodController unificado é bastante grande e repete a lógica de autenticação/autorização para alimentos e receitas — considere dividir as responsabilidades em controladores menores ou usar anotações do Spring Security como @PreAuthorize para simplificar o controle de acesso.
## Comentários Individuais
### Comentário 1
<location> `src/main/java/com/otavio/aifoodapp/service/FoodItemService.java:313` </location>
<code_context>
+ * @param user the user who owns the food item
+ * @return the created food item with AI-enhanced data
+ */
+ public FoodItem createWithAiEnhancement(FoodItemCreateDto createDto, User user) {
+ FoodItem foodItem = new FoodItem();
+ foodItem.setName(createDto.name());
+ foodItem.setQuantity(createDto.quantity());
+ foodItem.setExpiration(createDto.expiration());
+ foodItem.setUser(user);
+
+ // Enhance with AI (async)
+ foodAiService.determineNutritionalFacts(foodItem)
+ .subscribe(
</code_context>
<issue_to_address>
O aprimoramento de IA é realizado assincronamente, mas o método retorna antes que o aprimoramento seja concluído.
Os clientes podem receber itens alimentares com dados nutricionais incompletos. Considere tornar o aprimoramento síncrono ou implementar uma maneira para os clientes acessarem os dados aprimorados quando estiverem prontos.
</issue_to_address>
### Comentário 2
<location> `src/main/java/com/otavio/aifoodapp/service/FoodItemService.java:321` </location>
<code_context>
+ foodItem.setUser(user);
+
+ // Enhance with AI (async)
+ foodAiService.determineNutritionalFacts(foodItem)
+ .subscribe(
+ enhancedItem -> {
+ log.info("Successfully enhanced food item {} with AI", enhancedItem.getName());
+ foodItemRepository.save(enhancedItem);
+ },
+ error -> log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage())
+ );
+
</code_context>
<issue_to_address>
O tratamento de erros para o aprimoramento de IA apenas registra o erro e não informa o cliente.
Atualmente, as falhas no aprimoramento de IA são apenas registradas, e o cliente não é notificado. Por favor, considere atualizar a resposta para informar o cliente sobre a falha ou implementar um mecanismo de repetição.
Implementação sugerida:
```java
// Enhance with AI (async)
foodAiService.determineNutritionalFacts(foodItem)
.subscribe(
enhancedItem -> {
+ log.info("Successfully enhanced food item {} with AI", enhancedItem.getName());
+ foodItemRepository.save(enhancedItem);
},
error -> {
+ log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage());
+ throw new ResponseStatusException(
+ org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR,
+ "AI enhancement failed: " + error.getMessage()
+ );
}
);
```
Se o método for esperado para retornar imediatamente e o aprimoramento for realmente assíncrono, pode ser necessário refatorar o método para retornar um `Mono<FoodItem>` ou tipo reativo similar, para que as exceções possam ser propagadas para o cliente corretamente. Caso contrário, lançar uma exceção dentro do callback assíncrono pode não chegar ao cliente. Considere refatorar para um tipo de retorno reativo se sua pilha suportar.
</issue_to_address>
### Comentário 3
<location> `src/main/java/com/otavio/aifoodapp/mapper/RecipeMapper.java:108` </location>
<code_context>
private RecipeIngredientDto toIngredientDto(RecipeIngredient recipeIngredient) {
return new RecipeIngredientDto(
recipeIngredient.getId(),
- recipeIngredient.getName(),
+ recipeIngredient.getFoodItem() != null ? recipeIngredient.getFoodItem().getName() : "Unknown",
recipeIngredient.getQuantity(),
recipeIngredient.getUnit()
</code_context>
<issue_to_address>
Definir o nome do ingrediente como "Desconhecido" pode obscurecer problemas de dados.
Considere registrar um aviso ou lidar com nomes de itens alimentares ausentes de forma mais explícita para apoiar a depuração e manter a integridade dos dados.
Implementação sugerida:
```java
private RecipeIngredientDto toIngredientDto(RecipeIngredient recipeIngredient) {
String ingredientName;
if (recipeIngredient.getFoodItem() != null) {
ingredientName = recipeIngredient.getFoodItem().getName();
} else {
logger.warn("RecipeIngredient with ID {} has no associated FoodItem. Defaulting name to 'Unknown'.", recipeIngredient.getId());
ingredientName = "Unknown";
}
return new RecipeIngredientDto(
recipeIngredient.getId(),
ingredientName,
recipeIngredient.getQuantity(),
recipeIngredient.getUnit()
);
```
Se o `logger` ainda não estiver definido na classe, você deve adicioná-lo no início da classe:
```java
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(RecipeMapper.class);
```
</issue_to_address>
### Comentário 4
<location> `src/main/java/com/otavio/aifoodapp/controller/FoodController.java:196` </location>
<code_context>
+ * Generate recipes based on available food items
+ */
+ @GetMapping("/recipes/gen")
+ public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() {
+ User currentUser = getCurrentUser();
+ if (currentUser == null) {
+ return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
+ }
+
+ List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser);
+
+ return chatService.generateRecipe(foodItems)
+ .flatMap(recipes ->
+ Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes))
+ .subscribeOn(Schedulers.boundedElastic())
+ )
+ .map(ResponseEntity::ok)
+ .onErrorResume(e -> {
+ log.error("Error generating recipes", e);
</code_context>
<issue_to_address>
O tratamento de erros para a geração de receitas retorna uma lista vazia em caso de falha.
Considere retornar uma resposta que diferencie entre um erro e um resultado vazio, como incluir uma mensagem de erro ou código de status, para ajudar os clientes a lidar com falhas de forma apropriada.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() {
User currentUser = getCurrentUser();
if (currentUser == null) {
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
}
List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser);
return chatService.generateRecipe(foodItems)
.flatMap(recipes ->
Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes))
.subscribeOn(Schedulers.boundedElastic())
)
.map(ResponseEntity::ok)
.onErrorResume(e -> {
log.error("Error generating recipes", e);
return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(List.of()));
});
}
=======
public Mono<ResponseEntity<?>> generateRecipe() {
User currentUser = getCurrentUser();
if (currentUser == null) {
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED)
.body(Map.of("error", "Unauthorized")));
}
List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser);
return chatService.generateRecipe(foodItems)
.flatMap(recipes ->
Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes))
.subscribeOn(Schedulers.boundedElastic())
)
.map(recipeDtos -> ResponseEntity.ok(recipeDtos))
.onErrorResume(e -> {
log.error("Error generating recipes", e);
return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(Map.of(
"error", "Failed to generate recipes",
"details", e.getMessage()
)));
});
}
>>>>>>> REPLACE
</suggested_fix>
### Comentário 5
<location> `src/main/java/com/otavio/aifoodapp/controller/FoodController.java:90` </location>
<code_context>
+ * Get a specific food item by ID
+ */
+ @GetMapping("/food-items/{id}")
+ public ResponseEntity<?> getFoodById(@PathVariable Long id) {
+ try {
+ User currentUser = getCurrentUser();
+ if (currentUser == null) {
+ return unauthorized();
+ }
+
+ FoodItem foodItem = foodItemService.getById(id);
+ if (foodItem == null) {
+ return notFound("Food item", id);
+ }
+
+ if (!isAuthorized(foodItem.getUser().getId())) {
+ return unauthorized();
+ }
</code_context>
<issue_to_address>
A verificação de autorização assume que foodItem.getUser() não é nulo.
Valide que foodItem.getUser() não é nulo antes de acessar getId() para evitar uma NullPointerException.
</issue_to_address>
### Comentário 6
<location> `src/main/java/com/otavio/aifoodapp/controller/FoodController.java:44` </location>
<code_context>
+@RestController
+@RequestMapping("/api")
+@Slf4j
+public class FoodController extends BaseController {
+
+ private final FoodItemService foodItemService;
</code_context>
<issue_to_address>
Considere dividir o controlador e usar o tratamento de exceções global para reduzir a redundância e melhorar a clareza.
```markdown
Concordo que o `FoodController` consolidado está fazendo demais e repete muito código clichê (try/catch, verificações de autenticação, mapeamento de erros). Você pode simplificá-lo drasticamente:
1) Dividindo em dois controladores
2) Lançando exceções personalizadas (`UnauthorizedException`, `NotFoundException`, etc.) em vez de capturá-las localmente
3) Lidando com todas as exceções em um único `@RestControllerAdvice`
4) Injetando o usuário autenticado via Spring Security
---
### 1) Dividir controladores
```java
@RestController
@RequestMapping("/api/food-items")
public class FoodItemController {
private final FoodItemService svc;
private final FoodMapper mapper;
public FoodItemController(FoodItemService svc, FoodMapper mapper) {
this.svc = svc; this.mapper = mapper;
}
@GetMapping
public List<FoodDto> list(@AuthenticationPrincipal User user) {
return svc.listAllForUser(user).stream()
.map(mapper::map).toList();
}
@GetMapping("/{id}")
public FoodDto get(@PathVariable Long id,
@AuthenticationPrincipal User user) {
var item = svc.getById(id)
.orElseThrow(() -> new NotFoundException("Food", id));
if (!item.getUser().getId().equals(user.getId())) {
throw new UnauthorizedException();
}
return mapper.map(item);
}
// ...create, update, delete similarly...
}
```
```java
@RestController
@RequestMapping("/api/recipes")
public class RecipeController {
private final RecipeService svc;
// ...
// same pattern: throw NotFoundException/UnauthorizedException
}
```
### 2) Manipulador de exceções global
```java
@RestControllerAdvice
public class GlobalExceptionHandler {
@ExceptionHandler(UnauthorizedException.class)
public ResponseEntity<ErrorDto> onUnauthorized() {
return ResponseEntity.status(401)
.body(new ErrorDto("Unauthorized"));
}
@ExceptionHandler(NotFoundException.class)
public ResponseEntity<ErrorDto> onNotFound(NotFoundException ex) {
return ResponseEntity.status(404)
.body(new ErrorDto(ex.getMessage()));
}
@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorDto> onServerError(Exception ex) {
log.error("Unexpected error", ex);
return ResponseEntity.status(500)
.body(new ErrorDto("Internal server error"));
}
}
```
### 3) Remover try/catch por método
Após mudar para exceções + `@RestControllerAdvice`, você pode excluir todos os blocos `try { … } catch` em seus controladores, achatando drasticamente cada manipulador.
Isso mantém toda a funcionalidade, remove duplicação e torna cada controlador com menos de 50 linhas.
</issue_to_address>
### Comentário 7
<location> `src/main/java/com/otavio/aifoodapp/controller/base/BaseController.java:25` </location>
<code_context>
+ * and error handling
+ */
+@Slf4j
+public abstract class BaseController {
+
+ @Autowired
</code_context>
<issue_to_address>
Considere refatorar o BaseController usando o tratamento de exceções embutido do Spring, injeção de usuário e anotações de autorização para centralizar e simplificar a lógica comum.
Você pode reduzir drasticamente o `BaseController` ao:
1. Empurrar toda a lógica de exceção e resposta de erro para um único `@ControllerAdvice`
2. Deixar o Spring injetar o usuário atual com `@AuthenticationPrincipal` (ou um pequeno resolvedor de argumento `@CurrentUser` personalizado) em vez de chamar `SecurityContextHolder` em todos os lugares
3. Mover as regras de autorização para os métodos com `@PreAuthorize`
---
### 1) Manipulador global de exceções e erros
```java
// remove all error(), notFound(), unauthorized(), handleException() from BaseController
@ControllerAdvice
public class RestExceptionHandler extends ResponseEntityExceptionHandler {
@ExceptionHandler(ResourceNotFoundException.class)
protected ResponseEntity<ApiError> handleNotFound(ResourceNotFoundException ex) {
ApiError err = new ApiError(NOT_FOUND, ex.getMessage());
return new ResponseEntity<>(err, NOT_FOUND);
}
@ExceptionHandler(AccessDeniedException.class)
protected ResponseEntity<ApiError> handleDenied(AccessDeniedException ex) {
ApiError err = new ApiError(FORBIDDEN, "You are not authorized");
return new ResponseEntity<>(err, FORBIDDEN);
}
@ExceptionHandler(Exception.class)
protected ResponseEntity<ApiError> handleAll(Exception ex) {
ApiError err = new ApiError(INTERNAL_SERVER_ERROR, "An error occurred");
return new ResponseEntity<>(err, INTERNAL_SERVER_ERROR);
}
}
@Data
@AllArgsConstructor
class ApiError {
private HttpStatus status;
private String message;
}
```
### 2) Injetar usuário atual via `@AuthenticationPrincipal`
```java
// in your controller methods instead of getCurrentUser()/isAuthenticated()
@GetMapping("/me")
public ResponseEntity<UserDto> me(@AuthenticationPrincipal User currentUser) {
return ResponseEntity.ok(userService.toDto(currentUser));
}
```
Se você quiser um objeto principal personalizado:
```java
@Target(PARAMETER) @Retention(RUNTIME)
public @interface CurrentUser { }
@Component
public class CurrentUserResolver implements HandlerMethodArgumentResolver {
@Override
public boolean supportsParameter(MethodParameter p) {
return p.hasParameterAnnotation(CurrentUser.class)
&& p.getParameterType() == User.class;
}
@Override
public Object resolveArgument(...) {
return ((User) SecurityContextHolder.getContext().getAuthentication().getPrincipal());
}
}
// then in controllers
public ResponseEntity<?> foo(@CurrentUser User me) { … }
```
### 3) Autorização via anotações
```java
@PreAuthorize("hasRole('ADMIN') or #id == principal.id")
@GetMapping("/orders/{id}")
public ResponseEntity<OrderDto> getOrder(@PathVariable Long id) {
…
}
```
Isso remove todos os `isAuthorized()`, `isAuthenticated()`, verificações de função manuais em sua classe base e centraliza tudo em hooks fornecidos pela estrutura.
</issue_to_address>
### Comentário 8
<location> `src/main/java/com/otavio/aifoodapp/service/RecipeService.java:114` </location>
<code_context>
+ List<Recipe> recipes = recipeRepository.findByUserId(user.getId());
+
+ // Initialize lazy-loaded collections for each recipe
+ for (Recipe recipe : recipes) {
+ Hibernate.initialize(recipe.getInstructions());
+ Hibernate.initialize(recipe.getNutritionalInfo());
</code_context>
<issue_to_address>
Considere usar @EntityGraph do Spring Data JPA em métodos de repositório para lidar com o carregamento ansioso e remover a inicialização manual do Hibernate da camada de serviço.
```suggestion
// RecipeRepository.java
public interface RecipeRepository extends JpaRepository<Recipe, Long> {
@EntityGraph(attributePaths = {
"instructions",
"nutritionalInfo",
"ingredientsList",
"ingredientsList.foodItem"
})
Optional<Recipe> findWithDetailsById(Long id);
@EntityGraph(attributePaths = {
"instructions",
"nutritionalInfo",
"ingredientsList",
"ingredientsList.foodItem"
})
List<Recipe> findAllWithDetailsByUserId(Long userId);
}
```
```suggestion
// RecipeService.java
public Optional<RecipeDto> getById(Long id) {
return recipeRepository
.findWithDetailsById(id)
.map(recipeMapper::toDto);
}
public List<RecipeDto> getAllRecipesForUser(User user) {
if (user == null) {
throw new IllegalArgumentException("User cannot be null");
}
return recipeRepository
.findAllWithDetailsByUserId(user.getId())
.stream()
.map(recipeMapper::toDto)
.toList();
}
```
**O que isso faz:**
- Move todas as chamadas `Hibernate.initialize(...)` para um único método de repositório via `@EntityGraph` do Spring Data JPA.
- Elimina loops manuais e inicialização aninhada na camada de serviço.
- Mantém a funcionalidade existente intacta, reduzindo a redundância e a complexidade.
</issue_to_address>
Sourcery é gratuito para código aberto - se você gosta de nossas revisões, considere compartilhá-las ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- FoodItemService.createWithAiEnhancement uses an async subscribe then immediately returns a basic save, so AI enrichment may happen after the response—pick a consistent reactive or blocking approach to ensure returned entities always contain enriched data.
- There are inconsistencies between your documentation and controller mappings (e.g.
/recipes/genvs/recipes/generateand callingfindByIdwhen onlygetByIdexists) causing potential compile/runtime errors—please align paths and implement any missing methods. - The unified FoodController is quite large and repeats auth/authorization logic for foods and recipes—consider splitting responsibilities into smaller controllers or using Spring Security annotations like @PreAuthorize to simplify access control.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- FoodItemService.createWithAiEnhancement uses an async subscribe then immediately returns a basic save, so AI enrichment may happen after the response—pick a consistent reactive or blocking approach to ensure returned entities always contain enriched data.
- There are inconsistencies between your documentation and controller mappings (e.g. `/recipes/gen` vs `/recipes/generate` and calling `findById` when only `getById` exists) causing potential compile/runtime errors—please align paths and implement any missing methods.
- The unified FoodController is quite large and repeats auth/authorization logic for foods and recipes—consider splitting responsibilities into smaller controllers or using Spring Security annotations like @PreAuthorize to simplify access control.
## Individual Comments
### Comment 1
<location> `src/main/java/com/otavio/aifoodapp/service/FoodItemService.java:313` </location>
<code_context>
+ * @param user the user who owns the food item
+ * @return the created food item with AI-enhanced data
+ */
+ public FoodItem createWithAiEnhancement(FoodItemCreateDto createDto, User user) {
+ FoodItem foodItem = new FoodItem();
+ foodItem.setName(createDto.name());
+ foodItem.setQuantity(createDto.quantity());
+ foodItem.setExpiration(createDto.expiration());
+ foodItem.setUser(user);
+
+ // Enhance with AI (async)
+ foodAiService.determineNutritionalFacts(foodItem)
+ .subscribe(
</code_context>
<issue_to_address>
AI enhancement is performed asynchronously, but the method returns before enhancement completes.
Clients may receive food items with incomplete nutritional data. Consider making the enhancement synchronous or implementing a way for clients to access the enhanced data once it's ready.
</issue_to_address>
### Comment 2
<location> `src/main/java/com/otavio/aifoodapp/service/FoodItemService.java:321` </location>
<code_context>
+ foodItem.setUser(user);
+
+ // Enhance with AI (async)
+ foodAiService.determineNutritionalFacts(foodItem)
+ .subscribe(
+ enhancedItem -> {
+ log.info("Successfully enhanced food item {} with AI", enhancedItem.getName());
+ foodItemRepository.save(enhancedItem);
+ },
+ error -> log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage())
+ );
+
</code_context>
<issue_to_address>
Error handling for AI enhancement only logs the error and does not inform the client.
Currently, failures in AI enhancement are only logged, and the client is not notified. Please consider updating the response to inform the client of the failure or implement a retry mechanism.
Suggested implementation:
```java
// Enhance with AI (async)
foodAiService.determineNutritionalFacts(foodItem)
.subscribe(
enhancedItem -> {
log.info("Successfully enhanced food item {} with AI", enhancedItem.getName());
foodItemRepository.save(enhancedItem);
},
error -> {
log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage());
throw new ResponseStatusException(
org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR,
"AI enhancement failed: " + error.getMessage()
);
}
);
```
If the method is expected to return immediately and the enhancement is truly asynchronous, you may need to refactor the method to return a `Mono<FoodItem>` or similar reactive type, so that exceptions can be propagated to the client properly. Otherwise, throwing an exception inside the async callback may not reach the client. Consider refactoring to a reactive return type if your stack supports it.
</issue_to_address>
### Comment 3
<location> `src/main/java/com/otavio/aifoodapp/mapper/RecipeMapper.java:108` </location>
<code_context>
private RecipeIngredientDto toIngredientDto(RecipeIngredient recipeIngredient) {
return new RecipeIngredientDto(
recipeIngredient.getId(),
- recipeIngredient.getName(),
+ recipeIngredient.getFoodItem() != null ? recipeIngredient.getFoodItem().getName() : "Unknown",
recipeIngredient.getQuantity(),
recipeIngredient.getUnit()
</code_context>
<issue_to_address>
Defaulting ingredient name to "Unknown" may obscure data issues.
Consider logging a warning or handling missing food item names more explicitly to support debugging and maintain data integrity.
Suggested implementation:
```java
private RecipeIngredientDto toIngredientDto(RecipeIngredient recipeIngredient) {
String ingredientName;
if (recipeIngredient.getFoodItem() != null) {
ingredientName = recipeIngredient.getFoodItem().getName();
} else {
logger.warn("RecipeIngredient with ID {} has no associated FoodItem. Defaulting name to 'Unknown'.", recipeIngredient.getId());
ingredientName = "Unknown";
}
return new RecipeIngredientDto(
recipeIngredient.getId(),
ingredientName,
recipeIngredient.getQuantity(),
recipeIngredient.getUnit()
);
```
If the `logger` is not already defined in the class, you should add it at the top of the class:
```java
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(RecipeMapper.class);
```
</issue_to_address>
### Comment 4
<location> `src/main/java/com/otavio/aifoodapp/controller/FoodController.java:196` </location>
<code_context>
+ * Generate recipes based on available food items
+ */
+ @GetMapping("/recipes/gen")
+ public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() {
+ User currentUser = getCurrentUser();
+ if (currentUser == null) {
+ return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
+ }
+
+ List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser);
+
+ return chatService.generateRecipe(foodItems)
+ .flatMap(recipes ->
+ Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes))
+ .subscribeOn(Schedulers.boundedElastic())
+ )
+ .map(ResponseEntity::ok)
+ .onErrorResume(e -> {
+ log.error("Error generating recipes", e);
</code_context>
<issue_to_address>
Error handling for recipe generation returns an empty list on failure.
Consider returning a response that differentiates between an error and an empty result, such as including an error message or status code, to help clients handle failures appropriately.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() {
User currentUser = getCurrentUser();
if (currentUser == null) {
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build());
}
List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser);
return chatService.generateRecipe(foodItems)
.flatMap(recipes ->
Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes))
.subscribeOn(Schedulers.boundedElastic())
)
.map(ResponseEntity::ok)
.onErrorResume(e -> {
log.error("Error generating recipes", e);
return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(List.of()));
});
}
=======
public Mono<ResponseEntity<?>> generateRecipe() {
User currentUser = getCurrentUser();
if (currentUser == null) {
return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED)
.body(Map.of("error", "Unauthorized")));
}
List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser);
return chatService.generateRecipe(foodItems)
.flatMap(recipes ->
Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes))
.subscribeOn(Schedulers.boundedElastic())
)
.map(recipeDtos -> ResponseEntity.ok(recipeDtos))
.onErrorResume(e -> {
log.error("Error generating recipes", e);
return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(Map.of(
"error", "Failed to generate recipes",
"details", e.getMessage()
)));
});
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `src/main/java/com/otavio/aifoodapp/controller/FoodController.java:90` </location>
<code_context>
+ * Get a specific food item by ID
+ */
+ @GetMapping("/food-items/{id}")
+ public ResponseEntity<?> getFoodById(@PathVariable Long id) {
+ try {
+ User currentUser = getCurrentUser();
+ if (currentUser == null) {
+ return unauthorized();
+ }
+
+ FoodItem foodItem = foodItemService.getById(id);
+ if (foodItem == null) {
+ return notFound("Food item", id);
+ }
+
+ if (!isAuthorized(foodItem.getUser().getId())) {
+ return unauthorized();
+ }
</code_context>
<issue_to_address>
Authorization check assumes foodItem.getUser() is non-null.
Validate that foodItem.getUser() is not null before accessing getId() to prevent a NullPointerException.
</issue_to_address>
### Comment 6
<location> `src/main/java/com/otavio/aifoodapp/controller/FoodController.java:44` </location>
<code_context>
+@RestController
+@RequestMapping("/api")
+@Slf4j
+public class FoodController extends BaseController {
+
+ private final FoodItemService foodItemService;
</code_context>
<issue_to_address>
Consider splitting the controller and using global exception handling to reduce boilerplate and improve clarity.
```markdown
I agree the consolidated `FoodController` is doing too much and repeats a lot of boilerplate (try/catch, auth checks, error mapping). You can dramatically simplify it by:
1) Splitting into two controllers
2) Throwing custom exceptions (`UnauthorizedException`, `NotFoundException`, etc.) instead of catching them locally
3) Handling all exceptions in a single `@RestControllerAdvice`
4) Injecting the authenticated user via Spring Security
---
### 1) Split controllers
```java
@RestController
@RequestMapping("/api/food-items")
public class FoodItemController {
private final FoodItemService svc;
private final FoodMapper mapper;
public FoodItemController(FoodItemService svc, FoodMapper mapper) {
this.svc = svc; this.mapper = mapper;
}
@GetMapping
public List<FoodDto> list(@AuthenticationPrincipal User user) {
return svc.listAllForUser(user).stream()
.map(mapper::map).toList();
}
@GetMapping("/{id}")
public FoodDto get(@PathVariable Long id,
@AuthenticationPrincipal User user) {
var item = svc.getById(id)
.orElseThrow(() -> new NotFoundException("Food", id));
if (!item.getUser().getId().equals(user.getId())) {
throw new UnauthorizedException();
}
return mapper.map(item);
}
// ...create, update, delete similarly...
}
```
```java
@RestController
@RequestMapping("/api/recipes")
public class RecipeController {
private final RecipeService svc;
// ...
// same pattern: throw NotFoundException/UnauthorizedException
}
```
### 2) Global exception handler
```java
@RestControllerAdvice
public class GlobalExceptionHandler {
@ExceptionHandler(UnauthorizedException.class)
public ResponseEntity<ErrorDto> onUnauthorized() {
return ResponseEntity.status(401)
.body(new ErrorDto("Unauthorized"));
}
@ExceptionHandler(NotFoundException.class)
public ResponseEntity<ErrorDto> onNotFound(NotFoundException ex) {
return ResponseEntity.status(404)
.body(new ErrorDto(ex.getMessage()));
}
@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorDto> onServerError(Exception ex) {
log.error("Unexpected error", ex);
return ResponseEntity.status(500)
.body(new ErrorDto("Internal server error"));
}
}
```
### 3) Remove per-method try/catch
After moving to exceptions + `@RestControllerAdvice`, you can delete all `try { … } catch` blocks in your controllers, drastically flattening each handler.
This keeps all functionality, removes duplication, and makes each controller < 50 lines.
</issue_to_address>
### Comment 7
<location> `src/main/java/com/otavio/aifoodapp/controller/base/BaseController.java:25` </location>
<code_context>
+ * and error handling
+ */
+@Slf4j
+public abstract class BaseController {
+
+ @Autowired
</code_context>
<issue_to_address>
Consider refactoring BaseController by using Spring's built-in exception handling, user injection, and authorization annotations to centralize and simplify common logic.
You can drastically slim down `BaseController` by:
1. Pushing all exception‐ and error‐response logic into a single `@ControllerAdvice`
2. Letting Spring inject the current user with `@AuthenticationPrincipal` (or a small custom `@CurrentUser` argument resolver) instead of calling `SecurityContextHolder` everywhere
3. Moving authorization rules onto methods with `@PreAuthorize`
---
### 1) Global exception‐ & error‐handler
```java
// remove all error(), notFound(), unauthorized(), handleException() from BaseController
@ControllerAdvice
public class RestExceptionHandler extends ResponseEntityExceptionHandler {
@ExceptionHandler(ResourceNotFoundException.class)
protected ResponseEntity<ApiError> handleNotFound(ResourceNotFoundException ex) {
ApiError err = new ApiError(NOT_FOUND, ex.getMessage());
return new ResponseEntity<>(err, NOT_FOUND);
}
@ExceptionHandler(AccessDeniedException.class)
protected ResponseEntity<ApiError> handleDenied(AccessDeniedException ex) {
ApiError err = new ApiError(FORBIDDEN, "You are not authorized");
return new ResponseEntity<>(err, FORBIDDEN);
}
@ExceptionHandler(Exception.class)
protected ResponseEntity<ApiError> handleAll(Exception ex) {
ApiError err = new ApiError(INTERNAL_SERVER_ERROR, "An error occurred");
return new ResponseEntity<>(err, INTERNAL_SERVER_ERROR);
}
}
@Data
@AllArgsConstructor
class ApiError {
private HttpStatus status;
private String message;
}
```
### 2) Inject current user via `@AuthenticationPrincipal`
```java
// in your controller methods instead of getCurrentUser()/isAuthenticated()
@GetMapping("/me")
public ResponseEntity<UserDto> me(@AuthenticationPrincipal User currentUser) {
return ResponseEntity.ok(userService.toDto(currentUser));
}
```
If you want a custom principal object:
```java
@Target(PARAMETER) @Retention(RUNTIME)
public @interface CurrentUser { }
@Component
public class CurrentUserResolver implements HandlerMethodArgumentResolver {
@Override
public boolean supportsParameter(MethodParameter p) {
return p.hasParameterAnnotation(CurrentUser.class)
&& p.getParameterType() == User.class;
}
@Override
public Object resolveArgument(...) {
return ((User) SecurityContextHolder.getContext().getAuthentication().getPrincipal());
}
}
// then in controllers
public ResponseEntity<?> foo(@CurrentUser User me) { … }
```
### 3) Authorization via annotations
```java
@PreAuthorize("hasRole('ADMIN') or #id == principal.id")
@GetMapping("/orders/{id}")
public ResponseEntity<OrderDto> getOrder(@PathVariable Long id) {
…
}
```
This removes all of `isAuthorized()`, `isAuthenticated()`, manual role checks in your base class, and centralizes everything in framework–provided hooks.
</issue_to_address>
### Comment 8
<location> `src/main/java/com/otavio/aifoodapp/service/RecipeService.java:114` </location>
<code_context>
+ List<Recipe> recipes = recipeRepository.findByUserId(user.getId());
+
+ // Initialize lazy-loaded collections for each recipe
+ for (Recipe recipe : recipes) {
+ Hibernate.initialize(recipe.getInstructions());
+ Hibernate.initialize(recipe.getNutritionalInfo());
</code_context>
<issue_to_address>
Consider using Spring Data JPA's @EntityGraph in repository methods to handle eager loading and remove manual Hibernate initialization from the service layer.
```suggestion
// RecipeRepository.java
public interface RecipeRepository extends JpaRepository<Recipe, Long> {
@EntityGraph(attributePaths = {
"instructions",
"nutritionalInfo",
"ingredientsList",
"ingredientsList.foodItem"
})
Optional<Recipe> findWithDetailsById(Long id);
@EntityGraph(attributePaths = {
"instructions",
"nutritionalInfo",
"ingredientsList",
"ingredientsList.foodItem"
})
List<Recipe> findAllWithDetailsByUserId(Long userId);
}
```
```suggestion
// RecipeService.java
public Optional<RecipeDto> getById(Long id) {
return recipeRepository
.findWithDetailsById(id)
.map(recipeMapper::toDto);
}
public List<RecipeDto> getAllRecipesForUser(User user) {
if (user == null) {
throw new IllegalArgumentException("User cannot be null");
}
return recipeRepository
.findAllWithDetailsByUserId(user.getId())
.stream()
.map(recipeMapper::toDto)
.toList();
}
```
**What this does:**
- Moves all `Hibernate.initialize(...)` calls into a single repository method via Spring Data JPA’s `@EntityGraph`.
- Eliminates manual loops and nested initialization in the service layer.
- Keeps existing functionality intact while reducing boilerplate and complexity.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+313
to
+320
| public FoodItem createWithAiEnhancement(FoodItemCreateDto createDto, User user) { | ||
| FoodItem foodItem = new FoodItem(); | ||
| foodItem.setName(createDto.name()); | ||
| foodItem.setQuantity(createDto.quantity()); | ||
| foodItem.setExpiration(createDto.expiration()); | ||
| foodItem.setUser(user); | ||
|
|
||
| // Enhance with AI (async) |
There was a problem hiding this comment.
issue (bug_risk): O aprimoramento de IA é realizado assincronamente, mas o método retorna antes que o aprimoramento seja concluído.
Os clientes podem receber itens alimentares com dados nutricionais incompletos. Considere tornar o aprimoramento síncrono ou implementar uma maneira para os clientes acessarem os dados aprimorados quando estiverem prontos.
Original comment in English
issue (bug_risk): AI enhancement is performed asynchronously, but the method returns before enhancement completes.
Clients may receive food items with incomplete nutritional data. Consider making the enhancement synchronous or implementing a way for clients to access the enhanced data once it's ready.
Comment on lines
+321
to
+327
| foodAiService.determineNutritionalFacts(foodItem) | ||
| .subscribe( | ||
| enhancedItem -> { | ||
| log.info("Successfully enhanced food item {} with AI", enhancedItem.getName()); | ||
| foodItemRepository.save(enhancedItem); | ||
| }, | ||
| error -> log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage()) |
There was a problem hiding this comment.
suggestion (bug_risk): O tratamento de erros para o aprimoramento de IA apenas registra o erro e não informa o cliente.
Atualmente, as falhas no aprimoramento de IA são apenas registradas, e o cliente não é notificado. Por favor, considere atualizar a resposta para informar o cliente sobre a falha ou implementar um mecanismo de repetição.
Implementação sugerida:
// Enhance with AI (async)
foodAiService.determineNutritionalFacts(foodItem)
.subscribe(
enhancedItem -> {
log.info("Successfully enhanced food item {} with AI", enhancedItem.getName());
foodItemRepository.save(enhancedItem);
},
error -> {
log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage());
throw new ResponseStatusException(
org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR,
"AI enhancement failed: " + error.getMessage()
);
}
);Se o método for esperado para retornar imediatamente e o aprimoramento for realmente assíncrono, pode ser necessário refatorar o método para retornar um Mono<FoodItem> ou tipo reativo similar, para que as exceções possam ser propagadas para o cliente corretamente. Caso contrário, lançar uma exceção dentro do callback assíncrono pode não chegar ao cliente. Considere refatorar para um tipo de retorno reativo se sua pilha suportar.
Original comment in English
suggestion (bug_risk): Error handling for AI enhancement only logs the error and does not inform the client.
Currently, failures in AI enhancement are only logged, and the client is not notified. Please consider updating the response to inform the client of the failure or implement a retry mechanism.
Suggested implementation:
// Enhance with AI (async)
foodAiService.determineNutritionalFacts(foodItem)
.subscribe(
enhancedItem -> {
log.info("Successfully enhanced food item {} with AI", enhancedItem.getName());
foodItemRepository.save(enhancedItem);
},
error -> {
log.error("Failed to enhance food item {} with AI: {}", foodItem.getName(), error.getMessage());
throw new ResponseStatusException(
org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR,
"AI enhancement failed: " + error.getMessage()
);
}
);If the method is expected to return immediately and the enhancement is truly asynchronous, you may need to refactor the method to return a Mono<FoodItem> or similar reactive type, so that exceptions can be propagated to the client properly. Otherwise, throwing an exception inside the async callback may not reach the client. Consider refactoring to a reactive return type if your stack supports it.
Comment on lines
-108
to
109
| recipeIngredient.getId(), | ||
| recipeIngredient.getName(), | ||
| recipeIngredient.getFoodItem() != null ? recipeIngredient.getFoodItem().getName() : "Unknown", | ||
| recipeIngredient.getQuantity(), |
There was a problem hiding this comment.
suggestion: Definir o nome do ingrediente como "Desconhecido" pode obscurecer problemas de dados.
Considere registrar um aviso ou lidar com nomes de itens alimentares ausentes de forma mais explícita para apoiar a depuração e manter a integridade dos dados.
Implementação sugerida:
private RecipeIngredientDto toIngredientDto(RecipeIngredient recipeIngredient) {
String ingredientName;
if (recipeIngredient.getFoodItem() != null) {
ingredientName = recipeIngredient.getFoodItem().getName();
} else {
logger.warn("RecipeIngredient with ID {} has no associated FoodItem. Defaulting name to 'Unknown'.", recipeIngredient.getId());
ingredientName = "Unknown";
}
return new RecipeIngredientDto(
recipeIngredient.getId(),
ingredientName,
recipeIngredient.getQuantity(),
recipeIngredient.getUnit()
);Se o logger ainda não estiver definido na classe, você deve adicioná-lo no início da classe:
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(RecipeMapper.class);Original comment in English
suggestion: Defaulting ingredient name to "Unknown" may obscure data issues.
Consider logging a warning or handling missing food item names more explicitly to support debugging and maintain data integrity.
Suggested implementation:
private RecipeIngredientDto toIngredientDto(RecipeIngredient recipeIngredient) {
String ingredientName;
if (recipeIngredient.getFoodItem() != null) {
ingredientName = recipeIngredient.getFoodItem().getName();
} else {
logger.warn("RecipeIngredient with ID {} has no associated FoodItem. Defaulting name to 'Unknown'.", recipeIngredient.getId());
ingredientName = "Unknown";
}
return new RecipeIngredientDto(
recipeIngredient.getId(),
ingredientName,
recipeIngredient.getQuantity(),
recipeIngredient.getUnit()
);If the logger is not already defined in the class, you should add it at the top of the class:
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(RecipeMapper.class);
Comment on lines
+196
to
+215
| public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() { | ||
| User currentUser = getCurrentUser(); | ||
| if (currentUser == null) { | ||
| return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); | ||
| } | ||
|
|
||
| List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser); | ||
|
|
||
| return chatService.generateRecipe(foodItems) | ||
| .flatMap(recipes -> | ||
| Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes)) | ||
| .subscribeOn(Schedulers.boundedElastic()) | ||
| ) | ||
| .map(ResponseEntity::ok) | ||
| .onErrorResume(e -> { | ||
| log.error("Error generating recipes", e); | ||
| return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| .body(List.of())); | ||
| }); | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): O tratamento de erros para a geração de receitas retorna uma lista vazia em caso de falha.
Considere retornar uma resposta que diferencie entre um erro e um resultado vazio, como incluir uma mensagem de erro ou código de status, para ajudar os clientes a lidar com falhas de forma apropriada.
Suggested change
| public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() { | |
| User currentUser = getCurrentUser(); | |
| if (currentUser == null) { | |
| return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); | |
| } | |
| List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser); | |
| return chatService.generateRecipe(foodItems) | |
| .flatMap(recipes -> | |
| Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes)) | |
| .subscribeOn(Schedulers.boundedElastic()) | |
| ) | |
| .map(ResponseEntity::ok) | |
| .onErrorResume(e -> { | |
| log.error("Error generating recipes", e); | |
| return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | |
| .body(List.of())); | |
| }); | |
| } | |
| public Mono<ResponseEntity<?>> generateRecipe() { | |
| User currentUser = getCurrentUser(); | |
| if (currentUser == null) { | |
| return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED) | |
| .body(Map.of("error", "Unauthorized"))); | |
| } | |
| List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser); | |
| return chatService.generateRecipe(foodItems) | |
| .flatMap(recipes -> | |
| Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes)) | |
| .subscribeOn(Schedulers.boundedElastic()) | |
| ) | |
| .map(recipeDtos -> ResponseEntity.ok(recipeDtos)) | |
| .onErrorResume(e -> { | |
| log.error("Error generating recipes", e); | |
| return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | |
| .body(Map.of( | |
| "error", "Failed to generate recipes", | |
| "details", e.getMessage() | |
| ))); | |
| }); | |
| } |
Original comment in English
suggestion (bug_risk): Error handling for recipe generation returns an empty list on failure.
Consider returning a response that differentiates between an error and an empty result, such as including an error message or status code, to help clients handle failures appropriately.
Suggested change
| public Mono<ResponseEntity<List<RecipeDto>>> generateRecipe() { | |
| User currentUser = getCurrentUser(); | |
| if (currentUser == null) { | |
| return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()); | |
| } | |
| List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser); | |
| return chatService.generateRecipe(foodItems) | |
| .flatMap(recipes -> | |
| Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes)) | |
| .subscribeOn(Schedulers.boundedElastic()) | |
| ) | |
| .map(ResponseEntity::ok) | |
| .onErrorResume(e -> { | |
| log.error("Error generating recipes", e); | |
| return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | |
| .body(List.of())); | |
| }); | |
| } | |
| public Mono<ResponseEntity<?>> generateRecipe() { | |
| User currentUser = getCurrentUser(); | |
| if (currentUser == null) { | |
| return Mono.just(ResponseEntity.status(HttpStatus.UNAUTHORIZED) | |
| .body(Map.of("error", "Unauthorized"))); | |
| } | |
| List<FoodItem> foodItems = foodItemService.listAllForUser(currentUser); | |
| return chatService.generateRecipe(foodItems) | |
| .flatMap(recipes -> | |
| Mono.fromCallable(() -> recipeService.saveAndMapToDto(recipes)) | |
| .subscribeOn(Schedulers.boundedElastic()) | |
| ) | |
| .map(recipeDtos -> ResponseEntity.ok(recipeDtos)) | |
| .onErrorResume(e -> { | |
| log.error("Error generating recipes", e); | |
| return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | |
| .body(Map.of( | |
| "error", "Failed to generate recipes", | |
| "details", e.getMessage() | |
| ))); | |
| }); | |
| } |
Comment on lines
+90
to
+99
| public ResponseEntity<?> getFoodById(@PathVariable Long id) { | ||
| try { | ||
| User currentUser = getCurrentUser(); | ||
| if (currentUser == null) { | ||
| return unauthorized(); | ||
| } | ||
|
|
||
| FoodItem foodItem = foodItemService.getById(id); | ||
| if (foodItem == null) { | ||
| return notFound("Food item", id); |
There was a problem hiding this comment.
issue (bug_risk): A verificação de autorização assume que foodItem.getUser() não é nulo.
Valide que foodItem.getUser() não é nulo antes de acessar getId() para evitar uma NullPointerException.
Original comment in English
issue (bug_risk): Authorization check assumes foodItem.getUser() is non-null.
Validate that foodItem.getUser() is not null before accessing getId() to prevent a NullPointerException.
| @RestController | ||
| @RequestMapping("/api") | ||
| @Slf4j | ||
| public class FoodController extends BaseController { |
There was a problem hiding this comment.
issue (complexity): Considere dividir o controlador e usar o tratamento de exceções global para reduzir a redundância e melhorar a clareza.
Concordo que o `FoodController` consolidado está fazendo demais e repete muito código clichê (try/catch, verificações de autenticação, mapeamento de erros). Você pode simplificá-lo drasticamente:
1) Dividindo em dois controladores
2) Lançando exceções personalizadas (`UnauthorizedException`, `NotFoundException`, etc.) em vez de capturá-las localmente
3) Lidando com todas as exceções em um único `@RestControllerAdvice`
4) Injetando o usuário autenticado via Spring Security
---
### 1) Dividir controladores
```java
@RestController
@RequestMapping("/api/food-items")
public class FoodItemController {
private final FoodItemService svc;
private final FoodMapper mapper;
public FoodItemController(FoodItemService svc, FoodMapper mapper) {
this.svc = svc; this.mapper = mapper;
}
@GetMapping
public List<FoodDto> list(@AuthenticationPrincipal User user) {
return svc.listAllForUser(user).stream()
.map(mapper::map).toList();
}
@GetMapping("/{id}")
public FoodDto get(@PathVariable Long id,
@AuthenticationPrincipal User user) {
var item = svc.getById(id)
.orElseThrow(() -> new NotFoundException("Food", id));
if (!item.getUser().getId().equals(user.getId())) {
throw new UnauthorizedException();
}
return mapper.map(item);
}
// ...create, update, delete similarly...
}@RestController
@RequestMapping("/api/recipes")
public class RecipeController {
private final RecipeService svc;
// ...
// same pattern: throw NotFoundException/UnauthorizedException
}2) Manipulador de exceções global
@RestControllerAdvice
public class GlobalExceptionHandler {
@ExceptionHandler(UnauthorizedException.class)
public ResponseEntity<ErrorDto> onUnauthorized() {
return ResponseEntity.status(401)
.body(new ErrorDto("Unauthorized"));
}
@ExceptionHandler(NotFoundException.class)
public ResponseEntity<ErrorDto> onNotFound(NotFoundException ex) {
return ResponseEntity.status(404)
.body(new ErrorDto(ex.getMessage()));
}
@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorDto> onServerError(Exception ex) {
log.error("Unexpected error", ex);
return ResponseEntity.status(500)
.body(new ErrorDto("Internal server error"));
}
}3) Remover try/catch por método
Após mudar para exceções + @RestControllerAdvice, você pode excluir todos os blocos try { … } catch em seus controladores, achatando drasticamente cada manipulador.
Isso mantém toda a funcionalidade, remove duplicação e torna cada controlador com menos de 50 linhas.
Original comment in English
issue (complexity): Consider splitting the controller and using global exception handling to reduce boilerplate and improve clarity.
I agree the consolidated `FoodController` is doing too much and repeats a lot of boilerplate (try/catch, auth checks, error mapping). You can dramatically simplify it by:
1) Splitting into two controllers
2) Throwing custom exceptions (`UnauthorizedException`, `NotFoundException`, etc.) instead of catching them locally
3) Handling all exceptions in a single `@RestControllerAdvice`
4) Injecting the authenticated user via Spring Security
---
### 1) Split controllers
```java
@RestController
@RequestMapping("/api/food-items")
public class FoodItemController {
private final FoodItemService svc;
private final FoodMapper mapper;
public FoodItemController(FoodItemService svc, FoodMapper mapper) {
this.svc = svc; this.mapper = mapper;
}
@GetMapping
public List<FoodDto> list(@AuthenticationPrincipal User user) {
return svc.listAllForUser(user).stream()
.map(mapper::map).toList();
}
@GetMapping("/{id}")
public FoodDto get(@PathVariable Long id,
@AuthenticationPrincipal User user) {
var item = svc.getById(id)
.orElseThrow(() -> new NotFoundException("Food", id));
if (!item.getUser().getId().equals(user.getId())) {
throw new UnauthorizedException();
}
return mapper.map(item);
}
// ...create, update, delete similarly...
}@RestController
@RequestMapping("/api/recipes")
public class RecipeController {
private final RecipeService svc;
// ...
// same pattern: throw NotFoundException/UnauthorizedException
}2) Global exception handler
@RestControllerAdvice
public class GlobalExceptionHandler {
@ExceptionHandler(UnauthorizedException.class)
public ResponseEntity<ErrorDto> onUnauthorized() {
return ResponseEntity.status(401)
.body(new ErrorDto("Unauthorized"));
}
@ExceptionHandler(NotFoundException.class)
public ResponseEntity<ErrorDto> onNotFound(NotFoundException ex) {
return ResponseEntity.status(404)
.body(new ErrorDto(ex.getMessage()));
}
@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorDto> onServerError(Exception ex) {
log.error("Unexpected error", ex);
return ResponseEntity.status(500)
.body(new ErrorDto("Internal server error"));
}
}3) Remove per-method try/catch
After moving to exceptions + @RestControllerAdvice, you can delete all try { … } catch blocks in your controllers, drastically flattening each handler.
This keeps all functionality, removes duplication, and makes each controller < 50 lines.
| * and error handling | ||
| */ | ||
| @Slf4j | ||
| public abstract class BaseController { |
There was a problem hiding this comment.
issue (complexity): Considere refatorar o BaseController usando o tratamento de exceções embutido do Spring, injeção de usuário e anotações de autorização para centralizar e simplificar a lógica comum.
Você pode reduzir drasticamente o BaseController ao:
- Empurrar toda a lógica de exceção e resposta de erro para um único
@ControllerAdvice - Deixar o Spring injetar o usuário atual com
@AuthenticationPrincipal(ou um pequeno resolvedor de argumento@CurrentUserpersonalizado) em vez de chamarSecurityContextHolderem todos os lugares - Mover as regras de autorização para os métodos com
@PreAuthorize
1) Manipulador global de exceções e erros
// remove all error(), notFound(), unauthorized(), handleException() from BaseController
@ControllerAdvice
public class RestExceptionHandler extends ResponseEntityExceptionHandler {
@ExceptionHandler(ResourceNotFoundException.class)
protected ResponseEntity<ApiError> handleNotFound(ResourceNotFoundException ex) {
ApiError err = new ApiError(NOT_FOUND, ex.getMessage());
return new ResponseEntity<>(err, NOT_FOUND);
}
@ExceptionHandler(AccessDeniedException.class)
protected ResponseEntity<ApiError> handleDenied(AccessDeniedException ex) {
ApiError err = new ApiError(FORBIDDEN, "You are not authorized");
return new ResponseEntity<>(err, FORBIDDEN);
}
@ExceptionHandler(Exception.class)
protected ResponseEntity<ApiError> handleAll(Exception ex) {
ApiError err = new ApiError(INTERNAL_SERVER_ERROR, "An error occurred");
return new ResponseEntity<>(err, INTERNAL_SERVER_ERROR);
}
}
@Data
@AllArgsConstructor
class ApiError {
private HttpStatus status;
private String message;
}2) Injetar usuário atual via @AuthenticationPrincipal
// in your controller methods instead of getCurrentUser()/isAuthenticated()
@GetMapping("/me")
public ResponseEntity<UserDto> me(@AuthenticationPrincipal User currentUser) {
return ResponseEntity.ok(userService.toDto(currentUser));
}Se você quiser um objeto principal personalizado:
@Target(PARAMETER) @Retention(RUNTIME)
public @interface CurrentUser { }
@Component
public class CurrentUserResolver implements HandlerMethodArgumentResolver {
@Override
public boolean supportsParameter(MethodParameter p) {
return p.hasParameterAnnotation(CurrentUser.class)
&& p.getParameterType() == User.class;
}
@Override
public Object resolveArgument(...) {
return ((User) SecurityContextHolder.getContext().getAuthentication().getPrincipal());
}
}
// then in controllers
public ResponseEntity<?> foo(@CurrentUser User me) { … }3) Autorização via anotações
@PreAuthorize("hasRole('ADMIN') or #id == principal.id")
@GetMapping("/orders/{id}")
public ResponseEntity<OrderDto> getOrder(@PathVariable Long id) {
…
}Isso remove todos os isAuthorized(), isAuthenticated(), verificações de função manuais em sua classe base e centraliza tudo em hooks fornecidos pela estrutura.
Original comment in English
issue (complexity): Consider refactoring BaseController by using Spring's built-in exception handling, user injection, and authorization annotations to centralize and simplify common logic.
You can drastically slim down BaseController by:
- Pushing all exception‐ and error‐response logic into a single
@ControllerAdvice - Letting Spring inject the current user with
@AuthenticationPrincipal(or a small custom@CurrentUserargument resolver) instead of callingSecurityContextHoldereverywhere - Moving authorization rules onto methods with
@PreAuthorize
1) Global exception‐ & error‐handler
// remove all error(), notFound(), unauthorized(), handleException() from BaseController
@ControllerAdvice
public class RestExceptionHandler extends ResponseEntityExceptionHandler {
@ExceptionHandler(ResourceNotFoundException.class)
protected ResponseEntity<ApiError> handleNotFound(ResourceNotFoundException ex) {
ApiError err = new ApiError(NOT_FOUND, ex.getMessage());
return new ResponseEntity<>(err, NOT_FOUND);
}
@ExceptionHandler(AccessDeniedException.class)
protected ResponseEntity<ApiError> handleDenied(AccessDeniedException ex) {
ApiError err = new ApiError(FORBIDDEN, "You are not authorized");
return new ResponseEntity<>(err, FORBIDDEN);
}
@ExceptionHandler(Exception.class)
protected ResponseEntity<ApiError> handleAll(Exception ex) {
ApiError err = new ApiError(INTERNAL_SERVER_ERROR, "An error occurred");
return new ResponseEntity<>(err, INTERNAL_SERVER_ERROR);
}
}
@Data
@AllArgsConstructor
class ApiError {
private HttpStatus status;
private String message;
}2) Inject current user via @AuthenticationPrincipal
// in your controller methods instead of getCurrentUser()/isAuthenticated()
@GetMapping("/me")
public ResponseEntity<UserDto> me(@AuthenticationPrincipal User currentUser) {
return ResponseEntity.ok(userService.toDto(currentUser));
}If you want a custom principal object:
@Target(PARAMETER) @Retention(RUNTIME)
public @interface CurrentUser { }
@Component
public class CurrentUserResolver implements HandlerMethodArgumentResolver {
@Override
public boolean supportsParameter(MethodParameter p) {
return p.hasParameterAnnotation(CurrentUser.class)
&& p.getParameterType() == User.class;
}
@Override
public Object resolveArgument(...) {
return ((User) SecurityContextHolder.getContext().getAuthentication().getPrincipal());
}
}
// then in controllers
public ResponseEntity<?> foo(@CurrentUser User me) { … }3) Authorization via annotations
@PreAuthorize("hasRole('ADMIN') or #id == principal.id")
@GetMapping("/orders/{id}")
public ResponseEntity<OrderDto> getOrder(@PathVariable Long id) {
…
}This removes all of isAuthorized(), isAuthenticated(), manual role checks in your base class, and centralizes everything in framework–provided hooks.
| List<Recipe> recipes = recipeRepository.findByUserId(user.getId()); | ||
|
|
||
| // Initialize lazy-loaded collections for each recipe | ||
| for (Recipe recipe : recipes) { |
There was a problem hiding this comment.
issue (complexity): Considere usar @entitygraph do Spring Data JPA em métodos de repositório para lidar com o carregamento ansioso e remover a inicialização manual do Hibernate da camada de serviço.
Suggested change
| for (Recipe recipe : recipes) { | |
| // RecipeRepository.java | |
| public interface RecipeRepository extends JpaRepository<Recipe, Long> { | |
| @EntityGraph(attributePaths = { | |
| "instructions", | |
| "nutritionalInfo", | |
| "ingredientsList", | |
| "ingredientsList.foodItem" | |
| }) | |
| Optional<Recipe> findWithDetailsById(Long id); | |
| @EntityGraph(attributePaths = { | |
| "instructions", | |
| "nutritionalInfo", | |
| "ingredientsList", | |
| "ingredientsList.foodItem" | |
| }) | |
| List<Recipe> findAllWithDetailsByUserId(Long userId); | |
| } |
Suggested change
| for (Recipe recipe : recipes) { | |
| // RecipeService.java | |
| public Optional<RecipeDto> getById(Long id) { | |
| return recipeRepository | |
| .findWithDetailsById(id) | |
| .map(recipeMapper::toDto); | |
| } | |
| public List<RecipeDto> getAllRecipesForUser(User user) { | |
| if (user == null) { | |
| throw new IllegalArgumentException("User cannot be null"); | |
| } | |
| return recipeRepository | |
| .findAllWithDetailsByUserId(user.getId()) | |
| .stream() | |
| .map(recipeMapper::toDto) | |
| .toList(); | |
| } |
O que isso faz:
- Move todas as chamadas
Hibernate.initialize(...)para um único método de repositório via@EntityGraphdo Spring Data JPA. - Elimina loops manuais e inicialização aninhada na camada de serviço.
- Mantém a funcionalidade existente intacta, reduzindo a redundância e a complexidade.
Original comment in English
issue (complexity): Consider using Spring Data JPA's @entitygraph in repository methods to handle eager loading and remove manual Hibernate initialization from the service layer.
Suggested change
| for (Recipe recipe : recipes) { | |
| // RecipeRepository.java | |
| public interface RecipeRepository extends JpaRepository<Recipe, Long> { | |
| @EntityGraph(attributePaths = { | |
| "instructions", | |
| "nutritionalInfo", | |
| "ingredientsList", | |
| "ingredientsList.foodItem" | |
| }) | |
| Optional<Recipe> findWithDetailsById(Long id); | |
| @EntityGraph(attributePaths = { | |
| "instructions", | |
| "nutritionalInfo", | |
| "ingredientsList", | |
| "ingredientsList.foodItem" | |
| }) | |
| List<Recipe> findAllWithDetailsByUserId(Long userId); | |
| } |
Suggested change
| for (Recipe recipe : recipes) { | |
| // RecipeService.java | |
| public Optional<RecipeDto> getById(Long id) { | |
| return recipeRepository | |
| .findWithDetailsById(id) | |
| .map(recipeMapper::toDto); | |
| } | |
| public List<RecipeDto> getAllRecipesForUser(User user) { | |
| if (user == null) { | |
| throw new IllegalArgumentException("User cannot be null"); | |
| } | |
| return recipeRepository | |
| .findAllWithDetailsByUserId(user.getId()) | |
| .stream() | |
| .map(recipeMapper::toDto) | |
| .toList(); | |
| } |
What this does:
- Moves all
Hibernate.initialize(...)calls into a single repository method via Spring Data JPA’s@EntityGraph. - Eliminates manual loops and nested initialization in the service layer.
- Keeps existing functionality intact while reducing boilerplate and complexity.
OtavioXimarelli
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@sourcery-ai
Resumo por Sourcery
Consolidar e unificar controladores de API para itens alimentares e receitas, enriquecer configurações de serviço e segurança, introduzir controladores de usuário e base, expandir fluxos de criação de alimentos impulsionados por IA, atualizar versões de Java/Docker e atualizar a documentação da API com Guia de Início Rápido e detalhes técnicos.
Novos Recursos:
Melhorias:
Compilação:
Documentação:
Original summary in English
Summary by Sourcery
Consolidate and unify API controllers for food items and recipes, enrich service and security configurations, introduce user and base controllers, expand AI-powered food creation flows, bump Java/Docker versions, and update API documentation with Quick Start and technical details.
New Features:
Enhancements:
Build:
Documentation: