Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abdm M1 V3 Changes #52

Merged
merged 8 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/main/environment/common_ci.properties
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,15 @@ [email protected]_HEALTH_ID_BASE_URL@/api/v1/auth/confirmWithAadha
[email protected]_Account_ID_BASE_URL@/api/v1/account/profile

##ABDM Facility services
getAbdmFacilityServicies= @env.ABDM_HEALTH_ID_BASE_URL@/devservice/v1/bridges/getServices
getAbdmFacilityServicies= @env.ABDM_BASE_URL@/devservice/v1/bridges/getServices

##ABDM V3 APIs
getAuthCertPublicKey = @env.ABDM_HEALTH_ID_BASE_URL@/api/v1/auth/cert
requestOtpForEnrollment = @env.ABDM_BASE_URL@/abha/api/v3/enrollment/request/otp
abhaEnrollByAadhaar = @env.ABDM_BASE_URL@/abha/api/v3/enrollment/enrol/byAadhaar
printAbhaCard = @env.ABDM_BASE_URL@/abha/api/v3/profile/account/abha-card
abhaLoginRequestOtp = @env.ABDM_BASE_URL@/abha/api/v3/profile/login/request/otp
verifyAbhaLogin = @env.ABDM_BASE_URL@/abha/api/v3/profile/login/verify


abhaMode=sbx
Expand Down
7 changes: 7 additions & 0 deletions src/main/environment/common_dev.properties
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ [email protected]_Account_ID_BASE_URL@/api/v1/account/profile
##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices

##ABDM V3 APIs
getAuthCertPublicKey = https://healthidsbx.abdm.gov.in/api/v1/auth/cert
requestOtpForEnrollment = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/request/otp
abhaEnrollByAadhaar = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/enrol/byAadhaar
printAbhaCard = https://abhasbx.abdm.gov.in/abha/api/v3/profile/account/abha-card
abhaLoginRequestOtp = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/request/otp
verifyAbhaLogin = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/verify

abhaMode=sbx

Expand Down
8 changes: 8 additions & 0 deletions src/main/environment/common_example.properties
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ [email protected]_Account_ID_BASE_URL@/api/v1/account/profile
##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices

##ABDM V3 APIs
getAuthCertPublicKey = https://healthidsbx.abdm.gov.in/api/v1/auth/cert
requestOtpForEnrollment = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/request/otp
abhaEnrollByAadhaar = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/enrol/byAadhaar
printAbhaCard = https://abhasbx.abdm.gov.in/abha/api/v3/profile/account/abha-card
abhaLoginRequestOtp = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/request/otp
verifyAbhaLogin = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/verify

Comment on lines +87 to +94
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Consider extracting base URLs to environment variables

The new ABDM V3 API endpoints use hardcoded URLs with sandbox domains. For better environment management and security, consider:

  1. Extracting base URLs to environment variables
  2. Using different base URLs for different environments (dev, test, prod)

Apply this pattern for better configuration management:

##ABDM V3 APIs
-getAuthCertPublicKey = https://healthidsbx.abdm.gov.in/api/v1/auth/cert
-requestOtpForEnrollment = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/request/otp
-abhaEnrollByAadhaar = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/enrol/byAadhaar
-printAbhaCard = https://abhasbx.abdm.gov.in/abha/api/v3/profile/account/abha-card
-abhaLoginRequestOtp = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/request/otp
-verifyAbhaLogin = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/verify
+[email protected]_AUTH_BASE_URL@
+[email protected]_ABHA_BASE_URL@
+
+getAuthCertPublicKey=${abdm.v3.auth.base-url}/api/v1/auth/cert
+requestOtpForEnrollment=${abdm.v3.abha.base-url}/api/v3/enrollment/request/otp
+abhaEnrollByAadhaar=${abdm.v3.abha.base-url}/api/v3/enrollment/enrol/byAadhaar
+printAbhaCard=${abdm.v3.abha.base-url}/api/v3/profile/account/abha-card
+abhaLoginRequestOtp=${abdm.v3.abha.base-url}/api/v3/profile/login/request/otp
+verifyAbhaLogin=${abdm.v3.abha.base-url}/api/v3/profile/login/verify
πŸ“ 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.

