I have been working on making our swagger documentation better and one of the steps was to first of all ensure that the various modules that are by default part of our distribution are fixed so that we first of all get rid of the swagger definition errors. (ref) → [RESTWS-957] - Jira.
In the past week i embarked on upgrading the swagger core version from 1.6.2 to around swagger 2.2.23 which now runs an OpenAPI Specification (OAS) and the ticket to this upgrade is at [RESTWS-958] - Jira. I have added a couple of tickets to the same modules used in the ticket above to upgrade them to run on 2.46.0-SNAPSHOT (in local maven repository) which has the changes at RESTWS-958: Upgrade swagger to 2.2.23 by mherman22 · Pull Request #631 · openmrs/openmrs-module-webservices.rest · GitHub as my way of testing the new changes in an sdk environment.
However, the application seems to be failing on startup with the following error which to me isn’t descriptive enough or doesn’t point me exactly where i need to look i.e like which module could be misbehaving or something
So the “EntityManager is closed” message usually happens when the Spring context refresh fails for whatever reason. In this case, the message that looks interesting is this one:
Caused by: java.lang.ClassCastException: com.fasterxml.jackson.datatype.jsr310.JavaTimeModule cannot be cast to com.fasterxml.jackson.databind.Module
I suspect what this means is that we’re adding the wrong version of the JSR-310 Jackson module to the classpath, presumably because core doesn’t provide that module and we’re using the version included with the new version of Swagger?
Can we check:
If the JSR-310 module is included in the webservices.rest OMOD?
What version it is?
What other Jackson libraries are included in the OMOD?
This doesn’t answer your question, but OpenAPI 4.0 is due to come out “by the end of 2024”. Not sure how accurate it is, but given that we are manually constructing the OpenAPI swagger json (in SwaggerSpecificationCreator.java), maybe we can hold off and wait for 4.0 to come out?
And thanks so much for fixing those swagger definition errors. Do you have more ideas on how to improve the documentation? I have been looking at it a lot as well, but haven’t written anything up yet.
I have tried to exclude those dependencies within modules that could be having their own jackson dependencies they depend on but seemingly that exception persists.
i don’t seem to find any, probably because i excluded them?
The leap from swagger 1.6.2 to swagger 3 (OpenAPI 4) could be quite huge. The longer we wait, the more challenging the eventual upgrade becomes. OpenAPI 4.0’s release date isn’t guaranteed (correct me if i am wrong) where by even after release, it’s usually wise to wait for the ecosystem to stabilize in order to see community adoption and tool support before migrating.
The idea is to really first having it
upgraded,
clean up the SwaggerSpecificationCreator class by removing all unnecessary and deprecated methods,
Think of bringing on board all the modules that aren’t currently extending our standard rest web services (i.e appointments, etc).
Even if OpenAPI 4.0 comes out this year, it will take a bit for the tooling to catch up. And it will likely be easier to migrate one major version at a time rather than try to go 2 in one go (FWIW, we’re currently generating OpenAPI 2.0 specifications).
The leap from swagger 1.6.2 to swagger 3 (OpenAPI 4) could be quite huge. The longer we wait, the more challenging the eventual upgrade becomes. OpenAPI 4.0’s release date isn’t guaranteed (correct me if i am wrong) where by even after release, it’s usually wise to wait for the ecosystem to stabilize in order to see community adoption and tool support before migrating.
Even if OpenAPI 4.0 comes out this year, it will take a bit for the tooling to catch up. And it will likely be easier to migrate one major version at a time rather than try to go 2 in one go (FWIW, we’re currently generating OpenAPI 2.0 specifications).
That’s fair. If the upgrade to 3.0 is nontrivial though, I would suggest looking at the feature set between 2.0 and 3.0 and see if there is any feature we want (either in terms of improving the typings / generation of the json or the toolings consuming the json) before sinking in the work.
Thank you @dkayiwa , the commit you made helps us by-pass the above reported error. Now i have to deal with this new error which might require me to update some modules in my instance with these changes.
It would be easier if you narrow this down to just one module. For instance, if i run mvn jetty:run with openmrs-core, which one module do i need to add in addition to webservicesrest and legacyui?
only way to fix those compile errors (for now) is locally by running mvn clean install against the rest module with the upgraded swagger changes and it should be able to compile