Refactor and test note creation

cmd/new.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/mickael-menu/zk/core/note"
 	"github.com/mickael-menu/zk/core/zk"
+	"github.com/mickael-menu/zk/util"
 	"github.com/mickael-menu/zk/util/opt"
 )
 
@@ -17,25 +18,35 @@ type New struct {
 	Extra     map[string]string `help:"Extra variables passed to the templates"`
 }
 
+func (cmd *New) ConfigOverrides() zk.ConfigOverrides {
+	return zk.ConfigOverrides{
+		BodyTemplatePath: opt.NewNotEmptyString(cmd.Template),
+		Extra:            cmd.Extra,
+	}
+}
+
 func (cmd *New) Run(container *Container) error {
 	zk, err := zk.Open(".")
 	if err != nil {
 		return err
 	}
 
-	dir, err := zk.DirAt(cmd.Directory)
+	dir, err := zk.RequireDirAt(cmd.Directory, cmd.ConfigOverrides())
+	if err != nil {
+		return err
+	}
+
+	content, err := util.ReadStdinPipe()
 	if err != nil {
 		return err
 	}
 
 	opts := note.CreateOpts{
-		Dir:      *dir,
-		Title:    opt.NewNotEmptyString(cmd.Title),
-		Content:  opt.NullString,
-		Template: opt.NewNotEmptyString(cmd.Template),
-		Extra:    cmd.Extra,
+		Dir:     *dir,
+		Title:   opt.NewNotEmptyString(cmd.Title),
+		Content: content,
 	}
-	file, err := note.Create(zk, opts, container.TemplateLoader())
+	file, err := note.Create(opts, container.TemplateLoader())
 	if err != nil {
 		return err
 	}

core/note/create.go

@@ -20,135 +20,125 @@ type CreateOpts struct {
 	Title opt.String
 	// Initial content of the note, which will be injected in the template.
 	Content opt.String
-	// Custom template to use for the note, overriding the one declared in the config.
-	Template opt.String
-	// Extra template variables to expand.
-	Extra map[string]string
 }
 
 // Create generates a new note from the given options.
-func Create(zk *zk.Zk, opts CreateOpts, templateLoader core.TemplateLoader) (string, error) {
-	wrap := errors.Wrapper("note creation failed")
+// Returns the path of the newly created note.
+func Create(
+	opts CreateOpts,
+	templateLoader core.TemplateLoader,
+) (string, error) {
+	wrap := errors.Wrapperf("new note")
 
-	exists, err := paths.Exists(opts.Dir.Path)
+	filenameTemplate, err := templateLoader.Load(opts.Dir.Config.FilenameTemplate)
 	if err != nil {
-		return "", wrap(err)
+		return "", err
 	}
-	if !exists {
-		return "", wrap(fmt.Errorf("directory not found at %v", opts.Dir.Path))
+
+	var bodyTemplate core.Template = core.NullTemplate
+	if templatePath := opts.Dir.Config.BodyTemplatePath.Unwrap(); templatePath != "" {
+		bodyTemplate, err = templateLoader.LoadFile(templatePath)
+		if err != nil {
+			return "", wrap(err)
+		}
 	}
 
-	context, err := newRenderContext(zk, opts, templateLoader)
+	createdNote, err := create(opts, createDeps{
+		filenameTemplate: filenameTemplate,
+		bodyTemplate:     bodyTemplate,
+		genId:            rand.NewIDGenerator(opts.Dir.Config.IDOptions),
+		validatePath:     validatePath,
+	})
 	if err != nil {
 		return "", wrap(err)
 	}
 
-	templatePath := opts.Template.OrDefault(
-		zk.Template(opts.Dir).OrDefault(""),
-	)
-	if templatePath != "" {
-		template, err := templateLoader.LoadFile(templatePath)
-		if err != nil {
-			return "", wrap(err)
-		}
-		content, err := template.Render(context)
-		if err != nil {
-			return "", wrap(err)
-		}
-		err = paths.WriteString(context.Path, content)
-		if err != nil {
-			return "", wrap(err)
-		}
-
-		fmt.Printf("<<<\n%v\n<<<\n", content)
+	err = paths.WriteString(createdNote.path, createdNote.content)
+	if err != nil {
+		return "", wrap(err)
 	}
 
-	return context.Path, nil
+	return createdNote.path, nil
+}
+
+func validatePath(path string) (bool, error) {
+	exists, err := paths.Exists(path)
+	return !exists, err
+}
+
+type createdNote struct {
+	path    string
+	content string
 }
 
 // renderContext holds the placeholder values which will be expanded in the templates.
 type renderContext struct {
+	ID           string `handlebars:"id"`
 	Title        string
 	Content      string
-	Path         string
+	Dir          string
 	Filename     string
 	FilenameStem string `handlebars:"filename-stem"`
-	ID           string
 	Extra        map[string]string
 }
 
-func newRenderContext(zk *zk.Zk, opts CreateOpts, templateLoader core.TemplateLoader) (renderContext, error) {
-	if opts.Extra == nil {
-		opts.Extra = make(map[string]string)
-	}
-	for k, v := range zk.Extra(opts.Dir) {
-		if _, ok := opts.Extra[k]; !ok {
-			opts.Extra[k] = v
-		}
-	}
-
-	template, err := templateLoader.Load(zk.FilenameTemplate(opts.Dir))
-	if err != nil {
-		return renderContext{}, err
-	}
-
-	genContext := newRenderContextGenerator(template, opts)
-	genId := rand.NewIDGenerator(zk.IDOptions(opts.Dir))
-	// Attempts to generate a new render context until the generated filepath doesn't exist.
-	for {
-		context, err := genContext(genId())
-		if err != nil {
-			return context, err
-		}
-		exists, err := paths.Exists(context.Path)
-		if err != nil {
-			return context, err
-		}
-		if !exists {
-			return context, nil
-		}
-	}
+type createDeps struct {
+	filenameTemplate core.Template
+	bodyTemplate     core.Template
+	genId            func() string
+	validatePath     func(path string) (bool, error)
 }
 
-type renderContextGenerator func(id string) (renderContext, error)
-
-func newRenderContextGenerator(
-	filenameTemplate core.Template,
+func create(
 	opts CreateOpts,
-) renderContextGenerator {
+	deps createDeps,
+) (*createdNote, error) {
 	context := renderContext{
 		// FIXME Customize default title in config
 		Title:   opts.Title.OrDefault("Untitled"),
 		Content: opts.Content.Unwrap(),
-		Extra:   opts.Extra,
+		Dir:     opts.Dir.Name,
+		Extra:   opts.Dir.Config.Extra,
 	}
 
-	i := 0
-	return func(id string) (renderContext, error) {
-		i++
-		// Attempts 50ish tries before failing.
-		if i >= 50 {
-			return context, fmt.Errorf("%v: file already exists", context.Path)
-		}
+	path, context, err := genPath(context, opts.Dir.Path, deps)
+	if err != nil {
+		return nil, err
+	}
 
-		context.ID = id
+	content, err := deps.bodyTemplate.Render(context)
+	if err != nil {
+		return nil, err
+	}
 
-		filename, err := filenameTemplate.Render(context)
+	return &createdNote{path: path, content: content}, nil
+}
+
+func genPath(
+	context renderContext,
+	basePath string,
+	deps createDeps,
+) (string, renderContext, error) {
+	var path string
+	for i := 0; i < 50; i++ {
+		context.ID = deps.genId()
+
+		filename, err := deps.filenameTemplate.Render(context)
 		if err != nil {
-			return context, err
+			return "", context, err
 		}
 
 		// FIXME Customize extension in config
-		path := filepath.Join(opts.Dir.Path, filename+".md")
-		// Same path as before? We can fail because there's no random component
-		// in the filename template.
-		if context.Path == path {
-			return context, fmt.Errorf("%v: file already exists", path)
+		path = filepath.Join(basePath, filename+".md")
+		validPath, err := deps.validatePath(path)
+		if err != nil {
+			return "", context, err
+		} else if validPath {
+			context.Filename = filepath.Base(path)
+			context.FilenameStem = paths.FilenameStem(path)
+			return path, context, nil
 		}
-
-		context.Path = path
-		context.Filename = filepath.Base(path)
-		context.FilenameStem = paths.FilenameStem(path)
-		return context, nil
 	}
+
+	return "", context, fmt.Errorf("%v: note already exists", path)
 }

core/note/create_test.go

@@ -0,0 +1,171 @@
+package note
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/mickael-menu/zk/core"
+	"github.com/mickael-menu/zk/core/zk"
+	"github.com/mickael-menu/zk/util/assert"
+	"github.com/mickael-menu/zk/util/opt"
+)
+
+func TestCreate(t *testing.T) {
+	filenameTemplate := spyTemplateString("filename")
+	bodyTemplate := spyTemplateString("body")
+
+	res, err := create(
+		CreateOpts{
+			Dir: zk.Dir{
+				Name: "log",
+				Path: "/test/log",
+				Config: zk.DirConfig{
+					Extra: map[string]string{
+						"hello": "world",
+					},
+				},
+			},
+			Title:   opt.NewString("Note title"),
+			Content: opt.NewString("Note content"),
+		},
+		createDeps{
+			filenameTemplate: &filenameTemplate,
+			bodyTemplate:     &bodyTemplate,
+			genId:            func() string { return "abc" },
+			validatePath:     func(path string) (bool, error) { return true, nil },
+		},
+	)
+
+	// Check the created note.
+	assert.Nil(t, err)
+	assert.Equal(t, res, &createdNote{
+		path:    "/test/log/filename.md",
+		content: "body",
+	})
+
+	// Check that the templates received the proper render contexts.
+	assert.Equal(t, filenameTemplate.Contexts, []renderContext{{
+		ID:      "abc",
+		Title:   "Note title",
+		Content: "Note content",
+		Dir:     "log",
+		Extra: map[string]string{
+			"hello": "world",
+		},
+	}})
+	assert.Equal(t, bodyTemplate.Contexts, []renderContext{{
+		ID:           "abc",
+		Title:        "Note title",
+		Content:      "Note content",
+		Dir:          "log",
+		Filename:     "filename.md",
+		FilenameStem: "filename",
+		Extra: map[string]string{
+			"hello": "world",
+		},
+	}})
+}
+
+func TestCreateTriesUntilValidPath(t *testing.T) {
+	filenameTemplate := spyTemplate(func(context renderContext) string {
+		return context.ID
+	})
+	bodyTemplate := spyTemplateString("body")
+
+	res, err := create(
+		CreateOpts{
+			Dir: zk.Dir{
+				Name: "log",
+				Path: "/test/log",
+			},
+			Title: opt.NewString("Note title"),
+		},
+		createDeps{
+			filenameTemplate: &filenameTemplate,
+			bodyTemplate:     &bodyTemplate,
+			genId:            incrementingID(),
+			validatePath: func(path string) (bool, error) {
+				return path == "/test/log/3.md", nil
+			},
+		},
+	)
+
+	// Check the created note.
+	assert.Nil(t, err)
+	assert.Equal(t, res, &createdNote{
+		path:    "/test/log/3.md",
+		content: "body",
+	})
+
+	assert.Equal(t, filenameTemplate.Contexts, []renderContext{
+		{
+			ID:    "1",
+			Title: "Note title",
+			Dir:   "log",
+		},
+		{
+			ID:    "2",
+			Title: "Note title",
+			Dir:   "log",
+		},
+		{
+			ID:    "3",
+			Title: "Note title",
+			Dir:   "log",
+		},
+	})
+}
+
+func TestCreateErrorWhenNoValidPaths(t *testing.T) {
+	_, err := create(
+		CreateOpts{
+			Dir: zk.Dir{
+				Name: "log",
+				Path: "/test/log",
+			},
+		},
+		createDeps{
+			filenameTemplate: core.TemplateFunc(func(context interface{}) (string, error) {
+				return "filename", nil
+			}),
+			bodyTemplate: core.NullTemplate,
+			genId:        func() string { return "abc" },
+			validatePath: func(path string) (bool, error) { return false, nil },
+		},
+	)
+
+	assert.Err(t, err, "/test/log/filename.md: note already exists")
+}
+
+func spyTemplate(result func(renderContext) string) TemplateSpy {
+	return TemplateSpy{
+		Contexts: make([]renderContext, 0),
+		Result:   result,
+	}
+}
+
+func spyTemplateString(result string) TemplateSpy {
+	return TemplateSpy{
+		Contexts: make([]renderContext, 0),
+		Result:   func(_ renderContext) string { return result },
+	}
+}
+
+type TemplateSpy struct {
+	Result   func(renderContext) string
+	Contexts []renderContext
+}
+
+func (m *TemplateSpy) Render(context interface{}) (string, error) {
+	renderContext := context.(renderContext)
+	m.Contexts = append(m.Contexts, renderContext)
+	return m.Result(renderContext), nil
+}
+
+func incrementingID() func() string {
+	i := 0
+	return func() string {
+		i++
+		return fmt.Sprintf("%d", i)
+	}
+}

core/template.go

@@ -1,12 +1,28 @@
 package core
 
+// TemplateLoader parses a given string template.
+type TemplateLoader interface {
+	Load(template string) (Template, error)
+	LoadFile(path string) (Template, error)
+}
+
 // Template renders strings using a given context.
 type Template interface {
 	Render(context interface{}) (string, error)
 }
 
-// TemplateLoader parses a given string template.
-type TemplateLoader interface {
-	Load(template string) (Template, error)
-	LoadFile(path string) (Template, error)
+// TemplateFunc is an adapter to use a function as a Template.
+type TemplateFunc func(context interface{}) (string, error)
+
+func (f TemplateFunc) Render(context interface{}) (string, error) {
+	return f(context)
+}
+
+// NullTemplate is a Template returning always an empty string.
+var NullTemplate = nullTemplate{}
+
+type nullTemplate struct{}
+
+func (t nullTemplate) Render(context interface{}) (string, error) {
+	return "", nil
 }

util/assert/assert.go

@@ -3,6 +3,7 @@ package assert
 import (
 	"encoding/json"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -35,3 +36,9 @@ func toJSON(t *testing.T, obj interface{}) string {
 	Nil(t, err)
 	return string(json)
 }
+
+func Err(t *testing.T, err error, expected string) {
+	if !strings.Contains(err.Error(), expected) {
+		t.Errorf("Expected error `%v`, received `%v`", expected, err.Error())
+	}
+}

util/os.go

@@ -0,0 +1,29 @@
+package util
+
+import (
+	"bufio"
+	"io/ioutil"
+	"os"
+
+	"github.com/mickael-menu/zk/util/opt"
+)
+
+// ReadStdinPipe returns the content of any piped input.
+func ReadStdinPipe() (opt.String, error) {
+	fi, err := os.Stdin.Stat()
+	if err != nil {
+		return opt.NullString, err
+	}
+	if fi.Mode()&os.ModeNamedPipe == 0 {
+		// Not a pipe
+		return opt.NullString, nil
+	}
+
+	reader := bufio.NewReader(os.Stdin)
+	bytes, err := ioutil.ReadAll(reader)
+	if err != nil {
+		return opt.NullString, err
+	}
+
+	return opt.NewNotEmptyString(string(bytes)), nil
+}