From 02853c2957e93237564d3606284db0fdb917ae33 Mon Sep 17 00:00:00 2001 From: Kristian Borgwarth <10348902@pm.me> Date: Mon, 6 Apr 2026 16:00:23 +0200 Subject: [PATCH] feat(create_note): handle template read from temp name instead of path --- core/handlers/create_note_handler.go | 13 +- docs/uow-refactor.md | 124 ++++++++++++++++++ persistence/store/file_store.go | 6 +- .../create_note_handler_test.go | 54 +++++--- 4 files changed, 169 insertions(+), 28 deletions(-) create mode 100644 docs/uow-refactor.md diff --git a/core/handlers/create_note_handler.go b/core/handlers/create_note_handler.go index 48f4c5f..35d049a 100644 --- a/core/handlers/create_note_handler.go +++ b/core/handlers/create_note_handler.go @@ -3,6 +3,7 @@ package handlers import ( "context" "encoding/json" + "path/filepath" "github.com/KristianJBorgwarth/dendrite.daemon/core/frontmatter" "github.com/KristianJBorgwarth/dendrite.daemon/core/models" @@ -35,8 +36,12 @@ func (h *CreateNoteHandler) Handle(ctx context.Context, raw json.RawMessage) (an slug := frontmatter.Slugify(cmd.Title) - templatePath := store.GetVaultStore().GetTemplatePath(cmd.TemplateName) - notePath := store.GetVaultStore().Config.VaultPath() + "/" + cmd.Directory + "/" + slug + ".md" + var templatePath string + if cmd.TemplateName != "" { + templatePath = store.GetVaultStore().GetTemplatePath(cmd.TemplateName) + } + + notePath := filepath.Join(store.GetVaultStore().Config.VaultPath(), cmd.Directory, slug+".md") data, err := template.RenderTemplate(templatePath, cmd.Title, slug) if err != nil { @@ -93,11 +98,11 @@ func (h *CreateNoteHandler) Handle(ctx context.Context, raw json.RawMessage) (an return nil, err } - h.uow.FileStore.Stage(cmd.Directory, data) + h.uow.FileStore.Stage(notePath, data) if err = h.uow.Commit(); err != nil { return nil, err } - return cmd.Directory, nil + return notePath, nil } diff --git a/docs/uow-refactor.md b/docs/uow-refactor.md new file mode 100644 index 0000000..26d89e1 --- /dev/null +++ b/docs/uow-refactor.md @@ -0,0 +1,124 @@ +# Unit of Work — Refactor Notes + +## Current Issues + +### 1. Lifetime +`UnitOfWork` is created once in the handler constructor and reused across every `Handle` call. +It should be **per-request**, created inside `Handle`. + +### 2. Transaction leaks out +`Begin()` returns `*sql.Tx`, which is then passed manually to each repo constructor. +The UoW should own that wiring — callers should never touch the transaction directly. + +### 3. Eager instantiation (rejected solution) +Having UoW create all repos in `Begin()` solves the leaking transaction, but forces every handler +to pay for repos it doesn't need. Not acceptable. + +--- + +## Solution: Lazy Initialization + +UoW exposes accessor methods that initialize each repo on first access, wired to the internal transaction. +Unused repos are never instantiated. + +```go +type UnitOfWork struct { + FileStore *store.FileStore + tx *sql.Tx + tags ITagRepository + notes INoteRepository +} + +func NewUnitOfWork() *UnitOfWork { + return &UnitOfWork{FileStore: store.NewFileStore()} +} + +func (u *UnitOfWork) Begin() error { + tx, err := persistence.GetDBContext().DB.Begin() + if err != nil { + return err + } + u.tx = tx + return nil +} + +func (u *UnitOfWork) Tags() ITagRepository { + if u.tags == nil { + u.tags = NewTagRepository(u.tx) + } + return u.tags +} + +func (u *UnitOfWork) Notes() INoteRepository { + if u.notes == nil { + u.notes = NewNoteRepository(u.tx) + } + return u.notes +} + +func (u *UnitOfWork) Commit() error { + if err := u.FileStore.Flush(); err != nil { + u.tx.Rollback() + u.tx = nil + return err + } + if err := u.tx.Commit(); err != nil { + u.FileStore.Rollback() + u.tx = nil + return err + } + u.tx = nil + return nil +} + +func (u *UnitOfWork) Rollback() { + if u.tx == nil { + return + } + u.tx.Rollback() + u.FileStore.Rollback() + u.tx = nil +} +``` + +--- + +## Usage in a Handler + +```go +func (h *CreateNoteHandler) Handle(ctx context.Context, raw json.RawMessage) (any, error) { + var cmd createNoteCommand + if err := json.Unmarshal(raw, &cmd); err != nil { + return nil, err + } + + uow := repositories.NewUnitOfWork() + if err := uow.Begin(); err != nil { + return nil, err + } + defer uow.Rollback() + + // Notes() is never called — never instantiated + dbTags, err := uow.Tags().GetByNames(ctx, tags) + if err != nil { + return nil, err + } + + // ... + + uow.FileStore.Stage(cmd.Path, data) + return cmd.Path, uow.Commit() +} +``` + +--- + +## Properties of This Design + +| Property | Result | +|---|---| +| Transaction leaks out of UoW | No — `tx` is private | +| Repos instantiated when not needed | No — lazy on first access | +| Handler controls which repos it uses | Yes — only accessed repos are created | +| UoW created per-request | Yes — inside `Handle`, not constructor | +| Handler has no persistent state | Yes — can be zero-value struct | diff --git a/persistence/store/file_store.go b/persistence/store/file_store.go index 1eb94e9..4e0cb8c 100644 --- a/persistence/store/file_store.go +++ b/persistence/store/file_store.go @@ -1,6 +1,8 @@ package store -import "os" +import ( + "os" +) type FileStore struct { staged []stagedFile @@ -28,7 +30,6 @@ func (fs *FileStore) Flush() error { if fs.fileExists(file.path) { continue } - if err := os.WriteFile(file.path, file.data, 0o644); err != nil { for _, path := range writtenPaths { _ = os.Remove(path) @@ -58,4 +59,3 @@ func (fs *FileStore) fileExists(path string) bool { _, err := os.Stat(path) return err == nil } - diff --git a/test/test_integration/create_note_handler_test.go b/test/test_integration/create_note_handler_test.go index d182269..ec71470 100644 --- a/test/test_integration/create_note_handler_test.go +++ b/test/test_integration/create_note_handler_test.go @@ -14,12 +14,13 @@ import ( func TestCreateNoteHandler_NoTemplate_CreatesNoteFileAndReturnsPath(t *testing.T) { // Arrange handler := handlers.NewCreateNoteHandler() - - notePath := filepath.Join(t.TempDir(), "my-note.md") + + vaultPath := Fixture.VaultStore.Config.VaultPath() + notePath := filepath.Join(vaultPath, "my-note.md") params, _ := json.Marshal(map[string]any{ - "title": "My Note", + "title": "My Note", "templateName": "", - "directory": "", + "directory": "", }) // Act @@ -40,16 +41,23 @@ func TestCreateNoteHandler_NoTemplate_CreatesNoteFileAndReturnsPath(t *testing.T func TestCreateNoteHandler_WithTemplate_CreatesNoteFileWithTagsAndReturnsPath(t *testing.T) { // Arrange handler := handlers.NewCreateNoteHandler() - dir := t.TempDir() - templatePath := filepath.Join(dir, "template.md") + vaultPath := Fixture.VaultStore.Config.VaultPath() + templateDir := Fixture.VaultStore.Config.TemplateDirectory() + require.NoError(t, os.MkdirAll(templateDir, 0o755)) + + templatePath := filepath.Join(templateDir, "template.md") require.NoError(t, os.WriteFile(templatePath, []byte("---\ntitle: Template\ntags: [go, testing]\n---\n"), 0o644)) + defer os.Remove(templatePath) - notePath := filepath.Join(dir, "templated-note.md") + subDir := "subdir" + require.NoError(t, os.MkdirAll(filepath.Join(vaultPath, subDir), 0o755)) + defer os.RemoveAll(filepath.Join(vaultPath, subDir)) + notePath := filepath.Join(vaultPath, subDir, "templated-note.md") params, _ := json.Marshal(map[string]any{ "title": "Templated Note", - "path": notePath, - "templatePath": templatePath, + "templateName": "template.md", + "directory": subDir, }) // Act @@ -81,17 +89,21 @@ func TestCreateNoteHandler_WithTemplate_CreatesNoteFileWithTagsAndReturnsPath(t func TestCreateNoteHandler_DuplicateSlug_Upserts(t *testing.T) { // Arrange - dir := t.TempDir() handler := handlers.NewCreateNoteHandler() - - path1 := filepath.Join(dir, "dup-note.md") - params1, _ := json.Marshal(map[string]any{"title": "Dup Note", "path": path1}) + vaultPath := Fixture.VaultStore.Config.VaultPath() + subDir := "dup-dir" + require.NoError(t, os.MkdirAll(filepath.Join(vaultPath, subDir), 0o755)) + defer os.RemoveAll(filepath.Join(vaultPath, subDir)) + + params1, _ := json.Marshal(map[string]any{"title": "Dup Note", "templateName": "", "directory": subDir}) _, err := handler.Handle(Fixture.TestContext, params1) require.NoError(t, err) - path2 := filepath.Join(dir, "dup-note-moved.md") - params2, _ := json.Marshal(map[string]any{"title": "Dup Note", "path": path2}) + subDir2 := "dup-dir-moved" + require.NoError(t, os.MkdirAll(filepath.Join(vaultPath, subDir2), 0o755)) + defer os.RemoveAll(filepath.Join(vaultPath, subDir2)) + params2, _ := json.Marshal(map[string]any{"title": "Dup Note", "templateName": "", "directory": subDir2}) // Act _, err = handler.Handle(Fixture.TestContext, params2) @@ -103,15 +115,15 @@ func TestCreateNoteHandler_DuplicateSlug_Upserts(t *testing.T) { require.NoError(t, Fixture.DB.QueryRow(`SELECT COUNT(*) FROM notes WHERE slug = ?`, "dup-note").Scan(&count)) assert.Equal(t, 1, count) + expectedPath := filepath.Join(vaultPath, subDir2, "dup-note.md") var path string require.NoError(t, Fixture.DB.QueryRow(`SELECT path FROM notes WHERE slug = ?`, "dup-note").Scan(&path)) - assert.Equal(t, path2, path) + assert.Equal(t, expectedPath, path) } func TestCreateNoteHandler_InvalidJSON_ReturnsError(t *testing.T) { // Arrange handler := handlers.NewCreateNoteHandler() - // Act _, err := handler.Handle(Fixture.TestContext, json.RawMessage(`{invalid json}`)) @@ -120,14 +132,14 @@ func TestCreateNoteHandler_InvalidJSON_ReturnsError(t *testing.T) { assert.Error(t, err) } -func TestCreateNoteHandler_NonExistentTemplatePath_ReturnsError(t *testing.T) { +func TestCreateNoteHandler_NonExistentTemplateName_ReturnsError(t *testing.T) { // Arrange handler := handlers.NewCreateNoteHandler() - + params, _ := json.Marshal(map[string]any{ "title": "Ghost Note", - "path": filepath.Join(t.TempDir(), "ghost.md"), - "templatePath": "/non/existent/template.md", + "templateName": "non-existent-template", + "directory": "", }) // Act