refactor(core.DatasetRequests.Save): removed ambiguous Save method

having Init, Update, and Save methods on core.DatasetRequests is wrong. removed Save method. BREAKING CHANGE: all api methods now route through either Init or Update. This doesn't really matter, as no one was using this api anyway. But hey, it's good to practice documenting these things
qri-io · Nov 14, 2017 · f1972dc · f1972dc
1 parent 446906b
commit f1972dc
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 126 deletions.
diff --git a/api/handlers/datasets.go b/api/handlers/datasets.go
@@ -38,7 +38,7 @@ func (h *DatasetHandlers) DatasetsHandler(w http.ResponseWriter, r *http.Request
 	case "PUT":
 		h.updateDatasetHandler(w, r)
 	case "POST":
-		h.saveDatasetHandler(w, r)
+		h.initDatasetHandler(w, r)
 	default:
 		util.NotFoundHandler(w, r)
 	}
@@ -50,8 +50,6 @@ func (h *DatasetHandlers) DatasetHandler(w http.ResponseWriter, r *http.Request)
 		util.EmptyOkHandler(w, r)
 	case "GET":
 		h.getDatasetHandler(w, r)
-	case "POST":
-		h.saveDatasetHandler(w, r)
 	case "PUT":
 		h.updateDatasetHandler(w, r)
 	case "DELETE":
@@ -140,55 +138,6 @@ func (h *DatasetHandlers) getDatasetHandler(w http.ResponseWriter, r *http.Reque
 	util.WriteResponse(w, res.Dataset)
 }
 
