Skip to content

AMM-871 Abdm Facility fetch services changes #46

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.

Sign up for GitHub

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

Merged
ravishanigarapu merged 6 commits into from
Nov 27, 2024
Merged

AMM-871 Abdm Facility fetch services changes #46

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3814860
Abdm Facility fetch services changes
Sep 19, 2024
a2224b4
facility save after carecontext and patientcarecontextx update
Sep 26, 2024
1a28c2b
FacilityId save in mongo db and Variable declaration correction
Oct 8, 2024
a985f42
revert mistake changes HTTPRequestInterceptor.java
helenKaryamsetty Oct 8, 2024
5e6f9c8
Updating logger for src/main/java/com/wipro/fhir/service/common/Commo…
helenKaryamsetty Nov 20, 2024
4d37ef2
Update common_ci.properties
helenKaryamsetty Nov 27, 2024
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
16 changes: 12 additions & 4 deletions src/main/environment/common_ci.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ eAushadhiDummy=yes
##NDHM ABHA API
[email protected]_ABHA_CLIENT_ID@
[email protected]_ABHA_CLIENT_SECRET_KEY@
[email protected]_BASE_URL@/gateway/v1/sessions

##Default ABDM Facility ID
abdmFacilityId= @env.ABDM_FACILITY_ID@