Suggested change
##ABDM V3 APIs
getAuthCertPublicKey = https://healthidsbx.abdm.gov.in/api/v1/auth/cert
requestOtpForEnrollment = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/request/otp
abhaEnrollByAadhaar = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/enrol/byAadhaar
printAbhaCard = https://abhasbx.abdm.gov.in/abha/api/v3/profile/account/abha-card
abhaLoginRequestOtp = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/request/otp
verifyAbhaLogin = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/verify
##ABDM V3 APIs
abdm.v3.auth.base-url[email protected]_AUTH_BASE_URL@
abdm.v3.abha.base-url[email protected]_ABHA_BASE_URL@
getAuthCertPublicKey=${abdm.v3.auth.base-url}/api/v1/auth/cert
requestOtpForEnrollment=${abdm.v3.abha.base-url}/api/v3/enrollment/request/otp
abhaEnrollByAadhaar=${abdm.v3.abha.base-url}/api/v3/enrollment/enrol/byAadhaar
printAbhaCard=${abdm.v3.abha.base-url}/api/v3/profile/account/abha-card
abhaLoginRequestOtp=${abdm.v3.abha.base-url}/api/v3/profile/login/request/otp
verifyAbhaLogin=${abdm.v3.abha.base-url}/api/v3/profile/login/verify

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai other urls are not maintained with variables so not maintaining these also with variables and using direct urls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@helenKaryamsetty Thank you for the clarification.


✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/environment/common_example.properties:87-94
Timestamp: 2024-12-10T13:24:01.383Z
Learning: In the FHIR-API project, URLs are maintained directly in the properties files rather than using variables, so it's acceptable to use direct URLs instead of variables.

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.


abhaMode=sbx

Expand Down
8 changes: 8 additions & 0 deletions src/main/environment/common_test.properties
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ [email protected]_Account_ID_BASE_URL@/api/v1/account/profile
##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices

##ABDM V3 APIs
getAuthCertPublicKey = https://healthidsbx.abdm.gov.in/api/v1/auth/cert
requestOtpForEnrollment = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/request/otp
abhaEnrollByAadhaar = https://abhasbx.abdm.gov.in/abha/api/v3/enrollment/enrol/byAadhaar
printAbhaCard = https://abhasbx.abdm.gov.in/abha/api/v3/profile/account/abha-card
abhaLoginRequestOtp = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/request/otp
verifyAbhaLogin = https://abhasbx.abdm.gov.in/abha/api/v3/profile/login/verify


abhaMode=sbx

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package com.wipro.fhir.controller.v3.abha;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.CrossOrigin;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import com.wipro.fhir.service.ndhm.GenerateHealthID_CardServiceImpl;
import com.wipro.fhir.service.v3.abha.CreateAbhaV3Service;
import com.wipro.fhir.utils.exception.FHIRException;
import com.wipro.fhir.utils.response.OutputResponse;

import io.swagger.v3.oas.annotations.Operation;

@CrossOrigin
@RestController
@RequestMapping(value = "/abhaCreation", headers = "Authorization")
public class CreateAbhaV3Controller {

private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());

@Autowired
private CreateAbhaV3Service createAbhaV3Service;

@Autowired
private GenerateHealthID_CardServiceImpl generateHealthID_CardServiceImpl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Remove unused service injection.

The GenerateHealthID_CardServiceImpl is autowired but never used in this controller.

-@Autowired 
-private GenerateHealthID_CardServiceImpl generateHealthID_CardServiceImpl;


