b/162242995 Add named extension path to validation messages in schema validator Change-Id: Id444fb8e31bcc2baa52047167bfc438f40580be5
diff --git a/.gitignore b/.gitignore index d1d4fe8..473f8c5 100644 --- a/.gitignore +++ b/.gitignore
@@ -2,6 +2,7 @@ .gradle **/build/ **/out/ +**/generated/ !src/**/build !gradle-wrapper.jar
diff --git a/build.gradle b/build.gradle index 21d1f75..f82f37a 100644 --- a/build.gradle +++ b/build.gradle
@@ -18,6 +18,7 @@ // Core Libraries "guava": "29.0-jre", // 22 June 2020 "guice": "4.2.3", // 22 June 2020 + "autoValue": "1.7.4", // 2 August 2020 // Logging "flogger": "0.5.1", // 22 June 2020
diff --git a/oas-core/build.gradle b/oas-core/build.gradle index ff16ae4..908c2e3 100644 --- a/oas-core/build.gradle +++ b/oas-core/build.gradle
@@ -1,9 +1,13 @@ dependencies { implementation "org.openapi4j:openapi-parser:${libVersions.openapiParser}" implementation "org.openapi4j:openapi-core:${libVersions.openapiCore}" + implementation "com.google.auto.value:auto-value-annotations:${libVersions.autoValue}" implementation "com.google.inject.extensions:guice-assistedinject:${libVersions.guice}" implementation "com.networknt:json-schema-validator:${libVersions.jsonSchemaValidator}" implementation "org.slf4j:slf4j-api:${libVersions.slf4j}" testImplementation project(":oas-test") -} \ No newline at end of file + + annotationProcessor "com.google.auto.value:auto-value:${libVersions.autoValue}" +} +
diff --git a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtension.java b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtension.java index 3c456c7..5791cf2 100644 --- a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtension.java +++ b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtension.java
@@ -4,7 +4,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.inject.assistedinject.Assisted; -import com.networknt.schema.ValidationMessage; import java.util.Map; import java.util.Optional; import javax.inject.Inject; @@ -47,7 +46,7 @@ /** Validates the extension with given {@link ExtensionValidator}. */ @Override - public ImmutableSet<ValidationMessage> validate(ExtensionValidator extensionValidator) { + public ImmutableSet<ExtensionValidationMessage> validate(ExtensionValidator extensionValidator) { return extensionValidator.validate(this); } }
diff --git a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidator.java b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidator.java index d7095a3..edfa743 100644 --- a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidator.java +++ b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidator.java
@@ -2,6 +2,8 @@ import static com.apigee.security.oas.extendedvalidator.ExtensionName.valueOfExtensionName; import static com.apigee.security.oas.extendedvalidator.ExtensionValidators.defaultErrors; +import static com.apigee.security.oas.extendedvalidator.ExtensionValidators.prepareNamedPath; +import static com.apigee.security.oas.extendedvalidator.ExtensionValidators.toExtensionValidationMessages; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableSet; @@ -14,6 +16,7 @@ import java.net.URISyntaxException; import java.net.URL; import java.util.Optional; +import java.util.Set; import javax.inject.Inject; final class BaseExtensionSchemaValidator implements ExtensionSchemaValidator { @@ -31,50 +34,55 @@ this.jsonSchemaFactory = jsonSchemaFactory; } - private JsonSchema getJsonSchemaFromUrl(URL schemaUrl) throws URISyntaxException { - return jsonSchemaFactory.getSchema(schemaUrl.toURI()); - } - - /** Validates {@link Extension} by contrasting it against its appropriate schema. */ + /** + * Validates {@link Extension} by contrasting it against its appropriate schema. + * + * <p>The extension should be a part of {@link ExtensionName}, otherwise {@code + * UNSUPPORTED_EXTENSION} {@link ExtensionValidationMessage} is returned. + */ @Override - public ImmutableSet<ValidationMessage> validate(Extension extension) { - Optional<ExtensionName> extensionName = valueOfExtensionName(extension.getExtensionName()); - @Var ImmutableSet<ValidationMessage> errors = ImmutableSet.of(); + public ImmutableSet<ExtensionValidationMessage> validate(Extension extension) { + String extensionName = extension.getExtensionName(); + Optional<ExtensionName> extensionNameOptional = valueOfExtensionName(extensionName); + @Var ImmutableSet<ExtensionValidationMessage> errors = ImmutableSet.of(); - if (extensionName.isPresent()) { - JsonNode extensionContent = extension.getExtensionContent(); - switch (extensionName.get()) { + if (extensionNameOptional.isPresent()) { + switch (extensionNameOptional.get()) { case X_SECURITY_TYPE: case X_SECURITY_ALLOW: - errors = validateExtensionContent(LIST_WITH_STRINGS_SCHEMA_URL, extensionContent); + errors = validateExtensionContent(LIST_WITH_STRINGS_SCHEMA_URL, extension); break; case X_SECURITY_TYPE_DEFINITIONS: - errors = validateExtensionContent(SECURITY_DEFINITIONS_SCHEMA_URL, extensionContent); + errors = validateExtensionContent(SECURITY_DEFINITIONS_SCHEMA_URL, extension); break; } - - // TODO(b/162242995): Add named extension path to each validation message in errors. } else { errors = defaultErrors(extension); } - logger.atInfo().log("Schema Errors : %s", errors); + logger.atInfo().log( + "Schema Errors(%s at %s) : %s", extensionName, prepareNamedPath(extension), errors); return errors; } /** Validates an {@link JsonNode} against its {@link JsonSchema}. */ - private ImmutableSet<ValidationMessage> validateExtensionContent( - URL schemaUrl, JsonNode extensionContent) { + private ImmutableSet<ExtensionValidationMessage> validateExtensionContent( + URL schemaUrl, Extension extension) { try { JsonSchema schema = getJsonSchemaFromUrl(schemaUrl); - ImmutableSet<ValidationMessage> errors = - ImmutableSet.copyOf(schema.validate(extensionContent)); - return errors; + Set<ValidationMessage> errors = schema.validate(extension.getExtensionContent()); + + return toExtensionValidationMessages(errors, "INVALID_SCHEMA", prepareNamedPath(extension)); + } catch (URISyntaxException e) { - String errorMessage = "Failed to validate extension schema."; + String errorMessage = "Failed to validate extension schema"; logger.atSevere().log("%s due to %s", errorMessage, e.getMessage()); throw new ValidationException(errorMessage, e); } } + + private JsonSchema getJsonSchemaFromUrl(URL schemaUrl) throws URISyntaxException { + return jsonSchemaFactory.getSchema(schemaUrl.toURI()); + } }
diff --git a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/Extension.java b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/Extension.java index a18a880..1043b99 100644 --- a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/Extension.java +++ b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/Extension.java
@@ -3,7 +3,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.networknt.schema.ValidationMessage; import java.util.Map; import java.util.Optional; import org.openapi4j.parser.model.OpenApiSchema; @@ -12,7 +11,7 @@ public interface Extension { /** Validates the {@code Extension} with {@link ExtensionValidator}. */ - ImmutableSet<ValidationMessage> validate(ExtensionValidator extensionValidator); + ImmutableSet<ExtensionValidationMessage> validate(ExtensionValidator extensionValidator); /** Returns the content of the Extension. */ JsonNode getExtensionContent();
diff --git a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidationMessage.java b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidationMessage.java new file mode 100644 index 0000000..387da6c --- /dev/null +++ b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidationMessage.java
@@ -0,0 +1,29 @@ +package com.apigee.security.oas.extendedvalidator; + +import com.google.auto.value.AutoValue; + +/** Holds information about an error occurred during validation. */ +@AutoValue +abstract class ExtensionValidationMessage { + + static Builder builder() { + return new AutoValue_ExtensionValidationMessage.Builder(); + } + + @AutoValue.Builder + abstract static class Builder { + abstract Builder setType(String value); + + abstract Builder setMessage(String value); + + abstract Builder setPath(String value); + + abstract ExtensionValidationMessage build(); + } + + abstract String type(); + + abstract String message(); + + abstract String path(); +}
diff --git a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidator.java b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidator.java index 9d2d3a0..fc11cf0 100644 --- a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidator.java +++ b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidator.java
@@ -1,11 +1,10 @@ package com.apigee.security.oas.extendedvalidator; import com.google.common.collect.ImmutableSet; -import com.networknt.schema.ValidationMessage; /** Validates a {@link Extension} based on the type of {@code Extension}. */ public interface ExtensionValidator { /** Validates a {@link Extension}. */ - ImmutableSet<ValidationMessage> validate(Extension extension); + ImmutableSet<ExtensionValidationMessage> validate(Extension extension); }
diff --git a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidators.java b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidators.java index 6090e9b..d380ca7 100644 --- a/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidators.java +++ b/oas-core/src/main/java/com/apigee/security/oas/extendedvalidator/ExtensionValidators.java
@@ -2,14 +2,15 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.networknt.schema.CustomErrorMessageType; import com.networknt.schema.ValidationMessage; +import java.util.HashSet; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.openapi4j.parser.model.OpenApiSchema; /** - * Utility class to prepare path string and {@link ValidationMessage} for {@link + * Utility class to prepare path string and {@link ExtensionValidationMessage} for {@link * ExtensionValidator}. */ final class ExtensionValidators { @@ -17,44 +18,72 @@ /** * Converts an {@link ImmutableList} {@code extensionPath} into {@link String}. * - * <p>A named path is used for logging and passed as an parameter of {@link ValidationMessage} if - * an error occurs during validation. + * <p>A named path is used for logging and passed as an parameter of {@link + * ExtensionValidationMessage} if an error occurs during validation. */ - static String prepareNamedPath( - ImmutableList<Map.Entry<Class<? extends OpenApiSchema>, Optional<String>>> extensionPath) { + static String prepareNamedPath(Extension extension) { StringBuilder namedPath = new StringBuilder(); - if (extensionPath.size() > 0) { - for (Map.Entry<Class<? extends OpenApiSchema>, Optional<String>> pathMap : extensionPath) { - Optional<String> namedPathObject = pathMap.getValue(); + ImmutableList<Map.Entry<Class<? extends OpenApiSchema>, Optional<String>>> extensionPath = + extension.getExtensionPath(); + + if (!extensionPath.isEmpty()) { + for (Map.Entry<Class<? extends OpenApiSchema>, Optional<String>> pathMap : + extension.getExtensionPath()) { + pathMap + .getValue() + .ifPresentOrElse( + pathString -> namedPath.append(pathString), + () -> namedPath.append(pathMap.getKey().getSimpleName())); + namedPath.append(" -> "); - if (namedPathObject.isPresent()) { - namedPath.append(namedPathObject.get()); - } else { - namedPath.append(pathMap.getKey().getSimpleName()); - } } + + namedPath.append(extension.getExtensionName()); } return namedPath.toString(); } - static ValidationMessage prepareValidationMessage( - String errorCode, String errorType, String path) { - return ValidationMessage.of(errorType, CustomErrorMessageType.of(errorCode), path); + /** + * Converts a {@link ValidationMessage} Set to {@link ExtensionValidationMessage} ImmutableSet + * with absolute {@code path} and {@code error type} information. + * + * <p>This gives more clarity about the object path that the {@code ExtensionValidationMessage} is + * about and helps with better path logging for validation errors. + */ + static ImmutableSet<ExtensionValidationMessage> toExtensionValidationMessages( + Set<ValidationMessage> errors, String type, String path) { + if (errors.isEmpty()) { + return ImmutableSet.of(); + } + + Set<ExtensionValidationMessage> updatedErrors = new HashSet<>(); + for (ValidationMessage validationMessage : errors) { + String message = Optional.ofNullable(validationMessage.getMessage()).orElse(""); + String messagePath = Optional.ofNullable(validationMessage.getPath()).orElse(""); + + updatedErrors.add( + ExtensionValidationMessage.builder() + .setType(type) + .setMessage(message) + .setPath(messagePath.isEmpty() ? path : path + " -> " + messagePath) + .build()); + } + return ImmutableSet.copyOf(updatedErrors); } - /** - * Returns an {@link ImmutableSet} with {@code UNSUPPORTED_EXTENSION} error as a {@link - * ValidationMessage}. - * - * <p>Prepares a default error {@link ValidationMessage} for extensions without validators. - */ - static ImmutableSet<ValidationMessage> defaultErrors(Extension extension) { - String errorCode = "UNSUPPORTED_EXTENSION"; - String errorType = extension.getExtensionName() + " is not binded to the validator."; - String path = prepareNamedPath(extension.getExtensionPath()); - ValidationMessage message = prepareValidationMessage(errorCode, errorType, path); - return ImmutableSet.of(message); + // TODO(b/162938113) : Create validation error type and message constants. + /** Prepares {@link ExtensionValidationMessage} for extensions without validators. */ + static ImmutableSet<ExtensionValidationMessage> defaultErrors(Extension extension) { + String errorType = "UNSUPPORTED_EXTENSION"; + String errorMessage = extension.getExtensionName() + " is not binded to the validator."; + String path = prepareNamedPath(extension); + return ImmutableSet.of( + ExtensionValidationMessage.builder() + .setType(errorType) + .setMessage(errorMessage) + .setPath(path) + .build()); } private ExtensionValidators() {}
diff --git a/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidatorTest.java b/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidatorTest.java index 1cc69aa..b4e1e61 100644 --- a/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidatorTest.java +++ b/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/BaseExtensionSchemaValidatorTest.java
@@ -18,7 +18,6 @@ import com.google.inject.Guice; import com.google.inject.Injector; import com.networknt.schema.JsonSchemaFactory; -import com.networknt.schema.ValidationMessage; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -51,7 +50,6 @@ Resources.getResource("ListWithStringsSchema.json"); private static final URL SECURITY_DEFINITIONS_SCHEMA_URL = Resources.getResource("SecurityDefinitionsSchema.json"); - private static final Injector injector = Guice.createInjector(new ExtendedValidatorMainModule()); private static final JsonNode emptyContent = new ObjectMapper().valueToTree("[]"); private static final ImmutableList<Map.Entry<Class<? extends OpenApiSchema>, Optional<String>>> extensionPath = @@ -74,6 +72,7 @@ /** Setting up validator and factories. */ @Before public void setup() { + Injector injector = Guice.createInjector(new ExtendedValidatorMainModule()); validator = injector.getInstance(ExtensionSchemaValidator.class); factory = injector.getInstance(ExtensionFactory.class); } @@ -119,7 +118,7 @@ @Test public void validate_schemaParameterTests_returnsErrorSet() { Extension extension = factory.create(extensionName, content, extensionPath); - ImmutableSet<ValidationMessage> errors = validator.validate(extension); + ImmutableSet<ExtensionValidationMessage> errors = validator.validate(extension); assertThat(errors.isEmpty()).isEqualTo(isValid); } @@ -139,18 +138,19 @@ /** Sets up validator and extension factory. */ @Before public void setup() { + Injector injector = Guice.createInjector(new ExtendedValidatorMainModule()); factory = injector.getInstance(ExtensionFactory.class); } @Test - public void validate_unsupportedExtension_returnsUnsupportedExtensionErrorMessage() { + public void validate_unsupportedExtension_returnsUnsupportedExtensionErrorType() { Extension extension = factory.create("x-unsupported", emptyContent, extensionPath); - ImmutableSet<ValidationMessage> errors = validator.validate(extension); + ImmutableSet<ExtensionValidationMessage> errors = validator.validate(extension); assertThat(errors) .hasSize(1) .first() - .extracting(ValidationMessage::getCode) + .extracting(ExtensionValidationMessage::type) .isEqualTo("UNSUPPORTED_EXTENSION"); }
diff --git a/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/ExtensionValidatorsTest.java b/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/ExtensionValidatorsTest.java index 95155f2..671534d 100644 --- a/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/ExtensionValidatorsTest.java +++ b/oas-core/src/test/java/com/apigee/security/oas/extendedvalidator/ExtensionValidatorsTest.java
@@ -2,6 +2,7 @@ import static com.apigee.security.oas.extendedvalidator.ExtensionValidators.defaultErrors; import static com.apigee.security.oas.extendedvalidator.ExtensionValidators.prepareNamedPath; +import static com.apigee.security.oas.extendedvalidator.ExtensionValidators.toExtensionValidationMessages; import static org.assertj.core.api.Assertions.assertThat; import com.fasterxml.jackson.databind.JsonNode; @@ -13,53 +14,87 @@ import java.util.AbstractMap.SimpleImmutableEntry; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.openapi4j.parser.model.OpenApiSchema; +import org.openapi4j.parser.model.v3.OpenApi3; import org.openapi4j.parser.model.v3.Operation; import org.openapi4j.parser.model.v3.Path; @RunWith(JUnit4.class) public class ExtensionValidatorsTest { - private static final ExtensionFactory factory = - Guice.createInjector(new ExtensionModule()).getInstance(ExtensionFactory.class); - - private ImmutableList<Map.Entry<Class<? extends OpenApiSchema>, Optional<String>>> path; + private ExtensionFactory factory; + private JsonNode node; private Extension extension; /** Setting up {@link Extension} object. */ @Before public void setup() { + Map.Entry<Class<? extends OpenApiSchema>, Optional<String>> rootEntry = + new SimpleImmutableEntry<>(OpenApi3.class, Optional.empty()); Map.Entry<Class<? extends OpenApiSchema>, Optional<String>> pathEntry = - new SimpleImmutableEntry<Class<? extends OpenApiSchema>, Optional<String>>( - Path.class, Optional.of("/users")); + new SimpleImmutableEntry<>(Path.class, Optional.of("/users")); Map.Entry<Class<? extends OpenApiSchema>, Optional<String>> operationEntry = - new SimpleImmutableEntry<Class<? extends OpenApiSchema>, Optional<String>>( - Operation.class, Optional.of("get")); - path = ImmutableList.of(pathEntry, operationEntry); + new SimpleImmutableEntry<>(Operation.class, Optional.of("get")); + ImmutableList<Map.Entry<Class<? extends OpenApiSchema>, Optional<String>>> path = + ImmutableList.of(rootEntry, pathEntry, operationEntry); - JsonNode node = new ObjectMapper().valueToTree("[]"); + factory = Guice.createInjector(new ExtensionModule()).getInstance(ExtensionFactory.class); + node = new ObjectMapper().valueToTree("[]"); extension = factory.create("x-security", node, path); } @Test - public void prepareNamedPath_extensionPath_returnsStringWithNamedPath() { - String namedPath = prepareNamedPath(path); + public void prepareNamedPath_returnsStringWithNamedPath() { + String namedPath = prepareNamedPath(extension); assertThat(namedPath).contains("/users", "get"); } @Test - public void defaultErrors_returnsImmutableSetWithUnsupportedExtensionMessage() { - ImmutableSet<ValidationMessage> errors = defaultErrors(extension); + public void prepareNamedPath_emptyExtensionPath_returnsEmptyString() { + Extension extension = factory.create("x-security", node, ImmutableList.of()); + String namedPath = prepareNamedPath(extension); + + assertThat(namedPath).isEmpty(); + } + + @Test + public void toExtensionValidationMessages_messageWithPath_returnsMessagesWithUpdatedPath() { + ValidationMessage messageWithPath = new ValidationMessage.Builder().path("$[0]").build(); + ImmutableSet<ExtensionValidationMessage> errors = + toExtensionValidationMessages(Set.of(messageWithPath), "", "/users -> get"); assertThat(errors) + .first() + .extracting(ExtensionValidationMessage::path) + .asString() + .contains("/users", "get", "$[0]"); + } + + @Test + public void toExtensionValidationMessages_messageWithoutPath_returnsMessagesWithPassedPath() { + ValidationMessage messageWithoutPath = new ValidationMessage.Builder().build(); + ImmutableSet<ExtensionValidationMessage> errors = + toExtensionValidationMessages(Set.of(messageWithoutPath), "", "/users -> get"); + + assertThat(errors) + .first() + .extracting(ExtensionValidationMessage::path) + .asString() + .contains("/users", "get"); + } + + @Test + public void defaultErrors_returnsImmutableSetWithUnsupportedExtensionType() { + assertThat(defaultErrors(extension)) .hasSize(1) .first() - .extracting(ValidationMessage::getCode) + .extracting(ExtensionValidationMessage::type) .isEqualTo("UNSUPPORTED_EXTENSION"); } }