Skip to content

Commit

Permalink
Merge pull request #730 from roxspring/feature/gh-725-convenience-uti…
Browse files Browse the repository at this point in the history
…lities

AvoidTrailingSlashesRule & test use Context and new convenience utility methods
  • Loading branch information
maxim-tschumak committed Jun 19, 2018
2 parents 98e5d17 + 62d4c3b commit 63050e3
Show file tree
Hide file tree
Showing 10 changed files with 221 additions and 28 deletions.
40 changes: 40 additions & 0 deletions .gitattributes
@@ -0,0 +1,40 @@
# Set the default behavior, in case people don't have core.autocrlf set.
* text=auto

# Explicitly declare text files you want to always be normalized and converted
# to native line endings on checkout.

Dockerfile text

*.conf text
*.go text
*.gradle text
*.html text
*.java text
*.js text
*.json text
*.jsx text
*.kt text
*.lock text
*.md text
*.properties text
*.pug text
*.scss text
*.sql text
*.toml text
*.txt text
*.yaml text
*.yml text

# Declare files that will always have CRLF line endings on checkout.
*.bat crlf eol=crlf

# Declare files that will always have LF line endings on checkout.
gradlew text eol=lf
*.sh text eol=lf

# Denote all files that are truly binary and should not be modified.
*.ico binary
*.jar binary
*.jpg binary
*.png binary
4 changes: 2 additions & 2 deletions cli/zally/integration_test.go
Expand Up @@ -135,7 +135,7 @@ func TestIntegrationWithZallyApiDefinition(t *testing.T) {
t.Run("integrationWithZallyApiDefinition", func(t *testing.T) {
out, e := RunAppAndCaptureOutput([]string{"", "lint", "../../server/src/main/resources/api/zally-api.yaml"})

assert.Contains(t, out, "[36mHINT[0m [36mNot Specify Standard Error Codes[0m")
assert.Contains(t, out, "Not Specify Standard Error Codes")

assert.Contains(t, out, "MUST violations: 0")
assert.Contains(t, out, "SHOULD violations: 0")
Expand All @@ -150,7 +150,7 @@ func TestIntegrationDisplayRulesList(t *testing.T) {
t.Run("integrationDisplayRulesList", func(t *testing.T) {
out, e := RunAppAndCaptureOutput([]string{"", "rules"})

assert.Contains(t, out, "[31m166[0m [31mMUST[0m: Avoid Link in Header Rule")
assert.Contains(t, out, "Avoid Link in Header Rule")
assert.Contains(t, out, "https://zalando.github.io/restful-api-guidelines/#166")

assert.Nil(t, e)
Expand Down
35 changes: 35 additions & 0 deletions server/src/main/java/de/zalando/zally/rule/Context.kt
Expand Up @@ -8,6 +8,7 @@ import de.zalando.zally.util.ast.ReverseAst
import io.swagger.models.Swagger
import io.swagger.parser.SwaggerParser
import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.oas.models.PathItem
import io.swagger.v3.parser.OpenAPIV3Parser
import io.swagger.v3.parser.converter.SwaggerConverter
import io.swagger.v3.parser.core.models.ParseOptions
Expand All @@ -21,6 +22,40 @@ class Context(openApi: OpenAPI, swagger: Swagger? = null) {

val api = recorder.proxy

/**
* Convenience method for filtering and iterating over the paths in order to create Violations.
* @param pathFilter a filter selecting the paths to validate
* @param action the action to perform on filtered items
* @return a list of Violations and/or nulls where no violations are necessary
*/
fun validatePaths(
pathFilter: (Map.Entry<String, PathItem>) -> Boolean = { true },
action: (Map.Entry<String, PathItem>) -> List<Violation?>
): List<Violation> = api.paths
.orEmpty()
.filter(pathFilter)
.flatMap(action)
.filterNotNull()

/**
* Creates a List of one Violation with a pointer to the OpenAPI or Swagger model node specified,
* defaulting to the last recorded location.
* @param description the description of the Violation
* @param value the OpenAPI or Swagger model node
* @return the new Violation
*/
fun violations(description: String, value: Any): List<Violation> =
listOf(violation(description, value))

/**
* Creates a List of one Violation with the specified pointer, defaulting to the last recorded location.
* @param description the description of the Violation
* @param pointer an existing pointer or null
* @return the new Violation
*/
fun violations(description: String, pointer: JsonPointer?): List<Violation> =
listOf(violation(description, pointer))

/**
* Creates a Violation with a pointer to the OpenAPI or Swagger model node specified,
* defaulting to the last recorded location.
Expand Down
@@ -1,11 +1,11 @@
package de.zalando.zally.rule.zalando

import de.zalando.zally.rule.Context
import de.zalando.zally.rule.api.Check
import de.zalando.zally.rule.api.Rule
import de.zalando.zally.rule.api.Severity
import de.zalando.zally.rule.api.Violation
import de.zalando.zally.util.PatternUtil
import io.swagger.models.Swagger

@Rule(
ruleSet = ZalandoRuleSet::class,
Expand All @@ -17,8 +17,10 @@ class AvoidTrailingSlashesRule {
private val description = "Rule avoid trailing slashes is not followed"

@Check(severity = Severity.MUST)
fun validate(swagger: Swagger): Violation? {
val paths = swagger.paths.orEmpty().keys.filter { it != null && PatternUtil.hasTrailingSlash(it) }
return if (!paths.isEmpty()) Violation(description, paths) else null
}
fun validate(context: Context): List<Violation> =
context.validatePaths(
pathFilter = { PatternUtil.hasTrailingSlash(it.key) }
) {
context.violations(description, it.value)
}
}
33 changes: 33 additions & 0 deletions server/src/test/java/de/zalando/zally/rule/ViolationAssert.kt
@@ -0,0 +1,33 @@
package de.zalando.zally.rule

import de.zalando.zally.rule.api.Violation
import org.assertj.core.api.AbstractAssert
import org.assertj.core.api.StringAssert

@Suppress("UndocumentedPublicClass")
class ViolationAssert(actual: Violation?) : AbstractAssert<ViolationAssert, Violation?>(actual, ViolationAssert::class.java) {

fun descriptionEqualTo(description: String): ViolationAssert {
description().isEqualTo(description)
return this
}

fun descriptionMatches(description: String): ViolationAssert {
description().matches(description)
return this
}

private fun description() = StringAssert(actual?.description).`as`("description")

fun pointerEqualTo(pointer: String): ViolationAssert {
pointer().isEqualTo(pointer)
return this
}

fun pointerMatches(pointer: String): ViolationAssert {
pointer().matches(pointer)
return this
}

private fun pointer() = StringAssert(actual?.pointer?.toString()).`as`("pointer")
}
35 changes: 35 additions & 0 deletions server/src/test/java/de/zalando/zally/rule/ViolationsAssert.kt
@@ -0,0 +1,35 @@
package de.zalando.zally.rule

import de.zalando.zally.rule.api.Violation
import org.assertj.core.api.AbstractListAssert
import org.assertj.core.api.ListAssert
import org.assertj.core.api.ObjectAssert

@Suppress("UndocumentedPublicClass", "SpreadOperator")
class ViolationsAssert(violations: List<Violation>?) : AbstractListAssert<ViolationsAssert, List<Violation>, Violation, ObjectAssert<Violation>>(violations, ViolationsAssert::class.java) {

override fun toAssert(value: Violation?, description: String?): ObjectAssert<Violation> {
return ObjectAssert<Violation>(value).`as`(description)
}

fun descriptionsAllEqualTo(description: String): ViolationsAssert {
descriptions().containsOnly(description)
return this
}

fun descriptionsEqualTo(vararg descriptions: String): ViolationsAssert {
descriptions().containsExactly(*descriptions)
return this
}

private fun descriptions(): ListAssert<String> {
isNotNull
return ListAssert(actual.map { it.description }).`as`("descriptions")
}

fun pointersEqualTo(vararg pointers: String): ViolationsAssert {
isNotNull
ListAssert(actual.map { it.pointer?.toString() }).`as`("pointers").containsExactly(*pointers)
return this
}
}
13 changes: 13 additions & 0 deletions server/src/test/java/de/zalando/zally/rule/ZallyAssertions.kt
@@ -0,0 +1,13 @@
package de.zalando.zally.rule

import de.zalando.zally.rule.api.Violation

@Suppress("UndocumentedPublicClass")
class ZallyAssertions {
companion object {

fun assertThat(actual: Violation?): ViolationAssert = ViolationAssert(actual)

fun assertThat(actual: List<Violation>?): ViolationsAssert = ViolationsAssert(actual)
}
}
@@ -1,8 +1,8 @@
package de.zalando.zally.rule.zalando

import de.zalando.zally.rule.Context
import de.zalando.zally.rule.ZallyAssertions.Companion.assertThat
import de.zalando.zally.testConfig
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test

class ApiAudienceRuleTest {
Expand All @@ -13,25 +13,30 @@ class ApiAudienceRuleTest {
fun correctApiAudienceIsSet() {
val context = withAudience("company-internal")

assertThat(rule.validate(context)).isNull()
val violation = rule.validate(context)

assertThat(violation)
.isNull()
}

@Test
fun incorrectAudienceIsSet() {
val context = withAudience("not-existing-audience")
val violation = rule.validate(context)

assertThat(violation?.pointer).hasToString("/info/x-audience")
assertThat(violation?.description).matches(".*doesn't match.*")
assertThat(violation)
.pointerEqualTo("/info/x-audience")
.descriptionMatches(".*doesn't match.*")
}

@Test
fun noApiAudienceIsSet() {
val context = withAudience("null")
val violation = rule.validate(context)

assertThat(violation?.pointer).hasToString("/info/x-audience")
assertThat(violation?.description).matches(".*Audience must be provided.*")
assertThat(violation)
.pointerEqualTo("/info/x-audience")
.descriptionMatches(".*Audience must be provided.*")
}

private fun withAudience(apiAudience: String): Context {
Expand Down
@@ -1,7 +1,7 @@
package de.zalando.zally.rule.zalando

import de.zalando.zally.rule.Context
import org.assertj.core.api.Assertions.assertThat
import de.zalando.zally.rule.ZallyAssertions.Companion.assertThat
import org.junit.Test

class ApiIdentifierRuleTest {
Expand All @@ -12,25 +12,30 @@ class ApiIdentifierRuleTest {
fun correctApiIdIsSet() {
val context = withApiId("zally-api")

assertThat(rule.validate(context)).isNull()
val violation = rule.validate(context)

assertThat(violation)
.isNull()
}

@Test
fun incorrectIdIsSet() {
val context = withApiId("This?iS//some|Incorrect+&ApI)(id!!!")
val violation = rule.validate(context)!!

assertThat(violation.pointer).hasToString("/info/x-api-id")
assertThat(violation.description).matches(".*doesn't match.*")
assertThat(violation)
.pointerEqualTo("/info/x-api-id")
.descriptionMatches(".*doesn't match.*")
}

@Test
fun noApiIdIsSet() {
val context = withApiId("null")
val violation = rule.validate(context)!!

assertThat(violation.pointer).hasToString("/info/x-api-id")
assertThat(violation.description).matches(".*should be provided.*")
assertThat(violation)
.pointerEqualTo("/info/x-api-id")
.descriptionMatches(".*should be provided.*")
}

private fun withApiId(apiId: String): Context {
Expand Down
@@ -1,29 +1,54 @@
package de.zalando.zally.rule.zalando

import de.zalando.zally.swaggerWithPaths
import io.swagger.models.Swagger
import org.assertj.core.api.Assertions.assertThat
import de.zalando.zally.rule.Context
import de.zalando.zally.rule.ZallyAssertions.Companion.assertThat
import org.junit.Test

@Suppress("StringLiteralDuplication", "UndocumentedPublicClass", "UnsafeCallOnNullableType")
class AvoidTrailingSlashesRuleTest {

private val rule = AvoidTrailingSlashesRule()

@Test
fun emptySwagger() {
assertThat(rule.validate(Swagger())).isNull()
val context = Context.createOpenApiContext("""
openapi: '3.0.0'
""".trimIndent())!!

val violations = rule.validate(context)

assertThat(violations).isEmpty()
}

@Test
fun positiveCase() {
val testAPI = swaggerWithPaths("/api/test-api")
assertThat(rule.validate(testAPI)).isNull()
val context = Context.createOpenApiContext("""
openapi: '3.0.0'
paths:
/api/test-api: {}
""".trimIndent())!!

val violations = rule.validate(context)

assertThat(violations).isEmpty()
}

@Test
fun negativeCase() {
val testAPI = swaggerWithPaths("/api/test-api/", "/api/test", "/some/other/path", "/long/bad/path/with/slash/")
assertThat(rule.validate(testAPI)!!.paths).hasSameElementsAs(
listOf("/api/test-api/", "/long/bad/path/with/slash/"))
val context = Context.createOpenApiContext("""
openapi: '3.0.0'
paths:
/api/test-api/: {}
/api/test: {}
/some/other/path: {}
/long/bad/path/with/slash/: {}
""".trimIndent())!!

val violations = rule.validate(context)

assertThat(violations)
.descriptionsAllEqualTo("Rule avoid trailing slashes is not followed")
.pointersEqualTo("/paths/~1api~1test-api~1", "/paths/~1long~1bad~1path~1with~1slash~1")
.hasSize(2)
}
}

0 comments on commit 63050e3

Please sign in to comment.