Skip to content

AOM-175: Add validation checks in getProviderForUser method#245

Open
aadvik93 wants to merge 2 commits intoopenmrs:masterfrom
aadvik93:AOM-175
Open

AOM-175: Add validation checks in getProviderForUser method#245
aadvik93 wants to merge 2 commits intoopenmrs:masterfrom
aadvik93:AOM-175

Conversation

@aadvik93
Copy link

@aadvik93 aadvik93 commented Mar 7, 2026

Description

This PR adds defensive validation checks in the getProviderForUser method in LegacyUIImpl.java.

Changes

  • Added null check for user
  • Added validation for ProviderService availability
  • Added handling when no providers exist for the user

Related Issue

Fixes AOM-175

@sudhanshu-raj
Copy link

Do we really need the ps == null check here? In normal OpenMRS runtime scenarios, Context.getProviderService() is very unlikely to return null. If there is an issue such as the Context not being initialized or authenticated, or a problem with Spring wiring, it would typically fail earlier or throw an exception rather than returning a null service instance. Because of that, I am not sure this check provides much practical value unless there is a specific case where getProviderService() could return null, for example in certain test setups.

Also, since we are trying to make this method more robust, should we consider adding a defensive check for user.getPerson() == null as well? Otherwise there is still a possibility of a NPE when calling ps.getProvidersByPerson(user.getPerson(), true).

@aadvik93
Copy link
Author

aadvik93 commented Mar 8, 2026

Do we really need the ps == null check here? In normal OpenMRS runtime scenarios, Context.getProviderService() is very unlikely to return null. If there is an issue such as the Context not being initialized or authenticated, or a problem with Spring wiring, it would typically fail earlier or throw an exception rather than returning a null service instance. Because of that, I am not sure this check provides much practical value unless there is a specific case where getProviderService() could return null, for example in certain test setups.

Also, since we are trying to make this method more robust, should we consider adding a defensive check for user.getPerson() == null as well? Otherwise there is still a possibility of a NPE when calling ps.getProvidersByPerson(user.getPerson(), true).

@sudhanshu-raj,
Thanks for the review and the explanation!

You’re right that Context.getProviderService() should normally not return null in a properly initialized OpenMRS runtime, so the defensive check there didn’t add much value.

I’ve removed that check and added a validation for user.getPerson() == null to avoid a potential NPE when calling getProvidersByPerson().

This has been updated in the latest commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants