feat(test): Unit Tests for Controllers and utils for Common API #258
Conversation
WalkthroughThis update introduces a comprehensive suite of new unit and integration tests for a wide range of controllers, services, and utility classes within the codebase. The changes also add new test dependencies to the build configuration and correct a minor logging message typo in an existing controller. No changes were made to exported or public entities in the main source code. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Class
participant Controller as Controller
participant Service as Service (mocked)
participant Repo as Repository (mocked)
participant Util as Utility (mocked)
participant Logger as Logger (mocked)
Test->>Controller: Invoke endpoint/method (HTTP or direct)
Controller->>Service: Call service method (mocked)
Service-->>Controller: Return mock data or throw exception
Controller->>Logger: Log info/error (mocked)
Controller-->>Test: Return response (JSON, HTTP, etc.)
Test->>Test: Assert response, verify service/logger calls
Note: The above diagram represents the high-level structure of almost all newly added tests, which follow the pattern of invoking a controller or service, mocking dependencies, and asserting responses and interactions. Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🔭 Outside diff range comments (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
1-1043: Critical: Entire test file is commented outThis entire test file (1043 lines) is commented out, which means no tests are actually being executed for the
IEMRAdminController. This is particularly concerning given the size of this test file and the importance of testing authentication and user management functionality.Please either:
- Uncomment and fix the tests if they are ready
- Remove the file if the tests are not ready
- Explain why the tests are commented out if there's a specific reason
Having commented-out test files in the codebase creates confusion and provides no value.
♻️ Duplicate comments (1)
src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java (1)
131-131: Same error message format issue.This line has the same missing space issue between "URL" and the exception class name.
🧹 Nitpick comments (33)
src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellRegistrationTest.java (1)
1-42: LGTM! Well-documented test with clear intent.The test class is well-structured with:
- Proper mocking setup
- Clear comments explaining design decisions
- Focused verification of the core functionality
The comment about not mocking logging behavior (lines 39-40) is particularly helpful for understanding the test scope.
Consider removing the redundant
times(1)parameter sinceverify()defaults to verifying exactly one invocation:- verify(registrationService, times(1)).registerBeneficiary(); + verify(registrationService).registerBeneficiary();src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForEmailTest.java (1)
1-35: LGTM! Clean test with good AAA structure.The test effectively validates the email service scheduling functionality with:
- Proper mocking using
@ExtendWith(MockitoExtension.class)- Clear AAA (Arrange-Act-Assert) pattern with comments
- Focused verification of the expected behavior
For consistency with other test classes in this package, consider using the same setup pattern:
-@ExtendWith(MockitoExtension.class) -class ScheduleJobServiceForEmailTest { +class ScheduleJobServiceForEmailTest { + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + }Also, consider removing the redundant
times(1)parameter:- verify(emailService, times(1)).publishEmail(); + verify(emailService).publishEmail();src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForUnblockTest.java (1)
28-31: Remove unnecessary empty @beforeeach method.The empty
setUp()method is redundant since Mockito automatically injects mocks when using@ExtendWith(MockitoExtension.class),@InjectMocks, and@Mockannotations.- @BeforeEach - void setUp() { - // Mockito will inject the mocks automatically due to @InjectMocks and @Mock - } -src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForAvniRegistrationTest.java (1)
13-27: Consider using @ExtendWith(MockitoExtension.class) for consistency.Other test classes in this PR use
@ExtendWith(MockitoExtension.class)instead ofMockitoAnnotations.openMocks(this). Consider adopting the same approach for consistency across the test suite.+import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; +@ExtendWith(MockitoExtension.class) class ScheduleJobServiceForAvniRegistrationTest { @Mock private DoorToDoorService doorToDoorService; @Mock private JobExecutionContext jobExecutionContext; @InjectMocks private ScheduleJobServiceForAvniRegistration scheduleJobServiceForAvniRegistration; - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - }src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java (2)
19-30: Use @ExtendWith(MockitoExtension.class) for consistency.+import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; +@ExtendWith(MockitoExtension.class) class LanguageControllerTest { @Mock private LanguageService languageService; @InjectMocks private LanguageController languageController; - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - }
32-37: Remove redundant test method.The
setLanguageService_shouldSetServicetest is unnecessary. The@InjectMocksannotation already ensures that the mock is injected into the controller. This test doesn't add value to the test suite.- @Test - void setLanguageService_shouldSetService() { - // Verify that the service is injected by Mockito - assertNotNull(languageController.languageService, "LanguageService should be set by @InjectMocks"); - }src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (1)
139-139: Consider adding more edge case testsThe test coverage is comprehensive, but consider adding tests for:
- Null request body scenario
- Missing or invalid Authorization header
- Testing with different date formats in the request
src/test/java/com/iemr/common/controller/everwell/callhandle/EverwellCallControllerTest.java (1)
16-16: Consider using MockMvc for consistencyThis test class directly calls controller methods, while
BRDIntegrationControllerTestuses MockMvc. Consider adopting a consistent testing approach across controller tests.Also applies to: 30-30
src/test/java/com/iemr/common/controller/report/CustomerRelationshipReportsTest.java (1)
33-34: Remove unused mock fieldThe
mapperfield is mocked but never used in any test. Remove it to keep the test class clean.- @Mock - private Report1097Mapper mapper;src/test/java/com/iemr/common/controller/customization/CustomizationControllerTest.java (1)
95-97: Consider logger verification alternativesThe comments correctly note the limitation of mocking private final logger fields. If logger verification is important, consider:
- Using PowerMockito (though it has limitations with newer Java versions)
- Making the logger package-private for testing
- Using a LogCaptor or similar testing utility
- Accepting this limitation and focusing on behavior testing
Also applies to: 257-259, 491-493
src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java (1)
117-119: Consider using a more robust JSON comparison approach.While the JsonElement comparison works, consider using a JSON assertion library like JSONAssert for more robust JSON comparison that handles formatting differences and provides better error messages.
Example using JSONAssert:
import org.skyscreamer.jsonassert.JSONAssert; // Replace lines 117-119 with: JSONAssert.assertEquals(serviceResponse, output.getData(), false);src/test/java/com/iemr/common/controller/eausadha/EAusadhaControllerTest.java (3)
66-75: Consider using consistent JSON parsing approach.While JSONObject works, consider using the Gson approach established in other test files for consistency across the test suite. This would also leverage the
OutputResponseclass methods for cleaner assertions.-JSONObject jsonResult = new JSONObject(result); -assertEquals(OutputResponse.SUCCESS, jsonResult.getInt("statusCode")); -assertEquals("Success", jsonResult.getString("status")); -assertEquals("Success", jsonResult.getString("errorMessage")); +OutputResponse output = gson.fromJson(result, OutputResponse.class); +assertEquals(OutputResponse.SUCCESS, output.getStatusCode()); +assertEquals("Success", output.getStatus()); +assertEquals("Success", output.getErrorMessage());
77-77: Remove redundant verification qualifiers.The
times(1)parameter inverify()is redundant as it's the default behavior. This can be simplified for cleaner test code.-verify(eAusadhaService, times(1)).createEAusadha(eAusadhaDTO, authorization); +verify(eAusadhaService).createEAusadha(eAusadhaDTO, authorization);Also applies to: 110-110
98-101: Use OutputResponse constants for better maintainability.Consider using
OutputResponse.GENERIC_FAILUREconstant instead of the hardcoded value 5000 for better maintainability and consistency.-assertEquals(5000, jsonResult.getInt("statusCode")); +assertEquals(OutputResponse.GENERIC_FAILURE, jsonResult.getInt("statusCode"));src/test/java/com/iemr/common/controller/questionconfig/QuestionnaireControllerTest.java (1)
27-50: Reconsider the necessity of explicit setter testing.Testing setters explicitly is generally not recommended unless they contain business logic. The
@InjectMocksannotation already handles dependency injection, making explicit setter tests redundant.Consider removing this test or converting it to test actual business functionality if the setter has specific behavior that needs validation.
src/test/java/com/iemr/common/controller/mctshistory/OutboundHistoryControllerTest.java (2)
23-29: Remove redundant setter call in setUp methodThe explicit call to
setOutboundHistoryServiceon line 28 is redundant. The@InjectMocksannotation already handles dependency injection via the setter method.@BeforeEach void setUp() { MockitoAnnotations.openMocks(this); - // The @InjectMocks annotation handles the injection via the setter - // but we can explicitly call it to ensure it works as expected for the test method. - outboundHistoryController.setOutboundHistoryService(outboundHistoryService); }
31-41: Consider removing trivial setter testThis test doesn't verify that the setter actually works - it only checks that the controller is not null. Setter coverage should be achieved through functional tests that use the dependency.
If you must test the setter explicitly, verify its behavior:
@Test void testSetOutboundHistoryService() { - // Verify that the service was set. Since @InjectMocks already calls the setter, - // we can just assert that the controller instance is not null, implying setup worked. - assertNotNull(outboundHistoryController); - // A more direct way to test the setter would be to use reflection or a spy, - // but for a simple setter, ensuring the object is ready for other tests is sufficient. - // If we wanted to be super explicit, we could create a new controller instance - // and call the setter, then use reflection to check the private field. - // However, @InjectMocks already verifies the setter is called. + // Create a new controller instance + OutboundHistoryController controller = new OutboundHistoryController(); + OutboundHistoryService newService = mock(OutboundHistoryService.class); + + // Set the service + controller.setOutboundHistoryService(newService); + + // Verify by calling a method that uses the service + when(newService.getCallHistory(anyString())).thenReturn("test"); + String result = controller.getCallHistory("{}"); + + verify(newService).getCallHistory("{}"); }Or simply remove this test as setter coverage will be implicitly achieved through other tests.
src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java (3)
29-39: Improve or remove trivial setter testThis test creates a mock service but doesn't verify it was actually set. Either verify the setter's behavior or remove this test.
82-85: Use assertTrue instead of assertEquals for boolean assertions-assertEquals(true, actualStatus.startsWith("Failed with " + thrownException.getMessage() + " at "), "Status message should start correctly."); -assertEquals(true, actualStatus.endsWith(".Please try after some time. If error is still seen, contact your administrator."), "Status message should end correctly."); +assertTrue(actualStatus.startsWith("Failed with " + thrownException.getMessage() + " at "), "Status message should start correctly."); +assertTrue(actualStatus.endsWith(".Please try after some time. If error is still seen, contact your administrator."), "Status message should end correctly.");
89-92: Implement logger verification as indicated by TODOThe TODO comment indicates missing logger verification. Consider using a mocked logger or a test appender to verify error logging.
Would you like me to provide an example of how to verify logger calls using Logback's ListAppender or by mocking the logger?
src/test/java/com/iemr/common/controller/callhandling/CallControllerTest.java (1)
714-722: Use static imports to reduce verbosityThe test uses fully qualified class names which makes the code harder to read. Consider using static imports.
Add these static imports at the top of the file:
import static org.mockito.Mockito.mock; import com.iemr.common.model.beneficiary.CallRequestByIDModel; import com.iemr.common.model.beneficiary.BeneficiaryCallModel; import com.iemr.common.data.callhandling.BeneficiaryCall; import com.iemr.common.data.callhandling.OutboundCallRequest; import com.iemr.common.data.users.ProviderServiceMapping; import com.fasterxml.jackson.databind.ObjectMapper;Then simplify the code:
-com.iemr.common.model.beneficiary.CallRequestByIDModel requestModel = new com.iemr.common.model.beneficiary.CallRequestByIDModel(); -com.iemr.common.model.beneficiary.BeneficiaryCallModel callModel = new com.iemr.common.model.beneficiary.BeneficiaryCallModel(); +CallRequestByIDModel requestModel = new CallRequestByIDModel(); +BeneficiaryCallModel callModel = new BeneficiaryCallModel();Also applies to: 751-753, 766-768
src/test/java/com/iemr/common/controller/lonic/LonicControllerTest.java (3)
37-45: Consider using @mock with constructor injection instead of reflection.Using reflection to inject the mock logger is brittle and makes tests harder to maintain. Consider refactoring the
LonicControllerto accept the logger via constructor injection, which would eliminate the need for reflection in tests.@BeforeEach void setUp() throws NoSuchFieldException, IllegalAccessException { MockitoAnnotations.openMocks(this); - - mockLogger = Mockito.mock(Logger.class); - Field loggerField = LonicController.class.getDeclaredField("logger"); - loggerField.setAccessible(true); - loggerField.set(lonicController, mockLogger); }If constructor injection isn't feasible, consider using
ReflectionTestUtils.setField()from Spring Test for cleaner reflection-based field injection.
129-129: Remove trailing whitespace.There's trailing whitespace after the verify statement that should be removed for code cleanliness.
- verify(lonicService, times(0)).findLonicRecordList(any(LonicDescription.class)); - } + verify(lonicService, times(0)).findLonicRecordList(any(LonicDescription.class)); + }
156-156: Remove trailing whitespace.There's trailing whitespace after the verify statement that should be removed for code cleanliness.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java (1)
169-185: Fix indentation inconsistency.The
@Testannotation is not properly aligned with the rest of the class structure.-@Test + @Test void changeNotificationStatus_shouldReturnSuccess_whenMultipleNotifications() throws Exception {src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java (1)
97-114: Consider removing or enhancing this redundant test.This test appears to duplicate
shouldReturnESanjeevaniURL_whenServiceReturnsValidURLwith different test data. If the intent is to test logger coverage, consider:
- Using a logger mock to verify logging behavior
- Testing edge cases or different code paths
- Or remove this test if it doesn't add value beyond the existing success test
src/test/java/com/iemr/common/utils/validator/ValidatorTest.java (1)
208-215: Consider clarifying the test assertion message.The assertion at line 214 states "Result should contain 'loginIPAddress' when get session fails but set still happens". While the logic is correct, consider rephrasing for clarity, such as: "Result should preserve original 'loginIPAddress' when getSessionObject fails but setSessionObject succeeds".
src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (1)
155-172: Simplify exception handling in tests.The try-catch blocks in these test methods make them harder to understand. Consider:
- If testing malformed JSON handling, use valid JSON with invalid content rather than syntactically invalid JSON
- Let test exceptions propagate naturally - the test framework will handle them
- Use
@Test(expected = ...)orassertThrows()if you need to verify specific exceptions- try { - mockMvc.perform(post(ADD_SUPPORT_ACTION_URL, id) - .header(AUTH_HEADER, BEARER_TOKEN) - .contentType(MediaType.APPLICATION_JSON) - .content(malformedJson) - .accept(MediaType.APPLICATION_JSON)) - .andExpect(status().isOk()); // Should still return 200 even with errors - } catch (Exception e) { - // If malformed JSON causes issues before reaching controller, test with valid JSON - mockMvc.perform(post(ADD_SUPPORT_ACTION_URL, id) - .header(AUTH_HEADER, BEARER_TOKEN) - .contentType(MediaType.APPLICATION_JSON) - .content("{}") - .accept(MediaType.APPLICATION_JSON)) - .andExpect(status().isOk()) - .andExpect(content().contentType(MediaType.APPLICATION_JSON)); - } + // Test with empty JSON object instead of malformed JSON + mockMvc.perform(post(ADD_SUPPORT_ACTION_URL, id) + .header(AUTH_HEADER, BEARER_TOKEN) + .contentType(MediaType.APPLICATION_JSON) + .content("{}") + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON));Also applies to: 195-215
src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java (1)
303-319: Consider using @test(expected = ...) or assertThrows for exception testing.The current approach using try-catch is functional but could be more explicit using JUnit 5's
assertThrowsmethod for cleaner exception testing.@Test - void testSendSMS_JsonMappingException() throws JsonMappingException, JsonProcessingException { + void testSendSMS_JsonMappingException() { // Invalid JSON input to trigger JsonParseException String invalidRequestBody = "invalid json string"; - // This should throw JsonParseException (which extends JsonProcessingException) since JSON parsing happens outside the try-catch block - try { - String responseString = smsController.sendSMS(invalidRequestBody, httpServletRequest); - // If we get here, the test should fail because we expect an exception - // But let's handle this gracefully in case the behavior changes - assertNotNull(responseString); - } catch (JsonProcessingException e) { - // This is expected - the controller throws JsonParseException for invalid JSON - // JsonParseException is a subclass of JsonProcessingException - assertNotNull(e.getMessage()); - assertEquals(true, e instanceof com.fasterxml.jackson.core.JsonParseException); - } + JsonProcessingException exception = assertThrows(JsonProcessingException.class, () -> { + smsController.sendSMS(invalidRequestBody, httpServletRequest); + }); + + assertNotNull(exception.getMessage()); + assertTrue(exception instanceof com.fasterxml.jackson.core.JsonParseException); }src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java (1)
49-51: Fix typos in URL constants.The URL constants contain typos that should be corrected for consistency and readability.
- private static final String START_ASSESSMENT_URL = "/lungAssessment/startAssesment"; - private static final String GET_ASSESSMENT_URL = "/lungAssessment/getAssesment/{assessmentId}"; - private static final String GET_ASSESSMENT_DETAILS_URL = "/lungAssessment/getAssesmentDetails/{patientId}"; + private static final String START_ASSESSMENT_URL = "/lungAssessment/startAssessment"; + private static final String GET_ASSESSMENT_URL = "/lungAssessment/getAssessment/{assessmentId}"; + private static final String GET_ASSESSMENT_DETAILS_URL = "/lungAssessment/getAssessmentDetails/{patientId}";src/test/java/com/iemr/common/controller/scheme/SchemeControllerTest.java (2)
137-137: Use assertEquals for string comparisonFor better readability and clearer test intent, use
assertEqualsinstead ofassertTruewith.equals().- assertTrue(outputResponse.getData().equals("[]")); // Empty list toString() is "[]" + assertEquals("[]", outputResponse.getData()); // Empty list toString() is "[]"
156-156: Use assertEquals for status code comparisonFor better readability and clearer test intent, use
assertEqualsinstead ofassertTruewith==.- assertTrue(outputResponse.getStatusCode() == 5000); + assertEquals(5000, outputResponse.getStatusCode());Also applies to: 223-223
src/test/java/com/iemr/common/config/encryption/SecurePasswordTest.java (1)
20-22: Fix inconsistent lambda indentationThe lambda expressions have inconsistent indentation. Please format them consistently.
For lines 20-22:
NumberFormatException ex = assertThrows(NumberFormatException.class, () -> { - securePassword.validatePassword(originalPassword, storedPassword); -}); + securePassword.validatePassword(originalPassword, storedPassword); + });For lines 53-55:
-NumberFormatException ex = assertThrows(NumberFormatException.class, () -> { - securePassword.validatePassword(originalPassword, storedPassword); -}); + NumberFormatException ex = assertThrows(NumberFormatException.class, () -> { + securePassword.validatePassword(originalPassword, storedPassword); + });Also applies to: 53-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
pom.xml(2 hunks)src/test/java/com/iemr/common/config/encryption/SecurePasswordTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleForCallCentreTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellDataSyncTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellRegistrationTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobForNHMDashboardDataTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForAvniRegistrationTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForEmailTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForSMSTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForUnblockTest.java(1 hunks)src/test/java/com/iemr/common/controller/abdmfacility/AbdmFacilityControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/callhandling/CallControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/customization/CustomizationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/directory/DirectoryControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/eausadha/EAusadhaControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/everwell/callhandle/EverwellCallControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/honeywell/HoneywellControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/kmfilemanager/KMFileManagerControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/location/LocationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/lonic/LonicControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/mctshistory/OutboundHistoryControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/otp/OTPGatewayTest.java(1 hunks)src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/questionconfig/QuestionnaireControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/report/CustomerRelationshipReportsTest.java(1 hunks)src/test/java/com/iemr/common/controller/scheme/SchemeControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReportsTest.java(1 hunks)src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/users/EmployeeSignatureControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/version/VersionControllerTest.java(1 hunks)src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java(1 hunks)src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java(1 hunks)src/test/java/com/iemr/common/utils/CookieUtilTest.java(1 hunks)src/test/java/com/iemr/common/utils/CryptoUtilTest.java(1 hunks)src/test/java/com/iemr/common/utils/mapper/InputMapperTest.java(1 hunks)src/test/java/com/iemr/common/utils/validator/ValidatorTest.java(1 hunks)src/test/resources/git.properties(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
src/test/java/com/iemr/common/utils/CookieUtilTest.java (2)
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:40-47
Timestamp: 2025-02-21T07:42:36.497Z
Learning: In the Common-API project's CookieUtil class, JWT cookies are configured with SameSite=None to support cross-origin requests, which is required for the project's CORS functionality.
src/test/java/com/iemr/common/controller/abdmfacility/AbdmFacilityControllerTest.java (4)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/repository/abdmfacility/AbdmFacilityRepository.java:13-14
Timestamp: 2024-11-12T13:21:47.312Z
Learning: In the `getAbdamFacility` method in `AbdmFacilityRepository`, the ordering in the query is required even when returning a single object.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java (2)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
src/test/java/com/iemr/common/controller/users/EmployeeSignatureControllerTest.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (2)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
🧬 Code Graph Analysis (5)
src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
src/test/java/com/iemr/common/controller/lonic/LonicControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
src/test/java/com/iemr/common/controller/eausadha/EAusadhaControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
src/test/java/com/iemr/common/controller/users/EmployeeSignatureControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
🔇 Additional comments (47)
src/test/resources/git.properties (1)
1-7: LGTM! Well-structured test resource file.The Git properties file provides appropriate test data with realistic but generic values for testing the version controller functionality. The format and content are suitable for test scenarios.
pom.xml (1)
251-261: LGTM! Properly scoped Logback dependencies for testing.The Logback dependencies are correctly added with test scope, which is appropriate for testing infrastructure without affecting the production build.
src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellDataSyncTest.java (1)
1-38: LGTM! Well-structured unit test with proper mocking.The test class follows best practices:
- Proper use of JUnit 5 and Mockito annotations
- Clean setup with
MockitoAnnotations.openMocks()- Focused test that verifies the core functionality
- Clear verification of the expected service method call
The test effectively validates that the scheduled job triggers the data sync operation.
src/test/java/com/iemr/common/config/quartz/ScheduleForCallCentreTest.java (1)
1-36: Well-structured unit test with proper mocking.The test class follows JUnit 5 and Mockito best practices effectively. The test method is focused, clearly named, and verifies the expected behavior without unnecessary complexity.
src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForUnblockTest.java (1)
33-43: Excellent test implementation with helpful comments.The test method correctly verifies the core business logic while appropriately ignoring side effects like logging. The explanatory comments add valuable context for future maintainers.
src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForSMSTest.java (1)
15-38: Consistent and well-documented test implementation.The test maintains consistency with the other scheduled job tests while providing clear documentation about testing decisions. The comments about not stubbing
jobExecutionContextmethods are particularly valuable for understanding the test scope.src/test/java/com/iemr/common/controller/version/VersionControllerTest.java (2)
25-36: Clarify expected behavior when git.properties is missing.Both test methods use identical controller setup but expect different behaviors. The second test appears to expect the controller to handle missing
git.propertiesgracefully, but it's unclear how the same controller instance would behave differently without external state changes.Consider making the test scenarios more explicit about what conditions trigger each behavior, or mock the file system interactions to ensure predictable test outcomes.
11-23: Verified git.properties presence in test resourcesThe file
src/test/resources/git.propertiesexists, so the standalone controller test will locate it. No blocking issues remain.Optional improvements for greater reliability:
- Mock the file‐reading or inject a fake
GitPropertiesintoVersionControllerto decouple tests from external files.- Or switch to a Spring Boot test (
@SpringBootTest+@AutoConfigureMockMvc) so the application context (and its resources) is fully managed.src/test/java/com/iemr/common/utils/CryptoUtilTest.java (1)
1-70: Comprehensive test coverage with excellent edge case handling.This test class demonstrates excellent testing practices:
- Thorough padding scenarios: Tests strings requiring different amounts of padding (1 byte, 15 bytes, full block, etc.)
- Proper error handling: Verifies that invalid inputs return null rather than throwing exceptions
- Clear test organization: Well-named methods that clearly indicate what's being tested
- Security-focused: Appropriate for testing cryptographic utilities where robust error handling is crucial
The test coverage ensures both functionality correctness and resilience against malformed inputs.
src/test/java/com/iemr/common/utils/CookieUtilTest.java (4)
36-80: Comprehensive test coverage for getCookieValue!Excellent coverage of edge cases including null cookies and empty arrays.
82-114: Verify the SameSite attribute configuration is intentional.The tests show that production uses
SameSite=Strictwhile non-production usesSameSite=None. Based on the retrieved learnings, SameSite=None is required for cross-origin requests. Please confirm that usingStrictin production won't break CORS functionality.
116-160: Well-structured tests for JWT token retrieval!Good coverage of edge cases for the static method.
162-173: Case-insensitive cookie matching verifiedThe
getJwtTokenFromCookiemethod uses"Jwttoken".equalsIgnoreCase(cookie.getName()), ensuring cookie names are matched without regard to case. The existing test for"jwtTOKEN"will pass as intended.src/test/java/com/iemr/common/config/quartz/ScheduleJobForNHMDashboardDataTest.java (1)
15-64: Clean and focused test implementation!Good coverage of both success and exception scenarios for the scheduled job.
src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForAvniRegistrationTest.java (1)
29-57: Well-structured test cases!Good coverage of both success and exception scenarios.
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (1)
78-78: Verify the HTTP status code design patternThe tests expect HTTP 200 OK status even when the controller returns error responses. This is unusual as REST APIs typically return appropriate error status codes (4xx/5xx). Please verify if this is the intended behavior.
Also applies to: 93-93, 108-108, 136-136
src/test/java/com/iemr/common/controller/everwell/callhandle/EverwellCallControllerTest.java (1)
168-178: Excellent edge case coverage!Great job testing non-success service responses and null returns. This thorough approach helps ensure robust error handling.
Also applies to: 207-216
src/test/java/com/iemr/common/controller/report/CustomerRelationshipReportsTest.java (1)
71-85: Exception propagation is intentional and correctThe
CustomerRelationshipReportscontroller method declaresthrows Exceptionand does not catch service‐thrown exceptions (no try/catch or registered advice), so the test’s use ofassertThrowsto verify exception propagation aligns with the implementation.No changes required.
src/test/java/com/iemr/common/controller/customization/CustomizationControllerTest.java (1)
21-493: Excellent comprehensive test coverage!This test class provides thorough coverage of all controller methods with consistent success/exception test pairs. The structure is clean and maintainable.
src/test/java/com/iemr/common/utils/mapper/InputMapperTest.java (2)
58-85: Excellent comprehensive test coverage for JSON parsing scenarios.The tests effectively cover all major JSON input types (objects, arrays, primitives) with proper assertions. The exception handling in the try-catch blocks with
fail()is appropriate for this testing context.
127-133: Good exception testing pattern.Using
assertThrowsto verifyJsonSyntaxExceptionfor malformed JSON is the correct approach and follows JUnit 5 best practices.src/test/java/com/iemr/common/controller/abdmfacility/AbdmFacilityControllerTest.java (3)
29-37: Good test organization and setup.The use of constants for test data and proper MockMvc standalone setup is well-structured and maintainable.
77-87: Excellent authorization header validation test.Testing the missing Authorization header scenario is important for API security. The verification that no service interactions occur when the header is missing is a good practice.
58-75: No changes required: 200 OK on service exceptions is intentional
The controller’sgetMappedAbdmFacilitymethod deliberately catches all exceptions, sets the error on theOutputResponse, and returns its serialized form. By default Spring will send a 200 OK status when a handler method returns normally, so the test’s expectation ofstatus().isOk()with the error payload is correct and aligned with established patterns in this codebase.src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java (2)
40-56: Excellent test setup with proper Gson configuration.The reflection-based logger injection is appropriate for testing, and the Gson configuration correctly matches the
OutputResponse.toString()method's serialization settings. This ensures accurate JSON parsing in assertions.
94-100: Good comprehensive exception testing with logger verification.The exception handling tests properly verify both the response structure and logger interactions, ensuring complete coverage of the error handling flow.
src/test/java/com/iemr/common/controller/questionconfig/QuestionnaireControllerTest.java (2)
52-65: Good test structure for success scenario.The test properly mocks the service, verifies the response format using
OutputResponse, and confirms service interaction. This follows established patterns from the codebase.
80-87: Good acknowledgment of logger testing limitations.The comments appropriately acknowledge the challenges of testing logger interactions with final fields and suggest alternative approaches like test appenders, which shows good testing awareness.
src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java (1)
41-62: Well-structured test with proper JSON comparisonGood use of JsonElement comparison instead of string matching. This approach is more robust and handles field ordering correctly.
src/test/java/com/iemr/common/controller/callhandling/CallControllerTest.java (1)
1-944: Excellent comprehensive test coverageThis test class demonstrates thorough testing practices with comprehensive coverage of:
- Success scenarios
- Exception handling
- Null response handling
- Edge cases
- Input validation errors
Well done on the extensive test coverage!
src/test/java/com/iemr/common/controller/otp/OTPGatewayTest.java (1)
32-38: Good approach for logger verificationExcellent use of ReflectionTestUtils to inject a mock logger for verifying error logging behavior. This is a clean solution for testing logging in classes with private final logger fields.
src/test/java/com/iemr/common/controller/lonic/LonicControllerTest.java (1)
58-83: Excellent test coverage and verification approach.This test effectively covers the success scenario with proper verification of:
- Service method calls with correct arguments using
argThat()matchers- Response formatting and content
- Logging behavior
The use of argument matchers to verify the parsed DTO fields is particularly well done.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java (2)
33-57: Excellent test structure and constants usage.The use of constants for URLs, headers, and content types makes the tests more maintainable and readable. The standalone MockMvc setup is the right approach for unit testing controllers without loading the full Spring context.
358-404: Good coverage of authorization requirements.These tests properly verify that the endpoints return 404 when the required Authorization header is missing, which confirms the security requirements are properly enforced.
src/test/java/com/iemr/common/controller/kmfilemanager/KMFileManagerControllerTest.java (3)
51-61: Excellent dependency injection approach for testing.Using
ReflectionTestUtils.setField()to inject @Autowired dependencies is much cleaner than direct reflection. The manual controller instantiation ensures proper constructor injection while still allowing field-level dependency injection for testing.
63-68: Good use of helper method for test data creation.The
createSuccessOutput()helper method reduces code duplication and makes the tests more maintainable. This is a good pattern for standardizing test response creation.
70-96: Comprehensive test coverage for success and exception scenarios.These tests properly verify both the happy path and error handling, ensuring the controller behaves correctly in both scenarios. The use of Hamcrest matchers for partial string matching in error cases is appropriate.
src/test/java/com/iemr/common/controller/directory/DirectoryControllerTest.java (1)
91-136: Good MockMvc test structure for API endpoints.These tests properly use MockMvc to simulate HTTP requests and verify responses, which is the recommended approach for testing Spring MVC controllers. The test coverage includes both success and error scenarios.
src/test/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationControllerTest.java (1)
1-1081: Excellent comprehensive test coverage!This test class demonstrates thorough testing practices:
- Complete coverage of all CTI endpoints
- Consistent testing of both success and exception scenarios
- Proper verification of service interactions
- Edge case testing including header variations
- Clear test naming with @DisplayName annotations
src/test/java/com/iemr/common/utils/validator/ValidatorTest.java (1)
1-413: Excellent test implementation with comprehensive coverage!This test class demonstrates exceptional testing practices:
- Proper handling of static fields using reflection
- Comprehensive coverage of IP validation scenarios
- Thorough exception handling tests
- Good use of MockedStatic with try-with-resources
- Tests for various edge cases including null values and invalid JSON
src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (1)
36-102: Stub implementation detected in EverwellController – please confirm next steps
- All endpoints (
/everwell/getjson,/everwell/addSupportAction/{id},/everwell/editManualDoses/{id},/everwell/login) return hard-coded JSON strings viaOutputResponse.toString().- No service or repository is injected; the controller methods build and return static data.
- The existing tests in
src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java
correctly assert these hard-coded responses.Please confirm whether this stubbed behavior is intentional (e.g., for prototyping) or if you plan to replace it with real service integration. If it’s the latter, you’ll need to:
- Inject and call the appropriate service layer in
EverwellController.java.- Update or extend the tests to validate dynamic data instead of static JSON.
src/test/java/com/iemr/common/controller/honeywell/HoneywellControllerTest.java (1)
28-201: Well-structured test implementation!This test class demonstrates good testing practices:
- Mock logger injection for verifying logging behavior
- Comprehensive coverage of all endpoints
- Tests for various edge cases including null/empty request bodies
- Proper exception handling verification
- Clear test naming and structure
src/test/java/com/iemr/common/controller/users/EmployeeSignatureControllerTest.java (1)
1-161: Excellent test coverage and structure.This test class demonstrates comprehensive coverage of the
EmployeeSignatureControllerwith well-organized test methods covering all endpoints and error scenarios. The use of constants, helper methods, and clear test naming follows best practices.src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java (1)
44-61: Excellent static mocking setup for InputMapper.The proper setup and teardown of
MockedStatic<InputMapper>demonstrates good testing practices for handling static dependencies. The@AfterEachcleanup ensures test isolation.src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java (1)
54-73: Good helper method for response parsing.The
parseResponseStringmethod effectively reduces code duplication across test methods and makes the tests more readable.src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java (1)
58-65: Good multipart file creation helper.The
createTestFilemethod effectively creates consistent test data for multipart file upload testing.src/test/java/com/iemr/common/controller/location/LocationControllerTest.java (1)
62-85: Consistent error testing pattern across all endpoints.The test methods follow a consistent pattern of testing both success and exception scenarios for each endpoint, which provides good coverage and reliability.
src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/location/LocationControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/location/LocationControllerTest.java
Show resolved
Hide resolved
...ava/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReportsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
129-151: Same JSON parsing error issue as above.This test has the same problem - it's expecting a JSON parsing error instead of testing the actual controller functionality.
Apply the same fix as suggested for the
feedbackRequest_shouldReturnSuccess_whenValidRequesttest method.src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java (1)
1-270: Remove or properly manage the commented-out test file.The entire test file remains commented out. While I understand the socket connection timing concerns you mentioned, consider these alternatives instead of keeping commented code:
- Use
@Disabled("Socket tests - run with integration profile")annotation on the test class- Move to a separate integration test module that runs less frequently
- Use Maven/Gradle test profiles to conditionally include these tests
- If these tests won't be used, remove the file entirely
Keeping large blocks of commented code reduces maintainability and can cause confusion.
🧹 Nitpick comments (3)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
38-68: Consider adding test coverage for additional scenarios.The test class has good coverage but could benefit from additional edge case testing:
Consider adding tests for:
- Null request bodies
- Empty request bodies
- Boundary conditions (e.g., very large feedback IDs)
- Concurrent request handling
- Input validation for required fields
Example:
@Test void createFeedback_shouldReturnError_whenNullRequest() throws Exception { mockMvc.perform(post("/feedback/createFeedback") .header("Authorization", "Bearer test-token") .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isBadRequest()); } @Test void getFeedbackByPost_shouldReturnError_whenInvalidFeedbackID() throws Exception { Long invalidId = -1L; when(feedbackService.getFeedbackRequests(invalidId)).thenReturn(new ArrayList<>()); mockMvc.perform(post("/feedback/getfeedback/{feedbackID}", invalidId) .header("Authorization", "Bearer test-token")) .andExpect(status().isOk()) .andExpect(jsonPath("$.data").isEmpty()); }src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java (1)
37-42: Remove redundant service injection test.The
setLanguageService_shouldSetServicetest is unnecessary. The@InjectMocksannotation already ensures that the service is properly injected by Mockito. This test doesn't add value to your test suite.src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (1)
1-867: Exceptionally comprehensive test coverage!This test class provides outstanding coverage of the BeneficiaryRegistrationController with excellent testing practices:
- Comprehensive coverage of all controller endpoints with multiple scenarios
- Proper handling of MockMvc limitations by using direct controller method calls when needed
- Extensive error handling and exception testing
- Good boundary condition testing (empty responses, zero update counts)
- Well-structured mocking of all service dependencies
- Thorough testing of different search criteria and parameter combinations
The pragmatic approach of testing both via MockMvc and direct method calls ensures complete validation while working within framework constraints.
Consider breaking this large test class into smaller, feature-focused test classes (e.g.,
BeneficiarySearchControllerTest,BeneficiaryRegistrationControllerTest,BeneficiaryUpdateControllerTest) for better maintainability and organization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
pom.xml(1 hunks)src/main/java/com/iemr/common/controller/esanjeevani/ESanjeevaniController.java(1 hunks)src/main/java/com/iemr/common/controller/snomedct/SnomedController.java(2 hunks)src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerMinimalTest.java(1 hunks)src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/directory/DirectoryControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/honeywell/HoneywellControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/location/LocationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/mctshistory/OutboundHistoryControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/com/iemr/common/controller/esanjeevani/ESanjeevaniController.java
- src/main/java/com/iemr/common/controller/snomedct/SnomedController.java
🚧 Files skipped from review as they are similar to previous changes (8)
- pom.xml
- src/test/java/com/iemr/common/controller/mctshistory/OutboundHistoryControllerTest.java
- src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java
- src/test/java/com/iemr/common/controller/directory/DirectoryControllerTest.java
- src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java
- src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java
- src/test/java/com/iemr/common/controller/honeywell/HoneywellControllerTest.java
- src/test/java/com/iemr/common/controller/location/LocationControllerTest.java
🧰 Additional context used
🧠 Learnings (9)
src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerMinimalTest.java (2)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java (2)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (4)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#163
File: src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java:111-118
Timestamp: 2025-02-21T09:49:46.528Z
Learning: The `complaintResolution` and `remarks` fields in GrievanceDetails class are intentionally kept without validation constraints (@NotBlank, @Size) as per the team's decision.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (3)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
🔇 Additional comments (5)
src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerMinimalTest.java (1)
1-97: Well-structured minimal test suite!The test class effectively covers the essential HTTP layer behaviors while avoiding socket connection timeouts. The use of constants for URLs and headers improves maintainability, and the 3-second timeout annotation is a good safeguard against hanging tests.
src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java (1)
43-102: Excellent JSON parsing implementation!Great job implementing proper JSON parsing with ObjectMapper instead of string comparison. The tests comprehensively verify response structure, status codes, and data content for both success and exception scenarios.
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (1)
1-444: Exceptional test coverage!This test suite demonstrates excellent thoroughness with 24 comprehensive test cases covering:
- Standard success/failure scenarios
- Edge cases (null values, empty strings, missing fields)
- Malformed JSON handling
- Special characters and unicode support
- Large payload handling
- Authorization requirements
The consistent use of
toStringWithSerializeNulls()and proper JSON response verification makes these tests robust and maintainable.src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java (1)
1-391: Excellent comprehensive test coverage!This test class provides thorough coverage of the NotificationController with well-structured unit tests. The implementation correctly:
- Uses standalone MockMvc setup appropriate for unit testing
- Tests all endpoint variations with proper JSON request/response handling
- Covers both success and exception scenarios comprehensively
- Includes edge cases like invalid JSON, empty bodies, and missing headers
- Follows proper arrange-act-assert pattern throughout
The test approach aligns well with Spring Boot testing best practices and provides robust validation of the controller layer.
src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java (1)
1-258: Well-structured test class with proper static mocking!This test class demonstrates excellent testing practices:
- Proper static mocking of InputMapper with correct setup and teardown
- Comprehensive coverage including success, failure, and edge cases
- Good use of ReflectionTestUtils for private field access and verification
- Appropriate exception handling and logging verification
- Clean separation of concerns with dedicated setup/teardown methods
The static mock management is particularly well-implemented, ensuring no test interference.
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
70-92: Consider resolving the underlying JSON parsing configuration instead of testing error conditions.These tests expect JSON parsing failures due to "Java module system restrictions with Gson trying to access private fields in SimpleDateFormat." The use of
lenient()mocking indicates the service methods aren't actually being called, making these tests ineffective for validating business logic.However, if these parsing restrictions are truly unavoidable in your environment, the current test accurately reflects the actual system behavior.
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (2)
57-80: Remove tests that validate known bugs and focus on expected behavior.This test is disabled and validates a known serialization bug rather than testing expected behavior. Based on previous feedback, tests should verify correct behavior, not document bugs.
Consider removing all "KnownBug" test variants and keep only the "ExpectedBehavior" versions. This will:
- Reduce code duplication
- Focus on testing correct behavior
- Make the test suite more maintainable
- Prevent false confidence from validating incorrect behavior
- // Test 1: getInstitutesByLocation - Success (DISABLED due to known serialization bug) - @Test - @Disabled("Known serialization bug - BUG-2024-001: GSON fails to serialize dates due to Java module system restrictions with SimpleDateFormat") - void testGetInstitutesByLocation_Success_KnownBug() throws Exception { - // ... entire test method - }
274-334: Document the controller bug but don't validate it in tests.These tests are documenting a known controller bug where
responseObj.toString()is returned instead ofresponse.toString(), resulting in"{}"responses. While the bug documentation is useful, testing against incorrect behavior provides false confidence.Consider:
- Adding a comprehensive comment explaining the bug
- Marking these tests as
@Disabledwith a bug reference- Writing tests that expect the correct behavior (they would fail until the bug is fixed)
+ // Known Bug: Controller returns responseObj.toString() instead of response.toString() + // This causes empty JSON "{}" responses. See bug reference: BUG-2024-002 @Test + @Disabled("Bug BUG-2024-002: Controller returns wrong response object") void testGetInstituteByBranch_Success() throws Exception {
🧹 Nitpick comments (3)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
544-545: Remove duplicate status expectation.There's a duplicate
.andExpect(status().isOk())call on consecutive lines..andExpect(status().isOk()) - .andExpect(status().isOk()) .andExpected(jsonPath("$.statusCode").value(200))src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (2)
201-201: Remove debug print statements from production test code.Debug print statements should not be present in production test code as they create unnecessary console noise and are not part of the actual test assertions.
- System.out.println("DEBUG Empty Result - Actual response: " + responseBody); - System.out.println("DEBUG getInstituteTypes_ServiceException - Actual response: " + responseBody); - System.out.println("DEBUG getInstituteName_NullResult - Actual response: " + responseBody);Also applies to: 395-395, 458-458
710-723: Enhance helper methods for better test object creation.The helper methods are good but could be improved for better maintainability and flexibility.
// Helper method to create Designation objects - private Designation createDesignation(int id, String name) { + private Designation createDesignation(Integer id, String name) { Designation designation = new Designation(); designation.setDesignationID(id); designation.setDesignationName(name); + // Set other common fields with default values if needed return designation; } // Helper method to create InstituteType objects - private InstituteType createInstituteType(int id, String name) { + private InstituteType createInstituteType(Integer id, String name) { InstituteType instituteType = new InstituteType(); instituteType.setInstitutionTypeID(id); instituteType.setInstitutionType(name); + // Set other common fields with default values if needed return instituteType; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (6)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.070Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.659Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
🔇 Additional comments (8)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (4)
1-68: Excellent test class setup and structure.The test class follows Spring Boot testing best practices with proper MockitoExtension usage, comprehensive service mocking, and standalone MockMvc configuration. The package structure and imports are well-organized.
259-279: Test correctly reflects the controller's wrapper pattern for invalid JSON handling.This test accurately validates that the controller handles invalid JSON gracefully by returning HTTP 200 with success status, which aligns with the established wrapper pattern used throughout this codebase.
280-343: Excellent error handling test coverage following the wrapper pattern.These tests properly validate that service exceptions are caught and returned as structured error responses with HTTP 200 status and statusCode 5000, consistently following the established architectural pattern.
345-710: Outstanding comprehensive test coverage for all controller endpoints.This test suite provides excellent coverage of all FeedbackController endpoints with consistent validation of both success and error scenarios. The use of direct method calls for endpoints with different signatures (like
getAllFeedbackById1) shows good testing judgment.src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (4)
32-55: Well-structured test setup with comprehensive mocking.The test setup is well-organized with proper use of JUnit 5 annotations, Mockito for mocking dependencies, and MockMvc for web layer testing. The
@BeforeEachmethod properly initializes the test environment.
336-400: Comprehensive test coverage for InstituteTypes endpoint.These tests provide excellent coverage for the
getInstituteTypesendpoint, including success scenarios, empty requests, and service exceptions. The error handling validation is particularly thorough.
465-530: Thorough testing of Designations endpoint with proper error handling.The designation endpoint tests cover all major scenarios including success, empty results, service exceptions, and missing authorization headers. The test structure is consistent and well-organized.
592-594: Improve assertion logic for multiple possible outcomes.This assertion uses OR logic to check for multiple possible status codes, which makes the test less precise and harder to understand.
- assertTrue(responseBody.contains("\"statusCode\":200") || - responseBody.contains("\"statusCode\":5000")); + // Spring Boot is lenient with content types, should still parse JSON correctly + assertTrue(responseBody.contains("\"statusCode\":200"), + "Expected successful response even with wrong content type, but got: " + responseBody);⛔ Skipped due to learnings
Learnt from: Zapper9982 PR: PSMRI/Common-API#258 File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269 Timestamp: 2025-07-06T19:05:27.070Z Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (1)
216-233: Smart decision on extra fields testing based on Spring Boot behavior.Based on previous review feedback, you correctly decided not to test extra fields since Spring Boot's default Jackson configuration handles unknown properties gracefully. This avoids unnecessary test cases that don't add value.
src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (1)
22-22: Package naming convention violation.The package name
com.iemr.common.controller.everwellTestviolates Java naming conventions by using uppercase letters. It should becom.iemr.common.controller.everwell.src/test/java/com/iemr/common/controller/location/LocationControllerTest.java (2)
77-81: Potential issue with List.toString() in helper method.The
createSuccessOutputmethod callstoString()on the List, which may not produce the expected JSON format for comparison. Consider using a JSON serializer instead.
181-193: Incorrect service method used in getCity endpoint test.The
getCitytest correctly documents that the controller has a bug where it callslocationService.getDistrictBlocks()instead of the intendedgetCities()method. The test acknowledges this controller issue in the comments.src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (2)
91-113: Fix configuration issues instead of accepting them in tests.These tests expect failures due to "Java module system restrictions with Gson trying to access private fields in SimpleDateFormat". This indicates a configuration issue that should be resolved rather than accepted. Using
lenient()mocking suggests the mocks aren't being called, making these tests ineffective.Consider fixing the underlying module/dependency configuration to allow proper JSON parsing, or use proper test data that can be parsed correctly.
151-172: Fix configuration issues instead of accepting them in tests.This test has the same issue as the previous one - expecting failures due to Java module system restrictions instead of fixing the underlying configuration. The
lenient()mocking indicates the service methods aren't being called, making this test ineffective.Fix the JSON parsing configuration to enable proper testing of the actual business logic.
🧹 Nitpick comments (8)
src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java (2)
50-60: Remove unnecessary setter test for coverage.This test method only verifies that a setter method can be called without throwing an exception, which doesn't provide meaningful value. Setter methods with simple assignment logic don't require unit tests unless they contain complex validation or business logic.
Consider removing this test method:
- @Test - void testSetQuestionTypeService() { - // This test primarily ensures coverage for the setter method. - // @InjectMocks already handles the initial injection of the mock questionTypeService. - // For a simple setter, just calling it is sufficient to cover the method. - QuestionTypeService anotherMockService = mock(QuestionTypeService.class); - questionTypeController.setQuestionTypeService(anotherMockService); - // No explicit assertion is typically needed for a simple setter, - // as its primary function is assignment, which is assumed to work. - // If deep verification were needed, reflection would be required to access the private field. - }
110-113: Address incomplete logging verification TODOs.The TODO comments indicate that logging verification is incomplete. For production code, it's important to verify that error logging occurs correctly during exception scenarios.
Do you want me to help implement the logging verification using Logback's test utilities or provide guidance on testing logging behavior?
Also applies to: 159-162
src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java (1)
404-411: Consider removing unnecessary setter test.Similar to the QuestionTypeControllerTest, this setter test only verifies basic assignment functionality without meaningful business logic validation.
Consider removing this test method as it doesn't provide meaningful test coverage:
- @Test - void testSetNotificationService() { - // Test the setter method directly - NotificationService mockService = notificationService; - notificationController.setNotificationService(mockService); - - // The setter doesn't return anything, so we just verify it doesn't throw an exception - assertNotNull(notificationController); - }src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (1)
171-193: Simplify exception testing approach.The try-catch pattern for testing hardcoded controller methods adds unnecessary complexity. Since these methods return static responses, simpler tests focusing on the actual behavior would be more maintainable.
Consider simplifying exception tests for hardcoded methods:
- try { - mockMvc.perform(post(ADD_SUPPORT_ACTION_URL, id) - .header(AUTH_HEADER, BEARER_TOKEN) - .contentType(MediaType.APPLICATION_JSON) - .content(malformedJson) - .accept(MediaType.APPLICATION_JSON)) - .andExpect(status().isOk()); - } catch (Exception e) { - mockMvc.perform(post(ADD_SUPPORT_ACTION_URL, id) - .header(AUTH_HEADER, BEARER_TOKEN) - .contentType(MediaType.APPLICATION_JSON) - .content("{}") - .accept(MediaType.APPLICATION_JSON)) - .andExpect(status().isOk()) - .andExpect(content().contentType(MediaType.APPLICATION_JSON)); - } + mockMvc.perform(post(ADD_SUPPORT_ACTION_URL, id) + .header(AUTH_HEADER, BEARER_TOKEN) + .contentType(MediaType.APPLICATION_JSON) + .content("{}") + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.data.Id").value(123456789));src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (3)
221-221: Remove debug print statements from production test code.Debug print statements should be removed from production test code as they clutter the test output and are not necessary for automated testing.
- System.out.println("DEBUG Empty Result - Actual response: " + responseBody);
413-413: Remove debug print statements from production test code.Debug print statements should be removed from production test code as they clutter the test output and are not necessary for automated testing.
- System.out.println("DEBUG getInstituteTypes_ServiceException - Actual response: " + responseBody);
476-476: Remove debug print statements from production test code.Debug print statements should be removed from production test code as they clutter the test output and are not necessary for automated testing.
- System.out.println("DEBUG getInstituteName_NullResult - Actual response: " + responseBody);src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
565-567: Remove duplicate status expectation.Line 566 has a duplicate
.andExpect(status().isOk())call that should be removed.mockMvc.perform(post("/feedback/requestFeedback") .header("Authorization", "Bearer test-token") .contentType(MediaType.APPLICATION_JSON) .content(requestJson)) .andExpected(status().isOk()) - .andExpected(status().isOk()) .andExpected(jsonPath("$.statusCode").value(200)) .andExpected(jsonPath("$.status").value("Success")) .andExpected(jsonPath("$.data").exists());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
src/test/java/com/iemr/common/config/encryption/SecurePasswordTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleForCallCentreTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellDataSyncTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellRegistrationTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobForNHMDashboardDataTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForAvniRegistrationTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForEmailTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForSMSTest.java(1 hunks)src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForUnblockTest.java(1 hunks)src/test/java/com/iemr/common/controller/abdmfacility/AbdmFacilityControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/callhandling/CallControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerMinimalTest.java(1 hunks)src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/customization/CustomizationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/directory/DirectoryControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/eausadha/EAusadhaControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/everwell/callhandle/EverwellCallControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/honeywell/HoneywellControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/kmfilemanager/KMFileManagerControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/location/LocationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/lonic/LonicControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/mctshistory/OutboundHistoryControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/otp/OTPGatewayTest.java(1 hunks)src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/questionconfig/QuestionnaireControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/report/CustomerRelationshipReportsTest.java(1 hunks)src/test/java/com/iemr/common/controller/scheme/SchemeControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/users/EmployeeSignatureControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/version/VersionControllerTest.java(1 hunks)src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java(1 hunks)src/test/java/com/iemr/common/utils/CookieUtilTest.java(1 hunks)src/test/java/com/iemr/common/utils/CryptoUtilTest.java(1 hunks)src/test/java/com/iemr/common/utils/mapper/InputMapperTest.java(1 hunks)src/test/java/com/iemr/common/utils/validator/ValidatorTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (35)
- src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellRegistrationTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleForCallCentreTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForSMSTest.java
- src/test/java/com/iemr/common/utils/CookieUtilTest.java
- src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForUnblockTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleForEverwellDataSyncTest.java
- src/test/java/com/iemr/common/controller/carestream/CareStreamCreateOrderControllerMinimalTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForEmailTest.java
- src/test/java/com/iemr/common/controller/version/VersionControllerTest.java
- src/test/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardControllerTest.java
- src/test/java/com/iemr/common/utils/CryptoUtilTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleJobForNHMDashboardDataTest.java
- src/test/java/com/iemr/common/controller/everwell/callhandle/EverwellCallControllerTest.java
- src/test/java/com/iemr/common/controller/abdmfacility/AbdmFacilityControllerTest.java
- src/test/java/com/iemr/common/controller/eausadha/EAusadhaControllerTest.java
- src/test/java/com/iemr/common/controller/report/CustomerRelationshipReportsTest.java
- src/test/java/com/iemr/common/controller/questionconfig/QuestionnaireControllerTest.java
- src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java
- src/test/java/com/iemr/common/controller/lonic/LonicControllerTest.java
- src/test/java/com/iemr/common/controller/honeywell/HoneywellControllerTest.java
- src/test/java/com/iemr/common/config/quartz/ScheduleJobServiceForAvniRegistrationTest.java
- src/test/java/com/iemr/common/utils/mapper/InputMapperTest.java
- src/test/java/com/iemr/common/controller/customization/CustomizationControllerTest.java
- src/test/java/com/iemr/common/controller/language/LanguageControllerTest.java
- src/test/java/com/iemr/common/controller/kmfilemanager/KMFileManagerControllerTest.java
- src/test/java/com/iemr/common/controller/mctshistory/OutboundHistoryControllerTest.java
- src/test/java/com/iemr/common/controller/otp/OTPGatewayTest.java
- src/test/java/com/iemr/common/controller/lungassessment/LungAssessmentControllerTest.java
- src/test/java/com/iemr/common/controller/directory/DirectoryControllerTest.java
- src/test/java/com/iemr/common/controller/users/EmployeeSignatureControllerTest.java
- src/test/java/com/iemr/common/utils/validator/ValidatorTest.java
- src/test/java/com/iemr/common/controller/scheme/SchemeControllerTest.java
- src/test/java/com/iemr/common/controller/sms/SMSControllerTest.java
- src/test/java/com/iemr/common/config/encryption/SecurePasswordTest.java
🧰 Additional context used
🧠 Learnings (7)
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (5)
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:53:24.380Z
Learning: The user vishwab1 confirmed that their CAPTCHA implementation handles null token checking at the controller level before calling the service, which mitigates the direct security vulnerability of the service returning true for null tokens.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#163
File: src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java:111-118
Timestamp: 2025-02-21T09:49:46.528Z
Learning: The `complaintResolution` and `remarks` fields in GrievanceDetails class are intentionally kept without validation constraints (@NotBlank, @Size) as per the team's decision.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (3)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java (3)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (6)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.089Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (3)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
🧬 Code Graph Analysis (1)
src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
🔇 Additional comments (24)
src/test/java/com/iemr/common/controller/questionconfig/QuestionTypeControllerTest.java (1)
76-82: Good use of JSON comparison for response validation.The use of Gson's JsonParser to compare JSON responses as objects rather than strings is a good practice as it ignores field ordering and provides more reliable assertions.
src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java (2)
54-58: Excellent use of test constants for maintainability.The use of constants for repeated values like authorization headers and endpoints improves test maintainability and reduces duplication.
310-331: Comprehensive edge case coverage enhances robustness.The extensive edge case testing including large payloads, special characters, and boundary conditions demonstrates thorough consideration of real-world scenarios that could cause issues.
src/test/java/com/iemr/common/controller/callhandling/CallControllerTest.java (3)
51-53: Excellent use of @DisplayName for test clarity.The use of @DisplayName annotations makes the test suite more readable and provides clear descriptions of what each test is validating.
80-340: Comprehensive null response testing improves reliability.Testing null responses from service methods ensures the controller handles edge cases gracefully and provides appropriate fallback behavior when services return unexpected null values.
342-851: Thorough success scenario coverage validates core functionality.The extensive testing of success scenarios across all endpoints ensures that the controller correctly processes requests and returns expected responses under normal conditions.
src/test/java/com/iemr/common/controller/notification/NotificationControllerTest.java (3)
78-86: Good use of JsonNode for response validation.Using ObjectMapper and JsonNode to parse and validate JSON responses is a good practice that provides type-safe access to response fields and clearer assertions.
362-369: Proper testing of authorization requirements.Testing the missing authorization header scenario ensures that the security constraints are properly enforced and return appropriate HTTP status codes.
372-392: Excellent handling of invalid JSON edge case.The test properly validates that the controller handles malformed JSON gracefully and returns appropriate responses rather than throwing exceptions.
src/test/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationControllerTest.java (4)
47-66: Excellent test structure and setup.The test class demonstrates excellent practices with proper mocking setup, clear constants, and comprehensive MockMvc configuration. The use of
@DisplayNameannotations enhances test readability.
69-94: Well-implemented test pattern.The test follows a clear and consistent pattern: setup request data, mock service response, perform HTTP request, and verify both response content and service interaction. This approach ensures thorough validation of controller behavior.
192-215: Good exception handling test coverage.The test properly verifies that the controller handles service exceptions gracefully, returning HTTP 200 with error details in the response body rather than throwing the exception. This aligns with the controller's error handling strategy.
1059-1101: Comprehensive header testing.The tests for
X-FORWARDED-FORheader handling (both with value and empty) ensure robust request processing across different client configurations. This adds valuable coverage for real-world scenarios.src/test/java/com/iemr/common/controller/everwellTest/EverwellControllerTest.java (2)
57-68: Clean test implementation for hardcoded responses.The test effectively validates the hardcoded JSON response structure using JSONPath assertions. This approach is appropriate for testing static controller endpoints.
238-253: Good test for null handling.This test effectively validates the controller's null pointer exception handling when login fields are null, ensuring the error response structure is properly returned.
src/test/java/com/iemr/common/controller/snomedct/SnomedControllerTest.java (5)
67-82: Excellent static mocking setup.The proper setup and teardown of static
InputMappermocking with@BeforeEachand@AfterEachensures clean test isolation and prevents potential test interference.
84-96: Good coverage of setter method.Testing the setter method with reflection verification ensures the dependency injection works correctly, which is important for controller initialization.
98-125: Comprehensive success scenario testing.The test thoroughly validates the happy path with proper input mocking, service interaction verification, and logger statement validation. The Given-When-Then structure enhances readability.
153-180: Good edge case coverage.Testing the scenario where a record is found but has a null
conceptIDensures robust handling of data inconsistencies. This edge case testing improves overall reliability.
182-202: Proper exception handling verification.The test correctly verifies that exceptions are caught, logged appropriately, and result in proper error responses. The logger verification ensures debugging information is captured.
src/test/java/com/iemr/common/controller/institute/InstituteControllerTest.java (1)
78-124: Excellent approach to handling known bugs in tests.The pattern of using
@Disabledtests with bug references for known issues while maintaining separate "ExpectedBehavior" tests is exemplary. This approach:
- Documents current buggy behavior
- Prevents false confidence from tests validating bugs
- Prepares for future bug fixes
- Maintains clear separation between current and expected behavior
This establishes a maintainable testing strategy for handling controller serialization issues.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java (3)
54-78: Excellent test structure with standalone MockMvc approach.This test class demonstrates good practices:
- Uses standalone MockMvc setup to avoid full ApplicationContext loading
- Properly structured with MockitoExtension
- Clear constants for maintainability
- Comprehensive test coverage including edge cases
The approach is consistent with the broader testing strategy established in this PR.
80-104: Comprehensive test coverage for notification count endpoint.The tests properly cover both success and exception scenarios for the
getAlertsAndNotificationCountendpoint, with appropriate assertions for JSON response structure and status codes.Also applies to: 106-123
378-425: Good coverage of missing authorization header scenarios.These tests properly validate the behavior when the required Authorization header is missing, expecting 404 status as appropriate for the security configuration.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (3)
118-124: Inconsistent authorization header behavior expectations.This test expects
status().isBadRequest()(400) for missing Authorization header, but the test at line 450 expectsstatus().isNotFound()(404) for the same scenario. This inconsistency suggests unclear understanding of the framework behavior.Based on the retrieved learnings, this inconsistency was already flagged in a previous review. The behavior should be standardized across all tests.
326-349: Replace placeholder test with functional verification.Similar to the previous test, this one works around JSON parsing issues instead of testing actual functionality. This pattern is repeated throughout the test file.
These placeholder tests provide minimal value and should be replaced with proper functional tests that verify the expected behavior of the controller methods.
302-323: Replace JSON parsing workarounds with proper test implementation.This test acknowledges JSON parsing issues and tests error handling instead of the actual functionality. This creates a placeholder test that doesn't verify the intended behavior.
Consider these solutions:
- Fix the JSON parsing configuration by properly configuring Jackson ObjectMapper for test environment
- Use @WebMvcTest with proper Spring context instead of standalone MockMvc setup
- Test the controller method directly with proper input objects
@Test void shouldUpdateBeneficiary_whenValidDataProvided() throws Exception { - // Arrange - This method has complex JSON parsing that causes NoSuchMethod errors in unit tests - // due to JSONObject constructor limitations. We test the error handling path instead. - String requestJson = "{" - + "\"beneficiaryRegID\":123," - + "\"firstName\":\"Updated\"," - + "\"lastName\":\"Name\"" - + "}"; - - HttpServletRequest mockRequest = mock(HttpServletRequest.class); - when(mockRequest.getHeader("authorization")).thenReturn("Bearer test-token"); - - // Act - String result = beneficiaryRegistrationController.updateBenefciary(requestJson, mockRequest); - - // Assert - The method will fail due to JSONObject constructor issues, - // but we can verify it returns a proper error response - assertNotNull(result); - assertTrue(result.contains("statusCode") || result.contains("errorMessage") || result.contains("error")); - // Note: This test verifies the controller structure and error handling - // rather than the successful path due to JSONObject(Object) constructor not existing + // Arrange + when(registerBenificiaryService.updateBenificiary(any(BeneficiaryModel.class), anyString())) + .thenReturn(1); + + String requestJson = "{\"beneficiaryRegID\":123,\"firstName\":\"Updated\",\"lastName\":\"Name\"}"; + + // Act & Assert + mockMvc.perform(post("/beneficiary/updateBeneficiary") + .header("Authorization", "Bearer test-token") + .contentType(MediaType.APPLICATION_JSON) + .content(requestJson)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.statusCode").exists()); + + verify(registerBenificiaryService).updateBenificiary(any(BeneficiaryModel.class), anyString()); }
🧹 Nitpick comments (1)
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (1)
1-798: Consider architectural improvements for better test maintainability.While this test class is comprehensive, several patterns could be improved:
- Resolve JSON parsing issues systematically rather than working around them
- Use consistent testing approach (either MockMvc or direct method calls, not both)
- Implement proper test data builders for complex request objects
- Consider using @WebMvcTest for better Spring integration
The test class demonstrates good coverage but would benefit from addressing the underlying JSON parsing issues and adopting a more consistent testing strategy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (4)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
🔇 Additional comments (5)
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java (5)
64-94: Well-structured test setup with comprehensive mocking.The test class setup properly uses JUnit 5 with MockitoExtension and mocks all required dependencies. The MockMvc setup is appropriate for testing Spring MVC controllers.
246-274: Comprehensive reference data retrieval test.This test properly verifies that all required services are called when retrieving registration data, ensuring complete coverage of the endpoint's dependencies.
376-400: Good integration test pattern for complex operations.This test effectively combines service method verification with response validation, demonstrating proper testing of the update community/education endpoint.
599-665: Thorough error handling coverage.The comprehensive error handling tests cover various exception scenarios including RuntimeException and NumberFormatException, ensuring robust error handling verification.
126-146: Separate HTTP and unit testing concerns.This test method mixes MockMvc testing approach with direct controller method calls, which creates confusion about what exactly is being tested and makes the test harder to maintain.
The existing approach is better - keep HTTP tests using MockMvc and unit tests using direct method calls in separate test methods. This test should focus on one approach:
@Test -void shouldCreateBeneficiary_serviceInteraction_whenValidBeneficiaryModelProvided() throws Exception { +void shouldCreateBeneficiary_directCall_whenValidBeneficiaryModelProvided() throws Exception { // Arrange String mockResponse = "{\"statusCode\":200,\"data\":\"BEN123456\",\"status\":\"Success\"}"; when(registerBenificiaryService.save(any(BeneficiaryModel.class), any())).thenReturn(mockResponse); BeneficiaryModel beneficiaryModel = new BeneficiaryModel(); beneficiaryModel.setFirstName("John"); beneficiaryModel.setLastName("Doe"); // Act String result = beneficiaryRegistrationController.createBeneficiary(beneficiaryModel, null); // Assert verify(registerBenificiaryService).save(any(BeneficiaryModel.class), any()); assertNotNull(result); - // The controller wraps the response in an OutputResponse, so check for the wrapped content assertTrue(result.contains("BEN123456")); assertTrue(result.contains("statusCode")); }Likely an incorrect or invalid review comment.
src/test/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationControllerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
93-115: Resolve underlying configuration issues instead of accepting test failures.These tests are still expecting JSON parsing failures due to "Java module system restrictions with Gson". This indicates a configuration problem that should be resolved rather than accepted in tests.
Consider fixing the root cause by:
- Configuring Jackson/Gson properly for the test environment
- Using valid test data that can be parsed correctly
- Removing lenient() mocks that aren't being invoked
- // Mock for any scenario since JSON parsing will fail due to module restrictions - // Using lenient() because this mock won't be called due to JSON parsing error - lenient().when(feedbackService.getFeedbackRequests(any())).thenReturn(mockFeedbackList); + when(feedbackService.getFeedbackRequests(any())).thenReturn(mockFeedbackList); // Act & Assert - // Note: This test expects 5000 status code due to Java module system restrictions - // with Gson trying to access private fields in SimpleDateFormat mockMvc.perform(post("/feedback/beneficiaryRequests") .header("Authorization", "Bearer test-token") .contentType(MediaType.APPLICATION_JSON) .content(requestJson)) .andExpect(status().isOk()) - .andExpect(jsonPath("$.statusCode").value(5000)) - .andExpected(jsonPath("$.status").value(containsString("Failed making field"))) - .andExpect(jsonPath("$.errorMessage").exists()); + .andExpect(jsonPath("$.statusCode").value(200)) + .andExpect(jsonPath("$.status").value("Success")) + .andExpect(jsonPath("$.data").exists());Also applies to: 152-174
🧹 Nitpick comments (9)
src/test/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReportsTest.java (1)
26-34: Consider using Spring Test annotations instead of reflectionWhile the reflection-based dependency injection works, consider using Spring Test annotations for a cleaner approach:
-@ExtendWith(MockitoExtension.class) +@ExtendWith(MockitoExtension.class) +@MockBean +private SecondaryReportService secondaryReportService; -@BeforeEach -void setUp() throws Exception { - secondaryReportService = Mockito.mock(SecondaryReportService.class); - CustomerRelationshipSecondaryReports controller = new CustomerRelationshipSecondaryReports(); - java.lang.reflect.Field serviceField = CustomerRelationshipSecondaryReports.class.getDeclaredField("secondaryReportService"); - serviceField.setAccessible(true); - serviceField.set(controller, secondaryReportService); - mockMvc = MockMvcBuilders.standaloneSetup(controller).build(); -} +@BeforeEach +void setUp() { + mockMvc = MockMvcBuilders.standaloneSetup(new CustomerRelationshipSecondaryReports()).build(); +}src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java (2)
40-40: Consider using a proper data structure instead of hardcoded JSON string.The mock service response is hardcoded as a JSON string. While functional, consider using a proper data structure or constant for better maintainability.
+ private static final String MOCK_FACILITY_RESPONSE = "{\"id\":1,\"name\":\"Test Facility\"}"; + - String mockServiceResponseData = "{\"id\":1,\"name\":\"Test Facility\"}"; + String mockServiceResponseData = MOCK_FACILITY_RESPONSE;
107-107: Consider using more specific argument matchers for consistency.The test uses
anyString()matchers while other tests use specific values. For consistency and better test clarity, consider using the actual parameter values.- when(uptsuService.saveAppointmentDetails(anyString(), anyString())).thenThrow(new RuntimeException(errorMessage)); + when(uptsuService.saveAppointmentDetails(invalidRequestBody, authorizationHeader)).thenThrow(new RuntimeException(errorMessage));src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (1)
588-593: Remove duplicate status expectation.Line 589 contains a duplicate
.andExpected(status().isOk())call that should be removed.mockMvc.perform(post("/feedback/requestFeedback") .header("Authorization", "Bearer test-token") .contentType(MediaType.APPLICATION_JSON) .content(requestJson)) .andExpect(status().isOk()) - .andExpect(status().isOk()) .andExpect(jsonPath("$.statusCode").value(200)) .andExpect(jsonPath("$.status").value("Success")) .andExpect(jsonPath("$.data").exists());src/test/java/com/iemr/common/controller/videocall/VideoCallControllerTest.java (3)
90-91: Potential fragility in OutputResponse.toString() usage.The test relies on
OutputResponse.toString()for JSON comparison, which may break if the toString() implementation changes.Consider using a more robust JSON comparison approach:
-OutputResponse expectedOutputResponse = new OutputResponse(); -expectedOutputResponse.setError(new RuntimeException(errorMessage)); - -// ... later in test -.andExpect(content().json(expectedOutputResponse.toString())); +.andExpect(jsonPath("$.error").exists()) +.andExpect(jsonPath("$.error").value(containsString(errorMessage)));Also applies to: 100-100
45-47: Missing Authorization header validation.All tests include an Authorization header but there's no verification that it's actually being processed or validated by the controller.
Consider adding tests to verify authorization behavior:
@Test void shouldReturnUnauthorized_whenAuthorizationHeaderIsMissing() throws Exception { mockMvc.perform(post("/video-consultation/generate-link") .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isUnauthorized()); }Or if authorization is handled at a different layer, add a comment explaining why these headers are included in tests.
Also applies to: 59-61, 76-78, 95-97, 115-117, 130-132, 146-148, 168-170
1-177: Consider adding edge case test coverage.The current tests cover happy path and basic error scenarios but miss some edge cases.
Consider adding tests for:
- Invalid JSON payloads
- Null or empty request bodies
- Very large request payloads
- Concurrent access scenarios
- Network timeout scenarios
Example:
@Test void shouldReturnBadRequest_whenInvalidJsonProvided() throws Exception { mockMvc.perform(post("/video-consultation/send-link") .header("Authorization", "Bearer dummy_token") .contentType(MediaType.APPLICATION_JSON) .content("{invalid json}")) .andExpect(status().isBadRequest()); }src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java (2)
1-46: Organize imports for better readabilityThe import statements are scattered throughout the first 46 lines. Consider organizing them by grouping related imports together (Java core, third-party libraries, project imports, static imports).
524-534: Clarify AlertAndNotificationCount initialization approachThe commented setter calls suggest uncertainty about object initialization. Verify that the constructor properly initializes all required fields, and remove the commented code if setters are not needed.
AlertAndNotificationCount countA = new AlertAndNotificationCount(1, "TypeA", null, 5L); - // If setters are needed, use them as below: - // countA.setTypeId(1); - // countA.setTypeName("TypeA"); - // countA.setCount(5L); AlertAndNotificationCount countB = new AlertAndNotificationCount(2, "TypeB", null, 10L); - // countB.setTypeId(2); - // countB.setTypeName("TypeB"); - // countB.setCount(10L);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java(11 hunks)src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReportsTest.java(1 hunks)src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/videocall/VideoCallControllerTest.java(1 hunks)src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java (2)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.089Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java (2)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (6)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.089Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
🔇 Additional comments (18)
src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java (1)
77-77: LGTM: Proper HTTP status code standardizationThe changes correctly replace invalid HTTP status code
5000with the standard500(Internal Server Error) across all report endpoints. This ensures compliance with HTTP specifications and consistent error handling behavior.Also applies to: 103-103, 131-131, 156-156, 182-182, 206-206, 234-234, 261-261, 286-286, 313-313, 341-341
src/test/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReportsTest.java (2)
1-41: Comprehensive test setup with good coverageThe test class provides excellent coverage for all controller endpoints with both success and error scenarios. The use of JUnit 5 and Mockito is appropriate for unit testing.
42-339: Well-structured test methods with proper assertionsThe test methods effectively verify:
- HTTP status codes (200 for success, 500 for errors)
- Response headers (Content-Disposition with correct filenames)
- Content types (Excel format)
- Error message handling
This addresses the previous issue where the test file was commented out and now provides active test coverage.
src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java (2)
63-64: HTTP 200 status for error scenarios is consistent with codebase pattern.The test expects HTTP 200 even for error scenarios, which aligns with the codebase's approach of using
OutputResponsefor exception handling as mentioned in the retrieved learnings.
36-116: Test coverage is comprehensive and follows good practices.The test class provides excellent coverage with:
- Both successful and error scenarios for each endpoint
- Proper use of MockMvc for integration testing
- Consistent authorization header inclusion
- Clear test method naming following the should_when pattern
- Appropriate JSON path assertions
src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java (4)
1-92: LGTM! Well-structured test setup.The test class follows best practices with proper JUnit 5 configuration, MockMvc standalone setup for isolated testing, and appropriate mocking of all service dependencies.
282-302: LGTM! Test correctly reflects the controller's wrapper pattern behavior.This test appropriately validates the controller's handling of invalid JSON input. Based on the established architectural pattern in this codebase, the controller returns HTTP 200 with success indicators in the JSON response body, which this test correctly verifies.
305-1012: Excellent comprehensive test coverage!The test suite provides thorough coverage of all controller endpoints with both success and error scenarios. The tests correctly implement the controller's wrapper pattern expectations (HTTP 200 with statusCode field indicating actual success/failure) and include proper edge case handling.
Key strengths:
- Complete endpoint coverage
- Consistent error handling validation
- Proper mock configuration
- Edge case testing (null data, empty responses, etc.)
- Service exception handling verification
926-934: Empty body should return 400 as testedSpring’s default behavior for a required
@RequestBodyis to reject an absent or empty payload with HTTP 400 before entering the controller method. The wrapper pattern applies to exceptions thrown insidecreateFeedback(), but not to missing request bodies. Your test expectingstatus().isBadRequest()is correct—no change needed.Likely an incorrect or invalid review comment.
src/test/java/com/iemr/common/controller/videocall/VideoCallControllerTest.java (4)
54-66: Good error handling test coverage.The failure test properly verifies that exceptions are handled and return appropriate HTTP 500 status codes with error messages.
125-139: Well-structured validation tests.The validation tests properly verify that missing required fields return HTTP 400 Bad Request and that the service is not called when validation fails.
Also applies to: 141-155
157-176: Inconsistent error handling pattern repeated.Similar to the
sendVideoLinktest, theupdateCallStatusendpoint returns HTTP 200 even on service failures, which is inconsistent with thegenerateJitsiLinkbehavior.This reinforces the need to verify the intended error handling strategy across all video call endpoints.
86-103: Inconsistent error handling in VideoCallControllerIt looks like the two endpoints in VideoCallController use different approaches when an exception occurs:
- generateMeetingLink (
GET /video-consultation/generate-jitsi-link)
- Returns a
ResponseEntitywith HTTP 500 on error.- sendVideoLink (
POST /video-consultation/send-link)
- Returns a JSON
OutputResponsestring and always HTTP 200, embedding the error in the response body.Most controllers in this codebase follow the wrapper pattern (always HTTP 200 with a
statusCodefield in the JSON), including sendVideoLink. Please confirm how you’d like to standardize here:
- Should generateMeetingLink be refactored to use
OutputResponse(200 OK +statusCodein body), or- Should sendVideoLink adopt
ResponseEntitystatus codes to mirror generateMeetingLink?src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java (5)
52-77: Well-structured test helper classThe TestObject inner class is properly implemented for testing JSON serialization functionality.
262-277: Well-implemented spy-based test for querySelectorGood use of spy pattern to verify internal method calls based on status values.
224-236: Proper exception handling testsGood coverage of NullPointerException cases for null inputs.
405-453: Comprehensive coverage of getAlertAndNotificationCount scenariosExcellent test coverage including:
- Testing with and without workingLocationID
- Proper verification of repository method calls
- Good use of ArgumentMatchers
- Appropriate logger verification
47-783: Overall: Well-structured test class with minor cleanup neededThis test class provides comprehensive coverage of UserNotificationMappingService with good testing practices:
- Proper use of JUnit 5 and Mockito
- Good edge case coverage
- Appropriate use of spies for testing internal method calls
- Proper exception testing
After removing the duplicate test methods identified above, this will be a clean and maintainable test suite.
src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java
Outdated
Show resolved
Hide resolved
| @BeforeEach | ||
| void setUp() throws Exception { | ||
| videoCallService = Mockito.mock(VideoCallService.class); | ||
| VideoCallController controller = new VideoCallController(); | ||
| // Use reflection to inject the mock into the private field | ||
| java.lang.reflect.Field serviceField = VideoCallController.class.getDeclaredField("videoCallService"); | ||
| serviceField.setAccessible(true); | ||
| serviceField.set(controller, videoCallService); | ||
| mockMvc = MockMvcBuilders.standaloneSetup(controller).build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving dependency injection for better testability.
The reflection-based approach to inject the mock service indicates that the controller may not have proper dependency injection setup for testing. This makes the tests brittle and harder to maintain.
Consider refactoring the controller to use constructor injection or add a package-private setter for testing:
// In VideoCallController.java
+VideoCallController(VideoCallService videoCallService) {
+ this.videoCallService = videoCallService;
+}Then simplify the test setup:
-videoCallService = Mockito.mock(VideoCallService.class);
-VideoCallController controller = new VideoCallController();
-// Use reflection to inject the mock into the private field
-java.lang.reflect.Field serviceField = VideoCallController.class.getDeclaredField("videoCallService");
-serviceField.setAccessible(true);
-serviceField.set(controller, videoCallService);
+videoCallService = Mockito.mock(VideoCallService.class);
+VideoCallController controller = new VideoCallController(videoCallService);🤖 Prompt for AI Agents
In
src/test/java/com/iemr/common/controller/videocall/VideoCallControllerTest.java
around lines 28 to 37, the test uses reflection to inject the mock
VideoCallService into the controller, which is brittle and hard to maintain.
Refactor the VideoCallController to use constructor injection for the
VideoCallService or add a package-private setter method to allow injecting the
mock service. Then update the test setup to instantiate the controller by
passing the mock service directly or by calling the setter, removing the need
for reflection.
There was a problem hiding this comment.
Okay so currently the VideoCallController doesn't have a constructor so I wont make the change .
There was a problem hiding this comment.
@Zapper9982 Understood! That's a perfectly reasonable decision. Keeping the existing controller structure without a constructor is fine, and the reflection-based approach in the test works for your current needs. The test coverage you've achieved is comprehensive regardless of the injection method used.
| void getMarkQuery_shouldReturnEmptyStringForAnyDto() { | ||
| AlertAndNotificationChangeStatusDTO dto = new AlertAndNotificationChangeStatusDTO(); | ||
| // The content of DTO doesn't affect the current implementation of getMarkQuery | ||
| String result = userNotificationMappingService.getMarkQuery(dto); | ||
| assertEquals("", result); | ||
| } | ||
| @Test | ||
| void getMarkQuery_shouldReturnEmptyStringForNullDto() { | ||
| String result = userNotificationMappingService.getMarkQuery(null); | ||
| assertEquals("", result); | ||
| } |
There was a problem hiding this comment.
Remove duplicate getMarkQuery test methods
These test methods are duplicates of the tests at lines 326-336. Keep only one test for valid DTO and one for null DTO.
-@Test
-void getMarkQuery_shouldReturnEmptyStringForAnyDto() {
- AlertAndNotificationChangeStatusDTO dto = new AlertAndNotificationChangeStatusDTO();
- // The content of DTO doesn't affect the current implementation of getMarkQuery
- String result = userNotificationMappingService.getMarkQuery(dto);
- assertEquals("", result);
-}
-@Test
-void getMarkQuery_shouldReturnEmptyStringForNullDto() {
- String result = userNotificationMappingService.getMarkQuery(null);
- assertEquals("", result);
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void getMarkQuery_shouldReturnEmptyStringForAnyDto() { | |
| AlertAndNotificationChangeStatusDTO dto = new AlertAndNotificationChangeStatusDTO(); | |
| // The content of DTO doesn't affect the current implementation of getMarkQuery | |
| String result = userNotificationMappingService.getMarkQuery(dto); | |
| assertEquals("", result); | |
| } | |
| @Test | |
| void getMarkQuery_shouldReturnEmptyStringForNullDto() { | |
| String result = userNotificationMappingService.getMarkQuery(null); | |
| assertEquals("", result); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java
around lines 479 to 489, there are duplicate test methods for getMarkQuery that
already exist between lines 326 and 336. Remove the duplicate tests at lines
479-489, keeping only the original tests for a valid DTO and a null DTO to avoid
redundancy.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java
Outdated
Show resolved
Hide resolved
| void markNotificationList_shouldHandleNullListForMarkNotificationList() { | ||
| String status = "read"; | ||
| List<Integer> notificationIds = null; | ||
|
|
||
| String result = userNotificationMappingService.markNotificationList(status, notificationIds); | ||
|
|
||
| assertEquals("success", result); | ||
| verify(repo, times(1)).updateUserNotificationMappingList(status, null); | ||
| } |
There was a problem hiding this comment.
Remove duplicate markNotificationList test
This test method is a duplicate of the test at lines 394-402 with a slightly different name.
-@Test
-void markNotificationList_shouldHandleNullListForMarkNotificationList() {
- String status = "read";
- List<Integer> notificationIds = null;
-
- String result = userNotificationMappingService.markNotificationList(status, notificationIds);
-
- assertEquals("success", result);
- verify(repo, times(1)).updateUserNotificationMappingList(status, null);
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void markNotificationList_shouldHandleNullListForMarkNotificationList() { | |
| String status = "read"; | |
| List<Integer> notificationIds = null; | |
| String result = userNotificationMappingService.markNotificationList(status, notificationIds); | |
| assertEquals("success", result); | |
| verify(repo, times(1)).updateUserNotificationMappingList(status, null); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java
around lines 507 to 515, there is a duplicate test method for
markNotificationList that is very similar to the one at lines 394 to 402. Remove
this duplicate test method entirely to avoid redundancy and keep only the
original test.
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java
Outdated
Show resolved
Hide resolved
| void getDeleteQuery_shouldReturnEmptyStringForAnyDto() { | ||
| AlertAndNotificationChangeStatusDTO dto = new AlertAndNotificationChangeStatusDTO(); | ||
| // The content of DTO doesn't affect the current implementation of getDeleteQuery | ||
| String result = userNotificationMappingService.getDeleteQuery(dto); | ||
| assertEquals("", result); | ||
| } | ||
| @Test | ||
| void getDeleteQuery_shouldReturnEmptyStringForNullDto() { | ||
| String result = userNotificationMappingService.getDeleteQuery(null); | ||
| assertEquals("", result); | ||
| } |
There was a problem hiding this comment.
Remove duplicate getDeleteQuery test methods
These test methods are duplicates of the tests at lines 339-349. Keep only the original implementations.
-@Test
-void getDeleteQuery_shouldReturnEmptyStringForAnyDto() {
- AlertAndNotificationChangeStatusDTO dto = new AlertAndNotificationChangeStatusDTO();
- // The content of DTO doesn't affect the current implementation of getDeleteQuery
- String result = userNotificationMappingService.getDeleteQuery(dto);
- assertEquals("", result);
-}
-@Test
-void getDeleteQuery_shouldReturnEmptyStringForNullDto() {
- String result = userNotificationMappingService.getDeleteQuery(null);
- assertEquals("", result);
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void getDeleteQuery_shouldReturnEmptyStringForAnyDto() { | |
| AlertAndNotificationChangeStatusDTO dto = new AlertAndNotificationChangeStatusDTO(); | |
| // The content of DTO doesn't affect the current implementation of getDeleteQuery | |
| String result = userNotificationMappingService.getDeleteQuery(dto); | |
| assertEquals("", result); | |
| } | |
| @Test | |
| void getDeleteQuery_shouldReturnEmptyStringForNullDto() { | |
| String result = userNotificationMappingService.getDeleteQuery(null); | |
| assertEquals("", result); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java
around lines 491 to 501, there are duplicate test methods for getDeleteQuery
that already exist between lines 339 and 349. Remove the duplicate test methods
at lines 491 to 501 and keep only the original implementations to avoid
redundancy.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java (3)
468-479: Remove duplicate getMarkQuery test methods.These test methods are duplicates of the tests at lines 315-326.
480-491: Remove duplicate getDeleteQuery test methods.These test methods are duplicates of the tests at lines 328-339.
496-505: Remove duplicate markNotificationList test.This test method is a duplicate of the test at lines 383-392.
🧹 Nitpick comments (3)
src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java (3)
53-77: Consider making TestObject a static nested class.Since
TestObjectdoesn't access any instance members of the enclosing class, it should be declared asstaticfor better memory efficiency and clarity.- class TestObject { + static class TestObject {
237-250: Remove or modify test that assumes ObjectMapper configuration.This test creates its own
ObjectMapperinstance to verify the exact JSON string, but the actual service might use a differently configured ObjectMapper. This could lead to false test results.Consider either:
- Removing this test since lines 147-158 already test the logging behavior adequately
- Using argument matchers to verify the log contains expected patterns rather than exact JSON
- @Test - void getJsonAsString_shouldLogObjectAsJsonString() throws JsonProcessingException { - String name = "testObject"; - TestObject obj = new TestObject("value1", 123); // Use a simple test object - - // Call the method - userNotificationMappingService.getJsonAsString(name, obj); - - // Verify that logger.info was called with a string containing the object name and "toJSON" - // The exact JSON string is generated by ObjectMapper, so we verify the pattern. - // We can create an ObjectMapper instance in the test to get the expected JSON. - String expectedJson = new ObjectMapper().writeValueAsString(obj); - verify(mockLogger, times(1)).info("Object: " + name + " :toJSON: " + expectedJson); - }
1-766: Consider adding exception handling tests.The test suite would benefit from additional test cases covering exception scenarios, such as:
- Repository throwing exceptions during database operations
- JSON processing exceptions in the logging method
- Concurrent access scenarios
Would you like me to generate additional test cases for exception handling and error scenarios?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java(1 hunks)src/test/java/com/iemr/common/notification/agent/UserNotificationMappingServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/com/iemr/common/controller/uptsu/UPTechnicalSupportControllerTest.java
- src/test/java/com/iemr/common/controller/esanjeevani/ESanjeevaniControllerTest.java
|
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
src/test/java/com/iemr/common/controller/covid/CovidVaccinationControllerTest.java (3)
17-18: Remove unused imports.The
anyLongandanyStringargument matchers are imported but never used in the test methods. This suggests incomplete refactoring or copy-paste artifacts.-import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.anyString;
42-54: Verify JSON comparison reliability.The test uses
OutputResponse.toString()for JSON comparison, which may not produce reliable results if thetoString()method doesn't generate valid JSON or if field ordering varies.Consider using
ObjectMapperfor more reliable JSON comparison:+import com.fasterxml.jackson.databind.ObjectMapper; +private ObjectMapper objectMapper = new ObjectMapper(); - .andExpect(content().json(expectedOutputResponse.toString())); + .andExpect(content().json(objectMapper.writeValueAsString(expectedOutputResponse)));
108-123: Consider using test data builders for maintainability.The hardcoded JSON request body makes tests brittle and harder to maintain. Consider using test data builders or object serialization for better maintainability.
+private String createSaveRequestBody() { + return "{\"covidVSID\":1,\"beneficiaryRegID\":123,\"CovidVaccineTypeID\":1," + + "\"ProviderServiceMapID\":1,\"CreatedBy\":\"test\",\"ModifiedBy\":\"test\",\"VanID\":1}"; +} - String requestBody = "{\"covidVSID\":1,\"beneficiaryRegID\":123,\"CovidVaccineTypeID\":1,\"ProviderServiceMapID\":1,\"CreatedBy\":\"test\",\"ModifiedBy\":\"test\",\"VanID\":1}"; + String requestBody = createSaveRequestBody();Or better yet, use a proper test object with Jackson serialization.
src/test/java/com/iemr/common/controller/grievance/GrievanceControllerTest.java (3)
8-8: Remove unused imports.The
MockitoAnnotationsimport is not needed when using@ExtendWith(MockitoExtension.class), andUnallocationRequestis imported but never used in the tests.-import org.mockito.MockitoAnnotations; -import com.iemr.common.data.grievance.UnallocationRequest;Also applies to: 27-27
151-151: Consider using lenient JSON comparison for complex objects.Using strict JSON comparison (
trueparameter) with complex objects containing timestamps might make tests fragile. Consider using lenient comparison unless exact matching is required.- .andExpect(content().json(expectedJson, true)); + .andExpect(content().json(expectedJson, false));
97-113: Inconsistent exception handling patterns.The tests use different exception types (
IEMRExceptionvs genericException) inconsistently. Consider standardizing on the most appropriate exception type for each scenario.For consistency, consider using the same exception type pattern across similar test scenarios, or document why different exception types are used.
Also applies to: 154-171
src/test/java/com/iemr/common/controller/email/EmailControllerTest.java (2)
37-40: Remove redundant MockitoAnnotations.openMocks(this) call.Since you're using
@ExtendWith(MockitoExtension.class), theMockitoAnnotations.openMocks(this)call is redundant as the extension already handles mock initialization.@BeforeEach void setUp() { - MockitoAnnotations.openMocks(this); mockMvc = MockMvcBuilders.standaloneSetup(emailController).build(); }
112-124: Consider removing the reflection-based setter test.Testing simple setter methods using reflection adds complexity without significant value. The
@InjectMocksannotation already verifies field injection. Unless the setter contains business logic that needs verification, this test can be removed.If you want to keep setter testing, consider testing it through the controller's behavior rather than reflection:
-@Test -void setEmailService_shouldSetService() throws NoSuchFieldException, IllegalAccessException { - EmailService anotherMockEmailService = mock(EmailService.class); - - emailController.setEmailService(anotherMockEmailService); - - Field emailServiceField = EmailController.class.getDeclaredField("emailService"); - emailServiceField.setAccessible(true); - - EmailService actualEmailService = (EmailService) emailServiceField.get(emailController); - - assertEquals(anotherMockEmailService, actualEmailService); -}src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java (3)
39-39: Remove redundant MockitoAnnotations.openMocks() call.When using
@ExtendWith(MockitoExtension.class), mock initialization is handled automatically. TheMockitoAnnotations.openMocks(this)call is redundant.- MockitoAnnotations.openMocks(this);
81-81: Fix typo in error messages."exception occured" should be "exception occurred" in the error messages.
- expectedOutputResponse.setError(5000, "Unable to get user data, exception occured. " + serviceException.toString()); + expectedOutputResponse.setError(5000, "Unable to get user data, exception occurred. " + serviceException.toString());- expectedOutputResponse.setError(5000, "Error in getting suspected information, exception occured. " + serviceException.toString()); + expectedOutputResponse.setError(5000, "Error in getting suspected information, exception occurred. " + serviceException.toString());- .andExpect(jsonPath("$.status", containsString("Error in getting suspected information, exception occured."))) - .andExpect(jsonPath("$.errorMessage", containsString("Error in getting suspected information, exception occured."))); + .andExpect(jsonPath("$.status", containsString("Error in getting suspected information, exception occurred."))) + .andExpect(jsonPath("$.errorMessage", containsString("Error in getting suspected information, exception occurred.")));Also applies to: 131-131, 154-154
53-53: Consider removing Authorization header if not validated.All tests include an Authorization header, but there's no indication that authorization is being tested or validated. If the controller doesn't require authentication in these tests, the header adds unnecessary complexity.
Remove the Authorization header if not needed:
mockMvc.perform(post("/doortodoorapp/getUserDetails") - .header("Authorization", "Bearer token") .contentType("application/json") .content(requestJson))Or add a test to verify authorization is properly handled if it's required by the actual controller.
Also applies to: 69-69, 86-86, 103-103, 119-119, 136-136, 148-148
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
203-211: Consider improving testability to reduce reflection usage.The extensive use of reflection to access private methods and inject dependencies suggests the controller might have testability issues. Consider:
- Adding proper setter methods for dependencies
- Making helper methods package-private instead of private for testing
- Using constructor injection which is more testable
This would make the tests more maintainable and less brittle.
Also applies to: 944-990, 1426-1527
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java(2 hunks)src/test/java/com/iemr/common/controller/covid/CovidVaccinationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/email/EmailControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/grievance/GrievanceControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java
🧰 Additional context used
🧠 Learnings (3)
src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java (1)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
src/test/java/com/iemr/common/controller/grievance/GrievanceControllerTest.java (2)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java:139-140
Timestamp: 2025-03-03T05:09:33.578Z
Learning: The query method `fetchGrievanceWorklistByComplaintID` in GrievanceDataRepo does not require filtering for deleted records (g.deleted = false) as per developer's preference.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java:128-136
Timestamp: 2025-03-03T05:09:09.520Z
Learning: For the GrievanceDataRepo's fetchGrievanceDetailsBasedOnParams method, filtering out deleted records with "AND g.deleted = false" is not required as per the developer's decision.
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (3)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
🧬 Code Graph Analysis (1)
src/test/java/com/iemr/common/controller/grievance/GrievanceControllerTest.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
🔇 Additional comments (13)
src/test/java/com/iemr/common/controller/covid/CovidVaccinationControllerTest.java (3)
25-39: LGTM! Well-structured test setup.The test class setup follows best practices with proper JUnit 5 and Mockito annotations, and the MockMvc standalone setup is appropriate for unit testing controllers.
72-87: LGTM! Good test coverage for successful scenario.The test properly mocks the service method and verifies the controller's response handling. The beneficiaryRegID matching between request body and service mock is correct.
57-69: LGTM! Comprehensive exception handling tests.The exception handling tests properly verify that
IEMRExceptionis caught and converted to an appropriate error response. The expectation of HTTP 200 status with error details in the response body is consistent with the application's error handling pattern.Also applies to: 90-105, 126-141
src/test/java/com/iemr/common/controller/grievance/GrievanceControllerTest.java (6)
46-62: Test class setup follows good practices.The test class uses proper JUnit 5 and Mockito setup with standalone MockMvc configuration. The mock injection and setup are correctly implemented.
64-67: Good helper method for consistent test data.The helper method ensures consistent timestamp creation without milliseconds, which is important for reliable test assertions.
69-95: Well-structured success test with proper JSON assertion.The test correctly mocks the service layer, constructs the expected response, and uses appropriate Gson configuration for JSON comparison.
135-144: Good use of custom date serializer.The custom Gson date serializer ensures consistent date formatting in JSON responses, which is important for reliable test assertions.
173-423: Comprehensive test coverage for all endpoints.The test class provides thorough coverage of all controller endpoints with both success and failure scenarios. The test structure is consistent and follows good naming conventions.
396-397: No action needed: Mock signature matches implementation
The mocked callwhen(grievanceDataSync.fetchUnallocatedGrievanceCount( anyString(), any(Timestamp.class), any(Timestamp.class), anyInt())) .thenReturn(serviceResponseContent);correctly mirrors the service method signature:
public String fetchUnallocatedGrievanceCount( String preferredLanguage, Timestamp filterStartDate, Timestamp filterEndDate, int providerServiceMapID)All parameter types align, so no changes are required.
src/test/java/com/iemr/common/controller/email/EmailControllerTest.java (2)
42-57: Well-structured success test cases.The success test cases follow good testing practices with proper AAA pattern, appropriate mocking, and correct assertions. The MockMvc usage is also proper for testing Spring MVC controllers.
Also applies to: 76-92, 94-110
59-74: Local exception handling in EmailController is correct.Each endpoint in EmailController wraps its service call in a try/catch and returns an OutputResponse (with the error set) as a 200 OK. There are no global
@ControllerAdviceor@ExceptionHandlerclasses, so the test’s expectation of status 200 and matching JSON is valid.src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java (1)
57-57: No changes needed for JSON assertionsThe
toString()methods in bothOutputResponseclasses use Gson (with proper configuration) to serialize the object to JSON, so
.andExpect(content().json(expectedOutputResponse.toString()))
will indeed receive valid JSON. You can keep the current assertion; switching tojsonPath()is optional and not required for correctness.src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
161-1846: Good test coverage with comprehensive scenarios.The test class provides excellent coverage of the IEMRAdminController with tests for success cases, error handling, edge cases, and various authentication scenarios. The parameterized tests for mobile/non-mobile branches are particularly well done.
src/test/java/com/iemr/common/controller/email/EmailControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java
Show resolved
Hide resolved
src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java
Show resolved
Hide resolved
| org.json.JSONObject validatedObj = new org.json.JSONObject(); | ||
| validatedObj.put("jwtToken", jwtToken); | ||
| validatedObj.put("refreshToken", refreshToken); | ||
| validatedObj.put("previlegeObj", new org.json.JSONArray()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix typo: "previlegeObj" should be "privilegeObj".
There's a consistent typo throughout the file where "privilege" is misspelled as "previlege".
- validatedObj.put("previlegeObj", new org.json.JSONArray());
+ validatedObj.put("privilegeObj", new org.json.JSONArray());- .andExpect(jsonPath("$.previlegeObj").isArray());
+ .andExpect(jsonPath("$.privilegeObj").isArray());Also applies to: 48-48, 70-70
🤖 Prompt for AI Agents
In src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
at lines 27, 48, and 70, correct the typo "previlegeObj" to "privilegeObj" in
all occurrences to fix the consistent misspelling of "privilege".
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
Show resolved
Hide resolved
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(loginRequest))) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.errorMessage").value("Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null")); |
There was a problem hiding this comment.
Poor error handling: Controller returns NullPointerException messages.
Multiple tests expect responses containing NullPointerException messages like "Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null". This indicates the controller is not properly handling null cases and is leaking implementation details to the client.
The controller should validate inputs and return appropriate error responses instead of letting NullPointerExceptions propagate to the response.
Also applies to: 1086-1086, 1100-1100
🤖 Prompt for AI Agents
In src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
at lines 775, 1086, and 1100, the tests expect error responses containing
NullPointerException messages, indicating the controller leaks internal
exceptions to clients. To fix this, update the controller to validate inputs and
handle null cases gracefully by returning meaningful error responses with
appropriate HTTP status codes instead of allowing NullPointerExceptions to
propagate. Then, adjust the tests to expect these proper error responses rather
than raw exception messages.
There was a problem hiding this comment.
okay seems like theres a logical issue with the handling in the controller itself
There was a problem hiding this comment.
@Zapper9982, exactly! You've identified the core issue correctly. The controller is allowing NullPointerExceptions to propagate to the client instead of implementing proper error handling.
The problematic pattern appears to be:
- Controller methods aren't validating inputs properly
- When null objects are encountered, the code tries to call
.toString()on them - The resulting NPE gets serialized into the response, exposing internal implementation details
Recommended approach to fix this:
- Add input validation at the beginning of controller methods
- Implement null-safe operations for response object creation
- Return standardized error responses with appropriate HTTP status codes
- Use try-catch blocks to handle unexpected exceptions gracefully
For example, instead of directly calling responseObj.toString(), the controller should check if responseObj is null and return a proper error response like:
{
"statusCode": 400,
"errorMessage": "Invalid request parameters",
"status": "Bad Request"
}Once the controller is fixed, the tests can be updated to expect these proper error responses rather than raw exception messages.
✏️ Learnings added
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java:704-704
Timestamp: 2025-07-11T18:00:56.894Z
Learning: In the IEMRAdminController (and likely other controllers in this codebase), error handling is incomplete - NullPointerExceptions are being allowed to propagate to clients instead of being caught and converted to proper error responses. Tests expect error messages like "Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null", indicating a systemic issue with input validation and null handling in controller methods.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:53:24.380Z
Learning: The user vishwab1 confirmed that their CAPTCHA implementation handles null token checking at the controller level before calling the service, which mitigates the direct security vulnerability of the service returning true for null tokens.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.089Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
| @Test | ||
| void userAuthenticate_mobileDevice_branch_shouldReturnTokensAndStoreInRedis() throws Exception { | ||
| // Deprecated: replaced by parameterized test userAuthenticate_mobileAndNonMobileBranches_shouldReturnExpectedTokens | ||
| // ...existing code... | ||
| } | ||
|
|
||
| @Test | ||
| void superUserAuthenticate_mobileAndNonMobileBranches_shouldCoverJwtRefreshRedisCookieLogic() throws Exception { | ||
| // Deprecated: replaced by parameterized test superUserAuthenticate_mobileAndNonMobileBranches_shouldReturnExpectedTokens | ||
| // ...existing code... | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove deprecated test methods.
These test methods are marked as deprecated and have been replaced by parameterized tests. They should be removed to keep the codebase clean.
- @Test
- void userAuthenticate_mobileDevice_branch_shouldReturnTokensAndStoreInRedis() throws Exception {
- // Deprecated: replaced by parameterized test userAuthenticate_mobileAndNonMobileBranches_shouldReturnExpectedTokens
- // ...existing code...
- }
-
- @Test
- void superUserAuthenticate_mobileAndNonMobileBranches_shouldCoverJwtRefreshRedisCookieLogic() throws Exception {
- // Deprecated: replaced by parameterized test superUserAuthenticate_mobileAndNonMobileBranches_shouldReturnExpectedTokens
- // ...existing code...
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void userAuthenticate_mobileDevice_branch_shouldReturnTokensAndStoreInRedis() throws Exception { | |
| // Deprecated: replaced by parameterized test userAuthenticate_mobileAndNonMobileBranches_shouldReturnExpectedTokens | |
| // ...existing code... | |
| } | |
| @Test | |
| void superUserAuthenticate_mobileAndNonMobileBranches_shouldCoverJwtRefreshRedisCookieLogic() throws Exception { | |
| // Deprecated: replaced by parameterized test superUserAuthenticate_mobileAndNonMobileBranches_shouldReturnExpectedTokens | |
| // ...existing code... | |
| } |
🤖 Prompt for AI Agents
In src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
around lines 1833 to 1843, remove the two deprecated test methods
userAuthenticate_mobileDevice_branch_shouldReturnTokensAndStoreInRedis and
superUserAuthenticate_mobileAndNonMobileBranches_shouldCoverJwtRefreshRedisCookieLogic
entirely, as they have been replaced by parameterized tests, to keep the test
code clean and maintainable.
| .andExpect(content().json("{\"data\":{\"response\":\"Password Changed\"},\"statusCode\":200,\"errorMessage\":\"Success\",\"status\":\"Success\"}")); | ||
|
|
||
| verify(iemrAdminUserServiceImpl, times(1)).userExitsCheck("testUser"); | ||
| verify(aesUtil, times(1)).decrypt("Piramal12Piramal", "encryptedPwd"); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern: Hardcoded encryption key in tests.
The tests are verifying that AES decryption is called with a hardcoded key "Piramal12Piramal". If this reflects the actual implementation, it's a serious security vulnerability. Encryption keys should be stored securely and rotated regularly.
Also applies to: 623-623, 659-659
🏁 Script executed:
#!/bin/bash
# Description: Check if the hardcoded encryption key is used in the main source code
# This would indicate a security vulnerability
# Search for the hardcoded key in source files
rg -g '*.java' -g '!*Test*.java' "Piramal12Piramal" src/Length of output: 672
Critical Security Issue: Hardcoded AES Key in Production and Test Code
The literal key "Piramal12Piramal" is embedded in both your production controller and its tests, which exposes it in source control and at runtime. Encryption keys must never be hardcoded.
Locations requiring fixes:
- src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
• Lines where
String decryptPassword = aesUtil.decrypt("Piramal12Piramal", m_User.getPassword()); - src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
• Lines 586, 623, and 659 verifying calls with the same hardcoded key
Recommended remediation:
- Extract the AES key into a secure configuration (e.g., environment variable, encrypted vault, or a secrets manager).
- Inject the key at application startup (e.g., via Spring’s @ConfigurationProperties or similar).
- Update AES utility calls to use the injected key variable rather than a string literal.
- Adjust tests to mock or stub the key source instead of verifying against a hardcoded value.
Please externalize and secure this key before merging.
🤖 Prompt for AI Agents
In src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
at lines 586, 623, and 659, the AES key "Piramal12Piramal" is hardcoded in
verify calls, which is a critical security risk. To fix this, remove the
hardcoded key string and replace it with a reference to a securely injected key
variable or a mocked key source. Ensure the test setup mocks or stubs the key
retrieval mechanism so that the key is not exposed in the test code. Similarly,
update the production code in
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java to
inject the AES key from a secure configuration instead of hardcoding it.
There was a problem hiding this comment.
@drtechie is there something we can do about this?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| package com.iemr.common.controller.users; | ||
|
|
||
| import com.iemr.common.data.users.UserServiceRoleMapping; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import com.iemr.common.data.users.Role; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import java.sql.Timestamp; | ||
| import com.iemr.common.data.institute.Designation; | ||
| import com.iemr.common.data.users.UserLangMapping; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; | ||
| import static org.mockito.Mockito.never; | ||
| import static org.mockito.Mockito.lenient; | ||
| import static org.mockito.Mockito.verifyNoInteractions; | ||
| import static org.mockito.Mockito.times; | ||
| import static org.mockito.Mockito.verify; | ||
| import io.jsonwebtoken.ExpiredJwtException; | ||
| import java.util.List; | ||
| import java.util.ArrayList; | ||
| import com.iemr.common.data.users.ServiceRoleScreenMapping; | ||
| import com.iemr.common.data.users.UserSecurityQMapping; | ||
| import com.iemr.common.data.users.LoginSecurityQuestions; | ||
| import com.iemr.common.utils.response.OutputResponse; | ||
| import com.iemr.common.utils.sessionobject.SessionObject; | ||
| import com.iemr.common.utils.exception.IEMRException; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; | ||
| import static org.mockito.Mockito.doThrow; | ||
| import static org.mockito.Mockito.any; | ||
| import static org.mockito.Mockito.mock; | ||
| import io.jsonwebtoken.Claims; | ||
| import com.iemr.common.data.users.User; | ||
| import com.iemr.common.data.userbeneficiarydata.Status; | ||
| import org.springframework.data.redis.core.ValueOperations; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.springframework.test.web.servlet.MockMvc; | ||
| import org.springframework.test.web.servlet.setup.MockMvcBuilders; | ||
| import org.springframework.http.MediaType; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; | ||
| import org.mockito.Mockito; | ||
| import static org.mockito.Mockito.when; | ||
| import java.util.Map; | ||
| import java.util.HashMap; | ||
| import com.iemr.common.model.user.ForceLogoutRequestModel; | ||
| import com.iemr.common.model.user.LoginRequestModel; | ||
| import com.iemr.common.utils.JwtUtil; | ||
| import org.springframework.data.redis.core.RedisTemplate; | ||
|
|
||
| import static org.mockito.ArgumentMatchers.anyBoolean; | ||
| import static org.mockito.ArgumentMatchers.anyInt; | ||
| import static org.mockito.ArgumentMatchers.anyIterable; | ||
| import static org.mockito.ArgumentMatchers.anyLong; | ||
| import static org.mockito.ArgumentMatchers.anyString; | ||
| import static org.mockito.ArgumentMatchers.eq; | ||
| import static org.mockito.Mockito.doNothing; | ||
| import static org.mockito.Mockito.doReturn; | ||
|
|
||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.InjectMocks; | ||
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import com.iemr.common.controller.users.IEMRAdminController; | ||
| import com.iemr.common.service.users.IEMRAdminUserService; | ||
| import jakarta.servlet.http.Cookie; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import net.minidev.json.JSONArray; | ||
| import net.minidev.json.JSONObject; | ||
|
|
||
| import com.iemr.common.utils.encryption.AESUtil; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| // import com.google.common.net.MediaType; | ||
|
|
||
| import com.google.gson.JsonObject; | ||
| import java.util.Collections; | ||
| import java.util.Date; | ||
|
|
||
| import com.iemr.common.model.user.ChangePasswordModel; | ||
| import com.iemr.common.data.users.M_Role; | ||
| import com.iemr.common.utils.CookieUtil; | ||
| import com.iemr.common.data.users.ProviderServiceMapping; | ||
| import com.iemr.common.data.users.ServiceMaster; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) |
There was a problem hiding this comment.
Fix compilation error: Package declaration must be at the top of the file.
The package declaration and imports are placed after method definitions, which will cause a compilation error. Move these lines to the beginning of the file, before any class or method definitions.
+package com.iemr.common.controller.users;
+
+import com.iemr.common.data.users.UserServiceRoleMapping;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+// ... rest of imports ...
+
+@ExtendWith(MockitoExtension.class)
+class IEMRAdminControllerTest {
+
// --- Helpers for common mock setups ---
private User createMockUser(long id, String name, String status) {
-package com.iemr.common.controller.users;
-
-import com.iemr.common.data.users.UserServiceRoleMapping;
-// ... rest of misplaced imports ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| package com.iemr.common.controller.users; | |
| import com.iemr.common.data.users.UserServiceRoleMapping; | |
| import static org.junit.jupiter.api.Assertions.assertNotNull; | |
| import com.iemr.common.data.users.Role; | |
| import static org.junit.jupiter.api.Assertions.assertFalse; | |
| import static org.junit.jupiter.api.Assertions.assertTrue; | |
| import static org.junit.jupiter.api.Assertions.assertEquals; | |
| import java.sql.Timestamp; | |
| import com.iemr.common.data.institute.Designation; | |
| import com.iemr.common.data.users.UserLangMapping; | |
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; | |
| import static org.mockito.Mockito.never; | |
| import static org.mockito.Mockito.lenient; | |
| import static org.mockito.Mockito.verifyNoInteractions; | |
| import static org.mockito.Mockito.times; | |
| import static org.mockito.Mockito.verify; | |
| import io.jsonwebtoken.ExpiredJwtException; | |
| import java.util.List; | |
| import java.util.ArrayList; | |
| import com.iemr.common.data.users.ServiceRoleScreenMapping; | |
| import com.iemr.common.data.users.UserSecurityQMapping; | |
| import com.iemr.common.data.users.LoginSecurityQuestions; | |
| import com.iemr.common.utils.response.OutputResponse; | |
| import com.iemr.common.utils.sessionobject.SessionObject; | |
| import com.iemr.common.utils.exception.IEMRException; | |
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; | |
| import static org.mockito.Mockito.doThrow; | |
| import static org.mockito.Mockito.any; | |
| import static org.mockito.Mockito.mock; | |
| import io.jsonwebtoken.Claims; | |
| import com.iemr.common.data.users.User; | |
| import com.iemr.common.data.userbeneficiarydata.Status; | |
| import org.springframework.data.redis.core.ValueOperations; | |
| import java.util.concurrent.TimeUnit; | |
| import org.springframework.test.web.servlet.MockMvc; | |
| import org.springframework.test.web.servlet.setup.MockMvcBuilders; | |
| import org.springframework.http.MediaType; | |
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; | |
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | |
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; | |
| import org.mockito.Mockito; | |
| import static org.mockito.Mockito.when; | |
| import java.util.Map; | |
| import java.util.HashMap; | |
| import com.iemr.common.model.user.ForceLogoutRequestModel; | |
| import com.iemr.common.model.user.LoginRequestModel; | |
| import com.iemr.common.utils.JwtUtil; | |
| import org.springframework.data.redis.core.RedisTemplate; | |
| import static org.mockito.ArgumentMatchers.anyBoolean; | |
| import static org.mockito.ArgumentMatchers.anyInt; | |
| import static org.mockito.ArgumentMatchers.anyIterable; | |
| import static org.mockito.ArgumentMatchers.anyLong; | |
| import static org.mockito.ArgumentMatchers.anyString; | |
| import static org.mockito.ArgumentMatchers.eq; | |
| import static org.mockito.Mockito.doNothing; | |
| import static org.mockito.Mockito.doReturn; | |
| import org.junit.jupiter.api.BeforeEach; | |
| import org.junit.jupiter.api.Test; | |
| import org.junit.jupiter.api.extension.ExtendWith; | |
| import org.mockito.InjectMocks; | |
| import org.mockito.Mock; | |
| import org.mockito.junit.jupiter.MockitoExtension; | |
| import org.slf4j.Logger; | |
| import org.slf4j.LoggerFactory; | |
| import com.iemr.common.controller.users.IEMRAdminController; | |
| import com.iemr.common.service.users.IEMRAdminUserService; | |
| import jakarta.servlet.http.Cookie; | |
| import jakarta.servlet.http.HttpServletRequest; | |
| import net.minidev.json.JSONArray; | |
| import net.minidev.json.JSONObject; | |
| import com.iemr.common.utils.encryption.AESUtil; | |
| import com.fasterxml.jackson.databind.ObjectMapper; | |
| // import com.google.common.net.MediaType; | |
| import com.google.gson.JsonObject; | |
| import java.util.Collections; | |
| import java.util.Date; | |
| import com.iemr.common.model.user.ChangePasswordModel; | |
| import com.iemr.common.data.users.M_Role; | |
| import com.iemr.common.utils.CookieUtil; | |
| import com.iemr.common.data.users.ProviderServiceMapping; | |
| import com.iemr.common.data.users.ServiceMaster; | |
| @ExtendWith(MockitoExtension.class) | |
| // Move this block to the very top of the file: | |
| package com.iemr.common.controller.users; | |
| import com.iemr.common.data.users.UserServiceRoleMapping; | |
| import static org.junit.jupiter.api.Assertions.assertNotNull; | |
| // ... rest of imports ... | |
| @ExtendWith(MockitoExtension.class) | |
| class IEMRAdminControllerTest { | |
| // --- Helpers for common mock setups --- | |
| private User createMockUser(long id, String name, String status) { | |
| // existing helper implementation… | |
| } | |
| // … rest of the test class … |
🤖 Prompt for AI Agents
In src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java
between lines 72 and 160, the package declaration and import statements are
incorrectly placed after method definitions, causing a compilation error. Move
the package declaration and all import statements to the very top of the file,
before any class or method definitions, to fix the compilation issue.
There was a problem hiding this comment.
Sorry , seems like my file didnt save before
There was a problem hiding this comment.
@Zapper9982, no worries! File saving issues happen to the best of us.
The compilation error was correctly identified - the package declaration and imports need to be moved to the very top of the file before any class or method definitions. Once you fix that structure, the test file should compile properly.
Thanks for the clarification! 🐰
(\ /)
( ._.)
o_(")(")
🧠 Learnings used
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (5)
904-904: Fix typo: "previlegeObj" should be "privilegeObj".There's a consistent typo throughout the file where "privilege" is misspelled as "previlege".
Also applies to: 1337-1337, 1374-1374, 1375-1375, 1395-1395, 1396-1396, 1834-1834
515-515: Critical Security Issue: Hardcoded AES Key in Production and Test CodeThe literal key
"Piramal12Piramal"is embedded in test verifications, confirming it's hardcoded in production code. This is a critical security vulnerability.Also applies to: 552-552, 588-588, 625-625, 642-642, 675-675, 714-714
598-612: Incomplete test coverage: Missing response content assertions.Several test methods only verify HTTP status without asserting on the response content. This reduces the effectiveness of the tests.
Also applies to: 789-802, 805-810
704-704: Poor error handling: Controller returns NullPointerException messages.Multiple tests expect responses containing NullPointerException messages, indicating the controller is not properly handling null cases and is leaking implementation details to the client.
Also applies to: 1086-1086, 1100-1100
1763-1855: Remove deprecated test methods.These test methods are marked as deprecated and have been replaced by parameterized tests. They should be removed to keep the codebase clean.
🧹 Nitpick comments (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
244-372: Consider using parameterized tests to reduce duplication.Multiple test methods follow similar patterns (e.g., testing various error scenarios for refreshToken, different authentication scenarios). Consider using JUnit 5's
@ParameterizedTestto consolidate these and improve maintainability.Example for refreshToken error scenarios:
-@Test -void refreshToken_InvalidToken() throws Exception { - // test implementation -} - -@Test -void refreshToken_IncorrectTokenType() throws Exception { - // test implementation -} +@ParameterizedTest +@MethodSource("refreshTokenErrorScenarios") +void refreshToken_ErrorScenarios(String token, Claims claims, int expectedStatus, String expectedMessage) throws Exception { + // consolidated test implementation +} + +private static Stream<Arguments> refreshTokenErrorScenarios() { + return Stream.of( + Arguments.of("invalid_token", null, 401, "Unauthorized."), + Arguments.of("valid_token", mockClaimsWithType("access"), 401, "Unauthorized.") + ); +}Also applies to: 1135-1271
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (10)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.089Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:53:24.380Z
Learning: The user vishwab1 confirmed that their CAPTCHA implementation handles null token checking at the controller level before calling the service, which mitigates the direct security vulnerability of the service returning true for null tokens.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_example.properties:0-0
Timestamp: 2025-01-30T09:11:34.734Z
Learning: In example/template properties files, sensitive values like JWT secrets should use placeholders (e.g. `<Enter_Your_Secret_Key>`) instead of actual values to avoid exposing secrets in version control.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_test.properties:0-0
Timestamp: 2025-02-03T12:41:45.354Z
Learning: In Common-API, JWT secrets should be configured using placeholders (e.g., `<Enter_Your_Secret_Key>`) in properties files, similar to other sensitive credentials, to prevent accidental commit of actual secrets.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_uat.properties:0-0
Timestamp: 2025-02-03T12:41:59.244Z
Learning: In the Common-API project, JWT secret configuration should use environment variables (e.g., `jwt.secret=@env.JWT_SECRET_KEY@`) across all environments for better security.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
🔇 Additional comments (2)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (2)
1-89: LGTM! Well-organized imports and test setup.The imports are properly organized and the test class structure follows best practices with appropriate use of JUnit 5 and Mockito annotations.
198-242: Excellent test coverage and implementation!These test methods demonstrate comprehensive testing with:
- Proper mocking of dependencies
- Thorough verification of interactions
- Good edge case coverage
- Effective use of reflection for testing private methods
Also applies to: 374-393, 411-445, 483-498, 871-920, 1356-1456
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (5)
728-728: Poor error handling: NullPointerException messages exposed to clientsMultiple tests expect responses containing NPE messages like
"Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null". This indicates the controller is not properly handling null cases and is exposing internal implementation details to clients.The controller should validate inputs and return appropriate error responses instead of letting NullPointerExceptions propagate to the client.
Also applies to: 1039-1039, 1053-1053
928-928: Fix typo: "previlegeObj" should be "privilegeObj"There's a consistent typo throughout the file where "privilege" is misspelled as "previlege".
- validatedObj.put("previlegeObj", new org.json.JSONArray()); + validatedObj.put("privilegeObj", new org.json.JSONArray());Also applies to: 1363-1363, 1400-1400, 1421-1421, 1860-1860
1788-1830: Remove deprecated test methodsThese test methods contain comments indicating they are deprecated and have been replaced by parameterized tests. They should be removed to keep the codebase clean.
- @Test - void userAuthenticate_mobileDevice_branch_shouldReturnTokensAndStoreInRedis() throws Exception { - // Deprecated: replaced by parameterized test - // ...existing code... - } - - @Test - void superUserAuthenticate_mobileAndNonMobileBranches_shouldCoverJwtRefreshRedisCookieLogic() throws Exception { - // Deprecated: replaced by parameterized test - // ...existing code... - }Also applies to: 1832-1880
615-624: Improve test coverage: Add response content assertionsSeveral test methods only verify HTTP status without asserting on the response content. This reduces test effectiveness as they don't verify that endpoints return expected data.
Add assertions to verify response content:
@Test void userAuthenticateNew_Success() throws Exception { String jsonRequest = "{\"userName\":\"testUser\",\"password\":\"testPwd\"}"; mockMvc.perform(post("/user/userAuthenticateNew") .contentType(MediaType.APPLICATION_JSON) .content(jsonRequest)) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(jsonPath("$.statusCode").exists()) + .andExpect(jsonPath("$.data").exists()); }Also applies to: 626-636, 638-657, 732-744, 746-755, 757-764, 766-773, 775-786, 788-797, 799-808
537-537: Externalize Hardcoded AES Encryption Key in Production and TestsOur search confirms the literal
"Piramal12Piramal"is hardcoded in both the production controller and its tests, creating a critical security risk. Please externalize this key to a configuration property or environment variable and update tests accordingly.• Production occurrences (all in src/main/java/com/iemr/common/controller/users/IEMRAdminController.java):
- decrypt(“Piramal12Piramal”, m_User.getPassword())
- decrypt(“Piramal12Piramal”, m_user.getPassword())
- commented-out decrypt call
• Test occurrence (src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java, line 537):
verify(aesUtil, times(1)).decrypt("Piramal12Piramal", "encryptedPwd");Proposed changes:
In IEMRAdminController.java
- // hardcoded key - String decryptPassword = aesUtil.decrypt("Piramal12Piramal", m_User.getPassword()); + // injected from configuration + @Value("${aes.secret.key}") + private String aesSecretKey; + + String decryptPassword = aesUtil.decrypt(aesSecretKey, m_User.getPassword());In IEMRAdminControllerTest.java
- verify(aesUtil, times(1)).decrypt("Piramal12Piramal", "encryptedPwd"); + // use the same test key injected as a constant + private static final String TEST_AES_KEY = "testKeyPlaceholder"; + verify(aesUtil, times(1)).decrypt(TEST_AES_KEY, "encryptedPwd");• Remove any commented-out hardcoded calls.
• Define aes.secret.key in application.yml/properties or via an env var.
• Update your CI/run profiles to provide a secure value for aes.secret.key.
🧹 Nitpick comments (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
150-163: Consider improving dependency injection approachThe setUp method uses reflection to inject dependencies, which suggests the controller may not have proper setter methods. While this works for testing, it makes the code harder to maintain and indicates potential design issues.
Consider adding proper setter methods to the controller or using constructor injection to eliminate the need for reflection-based dependency injection in tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/test/java/com/iemr/common/controller/covid/CovidVaccinationControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/email/EmailControllerTest.java(1 hunks)src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/iemr/common/controller/email/EmailControllerTest.java
- src/test/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppControllerTest.java
- src/test/java/com/iemr/common/controller/covid/CovidVaccinationControllerTest.java
🧰 Additional context used
🧠 Learnings (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (14)
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java:704-704
Timestamp: 2025-07-11T18:00:56.894Z
Learning: In the IEMRAdminController (and likely other controllers in this codebase), error handling is incomplete - NullPointerExceptions are being allowed to propagate to clients instead of being caught and converted to proper error responses. Tests expect error messages like "Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null", indicating a systemic issue with input validation and null handling in controller methods.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:422-431
Timestamp: 2025-03-03T05:44:08.973Z
Learning: In the Common-API project, JsonNode from Jackson library has been found to not work properly for certain JSON parsing tasks, so JSONObject is preferred in those cases, particularly in the GrievanceHandlingServiceImpl class.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/feedback/FeedbackControllerTest.java:259-269
Timestamp: 2025-07-06T19:05:27.089Z
Learning: The FeedbackController (and likely other controllers in this codebase) uses a consistent wrapper pattern where all HTTP responses return status 200 OK, and the actual success/failure status is indicated by a `statusCode` field in the JSON response body (200 for success, 5000 for errors).
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:53:24.380Z
Learning: The user vishwab1 confirmed that their CAPTCHA implementation handles null token checking at the controller level before calling the service, which mitigates the direct security vulnerability of the service returning true for null tokens.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_example.properties:0-0
Timestamp: 2025-01-30T09:11:34.734Z
Learning: In example/template properties files, sensitive values like JWT secrets should use placeholders (e.g. `<Enter_Your_Secret_Key>`) instead of actual values to avoid exposing secrets in version control.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_test.properties:0-0
Timestamp: 2025-02-03T12:41:45.354Z
Learning: In Common-API, JWT secrets should be configured using placeholders (e.g., `<Enter_Your_Secret_Key>`) in properties files, similar to other sensitive credentials, to prevent accidental commit of actual secrets.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_uat.properties:0-0
Timestamp: 2025-02-03T12:41:59.244Z
Learning: In the Common-API project, JWT secret configuration should use environment variables (e.g., `jwt.secret=@env.JWT_SECRET_KEY@`) across all environments for better security.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Learnt from: srishtigrp78
PR: PSMRI/Common-API#159
File: src/main/java/com/iemr/common/data/grievance/GrievanceReallocationRequest.java:12-17
Timestamp: 2025-02-11T08:11:17.440Z
Learning: Validation annotations are not required for request DTOs in the Common-API project, as validation is handled elsewhere in the codebase.
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:21:53.819Z
Learning: The user vishwab1 expects CAPTCHA tokens to be optionally sent in request bodies during implementation, suggesting a gradual rollout approach where clients may or may not include CAPTCHA tokens initially.
Learnt from: Zapper9982
PR: PSMRI/Common-API#258
File: src/test/java/com/iemr/common/controller/brd/BRDIntegrationControllerTest.java:60-60
Timestamp: 2025-07-06T15:21:25.699Z
Learning: Spring Boot's default Jackson configuration ignores unknown/extra properties during JSON deserialization in REST controllers, making tests for extra fields in JSON input unnecessary as the framework handles this gracefully by default.
🔇 Additional comments (1)
src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java (1)
111-1883: Excellent test coverage and structureThis test file demonstrates comprehensive coverage of the IEMRAdminController functionality, including:
- Authentication scenarios (success, failure, multiple users)
- Token management (refresh, validation, expiration)
- Security question handling
- Error handling and edge cases
- Mobile vs non-mobile device handling
- Password management operations
The use of JUnit 5, Mockito, and MockMvc is appropriate and well-executed. The test methods are well-named and cover both positive and negative scenarios effectively.
|
Zapper9982
changed the title
A total of 793 testMethods are written with 17 Failures some in
|
|
@drtechie @vanitha1822 This PR is ready for a comprehensive review . |
| @@ -0,0 +1,292 @@ | |||
| /* | |||
| * AMRIT – Accessible Medical Records via Integrated Technology | |||
There was a problem hiding this comment.
@Zapper9982 Is this file required? If it doesn’t contain any valid code, please remove it.
There was a problem hiding this comment.
@vanitha1822 thats an extra test file , that I have added for the CareStreamCreateOrderControllerTest as a part of socket testing with the API . however since it performs socket connections testing with the socket connection , it causes about 40 seconds extra to build , so I added the minimal test along with it .
However if we wish we can remove the socket testing file ...
📋 Description
Added Unit tests for Utils , Mappers and some Controllers :
git.propertiesto simulate the version controller and test its all parameters .LogBacktesting dependency andJacoco Reportsupport .✅ Type of Change
ℹ️ Additional Information
Total current Test Methods :
417Proof that build is successful after testing :
Jacoco Report :
Summary by CodeRabbit