diff --git a/pkg/components/dashdiffs/formatter_basic.go b/pkg/components/dashdiffs/formatter_basic.go index 01c7757112d..af93ff42bf2 100644 --- a/pkg/components/dashdiffs/formatter_basic.go +++ b/pkg/components/dashdiffs/formatter_basic.go @@ -71,6 +71,8 @@ func NewBasicFormatter(left interface{}) *BasicFormatter { } } +// Format takes the diff of two JSON documents, and returns the difference +// between them summarized in an HTML document. func (b *BasicFormatter) Format(d diff.Diff) ([]byte, error) { // calling jsonDiff.Format(d) populates the JSON diff's "Lines" value, // which we use to compute the basic dif @@ -90,67 +92,45 @@ func (b *BasicFormatter) Format(d diff.Diff) ([]byte, error) { return buf.Bytes(), nil } -// Basic is V2 of the basic diff +// Basic transforms a slice of JSONLines into a slice of BasicBlocks. func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { // init an array you can append to for the basic "blocks" blocks := make([]*BasicBlock, 0) - // iterate through each line for _, line := range lines { - // TODO: this condition needs an explaination? what does it mean? - if b.LastIndent == 2 && line.Indent == 1 && line.Change == ChangeNil { + if b.returnToTopLevelKey(line) { if b.Block != nil { blocks = append(blocks, b.Block) } } + // Record the last indent level at each pass in case we need to + // check for a change in depth inside the JSON data structures. b.LastIndent = line.Indent - // TODO: why special handling for indent 2? if line.Indent == 1 { - switch line.Change { - case ChangeNil: - if line.Change == ChangeNil { - if line.Key != "" { - b.Block = &BasicBlock{ - Title: line.Key, - Change: line.Change, - } - } - } - - case ChangeAdded, ChangeDeleted: - blocks = append(blocks, &BasicBlock{ - Title: line.Key, - Change: line.Change, - New: line.Val, - LineStart: line.LineNum, - }) - - case ChangeOld: - b.Block = &BasicBlock{ - Title: line.Key, - Old: line.Val, - Change: line.Change, - LineStart: line.LineNum, - } - - case ChangeNew: - b.Block.New = line.Val - b.Block.LineEnd = line.LineNum - - // then write out the change - blocks = append(blocks, b.Block) - default: - // ok + if block, ok := b.handleTopLevelChange(line); ok { + blocks = append(blocks, block) } } - // TODO: why special handling for indent > 2 ? - // Other Lines + // Here is where we handle changes for all types, appending each change + // to the current block based on the value. + // + // Values which only occupy a single line in JSON (like a string or + // int, for example) are treated as "Basic Changes" that we append to + // the current block as soon as they're detected. + // + // Values which occupy multiple lines (either slices or maps) are + // treated as "Basic Summaries". When we detect the "ChangeNil" type, + // we know we've encountered one of these types, so we record the + // starting position as well the type of the change, and stop + // performing comparisons until we find the end of that change. Upon + // finding the change, we append it to the current block, and begin + // performing comparisons again. if line.Indent > 1 { - // Ensure single line change - if line.Key != "" && line.Val != nil && !b.writing { + // check to ensure a single line change + if b.isSingleLineChange(line) { switch line.Change { case ChangeAdded, ChangeDeleted: @@ -178,13 +158,31 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { //ok } + // otherwise, we're dealing with a change at a deeper level. We + // know there's a change somewhere in the JSON tree, but we + // don't know exactly where, so we go deeper. } else { + + // if the change is anything but unchanged, continue processing + // + // we keep "narrowing" the key as we go deeper, in order to + // correctly report the key name for changes found within an + // object or array. if line.Change != ChangeUnchanged { if line.Key != "" { b.narrow = line.Key b.keysIdent = line.Indent } + // if the change isn't nil, and we're not already writing + // out a change, then we've found something. + // + // First, try to determine the title of the embedded JSON + // object. If it's an empty string, then we're in an object + // or array, so we default to using the "narrowed" key. + // + // We also start recording the basic summary, until we find + // the next `ChangeUnchanged`. if line.Change != ChangeNil { if !b.writing { b.writing = true @@ -204,6 +202,17 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { } } } + // if we find a `ChangeUnchanged`, we do one of two things: + // + // - if we're recording a change already, then we know + // we've come to the end of that change block, so we write + // that change out be recording the line number of where + // that change ends, and append it to the current block's + // summary. + // + // - if we're not recording a change, then we do nothing, + // since the BasicDiff doesn't report on unchanged JSON + // values. } else { if b.writing { b.writing = false @@ -218,6 +227,81 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { return blocks } +// returnToTopLevelKey indicates that we've moved from a key at one level deep +// in the JSON document to a top level key. +// +// In order to produce distinct "blocks" when rendering the basic diff, +// we need a way to distinguish between differnt sections of data. +// To do this, we consider the value(s) of each top-level JSON key to +// represent a distinct block for Grafana's JSON data structure, so +// we perform this check to see if we've entered a new "block". If we +// have, we simply append the existing block to the array of blocks. +func (b *BasicDiff) returnToTopLevelKey(line *JSONLine) bool { + return b.LastIndent == 2 && line.Indent == 1 && line.Change == ChangeNil +} + +// handleTopLevelChange handles a change on one of the top-level keys on a JSON +// document. +// +// If the line's indentation is at level 1, then we know it's a top +// level key in the JSON document. As mentioned earlier, we treat these +// specially as they indicate their values belong to distinct blocks. +// +// At level 1, we only record single-line changes, ie, the "added", +// "deleted", "old" or "new" cases, since we know those values aren't +// arrays or maps. We only handle these cases at level 2 or deeper, +// since for those we either output a "change" or "summary". This is +// done for formatting reasons only, so we have logical "blocks" to +// display. +func (b *BasicDiff) handleTopLevelChange(line *JSONLine) (*BasicBlock, bool) { + switch line.Change { + case ChangeNil: + if line.Change == ChangeNil { + if line.Key != "" { + b.Block = &BasicBlock{ + Title: line.Key, + Change: line.Change, + } + } + } + + case ChangeAdded, ChangeDeleted: + return &BasicBlock{ + Title: line.Key, + Change: line.Change, + New: line.Val, + LineStart: line.LineNum, + }, true + + case ChangeOld: + b.Block = &BasicBlock{ + Title: line.Key, + Old: line.Val, + Change: line.Change, + LineStart: line.LineNum, + } + + case ChangeNew: + b.Block.New = line.Val + b.Block.LineEnd = line.LineNum + + // For every "old" change there is a corresponding "new", which + // is why we wait until we detect the "new" change before + // appending the change. + return b.Block, true + default: + // ok + } + + return nil, false +} + +// isSingleLineChange ensures we're iterating over a single line change (ie, +// either a single line or a old-new value pair was changed in the JSON file). +func (b *BasicDiff) isSingleLineChange(line *JSONLine) bool { + return line.Key != "" && line.Val != nil && !b.writing +} + // encStateMap is used in the template helper var ( encStateMap = map[ChangeType]string{ @@ -240,7 +324,9 @@ var ( ) var ( - // tplBlock is the whole thing + // tplBlock is the container for the basic diff. It iterates over each + // basic block, expanding each "change" and "summary" belonging to every + // block. tplBlock = `{{ define "block" -}} {{ range . }}
@@ -286,7 +372,7 @@ var ( {{ end }} {{ end }}` - // tplChange is the template for changes + // tplChange is the template for basic changes. tplChange = `{{ define "change" -}}
  • @@ -313,7 +399,7 @@ var (
    • {{ end }}` - // tplSummary is for basis summaries + // tplSummary is for basic summaries. tplSummary = `{{ define "summary" -}}
    diff --git a/pkg/components/dashdiffs/formatter_test.go b/pkg/components/dashdiffs/formatter_test.go new file mode 100644 index 00000000000..472d998feb5 --- /dev/null +++ b/pkg/components/dashdiffs/formatter_test.go @@ -0,0 +1,131 @@ +package dashdiffs + +import ( + "testing" + + "github.com/grafana/grafana/pkg/components/simplejson" + . "github.com/smartystreets/goconvey/convey" +) + +func TestDiff(t *testing.T) { + // Sample json docs for tests only + const ( + leftJSON = `{ + "key": "value", + "object": { + "key": "value", + "anotherObject": { + "same": "this field is the same in rightJSON", + "change": "this field should change in rightJSON", + "delete": "this field doesn't appear in rightJSON" + } + }, + "array": [ + "same", + "change", + "delete" + ], + "embeddedArray": { + "array": [ + "same", + "change", + "delete" + ] + } + }` + + rightJSON = `{ + "key": "differentValue", + "object": { + "key": "value", + "newKey": "value", + "anotherObject": { + "same": "this field is the same in rightJSON", + "change": "this field should change in rightJSON", + "add": "this field is added" + } + }, + "array": [ + "same", + "changed!", + "add" + ], + "embeddedArray": { + "array": [ + "same", + "changed!", + "add" + ] + } + }` + ) + + Convey("Testing dashboard diffs", t, func() { + + // Compute the diff between the two JSON objects + baseData, err := simplejson.NewJson([]byte(leftJSON)) + So(err, ShouldBeNil) + + newData, err := simplejson.NewJson([]byte(rightJSON)) + So(err, ShouldBeNil) + + left, jsonDiff, err := getDiff(baseData, newData) + So(err, ShouldBeNil) + + Convey("The JSONFormatter should produce the expected JSON tokens", func() { + f := NewJSONFormatter(left) + _, err := f.Format(jsonDiff) + So(err, ShouldBeNil) + + // Total up the change types. If the number of different change + // types is correct, it means that the diff is producing correct + // output to the template rendered. + changeCounts := make(map[ChangeType]int) + for _, line := range f.Lines { + changeCounts[line.Change]++ + } + + // The expectedChangeCounts here were determined by manually + // looking at the JSON + expectedChangeCounts := map[ChangeType]int{ + ChangeNil: 12, + ChangeAdded: 2, + ChangeDeleted: 1, + ChangeOld: 5, + ChangeNew: 5, + ChangeUnchanged: 5, + } + So(changeCounts, ShouldResemble, expectedChangeCounts) + }) + + Convey("The BasicFormatter should produce the expected BasicBlocks", func() { + f := NewBasicFormatter(left) + _, err := f.Format(jsonDiff) + So(err, ShouldBeNil) + + bd := &BasicDiff{} + blocks := bd.Basic(f.jsonDiff.Lines) + + changeCounts := make(map[ChangeType]int) + for _, block := range blocks { + for _, change := range block.Changes { + changeCounts[change.Change]++ + } + + for _, summary := range block.Summaries { + changeCounts[summary.Change]++ + } + + changeCounts[block.Change]++ + } + + expectedChangeCounts := map[ChangeType]int{ + ChangeNil: 3, + ChangeAdded: 2, + ChangeDeleted: 1, + ChangeOld: 3, + } + So(changeCounts, ShouldResemble, expectedChangeCounts) + }) + }) +}