feat: store created and modified time separately on database for book…

…marks (#896) * sqlite migrate script * create time just when bookmark added and modified update if change happen * show added and modified time in footer instead of header * add bun.lockb that missing * add migrate for postgres * add pg support of created time * change modifed to modifed_at and create to created_at in sqlite * change modifed to modifed_at and create to created_at in postgre * add created_at to mariadb * fix migration file names * better variable name and more clear code for add modified time if created and modified is not in same day * add unittest * add unittest to sure filters work as expected * index for created_at and modified_at * build new styles.css * update swagger documents * make styles * change Created and Modified to CreatedAt and ModifiedAt * fix missing Modified * fix typo * missing Modified * fix typo * make swagger * run tests parallel Co-authored-by: Felipe Martin <[email protected]> * remove t.Parallel() * remove dayjs dependency and combine two function * better unittest name * fix typo * diffrnt footer style for login and content page * use class instead of id * back parallel * change duplicate url * remvoe run Parallel * make styles --------- Co-authored-by: Felipe Martin <[email protected]>
go-shiori · Jun 26, 2024 · 4a5564d · 4a5564d
1 parent a3d4a68
commit 4a5564d
Show file tree

Hide file tree

Showing 26 changed files with 284 additions and 99 deletions.
diff --git a/docs/swagger/docs.go b/docs/swagger/docs.go
@@ -389,6 +389,9 @@ const docTemplate = `{
                     "description": "TODO: migrate outside the DTO",
                     "type": "boolean"
                 },
+                "createdAt": {
+                    "type": "string"
+                },
                 "excerpt": {
                     "type": "string"
                 },
@@ -410,7 +413,7 @@ const docTemplate = `{
                 "imageURL": {
                     "type": "string"
                 },
-                "modified": {
+                "modifiedAt": {
                     "type": "string"
                 },
                 "public": {

diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json
@@ -378,6 +378,9 @@
                     "description": "TODO: migrate outside the DTO",
                     "type": "boolean"
                 },
+                "createdAt": {
+                    "type": "string"
+                },
                 "excerpt": {
                     "type": "string"
                 },
@@ -399,7 +402,7 @@
                 "imageURL": {
                     "type": "string"
                 },
-                "modified": {
+                "modifiedAt": {
                     "type": "string"
                 },
                 "public": {

diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml
@@ -90,6 +90,8 @@ definitions:
       create_ebook:
         description: 'TODO: migrate outside the DTO'
         type: boolean
+      createdAt:
+        type: string
       excerpt:
         type: string
       hasArchive:
@@ -104,7 +106,7 @@ definitions:
         type: integer
       imageURL:
         type: string
-      modified:
+      modifiedAt:
         type: string
       public:
         type: integer

diff --git a/internal/cmd/export.go b/internal/cmd/export.go
@@ -62,7 +62,7 @@ func exportHandler(cmd *cobra.Command, args []string) {
 
 	for _, book := range bookmarks {
 		// Create Unix timestamp for bookmark
-		modifiedTime, err := time.Parse(model.DatabaseDateFormat, book.Modified)
+		modifiedTime, err := time.Parse(model.DatabaseDateFormat, book.ModifiedAt)
 		if err != nil {
 			modifiedTime = time.Now()
 		}

diff --git a/internal/cmd/import.go b/internal/cmd/import.go
@@ -136,10 +136,10 @@ func importHandler(cmd *cobra.Command, args []string) {
 
 		// Add item to list
 		bookmark := model.BookmarkDTO{
-			URL:      url,
-			Title:    title,
-			Tags:     tags,
-			Modified: modifiedDate.Format(model.DatabaseDateFormat),
+			URL:        url,
+			Title:      title,
+			Tags:       tags,
+			ModifiedAt: modifiedDate.Format(model.DatabaseDateFormat),
 		}
 
 		mapURL[url] = struct{}{}

diff --git a/internal/cmd/pocket.go b/internal/cmd/pocket.go
@@ -94,10 +94,10 @@ func pocketHandler(cmd *cobra.Command, args []string) {
 
 		// Add item to list
 		bookmark := model.BookmarkDTO{
-			URL:      url,
-			Title:    title,
-			Modified: modified.Format(model.DatabaseDateFormat),
-			Tags:     tags,
+			URL:        url,
+			Title:      title,
+			ModifiedAt: modified.Format(model.DatabaseDateFormat),
+			Tags:       tags,
 		}
 
 		mapURL[url] = struct{}{}

diff --git a/internal/core/processing.go b/internal/core/processing.go
@@ -120,6 +120,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
 		}
 
 		book.HasContent = book.Content != ""
+		book.ModifiedAt = ""
 	}
 
 	// Save article image to local disk
@@ -137,6 +138,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
 		}
 		if err == nil {
 			book.ImageURL = fp.Join("/", "bookmark", strID, "thumb")
+			book.ModifiedAt = ""
 			break
 		}
 	}
@@ -154,6 +156,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
 				return book, true, errors.Wrap(err, "failed to create ebook")
 			}
 			book.HasEbook = true
+			book.ModifiedAt = ""
 		}
 	}
 
@@ -186,6 +189,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
 		}
 
 		book.HasArchive = true
+		book.ModifiedAt = ""
 	}
 
 	return book, false, nil