@CrossOrigin
@Operation(summary = "Generate OTP for ABHA enrollment")
@PostMapping(value = { "/requestOtpForAbhaEnrollment" })
public String requestOtpForEnrollment(@RequestBody String request) {
logger.info("Generate OTP for ABHA enrollment API request " + request);
OutputResponse response = new OutputResponse();
try {
if (request != null) {
String s = createAbhaV3Service.getOtpForEnrollment(request);
response.setResponse(s);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR generate OTP for ABHA card API response " + response.toString());
return response.toString();
}

@CrossOrigin
@Operation(summary = "ABHA enrollment by Aadhaar")
@PostMapping(value = { "/abhaEnrollmentByAadhaar" })
public String abhaEnrollmentByAadhaar(@RequestBody String request) {
logger.info("ABHA enrollment BY Aadhaar API request " + request);
OutputResponse response = new OutputResponse();
String res = null;
try {
if (request != null) {
res = createAbhaV3Service.enrollmentByAadhaar(request);
response.setResponse(res);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR generate OTP for ABHA card API response " + response.toString());
return response.toString();
}
Comment on lines +51 to +68
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix logging and remove redundant variable in Aadhaar enrollment endpoint.

  1. Raw request logging poses security risks
  2. Response log message is incorrect (mentions OTP instead of enrollment)
  3. Variable 'res' is redundant
 @PostMapping(value = { "/abhaEnrollmentByAadhaar" })
 public String abhaEnrollmentByAadhaar(@RequestBody String request) {
-    logger.info("ABHA enrollment BY Aadhaar API request " + request);
+    logger.info("ABHA enrollment by Aadhaar API request received");
     OutputResponse response = new OutputResponse();
-    String res = null;
     try {
         if (request != null) {
-            res = createAbhaV3Service.enrollmentByAadhaar(request);
-            response.setResponse(res);
+            response.setResponse(createAbhaV3Service.enrollmentByAadhaar(request));
         } else
-            throw new FHIRException("NDHM_FHIR Empty request object");
+            throw new FHIRException("ABHA-002: Empty request object");
     } catch (FHIRException e) {
-        response.setError(5000, e.getMessage());
+        response.setError(4002, "Aadhaar Enrollment Failed: " + e.getMessage());
         logger.error(e.toString());
     }
-    logger.info("NDHM_FHIR generate OTP for ABHA card API response " + response.toString());
+    logger.info("Aadhaar enrollment completed with status: {}", response.hasError() ? "ERROR" : "SUCCESS");
     return response.toString();
 }
πŸ“ 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.

Suggested change
@PostMapping(value = { "/abhaEnrollmentByAadhaar" })
public String abhaEnrollmentByAadhaar(@RequestBody String request) {
logger.info("ABHA enrollment BY Aadhaar API request " + request);
OutputResponse response = new OutputResponse();
String res = null;
try {
if (request != null) {
res = createAbhaV3Service.enrollmentByAadhaar(request);
response.setResponse(res);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR generate OTP for ABHA card API response " + response.toString());
return response.toString();
}
@PostMapping(value = { "/abhaEnrollmentByAadhaar" })
public String abhaEnrollmentByAadhaar(@RequestBody String request) {
logger.info("ABHA enrollment by Aadhaar API request received");
OutputResponse response = new OutputResponse();
try {
if (request != null) {
response.setResponse(createAbhaV3Service.enrollmentByAadhaar(request));
} else
throw new FHIRException("ABHA-002: Empty request object");
} catch (FHIRException e) {
response.setError(4002, "Aadhaar Enrollment Failed: " + e.getMessage());
logger.error(e.toString());
}
logger.info("Aadhaar enrollment completed with status: {}", response.hasError() ? "ERROR" : "SUCCESS");
return response.toString();
}
🧰 Tools
πŸͺ› GitHub Check: SonarCloud

[notice] 53-53: Logging should not be vulnerable to injection attacks

Change this code to not log user-controlled data.

See more on SonarQube Cloud

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai kindly ignore all the logs related suggestions. Those are given according to our debugging usage

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@helenKaryamsetty, understood. I will ignore log-related suggestions in future reviews.

(・x・`)


✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/controller/v3/abha/CreateAbhaV3Controller.java:51-68
Timestamp: 2024-12-10T14:52:59.709Z
Learning: In the FHIR-API project, when reviewing code in Java controllers (e.g., `CreateAbhaV3Controller.java`), the team prefers to retain raw request logging for debugging purposes. Therefore, suggestions to remove or modify logging statements related to raw requests should be ignored.

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.


@CrossOrigin
@Operation(summary = "Verify Mobile OTP for ABHA enrollment")
@PostMapping(value = { "/verifyAuthByMobile" })
public String verifyMobileForAuth(@RequestBody String request) {
logger.info("Verify Mobile OTP for ABHA enrollment " + request);
OutputResponse response = new OutputResponse();
try {
if (request != null) {
String s = createAbhaV3Service.verifyAuthByMobile(request);
response.setResponse(s);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR generate OTP for ABHA card API response " + response.toString());
return response.toString();
}

@CrossOrigin
@Operation(summary = "Print Abha card")
@PostMapping(value = { "/printAbhaCard" })
public String printAbhaCard(@RequestBody String request) {
OutputResponse response = new OutputResponse();
try {
if (request != null) {
String s = createAbhaV3Service.getAbhaCardPrinted(request);
response.setResponse(s);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR generate OTP for ABHA card API respponse " + response.toString());
return response.toString();
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package com.wipro.fhir.controller.v3.abha;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.CrossOrigin;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import com.wipro.fhir.service.v3.abha.LoginAbhaV3Service;
import com.wipro.fhir.utils.exception.FHIRException;
import com.wipro.fhir.utils.response.OutputResponse;

import io.swagger.v3.oas.annotations.Operation;

@CrossOrigin
@RestController
@RequestMapping(value = "/abhaLogin", headers = "Authorization")
Comment on lines +18 to +20
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Review security implications of unrestricted CORS.

The @CrossOrigin annotation without any parameters allows requests from any origin, which could pose security risks. Consider restricting the allowed origins based on your deployment environment.

-@CrossOrigin
+@CrossOrigin(origins = "${allowed.origins}", allowedHeaders = "Authorization")

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai all over the project we are maintaining only crossOrigin only to maintain the commonality using it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@helenKaryamsetty Understood, thank you for the clarification. I will note that in this project, the @CrossOrigin annotation is intentionally used without parameters across all controllers to maintain consistency.


✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/controller/v3/abha/LoginAbhaV3Controller.java:18-20
Timestamp: 2024-12-09T16:09:31.378Z
Learning: In the Java Spring Boot project, all controllers, including `LoginAbhaV3Controller.java` at `src/main/java/com/wipro/fhir/controller/v3/abha/`, use the `@CrossOrigin` annotation without parameters to maintain consistency across the codebase.

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.

public class LoginAbhaV3Controller {

private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());

@Autowired
private LoginAbhaV3Service loginAbhaV3Service;

@CrossOrigin
@Operation(summary = "Request OTP for Abha LOgin")
@PostMapping(value = { "/abhaLoginRequestOtp" })
public String requestOtpForAbhaLogin(@RequestBody String request) {
logger.info("Generate OTP for ABHA Login API request " + request);
OutputResponse response = new OutputResponse();
try {
if (request != null) {
String s = loginAbhaV3Service.requestOtpForAbhaLogin(request);
response.setResponse(s);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR generate OTP for ABHA login API response " + response.toString());
return response.toString();
}

@CrossOrigin
@Operation(summary = "verify OTP for Abha LOgin")
@PostMapping(value = { "/verifyAbhaLogin" })
public String verifyAbhaLogin(@RequestBody String request) {
logger.info("verify OTP for ABHA Login API request " + request);
OutputResponse response = new OutputResponse();
try {
if (request != null) {
String s = loginAbhaV3Service.verifyAbhaLogin(request);
response.setResponse(s);
} else
throw new FHIRException("NDHM_FHIR Empty request object");
} catch (FHIRException e) {
response.setError(5000, e.getMessage());
logger.error(e.toString());
}
logger.info("NDHM_FHIR Verify abha login API response " + response.toString());
return response.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import java.util.ArrayList;

import org.bson.types.ObjectId;
import org.springframework.data.annotation.Id;
import org.springframework.data.mongodb.core.mapping.Document;
import org.springframework.data.mongodb.core.mapping.Field;
Expand All @@ -37,8 +38,8 @@ public class PatientCareContexts {

@Id
@Expose
@Field(value = "id")
private String id;
@Field(value = "_id")
private ObjectId _id;

@Expose
@Field(value = "phoneNumber")
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/com/wipro/fhir/data/v3/abhaCard/BioRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.wipro.fhir.data.v3.abhaCard;

import lombok.Data;

@Data
public class BioRequest {

private String timestamp;
private String txnId;
private String aadhaar;
private Object fingerPrintAuthPid;
private String mobile;

Comment on lines +5 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add input validation and security measures for sensitive data.

This class handles sensitive PII data (Aadhaar, mobile, biometric). Consider the following improvements:

  1. Add validation annotations for input fields
  2. Add documentation about data sensitivity and handling
  3. Replace generic Object type with a specific type for fingerPrintAuthPid

Apply these changes:

+import javax.validation.constraints.Pattern;
+import javax.validation.constraints.NotNull;
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;

+@ApiModel(description = "Request object for biometric authentication")
 @Data
 public class BioRequest {
 
+    @ApiModelProperty(value = "Timestamp of the request", required = true)
+    @NotNull(message = "Timestamp cannot be null")
     private String timestamp;
 
+    @ApiModelProperty(value = "Transaction ID", required = true)
+    @NotNull(message = "Transaction ID cannot be null")
     private String txnId;
 
+    @ApiModelProperty(value = "Aadhaar number", required = true)
+    @NotNull(message = "Aadhaar cannot be null")
+    @Pattern(regexp = "^[0-9]{12}$", message = "Invalid Aadhaar format")
     private String aadhaar;
 
+    @ApiModelProperty(value = "Fingerprint authentication PID", required = true)
+    @NotNull(message = "Fingerprint auth PID cannot be null")
-    private Object fingerPrintAuthPid;
+    private BiometricPID fingerPrintAuthPid;
 
+    @ApiModelProperty(value = "Mobile number", required = true)
+    @NotNull(message = "Mobile cannot be null")
+    @Pattern(regexp = "^[0-9]{10}$", message = "Invalid mobile number format")
     private String mobile;
 }

Committable suggestion skipped: line range outside the PR's diff.

}
10 changes: 10 additions & 0 deletions src/main/java/com/wipro/fhir/data/v3/abhaCard/ConsentRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.wipro.fhir.data.v3.abhaCard;

import lombok.Data;

@Data
public class ConsentRequest {

private String code;
private String version;
}
14 changes: 14 additions & 0 deletions src/main/java/com/wipro/fhir/data/v3/abhaCard/EnrollByAadhaar.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.wipro.fhir.data.v3.abhaCard;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import lombok.Data;

@Data
public class EnrollByAadhaar{
public Map<String, Object> authData;
public ConsentRequest consent;
}
Comment on lines +10 to +13
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Enhance encapsulation and add field validation

  1. The fields are declared as public, which breaks encapsulation. Consider making them private and relying on Lombok's @DaTa for access.
  2. Consider adding validation constraints for required fields using javax.validation annotations.
 @Data
 public class EnrollByAadhaar{
-    public Map<String, Object> authData;
-    public ConsentRequest consent;
+    @NotNull(message = "Authentication data is required")
+    private Map<String, Object> authData;
+
+    @NotNull(message = "Consent request is required")
+    private ConsentRequest consent;
 }

Don't forget to add the import:

import javax.validation.constraints.NotNull;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are not required fields and can be public

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Those are not required fields and can be public

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@helenKaryamsetty, understood, thank you for the clarification. I'll note that the fields authData and consent in the EnrollByAadhaar class are not required and can remain public.


✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/data/v3/abhaCard/EnrollByAadhaar.java:10-13
Timestamp: 2024-12-09T15:56:01.326Z
Learning: Fields `authData` and `consent` in the `EnrollByAadhaar` class (`src/main/java/com/wipro/fhir/data/v3/abhaCard/EnrollByAadhaar.java`) are not required and can remain public.

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.


Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.wipro.fhir.data.v3.abhaCard;

import lombok.Data;

@Data
public class LoginAbhaRequest {

private String loginId;
private String loginMethod;
private String loginHint;

}
16 changes: 16 additions & 0 deletions src/main/java/com/wipro/fhir/data/v3/abhaCard/LoginMethod.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.wipro.fhir.data.v3.abhaCard;

import lombok.Data;

@Data
public class LoginMethod {

private String loginId;
private String loginMethod;
private String pId;
private String tnxId;
private String mobileNumber;
private String createdBy;
private int providerServiceMapId;

}
15 changes: 15 additions & 0 deletions src/main/java/com/wipro/fhir/data/v3/abhaCard/OtpRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.wipro.fhir.data.v3.abhaCard;

import java.sql.Timestamp;

import lombok.Data;

@Data
public class OtpRequest {

private String timestamp;
private String txnId;
private String otpValue;
private String mobile;

Comment on lines +7 to +14
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Add field validations and documentation.

Consider enhancing the class with:

  1. Field validations using javax.validation annotations
  2. JavaDoc documentation for the class and fields
  3. Proper type for timestamp field

Here's the suggested implementation:

+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.Pattern;
+import java.time.LocalDateTime;
 import lombok.Data;

+/**
+ * Data Transfer Object for OTP request operations in ABHA card services.
+ */
 @Data
 public class OtpRequest {
-    private String timestamp;
-    private String txnId;
-    private String otpValue;
-    private String mobile;
+    private LocalDateTime timestamp;
+    
+    @NotBlank(message = "Transaction ID is required")
+    private String txnId;
+    
+    @NotBlank(message = "OTP value is required")
+    @Pattern(regexp = "^[0-9]{6}$", message = "OTP must be 6 digits")
+    private String otpValue;
+    
+    @NotBlank(message = "Mobile number is required")
+    @Pattern(regexp = "^[0-9]{10}$", message = "Mobile number must be 10 digits")
+    private String mobile;
πŸ“ 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.

Suggested change
@Data
public class OtpRequest {
private String timestamp;
private String txnId;
private String otpValue;
private String mobile;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.Pattern;
import java.time.LocalDateTime;
import lombok.Data;
/**
* Data Transfer Object for OTP request operations in ABHA card services.
*/
@Data
public class OtpRequest {
private LocalDateTime timestamp;
@NotBlank(message = "Transaction ID is required")
private String txnId;
@NotBlank(message = "OTP value is required")
@Pattern(regexp = "^[0-9]{6}$", message = "OTP must be 6 digits")
private String otpValue;
@NotBlank(message = "Mobile number is required")
@Pattern(regexp = "^[0-9]{10}$", message = "Mobile number must be 10 digits")
private String mobile;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pattern and not blank validations are all not required to check here they are validated in UI repos.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@helenKaryamsetty, understood. Thank you for the clarification. I've noted this for future reviews.


✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/data/v3/abhaCard/OtpRequest.java:7-14
Timestamp: 2024-12-09T15:37:18.039Z
Learning: Field validations such as `@NotBlank` and `@Pattern` annotations are not required in backend Java classes like `OtpRequest.java` in `com.wipro.fhir.data.v3.abhaCard`, because they are validated in the UI repositories.

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.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.wipro.fhir.data.v3.abhaCard;

import lombok.Data;

@Data
public class RequestOTPEnrollment {

String tnxId;
String[] scope;
String loginHint;
String loginId;
String otpSystem;

}
13 changes: 13 additions & 0 deletions src/main/java/com/wipro/fhir/data/v3/abhaCard/VerifyAbhaLogin.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.wipro.fhir.data.v3.abhaCard;

import java.util.Map;

import lombok.Data;

@Data
public class VerifyAbhaLogin {

public String[] scope;
public Map<String, Object> authData;
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Use Private Fields with Accessors

The fields scope and authData are declared as public. To encapsulate the data and follow best practices, declare them as private and use Lombok's @Data annotation to generate getters and setters.

Modify the field declarations:

- public String[] scope;
- public Map<String, Object> authData;
+ private String[] scope;
+ private Map<String, Object> authData;

Since Lombok's @Data annotation is used, getters and setters will be automatically generated.

Committable suggestion skipped: line range outside the PR's diff.


}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
package com.wipro.fhir.repo.common;

import java.math.BigInteger;

import org.springframework.data.repository.CrudRepository;
import org.springframework.data.rest.core.annotation.RestResource;
import org.springframework.stereotype.Repository;
Expand All @@ -29,7 +31,7 @@

@Repository
@RestResource(exported = false)
public interface AllergyIntoleranceDataModelRepo extends CrudRepository<AllergyIntoleranceDataModel, Long> {
public interface AllergyIntoleranceDataModelRepo extends CrudRepository<AllergyIntoleranceDataModel, BigInteger> {


}
Loading
Loading