SORMAS-Project icon indicating copy to clipboard operation
SORMAS-Project copied to clipboard

Fixes #11181 Extended API swagger documentation

Open tglimm-itk opened this issue 2 years ago • 8 comments

Update Swagger-Maven-Plugin & Extend API swagger documentation

Swagger Documentation

Added swagger documentation to all Rest-API resources and their corresponding Dto and Enum classes; see #11181.

Updated Dependencies

Addressing issue #11175, the swagger-maven-plugin provided by org.codehaus.mojo was replaced by io.swagger.core.v3.

~~Swagger-dependency version set to 2.2.3, which is the latest version that still supports the processJsonObjectMapper method overridden in SwaggerConfig.java to force the usage of ENUM.name() over ENUM.toString().~~ I stand corrected: Latest version 2.2.7 works just fine.

ToDo

The string "TBD_RESTAPI_SWAGGER_DOC" is used through-out the documentation to mark model members or endpoint parameters, where in-depth context knowledge about the application is needed. We kindly ask the the SORMAS Open-Source Community to complete these passages over time.

tglimm-itk avatar Dec 16 '22 09:12 tglimm-itk

Two further comments @tglimm-itk:

  1. Ignore the linting error in the CI https://github.com/hzi-braunschweig/SORMAS-Project/pull/11196 will most likely take care of this
  2. I played a little bit around with the versions, I was able to update everything to the most recent version of 2.2.7? Take a look at this diff:
