RESTWS-958: Upgrade swagger to 2.2.23 #631

mherman22 · 2024-10-14T09:24:27Z

Description of what I changed

Issue I worked on

see https://issues.openmrs.org/browse/RESTWS-958

Checklist: I completed these to help reviewers :)

My IDE is configured to follow the code style of this project.

No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)

No? -> write tests and add them to this commit git add . && git commit --amend
I ran mvn clean package right before creating this pull request and
added all formatting changes to my commit.

No? -> execute above command
All new and existing tests passed.

No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

mherman22 · 2024-10-18T05:08:34Z

omod-2.2/pom.xml

@@ -19,6 +19,28 @@
 			<groupId>${project.parent.groupId}</groupId>
 			<artifactId>${project.parent.artifactId}-omod-common</artifactId>
 			<version>${project.parent.version}</version>
+			<exclusions>
+				<exclusion>
+					<groupId>com.fasterxml.jackson.core</groupId>


hacky but i needed to add these exclusions because the jackson related dependencies coming from swagger-core 2.2.23 are quite high and causing conflicts with the related jackson stuff from spring

mherman22 · 2024-10-18T05:09:06Z

omod-2.3/pom.xml

+            <exclusions>
+                <exclusion>
+                    <groupId>com.fasterxml.jackson.core</groupId>
+                    <artifactId>jackson-annotations</artifactId>


mherman22 · 2024-10-18T05:09:23Z

omod-2.4/pom.xml