diff --git a/internal/database/database_test.go b/internal/database/database_test.go
@@ -3,6 +3,7 @@ package database
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/go-shiori/shiori/internal/model"
 	"github.com/stretchr/testify/assert"
@@ -15,19 +16,21 @@ type testDatabaseFactory func(t *testing.T, ctx context.Context) (DB, error)
 func testDatabase(t *testing.T, dbFactory testDatabaseFactory) {
 	tests := map[string]databaseTestCase{
 		// Bookmarks
-		"testBookmarkAutoIncrement":         testBookmarkAutoIncrement,
-		"testCreateBookmark":                testCreateBookmark,
-		"testCreateBookmarkWithContent":     testCreateBookmarkWithContent,
-		"testCreateBookmarkTwice":           testCreateBookmarkTwice,
-		"testCreateBookmarkWithTag":         testCreateBookmarkWithTag,
-		"testCreateTwoDifferentBookmarks":   testCreateTwoDifferentBookmarks,
-		"testUpdateBookmark":                testUpdateBookmark,
-		"testUpdateBookmarkWithContent":     testUpdateBookmarkWithContent,
-		"testGetBookmark":                   testGetBookmark,
-		"testGetBookmarkNotExistent":        testGetBookmarkNotExistent,
-		"testGetBookmarks":                  testGetBookmarks,
-		"testGetBookmarksWithSQLCharacters": testGetBookmarksWithSQLCharacters,
-		"testGetBookmarksCount":             testGetBookmarksCount,
+		"testBookmarkAutoIncrement":             testBookmarkAutoIncrement,
+		"testCreateBookmark":                    testCreateBookmark,
+		"testCreateBookmarkWithContent":         testCreateBookmarkWithContent,
+		"testCreateBookmarkTwice":               testCreateBookmarkTwice,
+		"testCreateBookmarkWithTag":             testCreateBookmarkWithTag,
+		"testCreateTwoDifferentBookmarks":       testCreateTwoDifferentBookmarks,
+		"testUpdateBookmark":                    testUpdateBookmark,
+		"testUpdateBookmarkUpdatesModifiedTime": testUpdateBookmarkUpdatesModifiedTime,
+		"testGetBoomarksWithTimeFilters":        testGetBoomarksWithTimeFilters,
+		"testUpdateBookmarkWithContent":         testUpdateBookmarkWithContent,
+		"testGetBookmark":                       testGetBookmark,
+		"testGetBookmarkNotExistent":            testGetBookmarkNotExistent,
+		"testGetBookmarks":                      testGetBookmarks,
+		"testGetBookmarksWithSQLCharacters":     testGetBookmarksWithSQLCharacters,
+		"testGetBookmarksCount":                 testGetBookmarksCount,
 		// Tags
 		"testCreateTag":  testCreateTag,
 		"testCreateTags": testCreateTags,
@@ -424,3 +427,92 @@ func testGetAccounts(t *testing.T, db DB) {
 		assert.Equal(t, 0, len(accounts))
 	})
 }