-func (h *DatasetHandlers) saveDatasetHandler(w http.ResponseWriter, r *http.Request) {
-	switch r.Header.Get("Content-Type") {
-	case "application/json":
-		h.saveStructureHandler(w, r)
-	default:
-		h.initDatasetHandler(w, r)
-	}
-}
-
-func (h *DatasetHandlers) updateDatasetHandler(w http.ResponseWriter, r *http.Request) {
-	switch r.Header.Get("Content-Type") {
-	case "application/json":
-		h.updateMetadataHandler(w, r)
-		// default:
-		//  h.initDatasetFileHandler(w, r)
-	}
-}
-
-func (h *DatasetHandlers) updateMetadataHandler(w http.ResponseWriter, r *http.Request) {
-	p := &core.UpdateParams{}
-	if err := json.NewDecoder(r.Body).Decode(p); err != nil {
-		util.WriteErrResponse(w, http.StatusBadRequest, err)
-		return
-	}
-	res := &repo.DatasetRef{}
-	if err := h.Update(p, res); err != nil {
-		h.log.Infof("error updating dataset: %s", err.Error())
-		util.WriteErrResponse(w, http.StatusInternalServerError, err)
-		return
-	}
-	util.WriteResponse(w, res)
-}
-
-func (h *DatasetHandlers) saveStructureHandler(w http.ResponseWriter, r *http.Request) {
-	p := &core.SaveParams{}
-	if err := json.NewDecoder(r.Body).Decode(p); err != nil {
-		util.WriteErrResponse(w, http.StatusBadRequest, err)
-		return
-	}
-	res := &dataset.Dataset{}
-	if err := h.Save(p, res); err != nil {
-		h.log.Infof("error saving dataset: %s", err.Error())
-		util.WriteErrResponse(w, http.StatusInternalServerError, err)
-		return
-	}
-
-	util.WriteResponse(w, res)
-}
-
 func (h *DatasetHandlers) initDatasetHandler(w http.ResponseWriter, r *http.Request) {
 	p := &core.InitDatasetParams{}
 	if r.Header.Get("Content-Type") == "application/json" {
@@ -220,6 +169,30 @@ func (h *DatasetHandlers) initDatasetHandler(w http.ResponseWriter, r *http.Requ
 	util.WriteResponse(w, res.Dataset)
 }
 
+func (h *DatasetHandlers) updateDatasetHandler(w http.ResponseWriter, r *http.Request) {
+	switch r.Header.Get("Content-Type") {
+	case "application/json":
+		h.updateMetadataHandler(w, r)
+		// default:
+		//  h.initDatasetFileHandler(w, r)
+	}
+}
+
+func (h *DatasetHandlers) updateMetadataHandler(w http.ResponseWriter, r *http.Request) {
+	p := &core.UpdateParams{}
+	if err := json.NewDecoder(r.Body).Decode(p); err != nil {
+		util.WriteErrResponse(w, http.StatusBadRequest, err)
+		return
+	}
+	res := &repo.DatasetRef{}
+	if err := h.Update(p, res); err != nil {
+		h.log.Infof("error updating dataset: %s", err.Error())
+		util.WriteErrResponse(w, http.StatusInternalServerError, err)
+		return
+	}
+	util.WriteResponse(w, res)
+}
+
 func (h *DatasetHandlers) deleteDatasetHandler(w http.ResponseWriter, r *http.Request) {
 	p := &core.DeleteParams{
 		Name: r.FormValue("name"),

diff --git a/core/datasets.go b/core/datasets.go
@@ -300,31 +300,6 @@ func (r *DatasetRequests) Update(p *UpdateParams, res *repo.DatasetRef) (err err
 	return nil
 }
 
-type SaveParams struct {
-	Name    string
-	Dataset *dataset.Dataset
-}
-
-// TODO - naming of "save" is ambiguous
-func (r *DatasetRequests) Save(p *SaveParams, res *dataset.Dataset) error {
-	ds := p.Dataset
-
-	path, err := dsfs.SaveDataset(r.repo.Store(), ds, true)
-	if err != nil {
-		return err
-	}
-
-	if err := r.repo.PutName(p.Name, path); err != nil {
-		return err
-	}
-	if err := r.repo.PutDataset(path, ds); err != nil {
-		return err
-	}
-
-	*res = *ds
-	return nil
-}
-
 type DeleteParams struct {
 	Path datastore.Key
 	Name string

diff --git a/core/datasets_test.go b/core/datasets_test.go
@@ -156,16 +156,16 @@ func TestDatasetRequestsUpdate(t *testing.T) {
 		t.Errorf("error allocating test repo: %s", err.Error())
 		return
 	}
-	path, err := mr.GetPath("movies")
-	if err != nil {
-		t.Errorf("error getting path: %s", err.Error())
-		return
-	}
-	moviesDs, err := dsfs.LoadDataset(mr.Store(), path)
-	if err != nil {
-		t.Errorf("error loading dataset: %s", err.Error())
-		return
-	}
+	// path, err := mr.GetPath("movies")
+	// if err != nil {
+	// 	t.Errorf("error getting path: %s", err.Error())
+	// 	return
+	// }
+	// moviesDs, err := dsfs.LoadDataset(mr.Store(), path)
+	// if err != nil {
+	// 	t.Errorf("error loading dataset: %s", err.Error())
+	// 	return
+	// }
 	cases := []struct {
 		p   *UpdateParams
 		res *repo.DatasetRef
@@ -192,45 +192,6 @@ func TestDatasetRequestsUpdate(t *testing.T) {
 	}
 }
 
-func TestDatasetRequestsSave(t *testing.T) {
-	mr, err := testrepo.NewTestRepo()
-	if err != nil {
-		t.Errorf("error allocating test repo: %s", err.Error())
-		return
-	}
-	path, err := mr.GetPath("movies")
-	if err != nil {
-		t.Errorf("error getting path: %s", err.Error())
-		return
-	}
-	moviesDs, err := dsfs.LoadDataset(mr.Store(), path)
-	if err != nil {
-		t.Errorf("error loading dataset: %s", err.Error())
-		return
-	}
-
-	cases := []struct {
-		p   *SaveParams
-		res *dataset.Dataset
-		err string
-	}{
-		//TODO find out why this fails second time but not first
-		{&SaveParams{Name: "ABC", Dataset: moviesDs}, nil, ""},
-		{&SaveParams{Name: "ABC", Dataset: moviesDs}, nil, "error marshaling dataset abstract structure to json: json: error calling MarshalJSON for type *dataset.Structure: json: error calling MarshalJSON for type dataset.DataFormat: Unknown Data Format"},
-	}
-
-	req := NewDatasetRequests(mr)
-	for i, c := range cases {
-		got := &dataset.Dataset{}
-		err := req.Save(c.p, got)
-
-		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
-			t.Errorf("case %d error mismatch: expected: %s, got: %s", i, c.err, err)
-			continue
-		}
-	}
-}
-
 func TestDatasetRequestsDelete(t *testing.T) {
 	mr, err := testrepo.NewTestRepo()
 	if err != nil {