feat: overhauled diff command

Merge pull request #698 from qri-io/diff

Makefile

@@ -7,6 +7,7 @@ github.com/olekukonko/tablewriter \
 github.com/qri-io/bleve \
 github.com/qri-io/dataset \
 github.com/qri-io/doggos \
+github.com/qri-io/deepdiff \
 github.com/qri-io/dsdiff \
 github.com/qri-io/varName \
 github.com/sergi/go-diff/diffmatchpatch \
@@ -55,6 +56,7 @@ update-qri-deps: require-gopath
 	cd $$GOPATH/src/github.com/qri-io/registry && git checkout master && git pull
 	cd $$GOPATH/src/github.com/qri-io/dataset && git checkout master && git pull
 	cd $$GOPATH/src/github.com/qri-io/varName && git checkout master && git pull
+	cd $$GOPATH/src/github.com/qri-io/deepdiff && git checkout master && git pull
 	cd $$GOPATH/src/github.com/qri-io/dsdiff && git checkout master && git pull
 	cd $$GOPATH/src/github.com/qri-io/jsonschema && git checkout master && git pull
 	cd $$GOPATH/src/github.com/qri-io/startf && git checkout master && git pull

api/api_test.go

@@ -97,12 +97,13 @@ type handlerTestCase struct {
 }
 
 // runHandlerTestCases executes a slice of handlerTestCase against a handler
-func runHandlerTestCases(t *testing.T, name string, h http.HandlerFunc, cases []handlerTestCase) {
+func runHandlerTestCases(t *testing.T, name string, h http.HandlerFunc, cases []handlerTestCase, jsonHeader bool) {
 	for i, c := range cases {
 		name := fmt.Sprintf("%s %s case %d: %s %s", t.Name(), name, i, c.method, c.endpoint)
 		req := httptest.NewRequest(c.method, c.endpoint, bytes.NewBuffer(c.body))
-		// TODO - make this settable with some sort of test case interface
-		req.Header.Set("Content-Type", "application/json")
+		if jsonHeader {
+			req.Header.Set("Content-Type", "application/json")
+		}
 		w := httptest.NewRecorder()
 
 		h(w, req)
@@ -155,13 +156,13 @@ func TestServerRoutes(t *testing.T) {
 		{"OPTIONS", "/", nil},
 		{"GET", "/", nil},
 	}
-	runHandlerTestCases(t, "root", h.Handler, rootCases)
+	runHandlerTestCases(t, "root", h.Handler, rootCases, true)
 
 	healthCheckCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"GET", "/", nil},
 	}
-	runHandlerTestCases(t, "health check", HealthCheckHandler, healthCheckCases)
+	runHandlerTestCases(t, "health check", HealthCheckHandler, healthCheckCases, true)
 }
 
 func TestServerReadOnlyRoutes(t *testing.T) {

api/dataset_test.go

@@ -21,22 +21,22 @@ func TestDatasetHandlers(t *testing.T) {
 		{"GET", "/", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "list", h.ListHandler, listCases)
+	runHandlerTestCases(t, "list", h.ListHandler, listCases, true)
 
 	// TODO: Remove this case, update API snapshot.
 	initCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"POST", "/", mustFile(t, "testdata/newRequestFromURL.json")},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "init", h.SaveHandler, initCases)
+	runHandlerTestCases(t, "init", h.SaveHandler, initCases, true)
 
 	saveCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"POST", "/", mustFile(t, "testdata/newRequestFromURL.json")},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "save", h.SaveHandler, saveCases)
+	runHandlerTestCases(t, "save", h.SaveHandler, saveCases, true)
 
 	getCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
@@ -45,37 +45,37 @@ func TestDatasetHandlers(t *testing.T) {
 		{"GET", "/at/map/QmPRjfgUFrH1GxBqujJ3sEvwV3gzHdux1j4g8SLyjbhwot", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "get", h.GetHandler, getCases)
+	runHandlerTestCases(t, "get", h.GetHandler, getCases, true)
 
 	bodyCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"GET", "/body/me/family_relationships", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "body", h.BodyHandler, bodyCases)
+	runHandlerTestCases(t, "body", h.BodyHandler, bodyCases, true)
 
 	renameCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"POST", "/rename", mustFile(t, "testdata/renameRequest.json")},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "rename", h.RenameHandler, renameCases)
+	runHandlerTestCases(t, "rename", h.RenameHandler, renameCases, true)
 
 	exportCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"GET", "/export/me/cities", nil},
 		{"GET", "/export/me/cities/at/map/QmPRjfgUFrH1GxBqujJ3sEvwV3gzHdux1j4g8SLyjbhwot", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "export", h.ZipDatasetHandler, exportCases)
