Merge pull request #721 from roxspring/feature/gh-715-typed-jsonpointer

Use Jackson's JsonPointer to represent violation locations

.travis.yml

@@ -23,17 +23,17 @@ install: true
 script:
   # Unit-test and build server
   - cd server/
-  - ./gradlew build --info
+  - ./gradlew --console=plain build --info
   - cd ..
 
   # Unit-test and build integration server
   - cd github-integration/
-  - ./gradlew build --info
+  - ./gradlew --console=plain build --info
   - cd ..
 
   # Launch local zally server
   - cd server/
-  - ./gradlew bootRun > /dev/null &
+  - ./gradlew --console=plain bootRun > /dev/null &
   - echo $! > /tmp/zally_server.pid
   - cd ..
 

cli/zally/domain/violation.go

@@ -6,5 +6,6 @@ type Violation struct {
 	Decription    string   `json:"description"`
 	ViolationType string   `json:"violation_type"`
 	RuleLink      string   `json:"rule_link"`
+	Pointer       string   `json:"pointer"`
 	Paths         []string `json:"paths"`
 }

cli/zally/domain/violations_test.go

@@ -12,6 +12,7 @@ func TestViolations(t *testing.T) {
 	mustViolation.RuleLink = "http://example.com/mustViolation"
 	mustViolation.ViolationType = "MUST"
 	mustViolation.Decription = "Must Description"
+	mustViolation.Pointer = "/pointer/1"
 	mustViolation.Paths = []string{"/path/one", "/path/two"}
 
 	var shouldViolation Violation
@@ -33,7 +34,7 @@ func TestViolations(t *testing.T) {
 	hintViolation.RuleLink = "http://example.com/hintViolation"
 	hintViolation.ViolationType = "HINT"
 	hintViolation.Decription = "Hint Description"
-	hintViolation.Paths = []string{"/path/seven", "/path/eight"}
+	hintViolation.Pointer = "/pointer/2"
 
 	var violationsCount ViolationsCount
 	violationsCount.Must = 1

cli/zally/utils/formatters/markdown_formatter.go

@@ -60,9 +60,15 @@ func (f *MarkdownFormatter) formatViolation(violation *domain.Violation) string
 
 	fmt.Fprintf(&buffer, "### `%s` [%s](%s)\n\n", violation.ViolationType, violation.Title, violation.RuleLink)
 	fmt.Fprintf(&buffer, "> %s\n\n", violation.Decription)
-	for _, path := range violation.Paths {
-		fmt.Fprintf(&buffer, "- %s\n", path)
+
+	if violation.Pointer != "" {
+		fmt.Fprintf(&buffer, "- %s\n", violation.Pointer)
+	} else {
+		for _, path := range violation.Paths {
+			fmt.Fprintf(&buffer, "- %s\n", path)
+		}
 	}
+
 	fmt.Fprintf(&buffer, "\n")
 
 	return buffer.String()

cli/zally/utils/formatters/markdown_formatter_test.go

@@ -109,13 +109,14 @@ func TestMarkdownFormatServerMessage(t *testing.T) {
 func TestMarkdownFormatViolation(t *testing.T) {
 	var markdownFormatter MarkdownFormatter
 
-	t.Run("Converts violation to string in pretty format", func(t *testing.T) {
+	var violation domain.Violation
+	violation.Title = "Test Title"
+	violation.RuleLink = "http://example.com/violation"
+	violation.ViolationType = "MUST"
+	violation.Decription = "Test Description"
+
+	t.Run("Converts violation to string in pretty format with just paths", func(t *testing.T) {
 
-		var violation domain.Violation
-		violation.Title = "Test Title"
-		violation.RuleLink = "http://example.com/violation"
-		violation.ViolationType = "MUST"
-		violation.Decription = "Test Description"
 		violation.Paths = []string{"/path/one", "/path/two"}
 
 		actualResult := markdownFormatter.formatViolation(&violation)
@@ -126,6 +127,31 @@ func TestMarkdownFormatViolation(t *testing.T) {
 
 		tests.AssertEquals(t, expectedResult, actualResult)
 	})
+
+	t.Run("Converts violation to string in pretty format with paths and pointer", func(t *testing.T) {
+
+		violation.Pointer = "/pointer/1"
+
+		actualResult := markdownFormatter.formatViolation(&violation)
+		expectedResult := "### `MUST` [Test Title](http://example.com/violation)\n\n" +
+			"> Test Description\n\n" +
+			"- /pointer/1\n\n"
+
+		tests.AssertEquals(t, expectedResult, actualResult)
+	})
+
+	t.Run("Converts violation to string in pretty format with just pointer", func(t *testing.T) {
+
+		violation.Paths = nil
+
+		actualResult := markdownFormatter.formatViolation(&violation)
+		expectedResult := "### `MUST` [Test Title](http://example.com/violation)\n\n" +
+			"> Test Description\n\n" +
+			"- /pointer/1\n\n"
+
+		tests.AssertEquals(t, expectedResult, actualResult)
+	})
+
 }
 
 func TestMarkdownFormatHeader(t *testing.T) {

cli/zally/utils/formatters/pretty_formatter.go

@@ -83,8 +83,12 @@ func (f *PrettyFormatter) formatViolation(violation *domain.Violation) string {
 	fmt.Fprintf(&buffer, "\t%s\n", violation.Decription)
 	fmt.Fprintf(&buffer, "\t%s\n", violation.RuleLink)
 
-	for _, path := range violation.Paths {
-		fmt.Fprintf(&buffer, "\t\t%s\n", path)
+	if violation.Pointer != "" {
+		fmt.Fprintf(&buffer, "\t\t%s\n", violation.Pointer)
+	} else {
+		for _, path := range violation.Paths {
+			fmt.Fprintf(&buffer, "\t\t%s\n", path)
+		}
 	}
 
 	fmt.Fprintf(&buffer, "\n")

cli/zally/utils/formatters/pretty_formatter_test.go

@@ -95,13 +95,14 @@ func TestPrettyFormatServerMessage(t *testing.T) {
 func TestPrettyFormatViolationInPrettyFormat(t *testing.T) {
 	prettyFormatter := NewPrettyFormatter(NewPrettyColorizer(true))
 
-	t.Run("Converts violation to string in pretty format", func(t *testing.T) {
+	var violation domain.Violation
+	violation.Title = "Test Title"
+	violation.RuleLink = "http://example.com/violation"
+	violation.ViolationType = "MUST"
+	violation.Decription = "Test Description"
+
+	t.Run("Converts violation to string in pretty format with just paths", func(t *testing.T) {
 
-		var violation domain.Violation
-		violation.Title = "Test Title"
-		violation.RuleLink = "http://example.com/violation"
-		violation.ViolationType = "MUST"
-		violation.Decription = "Test Description"
 		violation.Paths = []string{"/path/one", "/path/two"}
 
 		actualResult := prettyFormatter.formatViolation(&violation)
@@ -113,6 +114,32 @@ func TestPrettyFormatViolationInPrettyFormat(t *testing.T) {
 
 		tests.AssertEquals(t, expectedResult, actualResult)
 	})
+
+	t.Run("Converts violation to string in pretty format with paths and pointer", func(t *testing.T) {
+
+		violation.Pointer = "/pointer/1"
+
+		actualResult := prettyFormatter.formatViolation(&violation)
+		expectedResult := "\x1b[31mMUST\x1b[0m \x1b[31mTest Title\x1b[0m\n" +
+			"\tTest Description\n" +
+			"\thttp://example.com/violation\n" +
+			"\t\t/pointer/1\n\n"
+
+		tests.AssertEquals(t, expectedResult, actualResult)
+	})
+
+	t.Run("Converts violation to string in pretty format with just pointer", func(t *testing.T) {
+
+		violation.Paths = nil
+
+		actualResult := prettyFormatter.formatViolation(&violation)
+		expectedResult := "\x1b[31mMUST\x1b[0m \x1b[31mTest Title\x1b[0m\n" +
+			"\tTest Description\n" +
+			"\thttp://example.com/violation\n" +
+			"\t\t/pointer/1\n\n"
+
+		tests.AssertEquals(t, expectedResult, actualResult)
+	})
 }
 
 func TestPrettyFormatHeader(t *testing.T) {

server/src/main/java/de/zalando/zally/apireview/ApiViolationsController.java

@@ -95,7 +95,8 @@ private ViolationDTO toDto(Result violation) {
             violation.getDescription(),
             violation.getViolationType(),
             violation.getRuleSet().url(violation.getRule()).toString(),
-            violation.getPaths()
+            violation.getPaths(),
+            violation.getPointer() == null ? null : violation.getPointer().toString()
         );
     }
 

server/src/main/java/de/zalando/zally/dto/ViolationDTO.kt

@@ -3,7 +3,6 @@ package de.zalando.zally.dto
 import com.fasterxml.jackson.annotation.JsonInclude
 import com.fasterxml.jackson.annotation.JsonInclude.Include
 import de.zalando.zally.rule.api.Severity
-import java.util.Arrays.asList
 
 data class ViolationDTO(
     var title: String? = null,
@@ -12,28 +11,4 @@ data class ViolationDTO(
     var ruleLink: String? = null,
     @JsonInclude(Include.NON_NULL) @Deprecated("Use `pointer` instead.") var paths: List<String>? = null,
     @JsonInclude(Include.NON_NULL) var pointer: String? = null
-) {
-    constructor(
-        title: String?,
-        description: String?,
-        violationType: Severity?,
-        ruleLink: String?,
-        vararg paths: String
-    ) : this(title, description, violationType, ruleLink, asList(*paths))
-
-    @Deprecated("Use `pointer` instead.")
-    constructor(
-        title: String?,
-        description: String?,
-        violationType: Severity?,
-        ruleLink: String?,
-        paths: List<String>?
-    ) : this(
-        title,
-        description,
-        violationType,
-        ruleLink,
-        if (paths?.size == 1) null else paths,
-        if (paths?.size == 1) paths[0] else null
-    )
-}
+)

server/src/main/java/de/zalando/zally/rule/Context.kt

@@ -1,5 +1,7 @@
 package de.zalando.zally.rule
 
+import com.fasterxml.jackson.core.JsonPointer
+import de.zalando.zally.rule.api.Violation
 import de.zalando.zally.util.ast.JsonPointers
 import de.zalando.zally.util.ast.MethodCallRecorder
 import de.zalando.zally.util.ast.ReverseAst
@@ -19,13 +21,35 @@ class Context(openApi: OpenAPI, swagger: Swagger? = null) {
 
     val api = recorder.proxy
 
-    val currentPointer: String
-        get() = recorder.pointer
+    /**
+     * Creates a 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 violation(description: String, value: Any): Violation =
+        violation(description, pointerForValue(value))
 
-    fun isIgnored(pointer: String, ruleId: String): Boolean =
+    /**
+     * Creates a 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 violation(description: String, pointer: JsonPointer? = null): Violation =
+        Violation(description, pointer ?: recorder.pointer)
+
+    /**
+     * Check whether a location should be ignored by a specific rule.
+     * @param pointer the location to check
+     * @param ruleId the rule id to check
+     * @return true if the location should be ignored for this rule
+     */
+    fun isIgnored(pointer: JsonPointer, ruleId: String): Boolean =
         swaggerAst?.isIgnored(pointer, ruleId) ?: openApiAst.isIgnored(pointer, ruleId)
 
-    fun pointerForValue(value: Any): String? = if (swaggerAst != null) {
+    private fun pointerForValue(value: Any): JsonPointer? = if (swaggerAst != null) {
         val swaggerPointer = swaggerAst.getPointer(value)
         if (swaggerPointer != null)
             swaggerPointer

server/src/main/java/de/zalando/zally/rule/ContextRulesValidator.kt

@@ -1,5 +1,6 @@
 package de.zalando.zally.rule
 
+import com.fasterxml.jackson.core.JsonPointer
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 
@@ -13,5 +14,5 @@ class ContextRulesValidator(@Autowired rules: RulesManager) : RulesValidator<Con
     override fun parse(content: String): Context? =
             Context.createOpenApiContext(content) ?: Context.createSwaggerContext(content)
 
-    override fun ignore(root: Context, pointer: String, ruleId: String) = root.isIgnored(pointer, ruleId)
+    override fun ignore(root: Context, pointer: JsonPointer, ruleId: String) = root.isIgnored(pointer, ruleId)
 }

server/src/main/java/de/zalando/zally/rule/JsonRulesValidator.kt

@@ -1,5 +1,6 @@
 package de.zalando.zally.rule
 
+import com.fasterxml.jackson.core.JsonPointer
 import com.fasterxml.jackson.databind.JsonNode
 import de.zalando.zally.util.ast.ReverseAst
 import io.swagger.util.Json
@@ -28,5 +29,5 @@ class JsonRulesValidator(@Autowired rules: RulesManager) : RulesValidator<JsonNo
         ReverseAst.fromObject(it).build()
     }
 
-    override fun ignore(root: JsonNode, pointer: String, ruleId: String) = ast?.isIgnored(pointer, ruleId) ?: false
+    override fun ignore(root: JsonNode, pointer: JsonPointer, ruleId: String) = ast?.isIgnored(pointer, ruleId) ?: false
 }