Could not fetch the UserContext for a getPatientByUuid() Call from EMRAPI module

Hi,

When I view a patient through the patient dashboard, the EMRAPI module add that patient to the lastViewedPatients list which will be listed as RECENT in the findPatients dashboard.

The EMRAPI modules call the getPatientByUuid() method to get that patient information before adding that to lastViewedPatientsList. See here [1]

LocationBasedAccessControl Module: I’m working on this module to restrict the patients by the locations.

  • At very first Location based access control module will check the UserContext Location to get the logged in user location. See here[2]
  • If the UserContext Location is null, it will replace the actual patient object by a null object to restrict the patient from the user view - See here[3]

Actual Error Point: I clicked a patient from the findPatient dashboard to view the patient information to the dashboard. When I started to view the patient information, the EMRAPI module tried to add that patient into the lastViewedPatient list. At this point, I’m getting this error.


Here is the log : https://pastebin.com/GLmbi9YE

Actually the page called the getPatientByUuid() method for multiple times before showing all the information about the patients in the patient dashboard (for multiple reasons). At one point, It got an exception to validate the user context in the line - 37 (see the log). You can see all the getPatientByUuid() method is validated with the UserContext(you can see Location, administrative user, and roles). But for a call in Line-37, System Could not find the Usercontext information (still user context is not null, but the location and the administrative user is null, and role is Anonymous). Actually, this getPatientByUuid() method was called from that EMRAPI module.

When the location in the UserContext is null, Our module skips the checking (Since couldn’t find the user logged in session information) and return the null object instead of an actual patient object. So that the request method(In EMRAPI) could not find the patient since it is null. So It couldn’t add this patient to the last viewed patient list and throws the exception.

ERROR - PatientViewedEventListener$1.run(66) |2018-07-11 18:48:51,641| Failed to update the user's last viewed patients property
org.openmrs.api.APIException: failed to find a patient with uuid:5342904c-104c-4463-98e3-d60027792113 or the patient is not yet saved
    at org.openmrs.module.emrapi.event.PatientViewedEventListener.processMessage(PatientViewedEventListener.java:86)
    at org.openmrs.module.emrapi.event.PatientViewedEventListener$1.run(PatientViewedEventListener.java:63)
    at org.openmrs.api.context.Daemon$5.run(Daemon.java:286)

Could someone help me to find out the actual error on this part?

CC : @dkayiwa @wyclif @darius

See here : https://github.com/openmrs/openmrs-module-emrapi/blob/master/api/src/test/java/org/openmrs/module/emrapi/test/AuthenticatedUserTestHelper.java

EMRAPI module uses the AuthenticatedUserTestHelper to initialize the mockUserContext for the tests. So I think, the method calls from the EMRAPI module doesn’t contain a default autowiring with UserContext.

Any ideas from here?

CC : @dkayiwa @wyclif @darius

The getPatientByUuid() method calls from the EMRAPI module through an daemon thread. Is this a problem since it will differ from the actual user context?

Do you have a pull request of your changes to reproduce this problem?

Please have a look into this PR : https://github.com/openmrs/openmrs-module-locationbasedaccess/pull/12

Just added some Error based log lines. You need the updated appUI module including this commit to check this.

From the logic you pointed to, it’s clear that getPatientByUuid would return null if the authenticated user has no location defined on the user context object, you might to find out why locationId is null, can you check that the authenticated user has a location user property value defined for them?

Actually that UserContext at this point(When call from EMRAPI - Line 30) doesn’t contain the Authenticated User property(found as null). But for other objects which call the getPatientByUuid have the UserContext with all properties. (Eg : Line 2, 16 …)

See the log : https://pastebin.com/GLmbi9YE

Could you be dealing with a chicken egg dilemma here?

Yes :smile: But I don’t know what is happening at the back-end at that point only for this call.

You need to do some debugging, put a break point to find out when it gets called before the user context is setup.

Yah, I got it from remote debugging.

