Skip to content

Commit

Permalink
fix: code smells (#88)
Browse files Browse the repository at this point in the history
Signed-off-by: Pascal Krause <[email protected]>
  • Loading branch information
pk-work authored Nov 15, 2024
1 parent ff40810 commit 1eb5cc9
Show file tree
Hide file tree
Showing 16 changed files with 71 additions and 92 deletions.
2 changes: 1 addition & 1 deletion src/main/java/io/vertx/openapi/contract/Operation.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import io.vertx.codegen.annotations.Nullable;
import io.vertx.codegen.annotations.VertxGen;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.json.JsonObject;

import java.util.List;

Expand Down Expand Up @@ -80,6 +79,7 @@ public interface Operation extends OpenAPIObject {

/**
* Returns the applicable list of security requirements (scopes) or empty list.
*
* @return The related security requirement.
*/
List<SecurityRequirement> getSecurityRequirements();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toUnmodifiableList;

public class OpenAPIContractImpl implements OpenAPIContract {
private static final String KEY_SERVERS = "servers";
Expand Down Expand Up @@ -74,21 +75,17 @@ public OpenAPIContractImpl(JsonObject resolvedSpec, OpenAPIVersion version, Sche
this.version = version;
this.schemaRepository = schemaRepository;

servers = unmodifiableList(
resolvedSpec
.getJsonArray(KEY_SERVERS, EMPTY_JSON_ARRAY)
.stream()
.map(JsonObject.class::cast)
.map(ServerImpl::new)
.collect(toList()));
servers = resolvedSpec
.getJsonArray(KEY_SERVERS, EMPTY_JSON_ARRAY)
.stream()
.map(JsonObject.class::cast)
.map(ServerImpl::new).collect(toUnmodifiableList());

this.securityRequirements = unmodifiableList(
resolvedSpec
.getJsonArray(KEY_SECURITY, EMPTY_JSON_ARRAY)
.stream()
.map(JsonObject.class::cast)
.map(SecurityRequirementImpl::new)
.collect(toList()));
this.securityRequirements = resolvedSpec
.getJsonArray(KEY_SECURITY, EMPTY_JSON_ARRAY)
.stream()
.map(JsonObject.class::cast)
.map(SecurityRequirementImpl::new).collect(toUnmodifiableList());

if (servers.stream().collect(groupingBy(Server::getBasePath)).size() > 1) {
throw createUnsupportedFeature("Different base paths in server urls");
Expand Down Expand Up @@ -182,7 +179,7 @@ public String basePath() {

@Override
public List<Operation> operations() {
return unmodifiableList(new ArrayList<>(operations.values()));
return List.copyOf(operations.values());
}

@Override
Expand Down
14 changes: 6 additions & 8 deletions src/main/java/io/vertx/openapi/contract/impl/OperationImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import static io.vertx.openapi.impl.Utils.EMPTY_JSON_OBJECT;
import static java.util.Collections.unmodifiableList;
import static java.util.Collections.unmodifiableMap;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toUnmodifiableList;

public class OperationImpl implements Operation {
private static final Logger LOG = LoggerFactory.getLogger(OperationImpl.class);
Expand Down Expand Up @@ -80,16 +80,14 @@ public OperationImpl(String absolutePath, String path, HttpMethod method, JsonOb
this.extensions = unmodifiableMap(allExtensions);

this.tags =
unmodifiableList(operationModel.getJsonArray(KEY_TAGS, EMPTY_JSON_ARRAY).stream().map(Object::toString).collect(
toList()));
operationModel.getJsonArray(KEY_TAGS, EMPTY_JSON_ARRAY).stream().map(Object::toString).collect(toUnmodifiableList());

this.securityRequirements =
operationModel.containsKey(KEY_SECURITY) ?
unmodifiableList(
operationModel.getJsonArray(KEY_SECURITY).stream()
.map(JsonObject.class::cast)
.map(SecurityRequirementImpl::new)
.collect(toList())) :
operationModel.getJsonArray(KEY_SECURITY).stream()
.map(JsonObject.class::cast)
.map(SecurityRequirementImpl::new)
.collect(toUnmodifiableList()) :
globalSecReq;

List<Parameter> operationParameters =
Expand Down
25 changes: 11 additions & 14 deletions src/main/java/io/vertx/openapi/contract/impl/ResponseImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.List;
import java.util.Map;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE;
import static io.vertx.openapi.contract.Location.HEADER;
Expand All @@ -30,10 +29,10 @@
import static io.vertx.openapi.contract.OpenAPIContractException.createUnsupportedFeature;
import static io.vertx.openapi.impl.Utils.EMPTY_JSON_OBJECT;
import static java.lang.String.join;
import static java.util.Collections.unmodifiableList;
import static java.util.Collections.unmodifiableMap;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toUnmodifiableList;

public class ResponseImpl implements Response {
private static final String KEY_HEADERS = "headers";
Expand All @@ -51,18 +50,16 @@ public ResponseImpl(JsonObject responseModel, String operationId) {
this.responseModel = responseModel;

JsonObject headersObject = responseModel.getJsonObject(KEY_HEADERS, EMPTY_JSON_OBJECT);
this.headers = unmodifiableList(
headersObject
.fieldNames()
.stream()
.filter(JsonSchema.EXCLUDE_ANNOTATIONS)
.filter(FILTER_CONTENT_TYPE)
.map(name -> {
JsonObject headerModel = headersObject.getJsonObject(name).copy().put("name", name).put("in",
HEADER.toString());
return new ParameterImpl("", headerModel);
})
.collect(Collectors.toList()));
this.headers = headersObject
.fieldNames()
.stream()
.filter(JsonSchema.EXCLUDE_ANNOTATIONS)
.filter(FILTER_CONTENT_TYPE)
.map(name -> {
JsonObject headerModel = headersObject.getJsonObject(name).copy().put("name", name).put("in",
HEADER.toString());
return new ParameterImpl("", headerModel);
}).collect(toUnmodifiableList());

JsonObject contentObject = responseModel.getJsonObject(KEY_CONTENT, EMPTY_JSON_OBJECT);
this.content = unmodifiableMap(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,29 @@
import java.util.stream.Collectors;

import static io.vertx.openapi.impl.Utils.EMPTY_JSON_ARRAY;
import static java.util.Collections.*;
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static java.util.Collections.unmodifiableMap;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toUnmodifiableSet;

public class SecurityRequirementImpl implements SecurityRequirement {

private final JsonObject securityRequirementModel;
private final Set<String> names;
private final Map<String, List<String>> scopes;
private final boolean empty;

public SecurityRequirementImpl(JsonObject securityRequirementModel) {
this.securityRequirementModel = securityRequirementModel;
this.empty = securityRequirementModel.isEmpty();

if (securityRequirementModel.isEmpty()) {
this.names = emptySet();
this.scopes = emptyMap();
} else {
this.names = unmodifiableSet(
securityRequirementModel
.fieldNames()
.stream()
.filter(JsonSchema.EXCLUDE_ANNOTATIONS)
.collect(Collectors.toSet()));
this.names = securityRequirementModel
.fieldNames()
.stream()
.filter(JsonSchema.EXCLUDE_ANNOTATIONS).collect(toUnmodifiableSet());

this.scopes = unmodifiableMap(
this.names
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/io/vertx/openapi/validation/RequestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public static Future<ValidatableRequest> extract(HttpServerRequest request, Oper
* @return A {@link Future} holding the ValidatableRequest.
*/
public static Future<ValidatableRequest> extract(HttpServerRequest request, Operation operation,
Supplier<Future<Buffer>> bodySupplier) {
Supplier<Future<Buffer>> bodySupplier) {
Map<String, RequestParameter> cookies = new HashMap<>();
Map<String, RequestParameter> headers = new HashMap<>();
Map<String, RequestParameter> pathParams = new HashMap<>();
Expand Down Expand Up @@ -137,7 +137,7 @@ private static RequestParameter extractQuery(HttpServerRequest request, Paramete
}

private static RequestParameter joinFormValues(Collection<String> formValues, Parameter parameter,
Supplier<RequestParameter> explodedObjectSupplier) {
Supplier<RequestParameter> explodedObjectSupplier) {
if (formValues.isEmpty()) {
return EMPTY;
}
Expand Down Expand Up @@ -170,7 +170,7 @@ public static int findPathSegment(String templatePath, String parameterName) {

static String decodeUrl(String encoded) {
try {
return encoded == null ? null : URLDecoder.decode(encoded, StandardCharsets.UTF_8.name());
return encoded == null ? null : URLDecoder.decode(encoded, StandardCharsets.UTF_8);
} catch (Exception e) {
throw new ValidatorException("Can't decode URL value: " + encoded, ILLEGAL_VALUE, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ public static ContentAnalyser getContentAnalyser(MediaType mediaType, String con
}
}

protected String contentType;
protected Buffer content;
protected ValidationContext requestOrResponse;
protected final String contentType;
protected final Buffer content;
protected final ValidationContext requestOrResponse;

/**
* Creates a new content analyser.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ protected static Map<String, ResponseParameter> safeUnmodifiableMap(Map<String,
entry -> entry.getKey().toLowerCase(), Entry::getValue));

return Collections.unmodifiableMap(map == null ? Collections.emptyMap() :
new HashMap<String, ResponseParameter>(lowerCaseHeader) {
new HashMap<>(lowerCaseHeader) {
@Override
public ResponseParameter get(Object key) {
return super.get(key.toString().toLowerCase());
Expand Down
10 changes: 0 additions & 10 deletions src/test/java/io/vertx/tests/MockHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@

package io.vertx.tests;

import io.vertx.core.buffer.Buffer;
import io.vertx.json.schema.JsonSchema;
import io.vertx.json.schema.common.dsl.SchemaType;
import io.vertx.openapi.contract.Location;
import io.vertx.openapi.contract.Parameter;
import io.vertx.openapi.contract.Style;
import io.vertx.openapi.validation.ValidatableRequest;
import io.vertx.openapi.validation.impl.RequestParameterImpl;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -30,13 +27,6 @@ private MockHelper() {

}

public static ValidatableRequest mockValidatableRequest(Buffer body, String contentType) {
ValidatableRequest mockedRequest = mock(ValidatableRequest.class);
when(mockedRequest.getBody()).thenReturn(new RequestParameterImpl(body));
when(mockedRequest.getContentType()).thenReturn(contentType);
return mockedRequest;
}

public static Parameter mockParameter(String name, Location in, Style style, boolean explode, JsonSchema schema) {
return mockParameter(name, in, style, explode, schema, false);
}
Expand Down
16 changes: 8 additions & 8 deletions src/test/java/io/vertx/tests/contract/OpenAPIContractTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ void testValidJsonSchemaProvidedAsAdditionalSpecFiles(Vertx vertx, VertxTestCont
JsonObject validComponents = loadJson(vertx, resourcePath.resolve("components.json"));

Map<String, JsonObject> additionalValidSpecFiles = ImmutableMap.of("https://example.com/petstore", validComponents);
Map<String, JsonObject> additionalInvalidSpecFiles = ImmutableMap.of("https://example.com/petstore", invalidComponents);
Map<String, JsonObject> additionalInvalidSpecFiles = ImmutableMap.of("https://example.com/petstore",
invalidComponents);

OpenAPIContract.from(vertx, contract.copy(), additionalValidSpecFiles)
.compose(validResp -> Future.succeededFuture(validResp.getRawContract()))
Expand All @@ -185,7 +186,8 @@ void testMalformedJsonSchemaProvidedAsAdditionalSpecFiles(Vertx vertx, VertxTest
JsonObject contract = loadJson(vertx, resourcePath.resolve("petstore.json"));
JsonObject malformedComponents = loadJson(vertx, resourcePath.resolve("malformedComponents.json"));

Map<String, JsonObject> additionalMalformedSpecFiles = ImmutableMap.of("https://example.com/petstore", malformedComponents);
Map<String, JsonObject> additionalMalformedSpecFiles = ImmutableMap.of("https://example.com/petstore",
malformedComponents);

OpenAPIContract.from(vertx, contract.copy(), additionalMalformedSpecFiles)
.onComplete(handler -> testContext.verify(() -> {
Expand All @@ -208,12 +210,10 @@ public void testAdditionalSchemaFiles(Vertx vertx, VertxTestContext testContext)

Map<String, String> additionalSpecFiles = ImmutableMap.of("https://schemas/Name.yaml", componentsPath.toString());
OpenAPIContract.from(vertx, contractPath.toString(), additionalSpecFiles)
.onComplete(testContext.succeeding(c -> {
testContext.verify(() -> {
assertThat(c.getRawContract().toString()).isEqualTo(dereferenced.toString());
testContext.completeNow();
});
}));
.onComplete(testContext.succeeding(c -> testContext.verify(() -> {
assertThat(c.getRawContract().toString()).isEqualTo(dereferenced.toString());
testContext.completeNow();
})));
}

}
29 changes: 15 additions & 14 deletions src/test/java/io/vertx/tests/contract/OpenAPIVersionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import io.vertx.junit5.VertxTestContext;
import io.vertx.openapi.contract.OpenAPIContractException;
import io.vertx.openapi.contract.OpenAPIVersion;
import io.vertx.tests.ResourceHelper;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand All @@ -40,10 +39,10 @@
import java.util.stream.Stream;

import static com.google.common.truth.Truth.assertThat;
import static io.vertx.tests.ResourceHelper.TEST_RESOURCE_PATH;
import static io.vertx.openapi.impl.Utils.EMPTY_JSON_OBJECT;
import static io.vertx.openapi.contract.OpenAPIVersion.V3_0;
import static io.vertx.openapi.contract.OpenAPIVersion.V3_1;
import static io.vertx.openapi.impl.Utils.EMPTY_JSON_OBJECT;
import static io.vertx.tests.ResourceHelper.TEST_RESOURCE_PATH;
import static io.vertx.tests.ResourceHelper.getRelatedTestResourcePath;
import static io.vertx.tests.ResourceHelper.loadJson;
import static java.util.concurrent.TimeUnit.SECONDS;
Expand Down Expand Up @@ -91,7 +90,7 @@ void validateContractTest(OpenAPIVersion version, Path contractFile, Vertx vertx
@MethodSource(value = "provideVersionAndInvalidSpec")
@Timeout(value = 2, timeUnit = SECONDS)
void validateContractTestError(OpenAPIVersion version, Path contractFile, Consumer<OutputUnit> validator, Vertx vertx,
VertxTestContext testContext) {
VertxTestContext testContext) {
JsonObject contract = vertx.fileSystem().readFileBlocking(contractFile.toString()).toJsonObject();
version.getRepository(vertx, DUMMY_BASE_URI).compose(repo -> version.validateContract(vertx, repo, contract))
.onComplete(testContext.succeeding(res -> {
Expand Down Expand Up @@ -139,7 +138,8 @@ void testFromSpec(OpenAPIVersion version, Path specFile, Vertx vertx) {
@DisplayName("fromSpec should throw exception if field openapi doesn't exist or the version isn't supported")
void testFromSpecException() {
String expectedInvalidMsg = "The passed OpenAPI contract is invalid: Field \"openapi\" is missing";
Assertions.assertThrows(OpenAPIContractException.class, () -> OpenAPIVersion.fromContract(null), expectedInvalidMsg);
Assertions.assertThrows(OpenAPIContractException.class, () -> OpenAPIVersion.fromContract(null),
expectedInvalidMsg);
assertThrows(OpenAPIContractException.class, () -> OpenAPIVersion.fromContract(EMPTY_JSON_OBJECT),
expectedInvalidMsg);

Expand All @@ -148,6 +148,7 @@ void testFromSpecException() {
assertThrows(OpenAPIContractException.class, () -> OpenAPIVersion.fromContract(unsupportedContract),
expectedUnsupportedMsg);
}

@ParameterizedTest(name = "{index} should be able to validate additional files against the json schema for {0}")
@EnumSource(OpenAPIVersion.class)
@Timeout(value = 2, timeUnit = SECONDS)
Expand All @@ -158,15 +159,15 @@ public void testValidationOfAdditionalSchemaFiles(OpenAPIVersion version, Vertx


version.getRepository(vertx, "https://vertx.io")
.onSuccess(repository -> version.validateAdditionalContractFile(vertx, repository, validJsonSchema)
.onFailure(testContext::failNow)
.onSuccess(ignored -> version.validateAdditionalContractFile(vertx, repository, malformedJsonSchema)
.onComplete(handler -> testContext.verify(() -> {
assertThat(handler.failed()).isTrue();
assertThat(handler.cause()).isInstanceOf(JsonSchemaValidationException.class);
assertThat(handler.cause()).hasMessageThat().isEqualTo("-1 is less than 0");
testContext.completeNow();
}))));
.onSuccess(repository -> version.validateAdditionalContractFile(vertx, repository, validJsonSchema)
.onFailure(testContext::failNow)
.onSuccess(ignored -> version.validateAdditionalContractFile(vertx, repository, malformedJsonSchema)
.onComplete(handler -> testContext.verify(() -> {
assertThat(handler.failed()).isTrue();
assertThat(handler.cause()).isInstanceOf(JsonSchemaValidationException.class);
assertThat(handler.cause()).hasMessageThat().isEqualTo("-1 is less than 0");
testContext.completeNow();
}))));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import io.vertx.openapi.validation.impl.RequestValidatorImpl;
import org.junit.jupiter.api.Test;

import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import io.vertx.openapi.validation.impl.ResponseValidatorImpl;
import org.junit.jupiter.api.Test;

import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void testCheckSyntacticalCorrectnessThrows() {
ApplicationJsonAnalyser analyser = new ApplicationJsonAnalyser(APPLICATION_JSON.toString(), Buffer.buffer(
"\"foobar"), REQUEST);

ValidatorException exception = assertThrows(ValidatorException.class, () -> analyser.checkSyntacticalCorrectness());
ValidatorException exception = assertThrows(ValidatorException.class, analyser::checkSyntacticalCorrectness);
String expectedMsg = "The request body can't be decoded";
assertThat(exception.type()).isEqualTo(ILLEGAL_VALUE);
assertThat(exception).hasMessageThat().isEqualTo(expectedMsg);
Expand Down
Loading

0 comments on commit 1eb5cc9

Please sign in to comment.