Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Service;

import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@ public Integer updateBenificiary(BeneficiaryModel benificiaryDetails, String aut
Integer updatedRows = 0;

IdentityEditDTO identityEditDTO = identityBenEditMapper.BenToIdentityEditMapper(benificiaryDetails);
if(null != benificiaryDetails.getI_bendemographics())
if(null != benificiaryDetails.getI_bendemographics()) {
identityEditDTO.setCommunity(benificiaryDetails.getI_bendemographics().getCommunityName());
identityEditDTO.setCommunityName(benificiaryDetails.getI_bendemographics().getCommunityName());
}
Comment on lines +117 to +120
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

Redundant community field assignments.

The code sets both community and communityName fields with the same value. This appears redundant and could lead to maintenance issues.

Consider using only one of these fields consistently:

 if(null != benificiaryDetails.getI_bendemographics()) {
-    identityEditDTO.setCommunity(benificiaryDetails.getI_bendemographics().getCommunityName());
-    identityEditDTO.setCommunityName(benificiaryDetails.getI_bendemographics().getCommunityName());
+    identityEditDTO.setCommunityName(benificiaryDetails.getI_bendemographics().getCommunityName());
 }

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

if (benificiaryDetails.getBeneficiaryIdentities() != null
&& benificiaryDetails.getBeneficiaryIdentities().size() > 0) {
identityEditDTO.setIdentities(Identity.createIdentity(benificiaryDetails.getBeneficiaryIdentities(),
Expand Down Expand Up @@ -143,6 +145,9 @@ public String save(BeneficiaryModel beneficiaryModel, HttpServletRequest servlet
// logger.info("benificiaryDetails: " + beneficiaryModel);

CommonIdentityDTO identityDTO = identityMapper.beneficiaryModelCommonIdentityDTO(beneficiaryModel);
if(null != beneficiaryModel.getI_bendemographics()) {
identityDTO.setCommunity(beneficiaryModel.getI_bendemographics().getCommunityName());
}
// identityDTO.setOtherFields(beneficiaryModel.getOtherFields());
identityDTO.setFaceEmbedding(beneficiaryModel.getFaceEmbedding());
identityDTO.setEmergencyRegistration(beneficiaryModel.isEmergencyRegistration());
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/com/iemr/common/service/sms/SMSServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -801,8 +801,9 @@ private static String getSubstringInRange(String input, int startIndex, int endI
@Override
public void publishSMS() {
RestTemplate restTemplateLogin = new RestTemplate();
if (!SMSServiceImpl.publishingSMS) {
try {

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 12, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential race condition with static flag.

The static publishingSMS boolean flag is not thread-safe, which could lead to race conditions when multiple threads execute this @async method simultaneously.

Consider using an AtomicBoolean instead:

-private static Boolean publishingSMS = false;
+private static final AtomicBoolean publishingSMS = new AtomicBoolean(false);

Then update the flag checks:

-if (!SMSServiceImpl.publishingSMS) {
-  SMSServiceImpl.publishingSMS = true;
+if (SMSServiceImpl.publishingSMS.compareAndSet(false, true)) {

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an existing code. maintaining from long back

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!

try {
if (!SMSServiceImpl.publishingSMS) {
SMSServiceImpl.publishingSMS = true;
Boolean doSendSMS = ConfigProperties.getBoolean("send-sms");
String sendSMSURL = ConfigProperties.getPropertyByName("send-message-url");
Expand Down Expand Up @@ -875,11 +876,11 @@ public void publishSMS() {
sms = smsNotification.save(sms);
}
}
} catch (Exception e) {
logger.error("publishSMS failed with error " + e.getMessage());
} finally {
SMSServiceImpl.publishingSMS = false;
}
} catch (Exception e) {
logger.error("publishSMS failed with error " + e.getMessage());
} finally {
SMSServiceImpl.publishingSMS = false;
}
}

Expand Down
Loading