Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103) #45

josephbate · 2022-11-15T09:00:44Z

Issue in Question

(link:https://issues.openmrs.org/browse/PROV-103?src=confmacro)

getting Errors in deploying the module

i made changes in the omod config.xml however i didnt see the changes when i deployed my omod file in the server, i keted on getting the same error
so used print statements to check if the omad file was picked up but that didn't reflect in my logs

i release there was a worning of my server configuration not existing when i selected the providerSearch option
WARN - OpenmrsUtil.getDirectoryInApplicationDataDirectory(1155) |2022-11-14 18:17:07,548| 'C:\Users\toshiba\openmrs\server2\configuration' doesn't exist. Creating directories now.

i assume in some way the omod file is not detected by the server or there is something i missed
@dkayiwa @mozzy11 @ibacher @gitcliff

…via reflection for openmrs version 2.x.x solve a backward incompatibility change

…od is picked up

josephbate · 2022-11-18T09:51:50Z

#37
everyone
i managed to get the sdk working fine however using conditional resourcing didn't really solve the ticket so i decided to follow @dkayiwa suggestion on talk of using java reflection to load the class
i been trying to find a way to use the this method but could not get how to call the loaded class constructor in the model.addAttribute()

been trying out mostof the methods to see if i can use the object togetInstance()constructor into the model.AddAttribute() but i get method undefined in the type class error

heres the ticket in question PROV-103

please let me know if there's something am missing or any other step i can make to solve this ticket
@mozzy11 @dkayiwa @gitcliff @ibacher

dkayiwa · 2022-11-22T10:48:24Z

@josephbate can you include a link to the JIRA ticket as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

josephbate · 2022-11-24T10:02:20Z

@josephbate can you include a link to the JIRA ticket as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips

okay let me do that quickly

josephbate · 2022-11-25T09:59:49Z

sorry for the delay i had power issues but i have edited the post @dkayiwa

josephbate · 2022-11-28T10:00:28Z

here are my changes so far PROV-103
@dkayiwa @mozzy11

josephbate · 2022-11-30T07:58:56Z

heres the ticket in question PROV-103
i had edited the first comment and included the link

please let me know if you see it

gitcliff · 2022-12-01T12:12:17Z

heres the ticket in question PROV-103 i had edited the first comment and included the link

please let me know if you see it

@josephbate could you add it above in the description like here

mozzy11 · 2023-01-30T16:28:32Z

Thanks @josephbate .
have you tested your changes locally ??
you also need to provide a more meaningful commit message to reflect what you have done

josephbate · 2023-01-31T11:54:41Z

hello everyone
@mozzy11;
yes i tested it and i kept on geting them same errors and veiw exception
there are two method in the NameSupport class that's getInstance() and
getDefaultLayoutFormat()

My finding currently working on #37

now i understand i must somehow replicate the NameSupport.getInstance().getDefaultLayoutTemplate() syntax with the class i loaded via reflection but in my effort at the moment its close to impossible, i keep on getting illegalLanguage Exceptions and syntax errors
invoking the methods via reflection stills does no good
there is one approach i also tried out, of using the java new instance method to achieve the above call, below the are code snippets

`try {
Class<?> cls = Class.forName("org.openmrs.layout.name.NameSupport");

		Object obj = cls.newInstance();

                    NameSupport ns;
                    if (obj instanceof NameSupport){
                              ns = (NameSupport)obj;
                    } else {
                              return
                    }

		System.out.println(
				"8###################******************does this work******************######################## "
						+ ns.getInstance().getDefaultLayoutTemplate());

		model.addAttribute("layoutTemplate", ns.getInstance().getDefaultLayoutTemplate());
	} catch (Throwable e) {
		e.printStackTrace();
	}
	
}`

the problem with this approach without using the imported NameSuport class it cannot work, so that means the new version of that class when loaded cannot override the imported because casting the class in the new stance object makes it override it hence giving the same exception

so with that i am asking for assistance on this @dkayiwa @mozzy11
please let me know if there is something i missed or maybe i didn't understance because i am running out of ideas

ibacher · 2023-01-31T12:20:16Z

It would probably be better to do this with MethodHandles, but (without any error handling):

Class<?> cls = Class.forName("org.openmrs.layout.name.NameSupport");
Method getInstance = cls.getMethod("getInstance");
Object instance = getInstance.invoke(null);
Method getDefaultLayoutTemplate = cls.getDeclaredMethod("getDefaultLayoutTemplate");
Object layoutTemplate = getDefaultLayoutTemplate.invoke(instance);

josephbate · 2023-01-31T13:16:33Z

It would probably be better to do this with MethodHandles, but (without any error handling):

Class<?> cls = Class.forName("org.openmrs.layout.name.NameSupport");
Method getInstance = cls.getMethod("getInstance");
Object instance = getInstance.invoke(null);
Method getDefaultLayoutTemplate = cls.getDeclaredMethod("getDefaultLayoutTemplate");
Object layoutTemplate = getDefaultLayoutTemplate.invoke(instance);

thank you @ibacher
oooh, it being a static method it invokes no arguments
i really didnt think of that plus i didnt know that "getDefaultLayoutTemplate" could be called by the same class object
"Its an inherited method", my word!
this is an eye opener
let me try it out

josephbate · 2023-02-01T05:50:05Z

hello everyone
i finally solved the bug,

My finding as of #37

the code snippet @ibacher suggected was an eye opener, however using it made get noSuchMethod exception whilst trying to invoke getDefaultLayoutTemplate()
to solve it i also loaded the org.openmrs.layout.LayoutSupport so as to access and when i ran the sdk and test the module the error disapeared

thank you so much guys @dkayiwa @gitcliff @mozzy11 @ibacher

my issue is ready for review

mozzy11 · 2023-02-06T12:55:53Z

.../org/openmrs/module/providermanagement/fragment/controller/PersonNameFragmentController.java


+	public void controller(FragmentModel model) throws ClassNotFoundException, NoSuchMethodException, SecurityException, IllegalAccessException, IllegalArgumentException, InvocationTargetException {
+
+			Class<?> cls1 = Class.forName("org.openmrs.layout.LayoutSupport");


Thanks @josephbate .
can you make this backward compatible , this should only run conditionally in the higher OpenMRS version where we have this change in the class location ,or else we should directly import the class in the older versions of OpenMRS

mozzy11 · 2023-02-06T13:00:26Z

.../org/openmrs/module/providermanagement/fragment/controller/PersonNameFragmentController.java

@@ -14,13 +14,35 @@

 package org.openmrs.module.providermanagement.fragment.controller;

-import org.openmrs.layout.web.name.NameSupport;
+import java.lang.reflect.*;


see our java convetions for imports
https://wiki.openmrs.org/display/docs/Java+Conventions#JavaConventions-Imports

api/src/main/java/org/openmrs/module/providermanagement/ReflectString.java

mozzy11 · 2023-02-06T13:02:32Z

@josephbate you need to properly configure your IDE to avoid any unnecessary formatting
see https://wiki.openmrs.org/display/docs/Developer+How-To+Setup+And+Use+Eclipse#DeveloperHowToSetupAndUseEclipse-SetupCodeStyleAndFormatForOpenMRS

mozzy11 · 2023-02-08T16:54:10Z

@josephbate , you can use this constant to check the running Openmrs version
an example of where its used .

ezef · 2023-10-13T14:22:46Z

Hi Everyone, there is something that is holding this PR to be merged?

ibacher · 2023-10-14T19:32:50Z

.../org/openmrs/module/providermanagement/fragment/controller/PersonNameFragmentController.java

-    }
-
+
+	/**


If we're going to include Javadocs, they should actually provide some information.

ibacher · 2023-10-14T19:33:08Z

.../org/openmrs/module/providermanagement/fragment/controller/PersonNameFragmentController.java

+	 * @throws IllegalAccessException
+	 */
+
+	public void controller(FragmentModel model) throws ClassNotFoundException, NoSuchMethodException, SecurityException,


I think at this point, its just easier to say throws Exception, no?

Also, is there nothing we can do here to recover from these exceptions?

do you have any suggestions @ibacher

ibacher · 2023-10-14T19:33:44Z

.../org/openmrs/module/providermanagement/fragment/controller/PersonNameFragmentController.java

+		/*
+		 * backward compatibility
+		 */
+		Class<?> nameSurpport;


Suggested change

Class<?> nameSurpport;

Class<?> nameSupport;

hello @ibacher thank you for pointing them out let me try to solve them

josephbate · 2023-10-15T15:04:09Z

Hello @ibacher i think i i have resolved some of the problems you pointed out

dkayiwa · 2023-11-01T23:12:01Z

api/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>org.openmrs.module</groupId>
 		<artifactId>providermanagement</artifactId>
-		<version>2.14.0-SNAPSHOT</version>
+		<version>2.15.0-SNAPSHOT</version>


I would not expect version changes to be part of this pull request.

I would not expect version changes to be part of this pull request.

it think the verion i forked is base on 2.15.0- snapshot

Check what the current upstream version is.

dkayiwa · 2023-11-01T23:14:45Z

.../org/openmrs/module/providermanagement/fragment/controller/PersonNameFragmentController.java

+
+       /*
+        * backward compatibility
+        * dynamic class loading and reflection to interact with classes that provide layout templates for perso names in the system, allowing for flexibility and extensibility in managing name layouts.


I do not see value added by this comment.

hello @dkayiwa i have made some changes please find them in the commit below

dkayiwa · 2023-11-02T09:31:37Z

@josephbate did you address my comment about the version change?

josephbate · 2023-11-02T11:11:13Z

@josephbate did you address my comment about the version change?

done! @dkayiwa

dkayiwa · 2023-11-02T11:15:22Z

Can you fix the build errors?

dkayiwa · 2023-11-02T11:15:48Z

pom.xml

@@ -4,7 +4,7 @@

 	<groupId>org.openmrs.module</groupId>
 	<artifactId>providermanagement</artifactId>
-	<version>2.14.0-SNAPSHOT</version>
+	<version>2.15.0-SNAPSHOT</version>


What is this version change for?

dkayiwa · 2023-11-02T11:15:56Z

omod/pom.xml

@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>org.openmrs.module</groupId>
 		<artifactId>providermanagement</artifactId>
-		<version>2.14.0-SNAPSHOT</version>


What is this version change for?

#45 (comment)

i was reacting to this comment, and was restoring the prevoius version before my commits
intially i had som build failures thats why i made switch

josephbate · 2023-11-02T11:32:01Z

i think everything works fine!
@dkayiwa

dkayiwa · 2023-11-02T12:05:58Z

Can you look at these build failures?

dkayiwa · 2023-11-02T12:23:12Z

And also fix the build failures.

…45)" This reverts commit 8ee4396.

…45)" (#46) This reverts commit 8ee4396.

josephbate added 3 commits November 3, 2022 18:36

PROV-103: added a conditional resource to load the NameSupport class …

3085d83

…via reflection for openmrs version 2.x.x solve a backward incompatibility change

more changesin the ProviderManagementActivator to test whether the om…

ca18092

…od is picked up

using java reflection to load the class

76e8d3a

openmrs-bot added 2 commits November 21, 2022 19:16

[maven-release-plugin] prepare release 2.14.0

ed24628

[maven-release-plugin] prepare for next development iteration

a0b912a

josephbate closed this Nov 28, 2022

josephbate reopened this Nov 28, 2022

my third commit

151358d

josephbate changed the title ~~Prov 103~~ Prov 103:https://issues.openmrs.org/browse/PROV-103 Jan 30, 2023

josephbate changed the title ~~Prov 103:https://issues.openmrs.org/browse/PROV-103~~ Prov 103: https://issues.openmrs.org/browse/PROV-103 Jan 30, 2023

josephbate changed the title ~~Prov 103: https://issues.openmrs.org/browse/PROV-103~~ Prov 103 #https://issues.openmrs.org/browse/PROV-103 Jan 30, 2023

josephbate changed the title ~~Prov 103 #https://issues.openmrs.org/browse/PROV-103~~ Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103) Jan 31, 2023

my final commit

28113a3

mozzy11 reviewed Feb 6, 2023

View reviewed changes

api/src/main/java/org/openmrs/module/providermanagement/ReflectString.java Outdated Show resolved Hide resolved

final commit

79d7459

gracepotma requested review from ibacher and dkayiwa October 13, 2023 17:49

ibacher requested changes Oct 14, 2023

View reviewed changes

josephbate added 3 commits October 15, 2023 16:07

edited commit

d4e05db

Merge remote-tracking branch 'origin/PROV-103' into PROV-103

88dae7a

cleaning up the code

ddef0be

improving on java docs

4e20d3a

dkayiwa reviewed Nov 1, 2023

View reviewed changes

editing java docs give more info

b5e83a5

Correcting version

14d33cc

dkayiwa reviewed Nov 2, 2023

View reviewed changes

josephbate added 2 commits November 2, 2023 14:24

to solves errors

645eebe

clean version

c2d83c9

version for omod pom.xml

fb0b080

cleaning

746194f

dkayiwa merged commit 8ee4396 into openmrs:master Nov 8, 2023
1 check passed

dkayiwa added a commit that referenced this pull request Nov 8, 2023

Revert "Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103) (#…

452f7ab

…45)" This reverts commit 8ee4396.

dkayiwa mentioned this pull request Nov 8, 2023

Revert "Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103)" #46

Merged

dkayiwa added a commit that referenced this pull request Nov 8, 2023

Revert "Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103) (#…

c5f84fd

…45)" (#46) This reverts commit 8ee4396.


		public void controller(FragmentModel model) throws ClassNotFoundException, NoSuchMethodException, SecurityException, IllegalAccessException, IllegalArgumentException, InvocationTargetException {

		Class<?> cls1 = Class.forName("org.openmrs.layout.LayoutSupport");

Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103) #45

Prov 103 [issue](https://issues.openmrs.org/browse/PROV-103) #45

Conversation

josephbate commented Nov 15, 2022 • edited Loading

Issue in Question

getting Errors in deploying the module

josephbate commented Nov 18, 2022 • edited Loading

dkayiwa commented Nov 22, 2022

josephbate commented Nov 24, 2022

josephbate commented Nov 25, 2022

josephbate commented Nov 28, 2022

josephbate commented Nov 30, 2022

gitcliff commented Dec 1, 2022

mozzy11 commented Jan 30, 2023

josephbate commented Jan 31, 2023 • edited Loading

**My finding currently working on #37 **

ibacher commented Jan 31, 2023

josephbate commented Jan 31, 2023

josephbate commented Feb 1, 2023

My finding as of #37

mozzy11 Feb 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mozzy11 commented Feb 6, 2023

mozzy11 commented Feb 8, 2023

ezef commented Oct 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephbate commented Oct 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkayiwa commented Nov 2, 2023

josephbate commented Nov 2, 2023

dkayiwa commented Nov 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephbate commented Nov 2, 2023

dkayiwa commented Nov 2, 2023

dkayiwa commented Nov 2, 2023

josephbate commented Nov 15, 2022 •

edited

Loading

josephbate commented Nov 18, 2022 •

edited

Loading

josephbate commented Jan 31, 2023 •

edited

Loading

My finding currently working on #37

mozzy11 Feb 6, 2023 •

edited

Loading