+	runHandlerTestCases(t, "export", h.ZipDatasetHandler, exportCases, true)
 
 	publishCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"GET", "/publish/", nil},
 		{"POST", "/publish/me/cities", nil},
 		{"DELETE", "/publish/me/cities", nil},
 	}
-	runHandlerTestCases(t, "publish", h.PublishHandler, publishCases)
+	runHandlerTestCases(t, "publish", h.PublishHandler, publishCases, true)
 
 	updateCases := []handlerMimeMultipartTestCase{
 		{"OPTIONS", "/", nil, nil},
@@ -89,8 +89,8 @@ func TestDatasetHandlers(t *testing.T) {
 	}
 	runMimeMultipartHandlerTestCases(t, "update", h.UpdateHandler, updateCases)
 
-	// TODO: Perhaps add an option to runHandlerTestCases to set Content-Type, then combine
-	// `runHandlerZipPostTestCases` with `runHandlerTestCases`.
+	// TODO: Perhaps add an option to runHandlerTestCases to set Content-Type, then combin, truee
+	// `runHandlerZipPostTestCases` with `runHandlerTestCases`, true.
 	unpackCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"POST", "/unpack/", mustFile(t, "testdata/exported.zip")},
@@ -99,19 +99,18 @@ func TestDatasetHandlers(t *testing.T) {
 
 	diffCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
-		{"GET", "/", mustFile(t, "testdata/diffRequest.json")},
-		{"GET", "/", mustFile(t, "testdata/diffRequestPlusMinusColor.json")},
+		{"GET", "/?left_path=me/family_relationships&right_path=me/cities", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "diff", h.DiffHandler, diffCases)
+	runHandlerTestCases(t, "diff", h.DiffHandler, diffCases, false)
 
 	removeCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
 		{"GET", "/", nil},
 		{"POST", "/remove/me/cities", nil},
 		{"POST", "/remove/at/map/QmPRjfgUFrH1GxBqujJ3sEvwV3gzHdux1j4g8SLyjbhwot", nil},
 	}
