rename composite modification : update#812
Conversation
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
This comment was marked as low quality.
This comment was marked as low quality.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java (1)
739-739:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate that
getMapMessageValues()is non-null before serialization.Line 739 calls
compositeMetadata.getMapMessageValues()without checking for null. If it returns null, Jackson's behavior depends on configuration, potentially causing NPEs or serializing the string"null"unexpectedly.🛡️ Proposed fix: Add null check
private void renameCompositeModifications(CompositeModificationEntity compositeEntity, CompositeModificationInfos compositeMetadata) { if (compositeMetadata.getName() != null) { compositeEntity.setName(compositeMetadata.getName()); - compositeEntity.setMessageValues(new ObjectMapper().writeValueAsString(compositeMetadata.getMapMessageValues())); + Map<?, ?> messageValues = compositeMetadata.getMapMessageValues(); + if (messageValues != null) { + compositeEntity.setMessageValues(objectMapper.writeValueAsString(messageValues)); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java` at line 739, The code calls compositeMetadata.getMapMessageValues() and serializes it directly via new ObjectMapper().writeValueAsString(...), which can yield unexpected "null" or cause NPEs; update the block around compositeEntity.setMessageValues(...) to first check compositeMetadata.getMapMessageValues() for null and only call ObjectMapper().writeValueAsString when non-null, otherwise set compositeEntity.setMessageValues(...) to a sensible default (e.g., null or "{}") so serialization is deterministic; reference compositeMetadata.getMapMessageValues(), ObjectMapper.writeValueAsString, and compositeEntity.setMessageValues when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`:
- Line 735: Remove the `@SneakyThrows` annotation on NetworkModificationRepository
and replace it with explicit handling around the Jackson serialization call that
currently relies on it (the code inside the `@Transactional` method around line
~710). Catch JsonProcessingException (or IOException) where
objectMapper.writeValueAsString / readValue is used, log a clear error via the
repository/class logger including the exception, and rethrow a suitable
unchecked exception (e.g., a custom RepositoryException or RuntimeException)
with a descriptive message so the transaction rollback is visible and
recoverable instead of being silently suppressed by `@SneakyThrows`.
- Line 739: The code creates a new ObjectMapper() for every call when setting
compositeEntity.setMessageValues(...), causing unnecessary allocations; add a
private final ObjectMapper field to the NetworkModificationRepository class,
initialize it via constructor injection (or a single shared instance created
once in the class) and replace the inline new ObjectMapper() usage in the method
that iterates (around the compositeEntity.setMessageValues call) with that field
to reuse the same thread-safe ObjectMapper instance.
---
Duplicate comments:
In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`:
- Line 739: The code calls compositeMetadata.getMapMessageValues() and
serializes it directly via new ObjectMapper().writeValueAsString(...), which can
yield unexpected "null" or cause NPEs; update the block around
compositeEntity.setMessageValues(...) to first check
compositeMetadata.getMapMessageValues() for null and only call
ObjectMapper().writeValueAsString when non-null, otherwise set
compositeEntity.setMessageValues(...) to a sensible default (e.g., null or "{}")
so serialization is deterministic; reference
compositeMetadata.getMapMessageValues(), ObjectMapper.writeValueAsString, and
compositeEntity.setMessageValues when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e035ad4-f58d-4f2a-8b3f-a9bf9cfd9753
📒 Files selected for processing (1)
src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/gridsuite/modification/server/repositories/CompositeModificationRepository.java (1)
45-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid
@SneakyThrowshere and stop creatingObjectMapperper call.At Line 45-49, this hides serialization failures and allocates a new mapper on every update. Handle the Jackson exception explicitly and reuse a single mapper instance.
🔧 Suggested patch
+import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import lombok.SneakyThrows; import org.gridsuite.modification.dto.CompositeModificationInfos; import org.gridsuite.modification.server.entities.CompositeModificationEntity; @@ public interface CompositeModificationRepository extends JpaRepository<CompositeModificationEntity, UUID> { + ObjectMapper OBJECT_MAPPER = new ObjectMapper(); @@ - `@SneakyThrows` default void renameCompositeModifications(CompositeModificationEntity compositeEntity, CompositeModificationInfos compositeMetadata) { compositeEntity.setName(compositeMetadata.getName()); - compositeEntity.setMessageValues(new ObjectMapper().writeValueAsString(compositeMetadata.getMapMessageValues())); + try { + compositeEntity.setMessageValues(OBJECT_MAPPER.writeValueAsString(compositeMetadata.getMapMessageValues())); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Failed to serialize composite messageValues for " + compositeEntity.getId(), e); + } } }#!/bin/bash # Verify occurrences in repository layer: rg -n --type=java '@SneakyThrows|new ObjectMapper\(' src/main/java/org/gridsuite/modification/server/repositories🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/modification/server/repositories/CompositeModificationRepository.java` around lines 45 - 49, Remove the use of `@SneakyThrows` on renameCompositeModifications and stop instantiating new ObjectMapper each call: add a shared ObjectMapper instance (e.g., a private static final ObjectMapper) in the repository class and use it to serialize compositeMetadata.getMapMessageValues(); catch and handle the Jackson checked exception (JsonProcessingException) in renameCompositeModifications—either wrap it in a specific runtime exception or handle/log appropriately—and update CompositeModificationEntity.setMessageValues(...) only after successful serialization; reference the method name renameCompositeModifications, the types CompositeModificationEntity and CompositeModificationInfos, and the ObjectMapper usage when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/main/java/org/gridsuite/modification/server/repositories/CompositeModificationRepository.java`:
- Around line 45-49: Remove the use of `@SneakyThrows` on
renameCompositeModifications and stop instantiating new ObjectMapper each call:
add a shared ObjectMapper instance (e.g., a private static final ObjectMapper)
in the repository class and use it to serialize
compositeMetadata.getMapMessageValues(); catch and handle the Jackson checked
exception (JsonProcessingException) in renameCompositeModifications—either wrap
it in a specific runtime exception or handle/log appropriately—and update
CompositeModificationEntity.setMessageValues(...) only after successful
serialization; reference the method name renameCompositeModifications, the types
CompositeModificationEntity and CompositeModificationInfos, and the ObjectMapper
usage when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3a7ba662-6cc3-41f3-a111-d177dfb9c9ef
📒 Files selected for processing (2)
src/main/java/org/gridsuite/modification/server/repositories/CompositeModificationRepository.javasrc/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
|



PR Summary