Actually when we call the userContext, it checks for the Daemon thread or not before sending the object. Above getPatientByUuid() call was released from a Daemon thread (PatientViewedEventListener Class). So it will not contains the properties of the active user.

public static User getAuthenticatedUser() {
        return Daemon.isDaemonThread() ? Daemon.getDaemonThreadUser() : getUserContext().getAuthenticatedUser();
    }

See here, System is assigning the different user for the Daemon thread.

public static User getDaemonThreadUser() {
    if (isDaemonThread()) {
        User user = (User)daemonThreadUser.get();
        if (user == null) {
            user = Context.getContextDAO().getUserByUuid("A4F30A1B-5EB9-11DF-A648-37A07F9C90FB");
            daemonThreadUser.set(user);
        }

        return user;
    } else {
        return null;
    }
}

But I’m dealing with the logged in AuthenticatedUser property. This is the issue which caused to get the AuthenticatedUser as null at that point.

I think I reached the solution for my issue :smile:

Since that respected method from EMRAPI module called from a Daemon thread,

  • Identified as a separate Daemon thread user which is created from here - See here[1] and here[2]
  • System differs the actual logged in user from this Daemon thread user.
  • Since we only deal with the logged in user in the AppUI module to set the location in the UserContext, We can’t expect that property for a Daemon thread user. Since the Daemon thread user is common for all locations (common for the system). So we can’t add any restrictions for the Daemon thread user.

So we need to add an exception for the Daemon thread user to skip the location restrictions. We only have a UUID value for the Daemon thread user (See here).

/**
* The uuid defined for the daemon user object
*/
protected static final String DAEMON_USER_UUID = "A4F30A1B-5EB9-11DF-A648-37A07F9C90FB";

Finally, we need to make these following changes in Location Based Access Control module,

  • Get the UUID of the authenticated user at the beginning of the method invocation using Context.getAuthenticatedUser().getUuid().
  • If the user uuid is equal to the Daemon thread user, then skip the location checking
  • If the user uuid differs from the Daemon thread user, then continue the location checking (for normal users)

CC : @dkayiwa

I have added an exception for the Daemon user while checking the userContext properties. This commit contains the required code to exclude the Daemon thread user safely from the userContext checking.

Thanks everyone for the kind support :slight_smile:

I got similar error in the past OpenMRS login page and the fix was to set default location by clicking on the My Profile on the legacy OpenMRS UI. But obvously this need to be set automatically during installation.

What happens if there is no authenticated user, won’t you get a NPE when you call Daemon.isDaemonUser()?

Actually, there are two methods we can get the AuthenticatedUser.

  1. Context.getAuthenticateUser()
  2. Context.getUserContext().getAuthenticatedUser()

For the Deamon thread, the Context.getAuthenticateUser() will return the Daemon user(predefined user). But Context.getUserContext.getAuthenticatedUser() return the null object.

I think what he means is that there will be a NPE when Context.getAuthenticatedUser() returns null.

When Context.getAuthenticatedUser() returns null, then I will get an NPE. But it can happen when the logged in user is null not for the Daemon thread user. There will be no chances to get the Daemon thread user as a null object.

See here,

public static User getAuthenticatedUser() {
        return Daemon.isDaemonThread() ? Daemon.getDaemonThreadUser() : getUserContext().getAuthenticatedUser();
    }

And

public static User getDaemonThreadUser() {
    if (isDaemonThread()) {
        User user = (User)daemonThreadUser.get();
        if (user == null) {
            user = Context.getContextDAO().getUserByUuid("A4F30A1B-5EB9-11DF-A648-37A07F9C90FB");
            daemonThreadUser.set(user);
        }

        return user;
    } else {
        return null;
    }
}

If it is Daemon thread, then there will be no chances to get the null object for the Context.AuthenticatedUser() since it checks before returning the Daemon thread user (it will create a new user if this Daemon thread user is null). So I will not get the NPE anymore for the Daemon thread.

But we need to think about the normal user. Better to add a null check before checking this.

CC : @wyclif @dkayiwa

Let’s move the code discussion to the commit, I’ve added a comment