🐛 Fixing some issues with threading and NPE (#5107)

* 🐛 Fixing some issues with threading and NPE

After running Sonar on the master branch, some major analysis
opportunities were displayed.

This fixes the use of SimpleDateFormat stored as static fields.
SimpleDateFormat is not thread-safe, and may retain data across threads.
While there's no indicator that this has caused any issues (these are
mostly used for example code), we should follow these best practices.

This also fixes a handful of NPE and other minor issues such as
comparing Boolean.TRUE to strings and no wrapping some closeables in
try-with-resources.

* [cli] Unit test GenerateBatch custom deserialization helper

* Quiet batch mode in sonar.yml

* Suppress unnecessary warnings (ThreadLocals in static fields)
This commit is contained in:
Jim Schubert
2020-01-25 18:28:16 -05:00
committed by GitHub
parent ad4a9df328
commit c9ec084418
27 changed files with 380 additions and 110 deletions

View File

@@ -123,27 +123,12 @@ public class GenerateBatch implements Runnable {
}
}
LOGGER.info(String.format(Locale.ROOT, "Batch generation using up to %d threads.\nIncludes: %s\nRoot: %s", numThreads, includesDir.getAbsolutePath(), rootDir.toAbsolutePath().toString()));
// Create a module which loads our config files, but supports a special "!include" key which can point to an existing config file.
// This allows us to create a sort of meta-config which holds configs which are otherwise required at CLI time (via generate task).
// That is, this allows us to create a wrapper config for generatorName, inputSpec, outputDir, etc.
SimpleModule module = new SimpleModule("GenerateBatch");
module.setDeserializerModifier(new BeanDeserializerModifier() {
@Override
public JsonDeserializer<?> modifyDeserializer(DeserializationConfig config,
BeanDescription bd, JsonDeserializer<?> original) {
JsonDeserializer<?> result;
if (bd.getBeanClass() == DynamicSettings.class) {
result = new DynamicSettingsRefSupport(original, includesDir);
} else {
result = original;
}
return result;
}
});
SimpleModule module = getCustomDeserializationModel(includesDir);
List<CodegenConfigurator> configurators = configs.stream().map(config -> CodegenConfigurator.fromFile(config, module)).collect(Collectors.toList());
// it doesn't make sense to interleave INFO level logs, so limit these to only ERROR.
@@ -169,6 +154,8 @@ public class GenerateBatch implements Runnable {
System.out.println("COMPLETE.");
} catch (InterruptedException e) {
e.printStackTrace();
// re-interrupt
Thread.currentThread().interrupt();
}
}
@@ -227,6 +214,28 @@ public class GenerateBatch implements Runnable {
}
}
static SimpleModule getCustomDeserializationModel(final File includesDir) {
// Create a module which loads our config files, but supports a special "!include" key which can point to an existing config file.
// This allows us to create a sort of meta-config which holds configs which are otherwise required at CLI time (via generate task).
// That is, this allows us to create a wrapper config for generatorName, inputSpec, outputDir, etc.
SimpleModule module = new SimpleModule("GenerateBatch");
module.setDeserializerModifier(new BeanDeserializerModifier() {
@Override
public JsonDeserializer<?> modifyDeserializer(DeserializationConfig config,
BeanDescription bd, JsonDeserializer<?> original) {
JsonDeserializer<?> result;
if (bd.getBeanClass() == DynamicSettings.class) {
result = new DynamicSettingsRefSupport(original, includesDir);
} else {
result = original;
}
return result;
}
});
return module;
}
static class DynamicSettingsRefSupport extends DelegatingDeserializer {
private static final String INCLUDE = "!include";
private File scanDir;
@@ -255,11 +264,13 @@ public class GenerateBatch implements Runnable {
// load the file into the tree node and continue parsing as normal
((ObjectNode) node).remove(INCLUDE);
JsonParser includeParser = codec.getFactory().createParser(includeFile);
TreeNode includeNode = includeParser.readValueAsTree();
TreeNode includeNode;
try (JsonParser includeParser = codec.getFactory().createParser(includeFile)) {
includeNode = includeParser.readValueAsTree();
}
ObjectReader reader = codec.readerForUpdating(node);
TreeNode updated = reader.readValue(includeFile);
TreeNode updated = reader.readValue(includeNode.traverse());
JsonParser updatedParser = updated.traverse();
updatedParser.nextToken();
return super.deserialize(updatedParser, ctx);

View File

@@ -0,0 +1,102 @@
package org.openapitools.codegen.cmd;
import com.fasterxml.jackson.databind.module.SimpleModule;
import org.openapitools.codegen.config.CodegenConfigurator;
import org.openapitools.codegen.config.Context;
import org.openapitools.codegen.config.GeneratorSettings;
import org.openapitools.codegen.config.WorkflowSettings;
import org.testng.ITestContext;
import org.testng.TestRunner;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import static org.testng.Assert.*;
@SuppressWarnings("RedundantThrows")
public class GenerateBatchTest {
private static final String SPEC_FILE = "batch/specs/petstore.yaml";
private static final String JAXRS_DATELIB_J8_JSON = "jaxrs-datelib-j8.json";
private static final String JAXRS_DATELIB_J8_YAML = "jaxrs-datelib-j8.yaml";
private static final String JAXRS_DATELIB_J8_YAML_INCLUDE_JSON = "jaxrs-datelib-j8-yaml-include.json";
private static final String JAXRS_DATELIB_J8_JSON_INCLUDE_YAML = "jaxrs-datelib-j8-json-include.yaml";
Path workingDirectory;
@BeforeTest
public void setUp(ITestContext ctx) throws IOException {
workingDirectory = Paths.get("src", "test", "resources", "batch");
}
@DataProvider(name = "customIncludeDeserializerFiles")
public Object[][] customIncludeDeserializerFiles() {
return new Object[][] {
{JAXRS_DATELIB_J8_JSON},
{JAXRS_DATELIB_J8_YAML},
{JAXRS_DATELIB_J8_JSON_INCLUDE_YAML}
};
}
@Test(dataProvider = "customIncludeDeserializerFiles")
public void testDeserializerWithJsonInclude(String file) throws IOException {
String config = getTargetResourceAsFile(file).toString();
SimpleModule module = GenerateBatch.getCustomDeserializationModel(getIncludesDir());
CodegenConfigurator loaded = CodegenConfigurator.fromFile(config, module);
Map<String, Object> expectedAdditionalProperties = new HashMap<>();
expectedAdditionalProperties.put("serverPort", "8082");
expectedAdditionalProperties.put("dateLibrary", "java8");
expectedAdditionalProperties.put("hideGenerationTimestamp", true);
expectedAdditionalProperties.put("serializableModel", true);
expectedAdditionalProperties.put("withXml", true);
expectedAdditionalProperties.put("java8", true);
expectedAdditionalProperties.put("useBeanValidation", true);
assertNotNull(loaded);
Context<?> context = loaded.toContext();
WorkflowSettings workflowSettings = context.getWorkflowSettings();
GeneratorSettings generatorSettings = context.getGeneratorSettings();
assertNotNull(workflowSettings);
assertNotNull(generatorSettings);
assertEquals(generatorSettings.getGeneratorName(), "jaxrs-jersey");
assertEquals(workflowSettings.getOutputDir(), "outputDir");
assertEquals(workflowSettings.getInputSpec(), SPEC_FILE);
assertTrue(generatorSettings.getAdditionalProperties().size() >= 7);
Set<Map.Entry<String, Object>> actualSet = generatorSettings.getAdditionalProperties().entrySet();
assertTrue(actualSet.containsAll(expectedAdditionalProperties.entrySet()));
}
@SuppressWarnings("unused")
@Test(
expectedExceptions = { RuntimeException.class },
expectedExceptionsMessageRegExp = "Unable to deserialize config file: .*"
)
public void testInvalidDeserializerWithIncludeOption() {
// JSON is valid YAML, but not the other way around, so we can't load a YAML include from a JSON config
// to do so would require additional work.
String config = getTargetResourceAsFile(JAXRS_DATELIB_J8_YAML_INCLUDE_JSON).toString();
SimpleModule module = GenerateBatch.getCustomDeserializationModel(getIncludesDir());
CodegenConfigurator loaded = CodegenConfigurator.fromFile(config, module);
fail("Expected an exception when trying to load a YAML include from a JSON file");
}
private File getIncludesDir() {
// The includes directory would be "batch" under resources here, as everything is relative to this directory.
return workingDirectory.toFile();
}
private File getTargetResourceAsFile(String relative) {
return workingDirectory.resolve(relative).toAbsolutePath().toFile();
}
}

View File

@@ -0,0 +1,7 @@
{
"serializableModel": true,
"withXml": true,
"dateLibrary": "java8",
"java8": true,
"useBeanValidation": true
}

View File

@@ -0,0 +1,6 @@
---
serializableModel: true
withXml: true
dateLibrary: java8
java8: true
useBeanValidation: true

View File

@@ -0,0 +1,8 @@
---
"!include": common/jaxrs-datelib-j8.json
generatorName: jaxrs-jersey
inputSpec: batch/specs/petstore.yaml
outputDir: outputDir
additionalProperties:
hideGenerationTimestamp: true
serverPort: '8082'

View File

@@ -0,0 +1,10 @@
{
"!include": "common/jaxrs-datelib-j8.yaml",
"generatorName": "jaxrs-jersey",
"inputSpec": "batch/specs/petstore.yaml",
"outputDir": "outputDir",
"additionalProperties": {
"hideGenerationTimestamp": true,
"serverPort": "8082"
}
}

View File

@@ -0,0 +1,10 @@
{
"!include": "common/jaxrs-datelib-j8.json",
"generatorName": "jaxrs-jersey",
"inputSpec": "batch/specs/petstore.yaml",
"outputDir": "outputDir",
"additionalProperties": {
"hideGenerationTimestamp": true,
"serverPort": "8082"
}
}

View File

@@ -0,0 +1,8 @@
---
"!include": common/jaxrs-datelib-j8.yaml
generatorName: jaxrs-jersey
inputSpec: batch/specs/petstore.yaml
outputDir: outputDir
additionalProperties:
hideGenerationTimestamp: true
serverPort: '8082'

View File

@@ -0,0 +1,111 @@
openapi: "3.0.0"
info:
version: 1.0.0
title: Swagger Petstore
license:
name: MIT
servers:
- url: http://petstore.swagger.io/v1
paths:
/pets:
get:
summary: List all pets
operationId: listPets
tags:
- pets
parameters:
- name: limit
in: query
description: How many items to return at one time (max 100)
required: false
schema:
type: integer
format: int32
responses:
'200':
description: A paged array of pets
headers:
x-next:
description: A link to the next page of responses
schema:
type: string
content:
application/json:
schema:
$ref: "#/components/schemas/Pets"
default:
description: unexpected error
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
post:
summary: Create a pet
operationId: createPets
tags:
- pets
responses:
'201':
description: Null response
default:
description: unexpected error
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
/pets/{petId}:
get:
summary: Info for a specific pet
operationId: showPetById
tags:
- pets
parameters:
- name: petId
in: path
required: true
description: The id of the pet to retrieve
schema:
type: string
responses:
'200':
description: Expected response to a valid request
content:
application/json:
schema:
$ref: "#/components/schemas/Pet"
default:
description: unexpected error
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
components:
schemas:
Pet:
type: object
required:
- id
- name
properties:
id:
type: integer
format: int64
name:
type: string
tag:
type: string
Pets:
type: array
items:
$ref: "#/components/schemas/Pet"
Error:
type: object
required:
- code
- message
properties:
code:
type: integer
format: int32
message:
type: string