-	runHandlerTestCases(t, "remove", h.RemoveHandler, removeCases)
+	runHandlerTestCases(t, "remove", h.RemoveHandler, removeCases, true)
 
 	removeMimeCases := []handlerMimeMultipartTestCase{
 		{"POST", "/remove/me/cities",

api/datasets.go

@@ -15,7 +15,6 @@ import (
 	util "github.com/datatogether/api/apiutil"
 	"github.com/qri-io/dataset"
 	"github.com/qri-io/dataset/dsutil"
-	"github.com/qri-io/dsdiff"
 	"github.com/qri-io/qri/actions"
 	"github.com/qri-io/qri/lib"
 	"github.com/qri-io/qri/p2p"
@@ -318,58 +317,46 @@ func (h *DatasetHandlers) getHandler(w http.ResponseWriter, r *http.Request) {
 	util.WriteResponse(w, res.Dataset)
 }
 
-type diffAPIParams struct {
-	Left, Right string
-	Format      string
-}
-
 func (h *DatasetHandlers) diffHandler(w http.ResponseWriter, r *http.Request) {
-	d := &diffAPIParams{}
+	req := &lib.DiffParams{}
 	switch r.Header.Get("Content-Type") {
 	case "application/json":
-		if err := json.NewDecoder(r.Body).Decode(d); err != nil {
+		if err := json.NewDecoder(r.Body).Decode(req); err != nil {
 			util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error decoding body into params: %s", err.Error()))
 			return
 		}
 	default:
-		d.Left = r.FormValue("left")
-		d.Right = r.FormValue("right")
-		d.Format = r.FormValue("format")
+		req = &lib.DiffParams{
+			LeftPath:  r.FormValue("left_path"),
+			RightPath: r.FormValue("right_path"),
+			Selector:  r.FormValue("selector"),
+		}
 	}
 
-	left, err := DatasetRefFromPath(d.Left)
-	if err != nil {
-		util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error getting datasetRef from left path: %s", err.Error()))
+	res := &lib.DiffResponse{}
+	if err := h.Diff(req, res); err != nil {
+		fmt.Println(err)
+		util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error generating diff: %s", err.Error()))
 		return
 	}
 
-	right, err := DatasetRefFromPath(d.Right)
-	if err != nil {
-		util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error getting datasetRef from right path: %s", err.Error()))
-		return
+	env := map[string]interface{}{
+		"meta": map[string]interface{}{
+			"code": http.StatusOK,
+			"stat": res.Stat,
+		},
+		"data": res.Diff,
 	}
 
-	diffs := make(map[string]*dsdiff.SubDiff)
-	p := &lib.DiffParams{
-		Left:    left,
-		Right:   right,
-		DiffAll: true,
-	}
-
-	if err = h.Diff(p, &diffs); err != nil {
-		util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error diffing datasets: %s", err))
-	}
-
-	if d.Format != "" {
-		formattedDiffs, err := dsdiff.MapDiffsToString(diffs, d.Format)
-		if err != nil {
-			util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error formating diffs: %s", err))
-		}
-		util.WriteResponse(w, formattedDiffs)
+	resData, err := json.Marshal(env)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	util.WriteResponse(w, diffs)
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusOK)
+	w.Write(resData)
 }
 
 func (h *DatasetHandlers) peerListHandler(w http.ResponseWriter, r *http.Request) {

api/log_test.go

@@ -36,5 +36,5 @@ func TestHistoryHandlers(t *testing.T) {
 		{"GET", "/history/at/map/QmZrmGvTPMCkJYfqaagFZBUWuX5bkqSXu179eNnFfhCKze", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "log", h.LogHandler, logCases)
+	runHandlerTestCases(t, "log", h.LogHandler, logCases, true)
 }

api/peers_test.go

@@ -15,6 +15,6 @@ func TestPeerHandlers(t *testing.T) {
 		{"GET", "/", nil},
 		{"DELETE", "/", nil},
 	}
-	runHandlerTestCases(t, "connections", h.ConnectionsHandler, connectionsCases)
+	runHandlerTestCases(t, "connections", h.ConnectionsHandler, connectionsCases, true)
 
 }

api/profile_test.go

@@ -27,13 +27,13 @@ func TestProfileHandler(t *testing.T) {
 	}
 
 	proh := NewProfileHandlers(node, false)
-	runHandlerTestCases(t, "profile", proh.ProfileHandler, cases)
+	runHandlerTestCases(t, "profile", proh.ProfileHandler, cases, true)
 
 	readOnlyCases := []handlerTestCase{
 		{"GET", "/", nil},
 	}
 	proh.ReadOnly = true
-	runHandlerTestCases(t, "read-only", proh.ProfileHandler, readOnlyCases)
+	runHandlerTestCases(t, "read-only", proh.ProfileHandler, readOnlyCases, true)
 
 	mimeCases := []handlerMimeMultipartTestCase{
 		{"GET", "/",

api/registry_test.go

@@ -20,12 +20,12 @@ func TestRegistryHandlers(t *testing.T) {
 		{"DELETE", "/registry/me/counter", nil},
 		{"PATCH", "/", nil},
 	}
-	runHandlerTestCases(t, "registry", h.RegistryHandler, registryCases)
+	runHandlerTestCases(t, "registry", h.RegistryHandler, registryCases, true)
 
 	registryDatasetsCases := []handlerTestCase{
 		{"GET", "/registry/datasets", nil},
 	}
-	runHandlerTestCases(t, "registryDatasets", h.RegistryDatasetsHandler, registryDatasetsCases)
+	runHandlerTestCases(t, "registryDatasets", h.RegistryDatasetsHandler, registryDatasetsCases, true)
 }
 
 func TestRegistryGet(t *testing.T) {

api/render_test.go

@@ -13,5 +13,5 @@ func TestRenderHandler(t *testing.T) {
 	}
 
 	h := NewRenderHandlers(r)
-	runHandlerTestCases(t, "render", h.RenderHandler, cases)
+	runHandlerTestCases(t, "render", h.RenderHandler, cases, false)
 }

api/search_test.go

@@ -10,10 +10,11 @@ func TestSearchHandlers(t *testing.T) {
 
 	searchCases := []handlerTestCase{
 		{"OPTIONS", "/", nil},
+		// TODO (b5): lol wut Get requests don't have bodies
 		{"GET", "/", mustFile(t, "testdata/searchRequest.json")},
 		{"DELETE", "/", nil},
 	}
 
 	proh := NewSearchHandlers(node)
-	runHandlerTestCases(t, "search", proh.SearchHandler, searchCases)
+	runHandlerTestCases(t, "search", proh.SearchHandler, searchCases, true)
 }

api/testdata/diffRequest.json

@@ -1,4 +1,3 @@
 {
-  "left": "me/cities@/map/QmVU86zb7A6NvipimEJ7mQFu1jy2nk96o6f3uwHe92D8US",
-  "right": "me/cities@/map/QmPRjfgUFrH1GxBqujJ3sEvwV3gzHdux1j4g8SLyjbhwot"
+  "rightPath": "me/cities"
 }

cmd/cmd_test.go

@@ -170,7 +170,7 @@ func TestCommandsIntegration(t *testing.T) {
 		"qri list me/movies",
 		fmt.Sprintf("qri save --body=%s -t=commit_1 me/movies", movies2FilePath),
 		"qri log me/movies",
-		"qri diff me/movies me/movies2 -d=detail",
+		"qri diff me/movies me/movies2",
 		fmt.Sprintf("qri export -o=%s me/movies --zip", path),
 		fmt.Sprintf("qri export -o=%s/ds.yaml --format=yaml me/movies", path),
 		"qri publish me/movies",