Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: store created and modified time separately on database for bookmarks #896

Merged
merged 49 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
acb1e1c
sqlite migrate script
Monirzadeh May 1, 2024
e2aa97a
create time just when bookmark added and modified update if change ha…
Monirzadeh May 1, 2024
7d37ff8
Merge branch 'master' into add-created-time
Monirzadeh May 1, 2024
55673cf
show added and modified time in footer instead of header
Monirzadeh May 1, 2024
cada6fe
Merge branch 'add-created-time' of github.com:Monirzadeh/shiori into …
Monirzadeh May 1, 2024
bca2910
add bun.lockb that missing
Monirzadeh May 1, 2024
428ad58
add migrate for postgres
Monirzadeh May 5, 2024
3eabf23
Merge branch 'master' into add-created-time
Monirzadeh May 5, 2024
3083cc3
add pg support of created time
Monirzadeh May 5, 2024
524ef8f
Merge branch 'master' into add-created-time
Monirzadeh May 7, 2024
3d6c031
change modifed to modifed_at and create to created_at in sqlite
Monirzadeh May 7, 2024
164a9e2
change modifed to modifed_at and create to created_at in postgre
Monirzadeh May 7, 2024
1f53e5c
add created_at to mariadb
Monirzadeh May 7, 2024
e8ea52e
Merge branch 'master' into add-created-time
Monirzadeh May 12, 2024
c56eb54
fix migration file names
Monirzadeh May 12, 2024
bfc21c2
Merge branch 'master' into add-created-time
fmartingr May 12, 2024
2522d29
better variable name and more clear code for add modified time if cre…
Monirzadeh May 12, 2024
bfe3105
Merge branch 'master' into add-created-time
Monirzadeh May 14, 2024
7786437
add unittest
Monirzadeh May 15, 2024
16b2893
add unittest to sure filters work as expected
Monirzadeh May 15, 2024
ff71b6c
index for created_at and modified_at
Monirzadeh May 15, 2024
4ee3d2e
Merge branch 'master' into add-created-time
Monirzadeh May 15, 2024
7d72e91
Merge branch 'master' into add-created-time
Monirzadeh May 18, 2024
966acb1
Merge branch 'master' into add-created-time
Monirzadeh May 26, 2024
101e672
Merge branch 'master' into add-created-time
Monirzadeh May 30, 2024
2c990a3
Merge branch 'master' into add-created-time
Monirzadeh Jun 8, 2024
d6c02fd
build new styles.css
Monirzadeh Jun 8, 2024
243f84d
update swagger documents
Monirzadeh Jun 8, 2024
ffbee63
Merge branch 'master' into add-created-time
Monirzadeh Jun 8, 2024
bd65ec9
make styles
Monirzadeh Jun 8, 2024
c99350d
Merge branch 'master' into add-created-time
Monirzadeh Jun 9, 2024
5c5a54e
change Created and Modified to CreatedAt and ModifiedAt
Monirzadeh Jun 9, 2024
218257f
fix missing Modified
Monirzadeh Jun 9, 2024
9670b48
fix typo
Monirzadeh Jun 9, 2024
4df7436
missing Modified
Monirzadeh Jun 9, 2024
5549fe8
fix typo
Monirzadeh Jun 9, 2024
c17faf4
make swagger
Monirzadeh Jun 9, 2024
9c02e7b
run tests parallel
Monirzadeh Jun 9, 2024
7567214
remove t.Parallel()
Monirzadeh Jun 10, 2024
e1357ce
remove dayjs dependency and combine two function
Monirzadeh Jun 10, 2024
535fccc
better unittest name
Monirzadeh Jun 10, 2024
123d003
fix typo
Monirzadeh Jun 10, 2024
171cea9
diffrnt footer style for login and content page
Monirzadeh Jun 10, 2024
19b9221
use class instead of id
Monirzadeh Jun 10, 2024
aec5089
back parallel
Monirzadeh Jun 12, 2024
62c754b
change duplicate url
Monirzadeh Jun 12, 2024
6af16ee
remvoe run Parallel
Monirzadeh Jun 26, 2024
d11c0e3
Merge branch 'master' into add-created-time
Monirzadeh Jun 26, 2024
7db5cd9
make styles
Monirzadeh Jun 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/swagger/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ const docTemplate = `{
"description": "TODO: migrate outside the DTO",
"type": "boolean"
},
"createdAt": {
"type": "string"
},
"excerpt": {
"type": "string"
},
Expand All @@ -410,7 +413,7 @@ const docTemplate = `{
"imageURL": {
"type": "string"
},
"modified": {
"modifiedAt": {
"type": "string"
},
"public": {
Expand Down
5 changes: 4 additions & 1 deletion docs/swagger/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,9 @@
"description": "TODO: migrate outside the DTO",
"type": "boolean"
},
"createdAt": {
"type": "string"
},
"excerpt": {
"type": "string"
},
Expand All @@ -399,7 +402,7 @@
"imageURL": {
"type": "string"
},
"modified": {
"modifiedAt": {
"type": "string"
},
"public": {
Expand Down
4 changes: 3 additions & 1 deletion docs/swagger/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ definitions:
create_ebook:
description: 'TODO: migrate outside the DTO'
type: boolean
createdAt:
type: string
excerpt:
type: string
hasArchive:
Expand All @@ -104,7 +106,7 @@ definitions:
type: integer
imageURL:
type: string
modified:
modifiedAt:
type: string
public:
type: integer
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{}
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/pocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{}
Expand Down
4 changes: 4 additions & 0 deletions internal/core/processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
}

book.HasContent = book.Content != ""
book.ModifiedAt = ""
}

// Save article image to local disk
Expand All @@ -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
}
}
Expand All @@ -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 = ""
}
}

Expand Down Expand Up @@ -186,6 +189,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
}

book.HasArchive = true
book.ModifiedAt = ""
}

return book, false, nil
Expand Down
118 changes: 105 additions & 13 deletions internal/database/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package database
import (
"context"
"testing"
"time"

"github.com/go-shiori/shiori/internal/model"
"github.com/stretchr/testify/assert"
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE bookmark RENAME COLUMN modified to created_at;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE bookmark
MODIFY created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE bookmark
ADD COLUMN modified_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
UPDATE bookmark
SET modified_at = COALESCE(created_at, CURRENT_TIMESTAMP)
WHERE created_at IS NOT NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CREATE INDEX idx_created_at ON bookmark (created_at);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CREATE INDEX idx_modified_at ON bookmark (modified_at);
16 changes: 16 additions & 0 deletions internal/database/migrations/postgres/0002_created_time.up.sql
Original file line number Diff line number Diff line change
@@ -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);
12 changes: 12 additions & 0 deletions internal/database/migrations/sqlite/0004_created_time.up.sql
Original file line number Diff line number Diff line change
@@ -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);
Loading
Loading