-
Notifications
You must be signed in to change notification settings - Fork 141
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #805 from zalando/714-query-parameter-collection-f…
…ormat-rule-migration 714 query parameter collection format rule migration
- Loading branch information
Showing
7 changed files
with
134 additions
and
164 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 16 additions & 33 deletions
49
server/src/main/java/de/zalando/zally/rule/zalando/QueryParameterCollectionFormatRule.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,46 +1,29 @@ | ||
| package de.zalando.zally.rule.zalando | ||
|
|
||
| import de.zalando.zally.rule.api.Check | ||
| import de.zalando.zally.rule.api.Context | ||
| import de.zalando.zally.rule.api.Rule | ||
| import de.zalando.zally.rule.api.Severity | ||
| import de.zalando.zally.rule.api.Violation | ||
| import io.swagger.models.Swagger | ||
| import io.swagger.models.parameters.Parameter | ||
| import io.swagger.models.parameters.QueryParameter | ||
| import de.zalando.zally.util.getAllParameters | ||
| import io.swagger.v3.oas.models.parameters.Parameter | ||
|
|
||
| @Rule( | ||
| ruleSet = ZalandoRuleSet::class, | ||
| id = "154", | ||
| severity = Severity.SHOULD, | ||
| title = "Explicitly define the Collection Format of Query Parameters" | ||
| ruleSet = ZalandoRuleSet::class, | ||
| id = "154", | ||
| severity = Severity.SHOULD, | ||
| title = "Use and Specify Explicitly the Form-Style Query Format for Collection Parameters" | ||
| ) | ||
| class QueryParameterCollectionFormatRule { | ||
| private val formatsAllowed = listOf("csv", "multi") | ||
| private val violationDescription = "CollectionFormat should be one of: $formatsAllowed" | ||
| private val allowedStyle = Parameter.StyleEnum.FORM | ||
| private val description = "Parameter style have to be `form`" | ||
|
|
||
| @Check(severity = Severity.SHOULD) | ||
| fun validate(swagger: Swagger): Violation? { | ||
| fun Collection<Parameter>?.extractInvalidQueryParam(path: String) = | ||
| orEmpty().filterIsInstance<QueryParameter>() | ||
| .filter { it.`type` == "array" && it.collectionFormat !in formatsAllowed } | ||
| .map { path to it.name } | ||
|
|
||
| val fromParams = swagger.parameters.orEmpty().values.extractInvalidQueryParam("parameters") | ||
| val fromPaths = swagger.paths.orEmpty().entries.flatMap { (name, path) -> | ||
| path.parameters.extractInvalidQueryParam(name) + path.operations.flatMap { operation -> | ||
| operation.parameters.extractInvalidQueryParam(name) | ||
| } | ||
| } | ||
|
|
||
| val allHeaders = fromParams + fromPaths | ||
| val paths = allHeaders | ||
| .map { "${it.first} ${it.second}" } | ||
| .distinct() | ||
|
|
||
| return if (paths.isNotEmpty()) createViolation(paths) else null | ||
| } | ||
|
|
||
| fun createViolation(paths: List<String>): Violation { | ||
| return Violation(violationDescription, paths) | ||
| } | ||
| fun checkParametersCollectionFormat(context: Context): List<Violation> = | ||
| if (context.isOpenAPI3()) | ||
| context.api.getAllParameters().values | ||
| .filter { "query" == it.`in` && "array" == it.schema.type } | ||
| .filter { it.style == null || allowedStyle != it.style } | ||
| .map { context.violation(description, it) } | ||
| else emptyList() | ||
| } |
25 changes: 0 additions & 25 deletions
25
server/src/main/java/de/zalando/zally/util/OpenAPIObjectUtils.kt
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
139 changes: 60 additions & 79 deletions
139
server/src/test/java/de/zalando/zally/rule/zalando/QueryParameterCollectionFormatRuleTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,97 +1,78 @@ | ||
| package de.zalando.zally.rule.zalando | ||
|
|
||
| import io.swagger.models.Operation | ||
| import io.swagger.models.Path | ||
| import io.swagger.models.Swagger | ||
| import io.swagger.models.parameters.QueryParameter | ||
| import de.zalando.zally.getOpenApiContextFromContent | ||
| import de.zalando.zally.getSwaggerContextFromContent | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.intellij.lang.annotations.Language | ||
| import org.junit.Test | ||
|
|
||
| class QueryParameterCollectionFormatRuleTest { | ||
|
|
||
| private val rule = QueryParameterCollectionFormatRule() | ||
|
|
||
| @Test | ||
| fun negativeCaseCollectionFormatNotSupported() { | ||
| val swagger = Swagger().apply { | ||
| parameters = mapOf("test" to QueryParameter().apply { name = "test"; type = "array"; collectionFormat = "notSupported" }) | ||
| } | ||
|
|
||
| val result = rule.validate(swagger)!! | ||
| assertThat(result.description).isEqualTo("CollectionFormat should be one of: [csv, multi]") | ||
| assertThat(result.paths).isEqualTo(listOf("parameters test")) | ||
| } | ||
|
|
||
| @Test | ||
| fun negativeCaseCollectionFormatNotSupportedFromPath() { | ||
| val paramList = listOf(QueryParameter().apply { name = "test"; type = "array"; collectionFormat = "notSupported" }) | ||
| val swagger = Swagger().apply { | ||
| paths = mapOf("/apis" to Path().apply { get = Operation().apply { parameters = paramList } }) | ||
| } | ||
|
|
||
| val result = rule.validate(swagger)!! | ||
| assertThat(result.description).isEqualTo("CollectionFormat should be one of: [csv, multi]") | ||
| assertThat(result.paths).isEqualTo(listOf("/apis test")) | ||
| } | ||
|
|
||
| @Test | ||
| fun negativeCaseCollectionFormatNull() { | ||
| val swagger = Swagger().apply { | ||
| parameters = mapOf("test" to QueryParameter().apply { name = "test"; type = "array"; collectionFormat = null }) | ||
| } | ||
|
|
||
| val result = rule.validate(swagger)!! | ||
| assertThat(result.description).isEqualTo("CollectionFormat should be one of: [csv, multi]") | ||
| assertThat(result.paths).isEqualTo(listOf("parameters test")) | ||
| fun `checkParametersCollectionFormat should return violation if invalid collection format is set`() { | ||
| @Language("YAML") | ||
| val content = """ | ||
| openapi: 3.0.1 | ||
| components: | ||
| parameters: | ||
| filters: | ||
| in: query | ||
| style: spaceDelimited | ||
| schema: | ||
| type: array | ||
| """.trimIndent() | ||
| val context = getOpenApiContextFromContent(content) | ||
|
|
||
| val violations = rule.checkParametersCollectionFormat(context) | ||
|
|
||
| assertThat(violations).isNotEmpty | ||
| assertThat(violations.size).isEqualTo(1) | ||
| assertThat(violations[0].description).isEqualTo("Parameter style have to be `form`") | ||
| assertThat(violations[0].pointer.toString()).isEqualTo("/components/parameters/filters") | ||
| } | ||
|
|
||
| @Test | ||
| fun negativeCaseCollectionFormatNullFromPath() { | ||
| val paramList = listOf(QueryParameter().apply { name = "test"; type = "array"; collectionFormat = null }) | ||
| val swagger = Swagger().apply { | ||
| paths = mapOf("/apis" to Path().apply { get = Operation().apply { parameters = paramList } }) | ||
| } | ||
|
|
||
| val result = rule.validate(swagger)!! | ||
| assertThat(result.description).isEqualTo("CollectionFormat should be one of: [csv, multi]") | ||
| assertThat(result.paths).isEqualTo(listOf("/apis test")) | ||
| fun `checkParametersCollectionFormat should return no violation if valid collection format is set`() { | ||
| @Language("YAML") | ||
| val content = """ | ||
| openapi: 3.0.1 | ||
| components: | ||
| parameters: | ||
| filters: | ||
| in: query | ||
| style: form | ||
| schema: | ||
| type: array | ||
| """.trimIndent() | ||
| val context = getOpenApiContextFromContent(content) | ||
|
|
||
| val violations = rule.checkParametersCollectionFormat(context) | ||
|
|
||
| assertThat(violations).isEmpty() | ||
| } | ||
|
|
||
| @Test | ||
| fun positiveCaseCsv() { | ||
| val swagger = Swagger().apply { | ||
| parameters = mapOf("test" to QueryParameter().apply { name = "test"; type = "array"; collectionFormat = "csv" }) | ||
| } | ||
|
|
||
| assertThat(rule.validate(swagger)).isNull() | ||
| } | ||
|
|
||
| @Test | ||
| fun positiveCaseCsvFromPath() { | ||
| val paramList = listOf(QueryParameter().apply { name = "test"; type = "array"; collectionFormat = "csv" }) | ||
| val swagger = Swagger().apply { | ||
| paths = mapOf("/apis" to Path().apply { get = Operation().apply { parameters = paramList } }) | ||
| } | ||
|
|
||
| assertThat(rule.validate(swagger)).isNull() | ||
| } | ||
|
|
||
| @Test | ||
| fun positiveCaseMulti() { | ||
| val swagger = Swagger().apply { | ||
| parameters = mapOf("test" to QueryParameter().apply { name = "test"; type = "array"; collectionFormat = "multi" }) | ||
| } | ||
|
|
||
| assertThat(rule.validate(swagger)).isNull() | ||
| } | ||
|
|
||
| @Test | ||
| fun positiveCaseMultiFromPath() { | ||
| val paramList = listOf(QueryParameter().apply { name = "test"; type = "array"; collectionFormat = "multi" }) | ||
| val swagger = Swagger().apply { | ||
| paths = mapOf("/apis" to Path().apply { get = Operation().apply { parameters = paramList } }) | ||
| } | ||
|
|
||
| assertThat(rule.validate(swagger)).isNull() | ||
| fun `checkParametersCollectionFormat should ignore OpenAPI 2 (Swagger) specifications`() { | ||
| @Language("YAML") | ||
| val content = """ | ||
| swagger: 2.0 | ||
| info: | ||
| title: Old API | ||
| version: 1.0.0 | ||
| parameters: | ||
| filters: | ||
| name: filters | ||
| in: query | ||
| type: array | ||
| items: | ||
| type: string | ||
| """.trimIndent() | ||
| val context = getSwaggerContextFromContent(content) | ||
|
|
||
| val violations = rule.checkParametersCollectionFormat(context) | ||
|
|
||
| assertThat(violations).isEmpty() | ||
| } | ||
| } |