Diff
From f28ce100d218865ab69f4387128b6ab6b768e9cb Mon Sep 17 00:00:00 2001
From: Jonas Cirotzki <[email protected]>
Date: Mon, 19 Dec 2022 13:44:55 +0100
Subject: [PATCH] [#11181] bump everything swagger related to version 2.2.7

---
 sormas-api/pom.xml                            |  2 -
 sormas-base/pom.xml                           | 30 ++----
 sormas-rest/pom.xml                           |  3 -
 .../rest/swagger/AttributeConverter.java      | 95 ++++++++++---------
 4 files changed, 59 insertions(+), 71 deletions(-)

diff --git a/sormas-api/pom.xml b/sormas-api/pom.xml
index 05aa220156..647d1a7416 100644
--- a/sormas-api/pom.xml
+++ b/sormas-api/pom.xml
@@ -51,13 +51,11 @@
 		<dependency>
             <groupId>io.swagger.core.v3</groupId>
             <artifactId>swagger-jaxrs2</artifactId>
-			<version>2.2.3</version>
         </dependency>
 
         <dependency>
             <groupId>io.swagger.core.v3</groupId>
             <artifactId>swagger-jaxrs2-servlet-initializer-v2</artifactId>
-			<version>2.2.3</version>
         </dependency>
 
 		<dependency>
diff --git a/sormas-base/pom.xml b/sormas-base/pom.xml
index 8f1cf55d2b..8f2dc978b3 100644
--- a/sormas-base/pom.xml
+++ b/sormas-base/pom.xml
@@ -31,7 +31,7 @@
 		<vaadin.version.warning>TODO: Remove bootstrap.js in widgetset</vaadin.version.warning>
 		<vaadin.version>8.14.3</vaadin.version>
 		<vaadin.plugin.version>${vaadin.version}</vaadin.plugin.version>
-		<swagger.version>2.1.13</swagger.version>
+		<swagger.version>2.2.7</swagger.version>
 		<bouncycastle.version>1.70</bouncycastle.version>
 		<keycloak.version>18.0.1</keycloak.version>
 		<resteasy.version>3.15.3.Final</resteasy.version>
@@ -1217,27 +1217,7 @@
 					<version>1.0.8</version>
 				</plugin>
 
-				<!-- Swagger -->
-				<plugin>
-					<groupId>io.openapitools.swagger</groupId>
-					<artifactId>swagger-maven-plugin</artifactId>
-					<version>2.1.2</version>
-					<executions>
-						<execution>
-							<goals>
-								<goal>generate</goal>
-							</goals>
-						</execution>
-					</executions>
-					<dependencies>
-						<!-- Currently necessary to get the latest bugfixes in swagger-core -->
-						<dependency>
-							<groupId>io.swagger.core.v3</groupId>
-							<artifactId>swagger-jaxrs2</artifactId>
-							<version>${swagger.version}</version>
-						</dependency>
-					</dependencies>
-				</plugin>
+
 
 				<!-- *** Plugins for generate-reports *** -->
 				<plugin>
@@ -1330,6 +1310,12 @@
 						<commitIdGenerationMode>full</commitIdGenerationMode>
 					</configuration>
 				</plugin>
+				<plugin>
+					<groupId>io.swagger.core.v3</groupId>
+					<artifactId>swagger-maven-plugin</artifactId>
+					<version>${swagger.version}</version>
+				</plugin>
+
 				<!-- *** Plugins for generate-reports END *** -->
 
 			</plugins>
diff --git a/sormas-rest/pom.xml b/sormas-rest/pom.xml
index b5839e8ae4..aa70fad4f3 100644
--- a/sormas-rest/pom.xml
+++ b/sormas-rest/pom.xml
@@ -55,13 +55,11 @@
 		<dependency>
 			<groupId>io.swagger.core.v3</groupId>
 			<artifactId>swagger-jaxrs2</artifactId>
-			<version>2.2.3</version>
 		</dependency>
 
 		<dependency>
 			<groupId>io.swagger.core.v3</groupId>
 			<artifactId>swagger-jaxrs2-servlet-initializer-v2</artifactId>
-			<version>2.2.3</version>
 		</dependency>
 
 		<!-- Keycloak -->
@@ -134,7 +132,6 @@
 			<plugin>
 				<groupId>io.swagger.core.v3</groupId>
 				<artifactId>swagger-maven-plugin</artifactId>
-				<version>2.2.3</version>
 				<configuration>
 					<outputPath>${basedir}/target/</outputPath>
 					<outputFormat>JSONANDYAML</outputFormat>
diff --git a/sormas-rest/src/main/java/de/symeda/sormas/rest/swagger/AttributeConverter.java b/sormas-rest/src/main/java/de/symeda/sormas/rest/swagger/AttributeConverter.java
index 0deba1d365..24a6462538 100644
--- a/sormas-rest/src/main/java/de/symeda/sormas/rest/swagger/AttributeConverter.java
+++ b/sormas-rest/src/main/java/de/symeda/sormas/rest/swagger/AttributeConverter.java
@@ -1,54 +1,61 @@
 package de.symeda.sormas.rest.swagger;
 
+import java.lang.annotation.Annotation;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+
 import com.fasterxml.jackson.databind.ObjectMapper;
+
 import de.symeda.sormas.api.Disease;
-import de.symeda.sormas.api.utils.*;
+import de.symeda.sormas.api.utils.Complication;
+import de.symeda.sormas.api.utils.DependantOn;
+import de.symeda.sormas.api.utils.Diseases;
+import de.symeda.sormas.api.utils.HideForCountries;
+import de.symeda.sormas.api.utils.HideForCountriesExcept;
+import de.symeda.sormas.api.utils.Outbreaks;
+import de.symeda.sormas.api.utils.PersonalData;
 import io.swagger.v3.core.converter.AnnotatedType;
 import io.swagger.v3.core.converter.ModelConverter;
 import io.swagger.v3.core.converter.ModelConverterContext;
 import io.swagger.v3.core.jackson.ModelResolver;
 import io.swagger.v3.oas.models.media.Schema;
 
-import java.lang.annotation.Annotation;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-
 public class AttributeConverter extends ModelResolver {
-    public static final String XPROP_PREFIX = "x-sormas-";
-    public static final String XPROP_PERSONAL_DATA = XPROP_PREFIX + "personal-data";
-    public static final String XPROP_FOR_COUNTRIES = XPROP_PREFIX + "countries-for";
-    public static final String XPROP_EXCEPT_COUNTRIES = XPROP_PREFIX + "countries-except";
-    public static final String XPROP_DEPENDS_ON = XPROP_PREFIX + "depends-on";
-    public static final String XPROP_DISEASES = XPROP_PREFIX + "diseases";
-    public static final String XPROP_OUTBREAKS = XPROP_PREFIX + "outbreaks";
-    public static final String XPROP_COMPLICATIONS = XPROP_PREFIX + "complications";
-
-    public AttributeConverter(ObjectMapper mapper) {
-        super(mapper);
-    }
-
-    @Override
-    protected void applyBeanValidatorAnnotations(Schema property, Annotation[] annotations, Schema parent) {
-        super.applyBeanValidatorAnnotations(property, annotations, parent);
-        Map<String, Annotation> annos = new HashMap<String, Annotation>();
-        if (annotations != null) {
-            for (Annotation anno : annotations) {
-                annos.put(anno.annotationType().getName(), anno);
-            }
-        }
-        if (parent != null &&
-                annos.containsKey("de.symeda.sormas.api.utils.Required")) {
-            addRequiredItem(parent, property.getName());
-        }
-    }
-
-    @Override
-    public Schema<?> resolve(AnnotatedType annotatedType, ModelConverterContext modelConverterContext,
-            Iterator<ModelConverter> iterator) {
-        Schema schema = super.resolve(annotatedType, modelConverterContext, iterator);
-
-        //@formatter:off
+
+	public static final String XPROP_PREFIX = "x-sormas-";
+	public static final String XPROP_PERSONAL_DATA = XPROP_PREFIX + "personal-data";
+	public static final String XPROP_FOR_COUNTRIES = XPROP_PREFIX + "countries-for";
+	public static final String XPROP_EXCEPT_COUNTRIES = XPROP_PREFIX + "countries-except";
+	public static final String XPROP_DEPENDS_ON = XPROP_PREFIX + "depends-on";
+	public static final String XPROP_DISEASES = XPROP_PREFIX + "diseases";
+	public static final String XPROP_OUTBREAKS = XPROP_PREFIX + "outbreaks";
+	public static final String XPROP_COMPLICATIONS = XPROP_PREFIX + "complications";
+
+	public AttributeConverter(ObjectMapper mapper) {
+		super(mapper);
+	}
+
+	@Override
+	protected void applyBeanValidatorAnnotations(Schema property, Annotation[] annotations, Schema parent, boolean applyNotNullAnnotations) {
+		super.applyBeanValidatorAnnotations(property, annotations, parent, applyNotNullAnnotations);
+
+		Map<String, Annotation> annos = new HashMap<>();
+		if (annotations != null) {
+			for (Annotation anno : annotations) {
+				annos.put(anno.annotationType().getName(), anno);
+			}
+		}
+		if (parent != null && annos.containsKey("de.symeda.sormas.api.utils.Required")) {
+			addRequiredItem(parent, property.getName());
+		}
+	}
+
+	@Override
+	public Schema<?> resolve(AnnotatedType annotatedType, ModelConverterContext modelConverterContext, Iterator<ModelConverter> iterator) {
+		Schema schema = super.resolve(annotatedType, modelConverterContext, iterator);
+
+		//@formatter:off
         if (schema != null && annotatedType.getCtxAnnotations() != null
                 && annotatedType.isSchemaProperty()) {
 
@@ -91,9 +98,9 @@ public class AttributeConverter extends ModelResolver {
                 }
             }
             //@formatter:on
-        }
+		}
 
-        return schema;
+		return schema;
 
-    }
-}
\ No newline at end of file
+	}
+}
-- 
2.39.0