+
+// TODO: Consider using `t.Parallel()` once we have automated database tests spawning databases using testcontainers.
+func testUpdateBookmarkUpdatesModifiedTime(t *testing.T, db DB) {
+	ctx := context.TODO()
+
+	book := model.BookmarkDTO{
+		URL:   "https://github.com/go-shiori/shiori",
+		Title: "shiori",
+	}
+
+	resultBook, err := db.SaveBookmarks(ctx, true, book)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+
+	updatedBook := resultBook[0]
+	updatedBook.Title = "modified"
+	updatedBook.ModifiedAt = ""
+
+	time.Sleep(1 * time.Second)
+	resultUpdatedBooks, err := db.SaveBookmarks(ctx, false, updatedBook)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+
+	assert.NotEqual(t, resultBook[0].ModifiedAt, resultUpdatedBooks[0].ModifiedAt)
+	assert.Equal(t, resultBook[0].CreatedAt, resultUpdatedBooks[0].CreatedAt)
+	assert.Equal(t, resultBook[0].CreatedAt, resultBook[0].ModifiedAt)
+	assert.NoError(t, err, "Get bookmarks must not fail")
+
+	assert.Equal(t, updatedBook.Title, resultUpdatedBooks[0].Title, "Saved bookmark must have updated Title")
+}
+
+// TODO: Consider using `t.Parallel()` once we have automated database tests spawning databases using testcontainers.
+func testGetBoomarksWithTimeFilters(t *testing.T, db DB) {
+	ctx := context.TODO()
+
+	book1 := model.BookmarkDTO{
+		URL:   "https://github.com/go-shiori/shiori/one",
+		Title: "Added First but Modified Last",
+	}
+	book2 := model.BookmarkDTO{
+		URL:   "https://github.com/go-shiori/shiori/second",
+		Title: "Added Last but Modified First",
+	}
+
+	// create two new bookmark
+	resultBook1, err := db.SaveBookmarks(ctx, true, book1)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+	time.Sleep(1 * time.Second)
+	resultBook2, err := db.SaveBookmarks(ctx, true, book2)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+
+	// update those bookmarks
+	updatedBook1 := resultBook1[0]
+	updatedBook1.Title = "Added First but Modified Last Updated Title"
+	updatedBook1.ModifiedAt = ""
+
+	updatedBook2 := resultBook2[0]
+	updatedBook2.Title = "Last Added but modified First Updated Title"
+	updatedBook2.ModifiedAt = ""
+
+	// modified bookmark2 first after one second modified bookmark1
+	resultUpdatedBook2, err := db.SaveBookmarks(ctx, false, updatedBook2)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+	time.Sleep(1 * time.Second)
+	resultUpdatedBook1, err := db.SaveBookmarks(ctx, false, updatedBook1)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+
+	// get diffrent filteter combination
+	booksOrderByLastAdded, err := db.GetBookmarks(ctx, GetBookmarksOptions{
+		IDs:         []int{resultUpdatedBook1[0].ID, resultUpdatedBook2[0].ID},
+		OrderMethod: 1,
+	})
+	assert.NoError(t, err, "Get bookmarks must not fail")
+	booksOrderByLastModified, err := db.GetBookmarks(ctx, GetBookmarksOptions{
+		IDs:         []int{resultUpdatedBook1[0].ID, resultUpdatedBook2[0].ID},
+		OrderMethod: 2,
+	})
+	assert.NoError(t, err, "Get bookmarks must not fail")
+	booksOrderById, err := db.GetBookmarks(ctx, GetBookmarksOptions{
+		IDs:         []int{resultUpdatedBook1[0].ID, resultUpdatedBook2[0].ID},
+		OrderMethod: 0,
+	})
+	assert.NoError(t, err, "Get bookmarks must not fail")
+
+	// Check Last Added
+	assert.Equal(t, booksOrderByLastAdded[0].Title, updatedBook2.Title)
+	// Check Last Modified
+	assert.Equal(t, booksOrderByLastModified[0].Title, updatedBook1.Title)
+	// Second id should be 2 if order them by id
+	assert.Equal(t, booksOrderById[1].ID, 2)
+}
diff --git a/internal/database/migrations/mysql/0005_rename_to_created_at.up.sql b/internal/database/migrations/mysql/0005_rename_to_created_at.up.sql
@@ -0,0 +1 @@
+ALTER TABLE bookmark RENAME COLUMN modified to created_at;
diff --git a/internal/database/migrations/mysql/0006_change_created_at_settings.up.sql b/internal/database/migrations/mysql/0006_change_created_at_settings.up.sql
@@ -0,0 +1,2 @@
+ALTER TABLE bookmark
+MODIFY created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
diff --git a/internal/database/migrations/mysql/0007_add_modified_at.up.sql b/internal/database/migrations/mysql/0007_add_modified_at.up.sql
@@ -0,0 +1,2 @@
+ALTER TABLE bookmark
+ADD COLUMN modified_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
diff --git a/internal/database/migrations/mysql/0008_set_modified_at_equal_created_at.up.sql b/internal/database/migrations/mysql/0008_set_modified_at_equal_created_at.up.sql
@@ -0,0 +1,3 @@
+UPDATE bookmark
+SET modified_at = COALESCE(created_at, CURRENT_TIMESTAMP)
+WHERE created_at IS NOT NULL;
diff --git a/internal/database/migrations/mysql/0009_index_for_created_at.up.sql b/internal/database/migrations/mysql/0009_index_for_created_at.up.sql
@@ -0,0 +1 @@
+CREATE INDEX idx_created_at ON bookmark (created_at);
diff --git a/internal/database/migrations/mysql/0010_index_for_modified_at.up.sql b/internal/database/migrations/mysql/0010_index_for_modified_at.up.sql
@@ -0,0 +1 @@
+CREATE INDEX idx_modified_at ON bookmark (modified_at);
diff --git a/internal/database/migrations/postgres/0002_created_time.up.sql b/internal/database/migrations/postgres/0002_created_time.up.sql
@@ -0,0 +1,16 @@
+-- Rename "modified" column to "created_at"
+ALTER TABLE bookmark
+RENAME COLUMN modified to created_at;
+
+-- Add the "modified_at" column to the bookmark table
+ALTER TABLE bookmark
+ADD COLUMN modified_at TIMESTAMP(0) NOT NULL DEFAULT CURRENT_TIMESTAMP;
+
+-- Update the "modified_at" column with the value from the "created_at" column if it is not null
+UPDATE bookmark
+SET modified_at = COALESCE(created_at, CURRENT_TIMESTAMP)
+WHERE created_at IS NOT NULL;
+
+-- Index for "created_at" "modified_at""
+CREATE INDEX idx_created_at ON bookmark(created_at);
+CREATE INDEX idx_modified_at ON bookmark(modified_at);
diff --git a/internal/database/migrations/sqlite/0004_created_time.up.sql b/internal/database/migrations/sqlite/0004_created_time.up.sql
@@ -0,0 +1,12 @@
+ALTER TABLE bookmark
+RENAME COLUMN modified to created_at;
+
+ALTER TABLE bookmark
+ADD COLUMN modified_at TEXT NULL;
+
+UPDATE bookmark
+SET modified_at = bookmark.created_at
+WHERE created_at IS NOT NULL;
+
+CREATE INDEX idx_created_at ON bookmark(created_at);
+CREATE INDEX idx_modified_at ON bookmark(modified_at);