@@ -21,6 +21,28 @@
            <groupId>${project.parent.groupId}</groupId>
            <artifactId>${project.parent.artifactId}-omod-common</artifactId>
            <version>${project.parent.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>com.fasterxml.jackson.core</groupId>


mherman22 · 2024-10-18T05:09:40Z

omod-2.5/pom.xml

@@ -23,6 +23,28 @@
            <groupId>${project.parent.groupId}</groupId>
            <artifactId>${project.parent.artifactId}-omod-common</artifactId>
            <version>${project.parent.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>com.fasterxml.jackson.core</groupId>


ibacher · 2024-10-18T13:10:18Z

@mherman22 Rather than doing the exclusions in every POM, can we add the exclusion to the omod-common declaration of Swagger? That should remove it from everywhere...

mherman22 · 2024-10-18T13:42:38Z

@mherman22 Rather than doing the exclusions in every POM, can we add the exclusion to the omod-common declaration of Swagger? That should remove it from everywhere...

@ibacher i tried that before this approach however this bean here ->

openmrs-module-webservices.rest/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/DynamicBeanConfiguration.java

Line 39 in 61a27fa

@Bean(name = "jsonHttpMessageConverter")

causes a org.springframework.beans.factory.BeanDefinitionStoreException with something as

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'jsonHttpMessageConverter' defined in class path resource [org/openmrs/module/webservices/rest/web/DynamicBeanConfiguration.class]: Instantiation of bean failed; nested exception is org.springframework.beans.factory.BeanDefinitionStoreException: Factory method [public org.springframework.http.converter.HttpMessageConverter org.openmrs.module.webservices.rest.web.DynamicBeanConfiguration.getMappingJacksonHttpMessageConverter() throws java.lang.Exception] threw exception; nested exception is java.lang.NoClassDefFoundError: com/fasterxml/jackson/core/JsonProcessingException

for more context -> https://paste.ubuntu.com/p/tJ6mZxnKJJ/

ibacher · 2024-10-18T17:21:43Z

I see what you mean... Have you tried running this version in an SDK instance?

mherman22 · 2024-10-18T17:27:58Z

I see what you mean... Have you tried running this version in an SDK instance?

Version you mean the omod built with the changes of this PR?

If so, then no. I have an instance I setup using docker but then openmrs is failing on startup for example the queue module's 1xlegacyController class is the cause and its because i think I need to now go into each of those modules and update them to take up stuff in this PR.

However I can not do before we have this in.

Or is there a way I can actually test this on modules without having to first get this in?

ibacher · 2024-10-18T17:29:46Z

Technically, if you run mvn clean install and then update the module to depend on the SNAPSHOT version, you can test this all locally.

ibacher · 2024-10-18T17:30:59Z

install adds it to your local Maven repo copy and, at least until the next PR gets merged, that should be newer than the SNAPSHOT we've published (and you can always re-do the install if that happens), so Maven will pick-up your local copy as the "latest" SNAPSHOT.

mherman22 · 2024-10-18T17:46:30Z

install adds it to your local Maven repo copy and, at least until the next PR gets merged, that should be newer than the SNAPSHOT we've published (and you can always re-do the install if that happens), so Maven will pick-up your local copy as the "latest" SNAPSHOT.

Interesting, thanks. If I can test this locally then that's great. I will update the PR as and when I have tested thoroughly.

mherman22 · 2024-10-29T11:33:49Z

omod-common/src/main/java/org/openmrs/module/webservices/docs/swagger/SwaggerConstants.java

pulled this out of RestConstants because for soe reason, initialization of RestConstants was failing.

mherman22 · 2024-10-29T11:36:55Z

...n/src/main/java/org/openmrs/module/webservices/docs/swagger/SwaggerSpecificationCreator.java

+
+		openAPI = new OpenAPI();
+		BuildJSON();
+		cachedJson = createJSON();  // Cache the JSON string


The Spec was being reset when accessing cached data, causing
empty paths in the API documentation on subsequent requests. This occurred
because initOpenAPI() was creating a new empty Paths object while handling
cached responses.

mherman22 · 2024-10-29T11:51:09Z

these are the current visuals. I am lookng into the failing test

ibacher · 2024-11-14T04:21:04Z

omod-common/pom.xml

@@ -31,9 +31,9 @@
 		</dependency>

 		<dependency>
-			<groupId>io.swagger</groupId>
+			<groupId>io.swagger.core.v3</groupId>


Quick thing: since the versions are already defined in the root POM (in dependency management), they don't need to be specified here. That way we only need to update versions in a single place (dependency management doesn't make something a dependency, but it's configuration applies to a dependency with the same coordinates.

For the same reasons, we shouldn't need to replicate the exclusions here.

…er 2.2.23

…eModule cannot be cast to com.fasterxml.jackson.databind.Module

…alongside openmrs-core

dkayiwa · 2024-11-21T21:22:08Z

@mherman22 as we discussed on this week's platform call, did you get some time to check if swagger has an option that saves us from the burden of having to manually maintain resource documentation?

mherman22 · 2024-11-21T21:41:10Z

@mherman22 as we discussed on this week's platform call, did you get some time to check if swagger has an option that saves us from the burden of having to manually maintain resource documentation?

@dkayiwa I am still looking into that. But so far what i have landed on from the various docs that i have read indicate OpenAPI 3.0 itself may not provide a mechanism to automatically generate schema definitions from code. But i am looking into Springdoc trying to see if it works for us but it looks to be more of a springboot project.

Others that i have looked at are poorly maintained and it's been long (4 years) since the last commit.

I will take any suggestions before i read all books on the internet 😃

mherman22 · 2024-11-21T21:49:39Z

add springfox to that list.

…ss/interface

…enerate swagger

mherman22 force-pushed the RESTWS-958 branch 2 times, most recently from e47142d to 672ccc1 Compare October 15, 2024 19:02

mherman22 marked this pull request as ready for review October 17, 2024 20:50

mherman22 requested a review from ibacher October 17, 2024 20:52

mherman22 commented Oct 18, 2024

View reviewed changes

mherman22 commented Oct 29, 2024

View reviewed changes

mherman22 requested a review from dkayiwa October 30, 2024 07:33

ibacher reviewed Nov 14, 2024

View reviewed changes

mherman22 and others added 13 commits November 14, 2024 09:43

RESTWS-958: Ensure the REST Module Api Docs are upgraded to use Swagg…

0601289

…er 2.2.23

fix some tests

8c9c6a1

fix the jackson dependency conflict

80c8c89

debug

25ea287

fix all tests

9036374

Fix ClassCastException: com.fasterxml.jackson.datatype.jsr310.JavaTim…

eeaef69

…eModule cannot be cast to com.fasterxml.jackson.databind.Module

Add missing closing tag

71ec69e

Revert exclusion of jackson-datatype-jsr310

b44d856

Fix ClassCastException: com.fasterxml.jackson.datatype.jsr310.JavaTim…

d56c9b0

…eModule cannot be cast to com.fasterxml.jackson.databind.Module

Ensure proper caching of the documentation and ensure the spec works …

2ac8c28

…alongside openmrs-core

fix swagger specification test

599e10c

uuid should be a uuidschema

a61de43

organize dependencies

c59f9e0

mherman22 force-pushed the RESTWS-958 branch from 9b0b984 to c59f9e0 Compare November 14, 2024 08:48

mherman22 requested a review from ibacher November 18, 2024 07:19

mherman22 added 2 commits November 28, 2024 15:45

scan through the class hierarchy to identifier parameterized supercla…

74c740f

…ss/interface

remove the use of getGETSchema, getUPDATESchema, getCREATESchema to g…

95cb367

…enerate swagger

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RESTWS-958: Upgrade swagger to 2.2.23 #631

RESTWS-958: Upgrade swagger to 2.2.23 #631

mherman22 commented Oct 14, 2024 •

edited

Loading

mherman22 Oct 18, 2024 •

edited

Loading

mherman22 Oct 18, 2024

mherman22 Oct 18, 2024

mherman22 Oct 18, 2024

ibacher commented Oct 18, 2024

mherman22 commented Oct 18, 2024 •

edited

Loading

ibacher commented Oct 18, 2024

mherman22 commented Oct 18, 2024

ibacher commented Oct 18, 2024

ibacher commented Oct 18, 2024

mherman22 commented Oct 18, 2024

mherman22 Oct 29, 2024

mherman22 Oct 29, 2024

mherman22 commented Oct 29, 2024

ibacher Nov 14, 2024

mherman22 Nov 14, 2024

dkayiwa commented Nov 21, 2024

mherman22 commented Nov 21, 2024

mherman22 commented Nov 21, 2024

RESTWS-958: Upgrade swagger to 2.2.23 #631

Are you sure you want to change the base?

RESTWS-958: Upgrade swagger to 2.2.23 #631

Conversation

mherman22 commented Oct 14, 2024 • edited Loading

Description of what I changed

Issue I worked on

Checklist: I completed these to help reviewers :)

mherman22 Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

mherman22 Oct 18, 2024

Choose a reason for hiding this comment

mherman22 Oct 18, 2024

Choose a reason for hiding this comment

mherman22 Oct 18, 2024

Choose a reason for hiding this comment

ibacher commented Oct 18, 2024

mherman22 commented Oct 18, 2024 • edited Loading

ibacher commented Oct 18, 2024

mherman22 commented Oct 18, 2024

ibacher commented Oct 18, 2024

ibacher commented Oct 18, 2024

mherman22 commented Oct 18, 2024

mherman22 Oct 29, 2024

Choose a reason for hiding this comment

mherman22 Oct 29, 2024

Choose a reason for hiding this comment

mherman22 commented Oct 29, 2024

ibacher Nov 14, 2024

Choose a reason for hiding this comment

mherman22 Nov 14, 2024

Choose a reason for hiding this comment

dkayiwa commented Nov 21, 2024

mherman22 commented Nov 21, 2024

mherman22 commented Nov 21, 2024

mherman22 commented Oct 14, 2024 •

edited

Loading

mherman22 Oct 18, 2024 •

edited

Loading

mherman22 commented Oct 18, 2024 •

edited

Loading