Fix sorting indexed notes by their paths in the database

adapter/sqlite/db.go

@@ -50,6 +50,7 @@ func (db *DB) Migrate() error {
 				`CREATE TABLE IF NOT EXISTS notes (
 					id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
 					path TEXT NOT NULL,
+					sortable_path TEXT NOT NULL,
 					title TEXT DEFAULT('') NOT NULL,
 					lead TEXT DEFAULT('') NOT NULL,
 					body TEXT DEFAULT('') NOT NULL,

adapter/sqlite/db_test.go

@@ -3,8 +3,8 @@ package sqlite
 import (
 	"testing"
 
-	"github.com/mickael-menu/zk/util/test/assert"
 	"github.com/mickael-menu/zk/util/fixtures"
+	"github.com/mickael-menu/zk/util/test/assert"
 )
 
 func TestOpen(t *testing.T) {
@@ -36,8 +36,8 @@ func TestMigrateFrom0(t *testing.T) {
 		assert.Equal(t, version, 1)
 
 		_, err = tx.Exec(`
-			INSERT INTO notes (path, title, body, word_count, checksum)
-			VALUES ("ref/tx1.md", "A reference", "Content", 1, "qwfpg")
+			INSERT INTO notes (path, sortable_path, title, body, word_count, checksum)
+			VALUES ("ref/tx1.md", "reftx1.md", "A reference", "Content", 1, "qwfpg")
 		`)
 		assert.Nil(t, err)
 

adapter/sqlite/fixtures/default/notes.yml

@@ -1,5 +1,6 @@
 - id: 1
   path: "log/2021-01-03.md"
+  sortable_path: "log/2021-01-03.md"
   title: "January 3, 2021"
   lead: "A daily note"
   body: "A daily note\n\nWith lot of content"
@@ -11,6 +12,7 @@
 
 - id: 2
   path: "log/2021-01-04.md"
+  sortable_path: "log/2021-01-04.md"
   title: "January 4, 2021"
   lead: "A second daily note"
   body: "A second daily note"
@@ -22,6 +24,7 @@
 
 - id: 3
   path: "index.md"
+  sortable_path: "index.md"
   title: "Index"
   lead: "Index of the Zettelkasten"
   body: "Index of the Zettelkasten"
@@ -33,6 +36,7 @@
 
 - id: 4
   path: "f39c8.md"
+  sortable_path: "f39c8.md"
   title: "An interesting note"
   lead: "Its content will surprise you"
   body: "Its content will surprise you"
@@ -44,6 +48,7 @@
 
 - id: 5
   path: "ref/test/b.md"
+  sortable_path: "ref/test/b.md"
   title: "A nested note"
   lead: "This one is in a sub sub directory"
   body: "This one is in a sub sub directory"
@@ -55,6 +60,7 @@
 
 - id: 6
   path: "ref/test/a.md"
+  sortable_path: "ref/test/a.md"
   title: "Another nested note"
   lead: "It shall appear before b.md"
   body: "It shall appear before b.md"
@@ -66,6 +72,7 @@
 
 - id: 7
   path: "log/2021-02-04.md"
+  sortable_path: "log/2021-02-04.md"
   title: "February 4, 2021"
   lead: "A third daily note"
   body: "A third daily note"

adapter/sqlite/note_dao.go

@@ -33,11 +33,11 @@ func NewNoteDAO(tx Transaction, logger util.Logger) *NoteDAO {
 		logger: logger,
 		indexedStmt: tx.PrepareLazy(`
 			SELECT path, modified from notes
-			 ORDER BY path ASC
+			 ORDER BY sortable_path ASC
 		`),
 		addStmt: tx.PrepareLazy(`
-			INSERT INTO notes (path, title, lead, body, raw_content, word_count, checksum, created, modified)
-			VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
+			INSERT INTO notes (path, sortable_path, title, lead, body, raw_content, word_count, checksum, created, modified)
+			VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
 		`),
 		updateStmt: tx.PrepareLazy(`
 			UPDATE notes
@@ -93,8 +93,16 @@ func (d *NoteDAO) Indexed() (<-chan paths.Metadata, error) {
 }
 
 func (d *NoteDAO) Add(note note.Metadata) error {
+	// For sortable_path, we replace in path / by the shortest non printable
+	// character available to make it sortable. Without this, sorting by the
+	// path would be a lexicographical sort instead of being the same order
+	// returned by filepath.Walk.
+	// \x01 is used instead of \x00, because SQLite treats \x00 as and end of
+	// string.
+	sortablePath := strings.ReplaceAll(note.Path, "/", "\x01")
+
 	_, err := d.addStmt.Exec(
-		note.Path, note.Title, note.Lead, note.Body, note.RawContent, note.WordCount, note.Checksum,
+		note.Path, sortablePath, note.Title, note.Lead, note.Body, note.RawContent, note.WordCount, note.Checksum,
 		note.Created, note.Modified,
 	)
 	return errors.Wrapf(err, "%v: can't add note to the index", note.Path)

adapter/sqlite/note_dao_test.go

@@ -12,34 +12,78 @@ import (
 )
 
 func TestNoteDAOIndexed(t *testing.T) {
-	testNoteDAO(t, func(tx Transaction, dao *NoteDAO) {
-		expected := []paths.Metadata{
+	testNoteDAOWithoutFixtures(t, func(tx Transaction, dao *NoteDAO) {
+		for _, note := range []note.Metadata{
 			{
-				Path:     "f39c8.md",
+				Path:     "a.md",
 				Modified: time.Date(2020, 1, 20, 8, 52, 42, 0, time.UTC),
 			},
 			{
-				Path:     "index.md",
+				Path:     "dir1/a.md",
 				Modified: time.Date(2019, 12, 4, 12, 17, 21, 0, time.UTC),
 			},
 			{
-				Path:     "log/2021-01-03.md",
+				Path:     "b.md",
+				Modified: time.Date(2020, 11, 22, 16, 27, 45, 0, time.UTC),
+			},
+			{
+				Path:     "dir1/b.md",
+				Modified: time.Date(2020, 11, 29, 8, 20, 18, 0, time.UTC),
+			},
+			{
+				Path:     "dir1/dir1/a.md",
+				Modified: time.Date(2020, 11, 10, 8, 20, 18, 0, time.UTC),
+			},
+			{
+				Path:     "dir2/a.md",
+				Modified: time.Date(2019, 11, 20, 20, 34, 6, 0, time.UTC),
+			},
+			{
+				Path:     "dir1 a space/a.md",
+				Modified: time.Date(2019, 11, 20, 20, 34, 6, 0, time.UTC),
+			},
+			{
+				Path:     "Dir3/a.md",
+				Modified: time.Date(2019, 11, 12, 20, 34, 6, 0, time.UTC),
+			},
+		} {
+			assert.Nil(t, dao.Add(note))
+		}
+
+		// We check that the metadata are sorted by the path but not
+		// lexicographically. Instead it needs to be sorted on each path
+		// component, like filepath.Walk would.
+		expected := []paths.Metadata{
+			{
+				Path:     "Dir3/a.md",
+				Modified: time.Date(2019, 11, 12, 20, 34, 6, 0, time.UTC),
+			},
+			{
+				Path:     "a.md",
+				Modified: time.Date(2020, 1, 20, 8, 52, 42, 0, time.UTC),
+			},
+			{
+				Path:     "b.md",
 				Modified: time.Date(2020, 11, 22, 16, 27, 45, 0, time.UTC),
 			},
 			{
-				Path:     "log/2021-01-04.md",
+				Path:     "dir1/a.md",
+				Modified: time.Date(2019, 12, 4, 12, 17, 21, 0, time.UTC),
+			},
+			{
+				Path:     "dir1/b.md",
 				Modified: time.Date(2020, 11, 29, 8, 20, 18, 0, time.UTC),
 			},
 			{
-				Path:     "log/2021-02-04.md",
+				Path:     "dir1/dir1/a.md",
 				Modified: time.Date(2020, 11, 10, 8, 20, 18, 0, time.UTC),
 			},
 			{
-				Path:     "ref/test/a.md",
+				Path:     "dir1 a space/a.md",
 				Modified: time.Date(2019, 11, 20, 20, 34, 6, 0, time.UTC),
 			},
 			{
-				Path:     "ref/test/b.md",
+				Path:     "dir2/a.md",
 				Modified: time.Date(2019, 11, 20, 20, 34, 6, 0, time.UTC),
 			},
 		}
@@ -455,6 +499,12 @@ func testNoteDAO(t *testing.T, callback func(tx Transaction, dao *NoteDAO)) {
 	})
 }
 
+func testNoteDAOWithoutFixtures(t *testing.T, callback func(tx Transaction, dao *NoteDAO)) {
+	testTransactionWithoutFixtures(t, func(tx Transaction) {
+		callback(tx, NewNoteDAO(tx, &util.NullLogger))
+	})
+}
+
 type noteRow struct {
 	Path, Title, Lead, Body, RawContent, Checksum string
 	WordCount                                     int

adapter/sqlite/transaction_test.go

@@ -4,27 +4,42 @@ import (
 	"testing"
 
 	"github.com/go-testfixtures/testfixtures/v3"
+	"github.com/mickael-menu/zk/util/opt"
 	"github.com/mickael-menu/zk/util/test/assert"
 )
 
 // testTransaction is an utility function used to test a SQLite transaction to
-// the DB.
+// the DB, which loads the default set of DB fixtures.
 func testTransaction(t *testing.T, test func(tx Transaction)) {
+	testTransactionWithFixtures(t, opt.NewString("default"), test)
+}
+
+// testTransaction is an utility function used to test a SQLite transaction to
+// an empty DB.
+func testTransactionWithoutFixtures(t *testing.T, test func(tx Transaction)) {
+	testTransactionWithFixtures(t, opt.NullString, test)
+}
+
+// testTransactionWithFixtures is an utility function used to test a SQLite transaction to
+// the DB, which loads the given set of DB fixtures.
+func testTransactionWithFixtures(t *testing.T, fixturesDir opt.String, test func(tx Transaction)) {
 	db, err := OpenInMemory()
 	assert.Nil(t, err)
 	err = db.Migrate()
 	assert.Nil(t, err)
 
-	fixtures, err := testfixtures.New(
-		testfixtures.Database(db.db),
-		testfixtures.Dialect("sqlite"),
-		testfixtures.Directory("fixtures"),
-		// Necessary to work with an in-memory database.
-		testfixtures.DangerousSkipTestDatabaseCheck(),
-	)
-	assert.Nil(t, err)
-	err = fixtures.Load()
-	assert.Nil(t, err)
+	if !fixturesDir.IsNull() {
+		fixtures, err := testfixtures.New(
+			testfixtures.Database(db.db),
+			testfixtures.Dialect("sqlite"),
+			testfixtures.Directory("fixtures/"+fixturesDir.String()),
+			// Necessary to work with an in-memory database.
+			testfixtures.DangerousSkipTestDatabaseCheck(),
+		)
+		assert.Nil(t, err)
+		err = fixtures.Load()
+		assert.Nil(t, err)
+	}
 
 	err = db.WithTransaction(func(tx Transaction) error {
 		test(tx)

util/paths/walk_test.go

@@ -18,11 +18,13 @@ func TestWalk(t *testing.T) {
 	}
 
 	assert.Equal(t, actual, []string{
+		"Dir3/a.md",
 		"a.md",
 		"b.md",
 		"dir1/a.md",
 		"dir1/b.md",
 		"dir1/dir1/a.md",
+		"dir1 a space/a.md",
 		"dir2/a.md",
 	})
 }