The only signature changed was in AttributeConverter, but this is easily resolvable I argue. Please prove me wrong, but SwaggerConfig.java which was mentioned in the initial comments appears to working as expected? Feel free to use the diff, would be really good to have everything on the the latest version.

JonasCir avatar Dec 19 '22 12:12 JonasCir

Hi @JonasCir, thank you for taking the time to upgrade the version to 2.2.7. I tested the changs in your diff and will incorporate them.

  1. I agree with inheriting the swagger version from sormas-base. This is absolutely the way to go.

  2. The generated external_visits_API.yaml previously contained the endpoint documentation to ExternalVisitsResource only. However, with PR #11160 they were moved together with all other resources and now appear within swagger.yaml. The corresponding config and execution-step in the pom.xml will be removed.

tglimm-itk avatar Dec 20 '22 11:12 tglimm-itk

Rebased (as per guidelines) on to current development state to handle conflicts.

tglimm-itk avatar Dec 20 '22 11:12 tglimm-itk

@tglimm-itk Sorry for getting back to you so late - it seems like there are some issues with the build concerning the mobile app, I suspect that you've maybe upgraded Gradle, which is not supposed to happen in an external PR (because as you can see, it can break stuff). Can you please try to fix that before I take an in-depth look at the changes?

MateStrysewske avatar Jan 03 '23 12:01 MateStrysewske

@MateStrysewske Thanks for taking a look at the PR. To me knowledge, we didn't touch Gradle at all. The Swagger-doc is built using Maven. I'll perform another rebase to incorporate the latest changes from development. If you could take a look afterwards, that would be great.

tglimm-itk avatar Jan 03 '23 13:01 tglimm-itk

So far, I've not been able to locate the cause for the sormas-app transformDebugClassesWithAsm UnitTest failing.

I found Google's IssueTracker, which is probably the reason you are sticking to Gradle 7.0.2? As previously stated, Gradle was never touched (or even used) within the scope of this Pull Request.

The fact that sormas-app is affected is quite irritating, as 95% of the changes are only swagger descriptions in sormas-rest and sormas-api. There's not a single change to endpoint or dto code. The only real change is to the pom.xml of

  • sormas-base
  • sormas-rest
  • sormas-api for the updated swagger-plugin.

I'm unfortunately out of time budget to work on this topic since December, but I don't want to leave you hanging. Can I give you more information to support you in any way?

tglimm-itk avatar Jan 04 '23 11:01 tglimm-itk

@tglimm-itk I'll have a look at the PR and see whether I can find any cause - maybe we can fix it ourselves, we'll see. Thanks for you work! I'll keep you updated.

MateStrysewske avatar Jan 06 '23 10:01 MateStrysewske

@tglimm-itk We'll unfortunately have to postpone reviewing this because it will take us a significant amount of time (obviously) and the funding for this is not quite clear right now. We'll talk this through with the SORMAS Foundation this week.

MateStrysewske avatar Jan 09 '23 08:01 MateStrysewske