From 7324ec9e833eef340694ec4b2b09083871506312 Mon Sep 17 00:00:00 2001 From: kunal kumar <162681701+Kunxal@users.noreply.github.com> Date: Fri, 3 Apr 2026 13:02:49 +0530 Subject: [PATCH 1/2] Add logging to IncentiveServiceImpl methods # String Concatenation Logic Error in IncentiveServiceImpl ## Description There is a critical bug in the `saveIncentivesMaster` method of `IncentiveServiceImpl.java` where `String.concat()` is incorrectly used within a `forEach` loop. Due to Java's String immutability, the concatenation result is discarded, causing the method to always return an empty string for the saved records summary. ## Location **File:** `src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java` **Method:** `saveIncentivesMaster` **Lines:** 49-51 ## Current Code (Buggy) ```java @Override public String saveIncentivesMaster(List activityDTOS) { try { activityDTOS.forEach(activityDTO -> { IncentiveActivity activity = incentivesRepo.findIncentiveMasterByNameAndGroup(activityDTO.getName(), activityDTO.getGroup()); if (activity == null) { activity = new IncentiveActivity(); modelMapper.map(activityDTO, activity); } else { Long id = activity.getId(); modelMapper.map(activityDTO, activity); activity.setId(id); } incentivesRepo.save(activity); }); String saved = ""; activityDTOS.forEach(dto -> saved.concat(dto.getGroup() + ": " + dto.getName())); // BUG HERE return "saved master data for " + saved; // Always returns "saved master data for " } catch (Exception e) { } return null; } ``` ## The Problem In Java, `String` objects are **immutable**. The `concat()` method does not modify the original string; instead, it returns a **new** string with the concatenated result. Since the return value is not assigned back to `saved`, the variable remains empty. ### What Happens: | Iteration | `saved.concat(...)` Result | `saved` Variable Value | |-----------|---------------------------|------------------------| | 1 | `"Group1: Activity1"` | `""` (unchanged) | | 2 | `"Group2: Activity2"` | `""` (unchanged) | | Final Return | - | `"saved master data for "` | ## Impact * **Incorrect API Response:** The method always returns `"saved master data for "` without listing the actual saved records * **Poor User Experience:** API consumers cannot verify which incentive activities were saved * **Debugging Difficulty:** Developers cannot trace what data was processed from logs ## Proposed Fix Replace `String` with `StringBuilder` which is mutable and designed for such use cases: ```java @Override public String saveIncentivesMaster(List activityDTOS) { try { activityDTOS.forEach(activityDTO -> { IncentiveActivity activity = incentivesRepo.findIncentiveMasterByNameAndGroup(activityDTO.getName(), activityDTO.getGroup()); if (activity == null) { activity = new IncentiveActivity(); modelMapper.map(activityDTO, activity); } else { Long id = activity.getId(); modelMapper.map(activityDTO, activity); activity.setId(id); } incentivesRepo.save(activity); }); // FIX: Use StringBuilder for mutable string operations StringBuilder saved = new StringBuilder(); activityDTOS.forEach(dto -> saved.append(dto.getGroup()) .append(": ") .append(dto.getName()) .append(", ") ); // Remove trailing ", " if present if (saved.length() > 0) { saved.setLength(saved.length() - 2); } return "saved master data for " + saved.toString(); } catch (Exception e) { logger.error("Error saving incentive master data: " + e.getMessage(), e); } return null; } ``` ## Alternative Fix (Java 8+ Streams) ```java String saved = activityDTOS.stream() .map(dto -> dto.getGroup() + ": " + dto.getName()) .collect(Collectors.joining(", ")); return "saved master data for " + saved; ``` ## Additional Improvements Needed 1. **Empty Catch Block:** The catch block at lines 52-54 is empty and should log the exception: ```java } catch (Exception e) { logger.error("Error in saveIncentivesMaster: " + e.getMessage(), e); } ``` 2. **Return Value Consistency:** Consider returning an empty string or a proper error message instead of `null` on failure. ## Test Case to Verify ```java @Test public void testSaveIncentivesMaster_ReturnsCorrectSummary() { // Arrange List dtos = Arrays.asList( createDto("MATERNAL HEALTH", "ANC-1"), createDto("MATERNAL HEALTH", "ANC-FULL") ); // Act String result = incentiveService.saveIncentivesMaster(dtos); // Assert assertEquals("saved master data for MATERNAL HEALTH: ANC-1, MATERNAL HEALTH: ANC-FULL", result); } ``` ## Priority **Medium** - This is a functional bug that affects API response accuracy but does not cause system crashes or data corruption. ## Labels `bug`, `good first issue`, `help wanted`, `code-quality` --- ## References * [Java String Immutability](https://docs.oracle.com/javase/tutorial/java/data/strings.html) * [StringBuilder vs String Concatenation Performance](https://docs.oracle.com/javase/8/docs/api/java/lang/StringBuilder.html) --- .../flw/service/impl/IncentiveServiceImpl.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java b/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java index e031ef7c..d4d7b6bd 100644 --- a/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java +++ b/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java @@ -10,6 +10,8 @@ import com.iemr.flw.repo.iemr.IncentivesRepo; import com.iemr.flw.service.IncentiveService; import org.modelmapper.ModelMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -20,6 +22,7 @@ @Service public class IncentiveServiceImpl implements IncentiveService { + private final Logger logger = LoggerFactory.getLogger(IncentiveServiceImpl.class); ModelMapper modelMapper = new ModelMapper(); @@ -46,11 +49,15 @@ public String saveIncentivesMaster(List activityDTOS) { } incentivesRepo.save(activity); }); - String saved = ""; - activityDTOS.forEach(dto -> saved.concat(dto.getGroup() + ": " + dto.getName())); - return "saved master data for " + saved ; + StringBuilder saved = new StringBuilder(); + activityDTOS.forEach(dto -> saved.append(dto.getGroup()).append(": ").append(dto.getName()).append(", ")); + // Remove trailing ", " if present + if (saved.length() > 0) { + saved.setLength(saved.length() - 2); + } + return "saved master data for " + saved.toString(); } catch (Exception e) { - + logger.error("Error saving incentive master data: " + e.getMessage(), e); } return null; } @@ -66,7 +73,7 @@ public String getIncentiveMaster(IncentiveRequestDTO incentiveRequestDTO) { Gson gson = new GsonBuilder().setDateFormat("MMM dd, yyyy h:mm:ss a").create(); return gson.toJson(dtos); } catch (Exception e) { - + logger.error("Error getting incentive master data: " + e.getMessage(), e); } return null; } From 63f424169b46c8b4e80c6be35de117d8e2d1342e Mon Sep 17 00:00:00 2001 From: kunal kumar <162681701+Kunxal@users.noreply.github.com> Date: Fri, 3 Apr 2026 13:21:16 +0530 Subject: [PATCH 2/2] Handle null values in activityDTOS string concatenation Updated string concatenation to handle null values using Objects.toString. --- .../java/com/iemr/flw/service/impl/IncentiveServiceImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java b/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java index d4d7b6bd..31b37b0e 100644 --- a/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java +++ b/src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; @Service @@ -50,7 +51,7 @@ public String saveIncentivesMaster(List activityDTOS) { incentivesRepo.save(activity); }); StringBuilder saved = new StringBuilder(); - activityDTOS.forEach(dto -> saved.append(dto.getGroup()).append(": ").append(dto.getName()).append(", ")); + activityDTOS.forEach(dto -> saved.append(Objects.toString(dto.getGroup(), "")).append(": ").append(Objects.toString(dto.getName(), "")).append(", ")); // Remove trailing ", " if present if (saved.length() > 0) { saved.setLength(saved.length() - 2);