[email protected]_BASE_URL@/gateway/v0.5/sessions
[email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/mobile/generateOtp
[email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/mobile/verifyOtp
[email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/mobile/createHealthId
Expand All @@ -53,9 +57,9 @@ abdmcreateHealthIdWithPreVerified= @env.ABDM_HEALTH_ID_BASE_URL@/api/v1/registra


##CareContext Creation API
[email protected]_BASE_URL@/gateway/v1/users/auth/init
addCareContext = @env.ABDM_BASE_URL@/gateway/v1/links/link/add-contexts
[email protected]_BASE_URL@/gateway/v1/users/auth/confirm
[email protected]_BASE_URL@/gateway/v0.5/users/auth/init
addCareContext = @env.ABDM_BASE_URL@/gateway/v0.5/links/link/add-contexts
[email protected]_BASE_URL@/gateway/v0.5/users/auth/confirm

##ABHA Card API
## Auth init - aadhar / mobile
Expand All @@ -77,6 +81,10 @@ [email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/aadhaar/
[email protected]_HEALTH_ID_BASE_URL@/api/v1/auth/confirmWithAadhaarBio
[email protected]_Account_ID_BASE_URL@/api/v1/account/profile

##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Replace hardcoded development URL with environment variable.

The getAbdmFacilityServicies property has several issues:

  1. Contains a hardcoded development URL instead of using environment variables
  2. Uses API version v1 while other endpoints use v0.5
  3. Points to 'devservice' subdomain which suggests development environment

Apply this fix to use environment variables consistently:

-getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices
+[email protected]_BASE_URL@/gateway/v0.5/bridges/getServices

Also, please note there's a typo in the property name ('Servicies' should be 'Services').

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


abhaMode=sbx

#logging.pattern.level=DEBUG
Expand Down
16 changes: 12 additions & 4 deletions src/main/environment/common_dev.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ eAushadhiDummy=yes
##NDHM ABHA API
clientID=<Enter client ID>
clientSecret=<Enter client secret key>
ndhmuserAuthenticate=https://dev.abdm.gov.in/gateway/v1/sessions

##Default ABDM Facility ID
abdmFacilityId= <Enter ABDM Facility ID>

ndhmuserAuthenticate=https://dev.abdm.gov.in/gateway/v0.5/sessions
ndhmGenerateOTP=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/generateOtp
ndhmVerifyOTP=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/verifyOtp
ndhmCreateHealthID=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/createHealthId
Expand All @@ -53,9 +57,9 @@ abdmcreateHealthIdWithPreVerified= https://healthidsbx.abdm.gov.in/api/v1/regist


##CareContext Creation API
generateOTPForCareContext=https://dev.abdm.gov.in/gateway/v1/users/auth/init
addCareContext = https://dev.abdm.gov.in/gateway/v1/links/link/add-contexts
validateOTPForCareContext=https://dev.abdm.gov.in/gateway/v1/users/auth/confirm
generateOTPForCareContext=https://dev.abdm.gov.in/gateway/v0.5/users/auth/init
addCareContext = https://dev.abdm.gov.in/gateway/v0.5/links/link/add-contexts
validateOTPForCareContext=https://dev.abdm.gov.in/gateway/v0.5/users/auth/confirm

##ABHA Card API
## Auth init - aadhar / mobile
Expand All @@ -77,6 +81,10 @@ [email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/aadhaar/
[email protected]_HEALTH_ID_BASE_URL@/api/v1/auth/confirmWithAadhaarBio
[email protected]_Account_ID_BASE_URL@/api/v1/account/profile

##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices


abhaMode=sbx

#logging.pattern.level=DEBUG
Expand Down
16 changes: 12 additions & 4 deletions src/main/environment/common_example.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ eAushadhiDummy=yes
##NDHM ABHA API
clientID=<Enter client ID>
clientSecret=<Enter client secret key>
ndhmuserAuthenticate=https://dev.abdm.gov.in/gateway/v1/sessions

##Default ABDM Facility ID
abdmFacilityId= <Enter ABDM Facility ID>

ndhmuserAuthenticate=https://dev.abdm.gov.in/gateway/v0.5/sessions
ndhmGenerateOTP=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/generateOtp
ndhmVerifyOTP=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/verifyOtp
ndhmCreateHealthID=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/createHealthId
Expand All @@ -53,9 +57,9 @@ abdmcreateHealthIdWithPreVerified= https://healthidsbx.abdm.gov.in/api/v1/regist


##CareContext Creation API
generateOTPForCareContext=https://dev.abdm.gov.in/gateway/v1/users/auth/init
addCareContext = https://dev.abdm.gov.in/gateway/v1/links/link/add-contexts
validateOTPForCareContext=https://dev.abdm.gov.in/gateway/v1/users/auth/confirm
generateOTPForCareContext=https://dev.abdm.gov.in/gateway/v0.5/users/auth/init
addCareContext = https://dev.abdm.gov.in/gateway/v0.5/links/link/add-contexts
validateOTPForCareContext=https://dev.abdm.gov.in/gateway/v0.5/users/auth/confirm

##ABHA Card API
## Auth init - aadhar / mobile
Expand All @@ -77,6 +81,10 @@ [email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/aadhaar/
[email protected]_HEALTH_ID_BASE_URL@/api/v1/auth/confirmWithAadhaarBio
[email protected]_Account_ID_BASE_URL@/api/v1/account/profile

##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices


abhaMode=sbx

#logging.pattern.level=DEBUG
Expand Down
16 changes: 12 additions & 4 deletions src/main/environment/common_test.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ eAushadhiDummy=yes
##NDHM ABHA API
clientID=<Enter client ID>
clientSecret=<Enter client secret key>
ndhmuserAuthenticate=https://dev.abdm.gov.in/gateway/v1/sessions

##Default ABDM Facility ID
abdmFacilityId= <Enter ABDM Facility ID>

ndhmuserAuthenticate=https://dev.abdm.gov.in/gateway/v0.5/sessions
ndhmGenerateOTP=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/generateOtp
ndhmVerifyOTP=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/verifyOtp
ndhmCreateHealthID=https://healthidsbx.abdm.gov.in/api/v1/registration/mobile/createHealthId
Expand All @@ -53,9 +57,9 @@ abdmcreateHealthIdWithPreVerified= https://healthidsbx.abdm.gov.in/api/v1/regist


##CareContext Creation API
generateOTPForCareContext=https://dev.abdm.gov.in/gateway/v1/users/auth/init
addCareContext = https://dev.abdm.gov.in/gateway/v1/links/link/add-contexts
validateOTPForCareContext=https://dev.abdm.gov.in/gateway/v1/users/auth/confirm
generateOTPForCareContext=https://dev.abdm.gov.in/gateway/v0.5/users/auth/init
addCareContext = https://dev.abdm.gov.in/gateway/v0.5/links/link/add-contexts
validateOTPForCareContext=https://dev.abdm.gov.in/gateway/v0.5/users/auth/confirm

##ABHA Card API
## Auth init - aadhar / mobile
Expand All @@ -77,6 +81,10 @@ [email protected]_HEALTH_ID_BASE_URL@/api/v1/registration/aadhaar/
[email protected]_HEALTH_ID_BASE_URL@/api/v1/auth/confirmWithAadhaarBio
[email protected]_Account_ID_BASE_URL@/api/v1/account/profile

##ABDM Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices
Comment on lines +84 to +85
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Maintain consistent API versioning and naming conventions.

Two observations:

  1. The new endpoint uses v1 while related endpoints were changed to v0.5
  2. The property name has a typo: getAbdmFacilityServicies should be getAbdmFacilityServices
##ABDM Facility services
-getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices
+getAbdmFacilityServices=https://dev.abdm.gov.in/devservice/v0.5/bridges/getServices
📝 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 Facility services
getAbdmFacilityServicies= https://dev.abdm.gov.in/devservice/v1/bridges/getServices
##ABDM Facility services
getAbdmFacilityServices=https://dev.abdm.gov.in/devservice/v0.5/bridges/getServices



abhaMode=sbx

#logging.pattern.level=DEBUG
Expand Down
33 changes: 0 additions & 33 deletions src/main/java/com/wipro/fhir/config/BlockingHttpMethodInterceptor.java
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/main/java/com/wipro/fhir/controller/carecontext/CareContextController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class CareContextController {
@Operation(summary = "Generate OTP for care context linking")
@PostMapping(value = { "/generateOTPForCareContext" })
public String generateOTP(
@Param(value = "{\"healthID\":\"String\",\"authenticationMode\":\"String\",\"healthIdNumber\":\"String\"}") @RequestBody String request,
@Param(value = "{\"healthID\":\"String\",\"authenticationMode\":\"String\",\"healthIdNumber\":\"String\", \"abdmFacilityId\":\"String\", \"abdmFacilityName\":\"String\" }") @RequestBody String request,
@RequestHeader(value = "Authorization") String Authorization) {
Comment on lines +53 to 54
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve request parameter documentation and validation.

The current @param annotation uses a JSON string to define the schema, which could be better represented using a proper request DTO class. Additionally, validation for the new required fields should be added.

Consider refactoring to use a DTO:

public class GenerateOTPRequest {
    @NotNull
    private String healthID;
    @NotNull
    private String authenticationMode;
    @NotNull
    private String healthIdNumber;
    @NotNull
    private String abdmFacilityId;
    @NotNull
    private String abdmFacilityName;
    // getters and setters
}

@PostMapping(value = "/generateOTPForCareContext")
public String generateOTP(
    @Valid @RequestBody GenerateOTPRequest request,
    @RequestHeader(value = "Authorization") String authorization) {
    // ... rest of the method
}

💡 Codebase verification

ABDM facility validation is partially implemented but needs enhancement

The code shows basic null checks for ABDM facility parameters but lacks comprehensive validation against the ABDM facility registry:

  • LinkCareContext_NDHMServiceImpl only performs null checks: if(null != genOTP.getAbdmFacilityId() && null != genOTP.getAbdmFacilityName())
  • The facility ID is configured via properties (@Value("${abdmFacilityId}")) but there's no validation logic to verify it against ABDM registry
  • No validation exists to ensure the facility ID and name pair is valid and active in ABDM's system
🔗 Analysis chain

Verify ABDM facility integration requirements.

The addition of abdmFacilityId and abdmFacilityName parameters suggests integration with ABDM facility services. Ensure that these values are properly validated against the ABDM facility registry.

Let me check the validation logic in the facility service implementation.

Let me check the NDHM service implementation for facility validation since the code shows integration with NDHM services.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for ABDM facility validation logic
echo "Searching for ABDM facility validation..."
rg -l "abdmFacility" --type java
rg -l "FacilityService" --type java

Length of output: 1282

Script:

#!/bin/bash
# Check facility validation implementation
rg -A 10 "abdmFacility" src/main/java/com/wipro/fhir/service/facility/FacilityServiceImpl.java

# Check care context service implementation for facility validation
rg -A 10 "abdmFacility" src/main/java/com/wipro/fhir/service/care_context/CareContextServiceImpl.java

# Look for ABDM facility validation patterns
ast-grep --pattern 'if ($_ != null && $_.contains("abdmFacility")) { $$$ }'

Length of output: 1291

Script:

#!/bin/bash
# Check NDHM service implementation for facility validation
rg -A 10 "LinkCareContext_NDHMService" src/main/java/com/wipro/fhir/service/ndhm/LinkCareContext_NDHMServiceImpl.java

# Look for facility validation in request processing
rg -A 10 "generateOTPForCareContext" src/main/java/com/wipro/fhir/service/ndhm/LinkCareContext_NDHMServiceImpl.java

# Check for any validation annotations or methods
ast-grep --pattern 'validate$_($$$)'

Length of output: 2350


OutputResponse response = new OutputResponse();
Expand Down
78 changes: 78 additions & 0 deletions src/main/java/com/wipro/fhir/controller/facility/FacilityController.java
View file
Open in desktop
Copy link
Member

@drtechie drtechie Nov 4, 2024

Choose a reason for hiding this comment

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

Indents are all over the place @helenKaryamsetty

Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package com.wipro.fhir.controller.facility;

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.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

import com.wipro.fhir.data.request_handler.ResourceRequestHandler;
import com.wipro.fhir.service.facility.FacilityService;
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 = "/facility", headers = "Authorization")
public class FacilityController {


@Autowired
private FacilityService facilityService;

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

@CrossOrigin
@Operation(summary = "Get ABDM Registered Facilities")
@GetMapping(value = { "/getAbdmRegisteredFacilities" })
public String getAbdmRegisteredFacilities(@RequestHeader(value = "Authorization") String Authorization) {

OutputResponse response = new OutputResponse();

try {

String resp = facilityService.fetchRegisteredFacilities();

response.setResponse(resp);

} catch (FHIRException e) {

response.setError(5000, e.getMessage());
logger.error(e.getMessage());
}
logger.info("Get ABDM Registered facilities API response" + response.toString());
return response.toString();
}
Comment on lines +34 to +54
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling and response structure.

Several improvements can be made to this endpoint:

  1. Consider using a proper response DTO instead of String
  2. Add specific error codes for different scenarios
  3. Use proper string formatting in logger
  4. Add Authorization header validation

Apply these improvements:

 @GetMapping(value = { "/getAbdmRegisteredFacilities" })
-public String getAbdmRegisteredFacilities(@RequestHeader(value = "Authorization") String Authorization) {
+public ResponseEntity<FacilityResponse> getAbdmRegisteredFacilities(
+    @RequestHeader(value = "Authorization") String authorization) {
+    if (StringUtils.isBlank(authorization)) {
+        return ResponseEntity.badRequest().body(
+            new FacilityResponse(ErrorCodes.INVALID_AUTH_HEADER, "Authorization header is required"));
+    }
 
     OutputResponse response = new OutputResponse();
 
     try {
-        String resp = facilityService.fetchRegisteredFacilities();
-        response.setResponse(resp);
+        FacilityDTO resp = facilityService.fetchRegisteredFacilities();
+        return ResponseEntity.ok(new FacilityResponse(resp));
     } catch (FHIRException e) {
-        response.setError(5000, e.getMessage());
-        logger.error(e.getMessage());
+        logger.error("Failed to fetch registered facilities: {}", e.getMessage(), e);
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .body(new FacilityResponse(ErrorCodes.FACILITY_FETCH_ERROR, e.getMessage()));
     }
-    logger.info("Get ABDM Registered facilities API response" + response.toString());
-    return response.toString();
 }

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



@CrossOrigin
@Operation(summary = "Get ABDM Registered Facilities")
@PostMapping(value = { "/saveAbdmFacilityId" })
public String saveAbdmFacilityForVisit(@RequestHeader(value = "Authorization") String Authorization, @RequestBody() String reqObj) {

OutputResponse response = new OutputResponse();

try {

String resp = facilityService.saveAbdmFacilityId(reqObj);

response.setResponse(resp);
} catch (FHIRException e) {

response.setError(5000, e.getMessage());
logger.error(e.getMessage());
}
logger.info("ABDM save Facility ID response" + response.toString());
return response.toString();
}
Comment on lines +58 to +76
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Swagger documentation and enhance endpoint implementation.

Several issues need attention:

  1. Incorrect Swagger operation summary (copied from GET endpoint)
  2. Missing request body validation
  3. Inconsistent method naming between endpoint and service call
  4. Same structural issues as the GET endpoint

Apply these improvements:

-@Operation(summary = "Get ABDM Registered Facilities")
+@Operation(summary = "Save ABDM Facility ID for Visit")
 @PostMapping(value = { "/saveAbdmFacilityId" })
-public String saveAbdmFacilityForVisit(@RequestHeader(value = "Authorization") String Authorization,
-    @RequestBody() String reqObj) {
+public ResponseEntity<FacilityResponse> saveAbdmFacilityId(
+    @RequestHeader(value = "Authorization") String authorization,
+    @Valid @RequestBody FacilityRequest request) {
+    
+    if (StringUtils.isBlank(authorization)) {
+        return ResponseEntity.badRequest().body(
+            new FacilityResponse(ErrorCodes.INVALID_AUTH_HEADER, "Authorization header is required"));
+    }
 
-    OutputResponse response = new OutputResponse();
     try {
-        String resp = facilityService.saveAbdmFacilityId(reqObj);
-        response.setResponse(resp);
+        FacilityDTO resp = facilityService.saveAbdmFacilityId(request);
+        return ResponseEntity.ok(new FacilityResponse(resp));
     } catch (FHIRException e) {
-        response.setError(5000, e.getMessage());
-        logger.error(e.getMessage());
+        logger.error("Failed to save facility ID: {}", e.getMessage(), e);
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .body(new FacilityResponse(ErrorCodes.FACILITY_SAVE_ERROR, e.getMessage()));
     }
-    logger.info("ABDM save Facility ID response" + response.toString());
-    return response.toString();
 }

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


}
Loading