Skip to content

Commit

Permalink
refactor(core.Add, core.Save): refactor input params, folding into a …
Browse files Browse the repository at this point in the history 
…DatasetPod

This change is a first example of eliminating the need for DatasetRef by leveraging fields present in dataset.DatasetPod. DatasetPod contains all the fields necessary to submit changes to a dataset, so Add & Save methods can now be refactored to accept a file that fully encapsulates the dataset to create.

BREAKING CHANGES:
add & save on both API & CLI now accept a "file" which is the full dataset
  • Loading branch information
@b5
b5 committed May 23, 2018
1 parent e69a6c2 commit e63694f
Show file tree
Hide file tree
Showing 11 changed files with 414 additions and 326 deletions.
96 changes: 48 additions & 48 deletions api/datasets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,25 +336,24 @@ func (h *DatasetHandlers) peerListHandler(w http.ResponseWriter, r *http.Request
}

func (h *DatasetHandlers) initHandler(w http.ResponseWriter, r *http.Request) {
p := &core.InitParams{}
dsp := &dataset.DatasetPod{}
switch r.Header.Get("Content-Type") {
case "application/json":
if err := json.NewDecoder(r.Body).Decode(p); err != nil {
if err := json.NewDecoder(r.Body).Decode(dsp); err != nil {
util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("error decoding body into params: %s", err.Error()))
return
}

if p.DataURL == "" {
util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("if adding dataset using json, body of request must have 'dataUrl' field"))
if dsp.DataPath == "" {
util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("if adding dataset using json, body of request must have 'dataPath' field"))
return
}

default:
p = &core.InitParams{
dsp = &dataset.DatasetPod{
Peername: r.FormValue("peername"),
Name: r.FormValue("name"),
DataURL: r.FormValue("url"),
Private: r.FormValue("private") == "true",
DataPath: r.FormValue("data_path"),
}

infile, fileHeader, err := r.FormFile("file")
Expand All @@ -371,7 +370,7 @@ func (h *DatasetHandlers) initHandler(w http.ResponseWriter, r *http.Request) {
defer os.Remove(path)
io.Copy(f, infile)
f.Close()
p.DataPath = path
dsp.DataPath = path
}

// metadatafile, metadataHeader, err := r.FormFile("metadata")
Expand All @@ -396,6 +395,10 @@ func (h *DatasetHandlers) initHandler(w http.ResponseWriter, r *http.Request) {
}

res := &repo.DatasetRef{}
p := &core.SaveParams{
Dataset: dsp,
Private: r.FormValue("private") == "true",
}
if err := h.Init(p, res); err != nil {
log.Infof("error initializing dataset: %s", err.Error())
util.WriteErrResponse(w, http.StatusInternalServerError, err)
Expand Down Expand Up @@ -426,23 +429,22 @@ func (h *DatasetHandlers) addHandler(w http.ResponseWriter, r *http.Request) {
util.WriteResponse(w, res)
}

type saveParamsJSON struct {
Peername string `json:"peername,omitempty"`
Name string `json:"name,omitempty"`
Title string `json:"title,omitempty"`
Message string `json:"message,omitempty"`
Data json.RawMessage `json:"data,omitempty"`
Meta *dataset.Meta `json:"meta,omitempty"`
Structure *dataset.StructurePod `json:"structure,omitempty"`
Commit *dataset.CommitPod `json:"commit,omitempty"`
}
// type saveParamsJSON struct {
// Peername string `json:"peername,omitempty"`
// Name string `json:"name,omitempty"`
// Title string `json:"title,omitempty"`
// Message string `json:"message,omitempty"`
// Data json.RawMessage `json:"data,omitempty"`
// Meta *dataset.Meta `json:"meta,omitempty"`
// Structure *dataset.StructurePod `json:"structure,omitempty"`
// Commit *dataset.CommitPod `json:"commit,omitempty"`
// }

func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
save := &core.SaveParams{}
dsp := &dataset.DatasetPod{}

if r.Header.Get("Content-Type") == "application/json" {
saveParams := &saveParamsJSON{}
err := json.NewDecoder(r.Body).Decode(saveParams)
err := json.NewDecoder(r.Body).Decode(dsp)
if err != nil {
util.WriteErrResponse(w, http.StatusBadRequest, err)
return
Expand All @@ -455,22 +457,22 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
return
}
if args.Peername != "" {
saveParams.Peername = args.Peername
saveParams.Name = args.Name
dsp.Peername = args.Peername
dsp.Name = args.Name
}
}

save = &core.SaveParams{
Peername: saveParams.Peername,
Name: saveParams.Name,
Title: saveParams.Title,
Message: saveParams.Message,
Dataset: &dataset.DatasetPod{
Commit: saveParams.Commit,
Meta: saveParams.Meta,
Structure: saveParams.Structure,
},
}
// save = &core.SaveParams{
// Peername: saveParams.Peername,
// Name: saveParams.Name,
// Title: saveParams.Title,
// Message: saveParams.Message,
// Dataset: &dataset.DatasetPod{
// Commit: saveParams.Commit,
// Meta: saveParams.Meta,
// Structure: saveParams.Structure,
// },
// }

// if len(saveParams.Data) != 0 {
// util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("cannot accept data files using Content-Type: application/json. must make a mime/multipart request"))
Expand All @@ -490,13 +492,13 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
// save.StructureFilename = "structure.json"
// }
} else {
save = &core.SaveParams{
Peername: r.FormValue("peername"),
DataURL: r.FormValue("data_url"),
Name: r.FormValue("name"),
Title: r.FormValue("title"),
Message: r.FormValue("message"),
}
// save = &core.SaveParams{
// Peername: r.FormValue("peername"),
// DataURL: r.FormValue("data_path"),
// Name: r.FormValue("name"),
// Title: r.FormValue("title"),
// Message: r.FormValue("message"),
// }

// infile, fileHeader, err := r.FormFile("file")
// if err != nil && err != http.ErrMissingFile {
Expand Down Expand Up @@ -529,14 +531,12 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
// }
}

// prev := &repo.DatasetRef{}
// if err := h.Get(&repo.DatasetRef{Peername: save.Peername, Name: save.Name}, prev); err != nil {
// panic(err)
// }
// fmt.Println("PREV: ", prev.Dataset)

res := &repo.DatasetRef{}
if err := h.Save(save, res); err != nil {
p := &core.SaveParams{
Dataset: dsp,
Private: r.FormValue("private") == "true",
}
if err := h.Save(p, res); err != nil {
util.WriteErrResponse(w, http.StatusInternalServerError, err)
return
}
Expand Down
2 changes: 1 addition & 1 deletion api/testdata/addRequestFromURL.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"peername":"peer",
"name":"family_relationships",
"dataUrl":"http://insight.dev.schoolwires.com/HelpAssets/C2Assets/C2Files/C2ImportFamRelSample.csv"
"dataPath":"http://insight.dev.schoolwires.com/HelpAssets/C2Assets/C2Files/C2ImportFamRelSample.csv"
}
91 changes: 37 additions & 54 deletions cmd/add.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"encoding/json"
"fmt"
// "gopkg.in/yaml.v2"
"os"
"path/filepath"
"strings"
Expand All @@ -17,11 +16,13 @@ import (
)

var (
addDsDataFilepath string
addDsFile string
addDsDataPath string
addDsMetaFilepath string
addDsStructureFilepath string
addDsName string
addDsURL string
addDsTitle string
addDsMessage string
addDsPassive bool
addDsShowValidation bool
addDsPrivate bool
Expand Down Expand Up @@ -56,14 +57,9 @@ changes to qri.`,
PreRun: func(cmd *cobra.Command, args []string) {
loadConfig()
},
Args: cobra.MinimumNArgs(1),
Run: func(cmd *cobra.Command, args []string) {

ingest := (addDsDataFilepath != "" || addDsMetaFilepath != "" || addDsStructureFilepath != "" || addDsURL != "")

if ingest && len(args) != 1 {
ErrExit(fmt.Errorf("adding datasets with --structure, --meta, or --data requires exactly 1 argument for the new dataset name"))
}
ingest := (addDsFile != "" || addDsDataPath != "" || addDsMetaFilepath != "" || addDsStructureFilepath != "")

if ingest {
ref, err := repo.ParseDatasetRef(args[0])
Expand All @@ -74,10 +70,6 @@ changes to qri.`,
}

for _, arg := range args {
if addDsPrivate {
ErrExit(fmt.Errorf("option to make dataset private not yet implimented, refer to https://github.com/qri-io/qri/issues/291 for updates"))
}

ref, err := repo.ParseDatasetRef(arg)
ExitIfErr(err)

Expand All @@ -96,54 +88,44 @@ changes to qri.`,
func initDataset(name repo.DatasetRef, cmd *cobra.Command) {
var err error

// metaFile, err = loadFileIfPath(addDsMetaFilepath)
// ExitIfErr(err)
// structureFile, err = loadFileIfPath(addDsStructureFilepath)
// ExitIfErr(err)

if addDsDataFilepath != "" {
addDsDataFilepath, err = filepath.Abs(addDsDataFilepath)
ExitIfErr(err)
}

p := &core.InitParams{
Peername: name.Peername,
Name: name.Name,
DataURL: addDsURL,
DataPath: addDsDataFilepath,
Private: addDsPrivate,
}

if dspath, err := cmd.Flags().GetString("dataset"); err == nil && dspath != "" {
ds := &dataset.DatasetPod{}
f, err := os.Open(dspath)
dsp := &dataset.DatasetPod{}
if addDsFile != "" {
f, err := os.Open(addDsFile)
ExitIfErr(err)

switch strings.ToLower(filepath.Ext(dspath)) {
switch strings.ToLower(filepath.Ext(addDsFile)) {
case ".yaml", ".yml":
data, err := ioutil.ReadAll(f)
ExitIfErr(err)
// err = UnmarshalYAML(data, ds)
err = dsutil.UnmarshalYAMLDatasetPod(data, ds)
err = dsutil.UnmarshalYAMLDatasetPod(data, dsp)
ExitIfErr(err)
case ".json":
err = json.NewDecoder(f).Decode(ds)
err = json.NewDecoder(f).Decode(dsp)
ExitIfErr(err)
}
p.Dataset = ds
}

// this is because passing nil to interfaces is bad
// see: https://golang.org/doc/faq#nil_error
// if dataFile != nil {
// p.Data = dataFile
// }
// if metaFile != nil {
// p.Metadata = metaFile
// }
// if structureFile != nil {
// p.Structure = structureFile
// }
if name.Peername != "" {
dsp.Name = name.Name
}
if name.Peername != "" {
dsp.Peername = name.Peername
}
if addDsDataPath != "" {
addDsDataPath, err = filepath.Abs(addDsDataPath)
ExitIfErr(err)
dsp.DataPath = addDsDataPath
}

// metaFile, err = loadFileIfPath(addDsMetaFilepath)
// ExitIfErr(err)
// structureFile, err = loadFileIfPath(addDsStructureFilepath)
// ExitIfErr(err)

p := &core.SaveParams{
Dataset: dsp,
Private: addDsPrivate,
}

req, err := datasetRequests(false)
ExitIfErr(err)
Expand Down Expand Up @@ -175,12 +157,13 @@ func initDataset(name repo.DatasetRef, cmd *cobra.Command) {
}

func init() {
datasetAddCmd.Flags().StringP("dataset", "", "", "dataset data file")
datasetAddCmd.Flags().StringVarP(&addDsURL, "url", "", "", "url of file to initialize from")
datasetAddCmd.Flags().StringVarP(&addDsDataFilepath, "data", "", "", "data file to initialize from")
datasetAddCmd.Flags().StringVarP(&addDsFile, "file", "f", "", "dataset data file in either (.yaml or .json) file")
datasetAddCmd.Flags().StringVarP(&addDsDataPath, "data", "d", "", "path to file or url to initialize from")
datasetAddCmd.Flags().StringVarP(&addDsTitle, "title", "t", "", "commit title")
datasetAddCmd.Flags().StringVarP(&addDsMessage, "messsage", "m", "", "commit message")
// datasetAddCmd.Flags().StringVarP(&addDsStructureFilepath, "structure", "", "", "dataset structure JSON file")
// datasetAddCmd.Flags().StringVarP(&addDsMetaFilepath, "meta", "", "", "dataset metadata JSON file")
datasetAddCmd.Flags().BoolVarP(&addDsPrivate, "private", "", false, "make dataset private. WARNING: not yet implimented. Please refer to https://github.com/qri-io/qri/issues/291 for updates")
datasetAddCmd.Flags().BoolVarP(&addDsShowValidation, "show-validation", "s", false, "display a list of validation errors upon adding")
// datasetAddCmd.Flags().BoolVarP(&addDsShowValidation, "show-validation", "s", false, "display a list of validation errors upon adding")
RootCmd.AddCommand(datasetAddCmd)
}
14 changes: 7 additions & 7 deletions cmd/cmd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func TestCommandsIntegration(t *testing.T) {
{"help"},
{"version"},
{"setup", "--peername=" + "alan", "--registry=" + registryServer.URL},
{"config", "get"},
{"config", "get", "-c"},
{"config", "get", "profile"},
{"config", "set", "webapp.port", "3505"},
// TODO - add setting whole config via a file
Expand All @@ -170,12 +170,12 @@ func TestCommandsIntegration(t *testing.T) {

for i, args := range commands {
func() {
// defer func() {
// if e := recover(); e != nil {
// t.Errorf("case %d unexpected panic executing command\n%s\n%s", i, strings.Join(args, " "), e)
// return
// }
// }()
defer func() {
if e := recover(); e != nil {
t.Errorf("case %d unexpected panic executing command\n%s\n%s", i, strings.Join(args, " "), e)
return
}
}()
_, err := executeCommand(RootCmd, args...)
if err != nil {
t.Errorf("case %d unexpected error executing command\n%s\n%s", i, strings.Join(args, " "), err.Error())
Expand Down
Loading

0 comments on commit e63694f

Please sign in to comment.