GSoC 2018 - OAuth Module Enhancements And SMART Apps Support Project

Tags: #<Tag:0x00007f1f8044d800>

Hello everyone!

I am starting this thread to discuss all the project related stuff.

Everyone please free to help and give suggestions!

Thank you! :slight_smile:

Great @pkatopenmrs you can post all the project updates and queries here that way you’ll have a greater audience. Also, I highly suggest that you have OAuth module up and running on your local machine.

Sure @mavrk!

Can you please post a link of working module here?

Refer to this documentation here to check all the endpoints

Once you are done with checking the endpoints, you can check the functionality of a SMART app from here

Hey @mavrk I just wrote a test for metadata controller and i am getting an error. Can you please help me with this : ?

Why there is a error in autowiring the beans?

Can you please give a link to your commit as well. I want to see the changes you’ve made in the code.

Also as you can see in line 98, it’s caused due to failed autowiring of ClientDAO. The metadata controller is independent of ClientDAO. Look out for this issue and why it arose for a starting point.

here is my commit :

please look into it :slight_smile:

Our weekly meeting notes

Hey @mavrk here is my plan for implementing the EHR launch flow. Please go through it and tomorrow maybe on a call we can discuss about what changes are needed. :slight_smile:

1.Add the tables “oauth2_smart_app” and “oauth2_smart_app_launch” as decided in the proposal phase.

2.Make a list of required forms/pages.

  • We need to show the required functions on the “Administration page”.

  • The “Manage SMART apps page” under “oauth2/manageSmartApps”.This page shows the registered smart apps list also an option to add new smart app.

  • “Register SMART app” page under “oauth2/manageSmartApps/register”

  • “Run SMART app” page shows the registered smart apps and option to run them.

3.Write controllers for the above pages. The following controllers are required with the following mappings :

  • We already have the controller for the “Administrator page”, so we will use that to show the “Administrator page” contents.

  • A “manageSmartAppController” under the mapping “oauth2/manageSmartApps” with GET mapping to show the “Manage SMART apps” page. Also when the user clicks “Remove” button, the respective client must be deleted from the db and the page must refresh. When the user clicks “Add New SMART Apps” link then redirect to the “oauth2/manageSmartApps/register” mapping.

  • A “registerSmartAppController” under the mapping “oauth2/manageSmartApps/register” with GET mapping to show the registration form and POST mapping whenever the user clicks “Register” button. The POST mapping function will generate a new oauth client and save it in db. I can take help from the clientRegistrationFormController for the same.

  • A “runSmartAppController” under the mapping “oauth2/runSmartApps” with GET mapping to show the available registered SMART apps. Also when the user clicks “Run” then we must first creat a launch code that is the “launch” parameter and save in db. Then redirect to the URL of the format “https://{app_launch_url}?iss=https://localhost:8080/openmrs/ws/fhir&launch=xyz123”. Further the SMART app will run as usual.

Regards Prabodh

Hi @pkatopenmrs sounds good for now. Let’s discuss more about this over a call tomorrow. Also, try to make a DB schema before the call, that way it’ll be more clear what and how we are going to store all the meta, stats, etc.

Good work with OA-8 OA-9 and OA-10

A little suggestion for you, try to be more general with your tickets from next time, for eg. there was no need to make two separate ticket for MVC Classes. For next issues be more general “create dao classes for api layer”, “setup implementation classes and interfaces for api layer” etc. This way it’d be much easier to see work on a larger scale.

Keep up the good work though :smile:

1 Like

Thanks @mavrk. Yep i will make it general. :slight_smile:

@mavrk would it be better if we put everything related to smart apps under a package api/smart? i.e. api/smart/model… api/smart/db… api/smart/impl… ?? because this is better than making a smart package in every directory :slight_smile:

Yup, we can do it that way:slightly_smiling_face:

ok :slight_smile:

Hey @mavrk yesterday when i was adding the method loadSmartAppsForClientDeveloper(), I needed to autowire the ClientDAO bean. But when I was autowiring the bean in SmartAppManagementServiceImpl, the ClientDAO bean in ClientRegisterationServiceImpl was not able to initialized properly and was throwing a NullPointerException. Then I explicitly gave the Bean Configuration for the bean in SmartAppManagementServiceImpl, look here , and the problem was solved. Now I am not getting any errors and everything seems to be working fine. But I still dont understand why the @Autowired annotation was failing in ClientRegistrationServiceImpl when I was Autowiring the bean in SmartAppManagementServiceImpl.

I searched on google, and all the answers were revolving around defining properly packages-to-scan and not to initialize bean with “new”. Can you please have a look and tell me?

Hi @pkatopenmrs, if you read Spring documentation on @Autowire you’ll know what was happening. Multiple bean definitions within the container may match the type specified by the setter method or constructor argument to be autowired (in this case, both SmartAppManagementServiceImpl and ClientRegistraionServiceImpl have the same definition i.e the same constructor for ClientDAO). If no unique bean definition is available, an exception is thrown. This was the reason why you were getting a NullPointerException . In your fix, what you have done is explicitly make definitions in the XML file and define a constructor in your class which resolves the problem. By default, all explicit definitions will override any @Autowire annotation for that class.

This has to do with Spring created to be as unambiguous as possible. That’s why Spring played safe there and for the same bean definitions in our container, @Autowire threw an exception.