RESTWS-568 - Optimize swagger spec creation (openmrs#294)

Generate and cache Swagger spec and clear it once every context refresh. If a cache exists SwaggerSpecificationCreator will serve the cached version instead of regenerating the spec.
oslynn · Oct 15, 2017 · 8545932 · 8545932
1 parent 799057d
commit 8545932
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 11 deletions.
diff --git a/...rc/main/java/org/openmrs/module/webservices/docs/swagger/SwaggerSpecificationCreator.java b/...rc/main/java/org/openmrs/module/webservices/docs/swagger/SwaggerSpecificationCreator.java
@@ -73,7 +73,7 @@
 
 public class SwaggerSpecificationCreator {
 
-	private Swagger swagger = new Swagger();
+	private static Swagger swagger;
 
 	private String host;
 
@@ -116,7 +116,10 @@ public SwaggerSpecificationCreator scheme(Scheme scheme) {
 		return this;
 	}
 
-	public String BuildJSON() {
+	/**
+	 * Regenerate the swagger spec from scratch
+	 */
+	private void BuildJSON() {
 		synchronized (this) {
 			log.info("Initiating Swagger specification creation");
 			toggleLogs(RestConstants.SWAGGER_LOGS_OFF);
@@ -134,6 +137,16 @@ public String BuildJSON() {
 				log.info("Swagger specification creation complete");
 			}
 		}
+	}
+
+	public String getJSON() {
+		if (isCached()) {
+			log.info("Returning a cached copy of Swagger specification");
+			initSwagger();
+		} else {
+			swagger = new Swagger();
+			BuildJSON();
+		}
 		return createJSON();
 	}
 
@@ -200,7 +213,7 @@ private void initSwagger() {
 		        .contact(new Contact().name("OpenMRS").url("http://openmrs.org"))
 		        .license(new License().name("MPL-2.0 w/ HD").url("http://openmrs.org/license"));
 
-		this.swagger
+		swagger
 		        .info(info)
 		        .host(this.host)
 		        .basePath(this.basePath)
@@ -849,12 +862,10 @@ private List<org.openmrs.module.webservices.docs.swagger.Parameter> getParameter
 					parameter.setRequired(false);
 					parameters.add(parameter);
 				}
-
 				break;
 			}
 		}
 		return parameters;
-
 	}
 
 	private String createJSON() {
@@ -1212,6 +1223,17 @@ public Swagger getSwagger() {
 		return swagger;
 	}
 
+	/**
+	 * @return true if and only if swagger is not null, and its paths are also set.
+	 */
+	public static boolean isCached() {
+		return swagger != null && swagger.getPaths() != null;
+	}
+
+	public static void clearCache() {
+		swagger = null;
+	}
+
 	//FIXME: move to separate util calls
 	// see: https://stackoverflow.com/q/13783295/3647002
 	public static String[] getEnums(Class<? extends Enum<?>> e) {

diff --git a/omod/src/main/java/org/openmrs/module/webservices/rest/Activator.java b/omod/src/main/java/org/openmrs/module/webservices/rest/Activator.java
@@ -14,6 +14,7 @@
 import org.openmrs.api.context.Context;
 import org.openmrs.module.BaseModuleActivator;
 import org.openmrs.module.ModuleActivator;
+import org.openmrs.module.webservices.docs.swagger.SwaggerSpecificationCreator;
 import org.openmrs.module.webservices.rest.util.ReflectionUtil;
 import org.openmrs.module.webservices.rest.web.ConversionUtil;
 import org.openmrs.module.webservices.rest.web.api.RestService;
@@ -44,6 +45,7 @@ public void contextRefreshed() {
 
 		ConversionUtil.clearCache();
 		ReflectionUtil.clearCaches();
+		SwaggerSpecificationCreator.clearCache();
 	}
 
 }
diff --git a/...va/org/openmrs/module/webservices/rest/web/controller/SwaggerSpecificationController.java b/...va/org/openmrs/module/webservices/rest/web/controller/SwaggerSpecificationController.java
@@ -32,7 +32,7 @@ String getSwaggerSpecification(HttpServletRequest request) throws Exception {
 		        .basePath(request.getContextPath() + "/ws/rest/v1")
 		        .scheme(Scheme.forValue(request.getScheme()))
 
-		        .BuildJSON();
+		        .getJSON();
 	}
 
 }
diff --git a/...rc/test/java/org/openmrs/module/webservices/rest/doc/SwaggerSpecificationCreatorTest.java b/...rc/test/java/org/openmrs/module/webservices/rest/doc/SwaggerSpecificationCreatorTest.java
@@ -51,7 +51,7 @@ public class SwaggerSpecificationCreatorTest extends BaseModuleWebContextSensiti
 
 	@Test
 	public void mainTest() {
-		String str = new SwaggerSpecificationCreator().BuildJSON();
+		String str = new SwaggerSpecificationCreator().getJSON();
 		assertNotNull(str);
 	}
 
@@ -65,6 +65,16 @@ public void hasSearchHandler() {
 		assertFalse(creator.hasSearchHandler("description", "concept"));
 	}
 
+	@Test
+	public void cacheTest() throws Exception {
+		if (SwaggerSpecificationCreator.isCached()) {
+			SwaggerSpecificationCreator.clearCache();
+		}
+		assertFalse(SwaggerSpecificationCreator.isCached());
+		new SwaggerSpecificationCreator().getJSON();
+		assertTrue(SwaggerSpecificationCreator.isCached());
+	}
+
 	@Test
 	public void modelResolveTest() {
 		final ModelResolver modelResolver = new ModelResolver(new ObjectMapper());
@@ -123,7 +133,7 @@ public void init() throws Exception {
 	@Test
 	public void checkNoDatabaseChanges() throws Exception {
 		SwaggerSpecificationCreator ssc = new SwaggerSpecificationCreator();
-		ssc.BuildJSON();
+		ssc.getJSON();
 
 		Map<String, Integer> afterCounts = getRowCounts();
 
@@ -151,7 +161,7 @@ public void checkOperationIdsSet() throws Exception {
 		List<String> operationIds = new ArrayList<String>();
 
 		SwaggerSpecificationCreator ssc = new SwaggerSpecificationCreator();
-		ssc.BuildJSON();
+		ssc.getJSON();
 		Swagger spec = ssc.getSwagger();
 
 		for (Path p : spec.getPaths().values()) {
@@ -166,7 +176,7 @@ public void checkOperationIdsSet() throws Exception {
 	@Test
 	public void checkRepresentationParamExists() throws Exception {
 		SwaggerSpecificationCreator ssc = new SwaggerSpecificationCreator();
-		ssc.BuildJSON();
+		ssc.getJSON();
 		Swagger spec = ssc.getSwagger();
 
 		for (Path p : spec.getPaths().values()) {
@@ -193,7 +203,7 @@ private boolean operationHasRepresentationParam(Operation o) {
 	@Test
 	public void checkPagingParamsExist() throws Exception {
 		SwaggerSpecificationCreator ssc = new SwaggerSpecificationCreator();
-		ssc.BuildJSON();
+		ssc.getJSON();
 		Swagger spec = ssc.getSwagger();
 
 		for (Path p : spec.getPaths().values()) {