Skip to content

Commit

Permalink
refactor: remove afero
Browse files Browse the repository at this point in the history
Created a simple local filesystem abstrantion in the
StorageDomain while removing afero, this has been done
because afero was causing some problem under Windows
hosts.

Fixes #822
  • Loading branch information
fmartingr committed Jan 28, 2024
1 parent 84e5b09 commit bbfc9ba
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 99 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ require (
github.com/sethvargo/go-envconfig v0.9.0
github.com/shurcooL/vfsgen v0.0.0-20230704071429-0000e147ea92
github.com/sirupsen/logrus v1.9.3
github.com/spf13/afero v1.11.0
github.com/spf13/cobra v1.8.0
github.com/stretchr/testify v1.8.4
github.com/swaggo/files v1.0.1
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,6 @@ github.com/shurcooL/vfsgen v0.0.0-20230704071429-0000e147ea92/go.mod h1:7/OT02F6
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/spf13/afero v1.11.0 h1:WJQKhtpdm3v2IzqG8VMqrr6Rf3UYpEF239Jy9wNepM8=
github.com/spf13/afero v1.11.0/go.mod h1:GH9Y3pIexgf1MTIWtNGyogA5MwRIDXGUr+hbWNoBjkY=
github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0=
github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
Expand Down
10 changes: 5 additions & 5 deletions internal/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import (
fp "path/filepath"
"time"

"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"golang.org/x/net/context"

"github.com/go-shiori/shiori/internal/config"
"github.com/go-shiori/shiori/internal/database"
"github.com/go-shiori/shiori/internal/dependencies"
"github.com/go-shiori/shiori/internal/domains"
"github.com/go-shiori/shiori/internal/model"
"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"golang.org/x/net/context"
)

// ShioriCmd returns the root command for shiori
Expand Down Expand Up @@ -103,7 +103,7 @@ func initShiori(ctx context.Context, cmd *cobra.Command) (*config.Config, *depen
dependencies.Domains.Auth = domains.NewAccountsDomain(dependencies)
dependencies.Domains.Archiver = domains.NewArchiverDomain(dependencies)
dependencies.Domains.Bookmarks = domains.NewBookmarksDomain(dependencies)
dependencies.Domains.Storage = domains.NewStorageDomain(dependencies, afero.NewBasePathFs(afero.NewOsFs(), cfg.Storage.DataDir))
dependencies.Domains.Storage = domains.NewStorageDomain(dependencies, cfg.Storage.DataDir)

Check warning on line 106 in internal/cmd/root.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/root.go#L106

Added line #L106 was not covered by tests

// Workaround: Get accounts to make sure at least one is present in the database.
// If there's no accounts in the database, create the shiori/gopher account the legacy api
Expand Down
26 changes: 13 additions & 13 deletions internal/core/ebook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import (
fp "path/filepath"
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"

"github.com/go-shiori/shiori/internal/core"
"github.com/go-shiori/shiori/internal/domains"
"github.com/go-shiori/shiori/internal/model"
"github.com/go-shiori/shiori/internal/testutil"
"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
)

func TestGenerateEbook(t *testing.T) {
Expand All @@ -25,7 +25,7 @@ func TestGenerateEbook(t *testing.T) {
tempDir := t.TempDir()
dstDir := t.TempDir()

deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
deps.Domains.Storage = domains.NewStorageDomain(deps, dstDir)

mockRequest := core.ProcessRequest{
Bookmark: model.BookmarkDTO{
Expand All @@ -46,7 +46,7 @@ func TestGenerateEbook(t *testing.T) {
t.Run("ebook generate with valid BookmarkID EbookExist ImagePathExist ReturnWithHasEbookTrue", func(t *testing.T) {
dstDir := t.TempDir()

deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
deps.Domains.Storage = domains.NewStorageDomain(deps, dstDir)

bookmark := model.BookmarkDTO{
ID: 2,
Expand All @@ -59,8 +59,8 @@ func TestGenerateEbook(t *testing.T) {
}
// Create the thumbnail file
imagePath := model.GetThumbnailPath(&bookmark)
deps.Domains.Storage.FS().MkdirAll(fp.Dir(imagePath), os.ModePerm)
file, err := deps.Domains.Storage.FS().Create(imagePath)
deps.Domains.Storage.MkDirAll(fp.Dir(imagePath), os.ModePerm)
file, err := deps.Domains.Storage.Create(imagePath)
if err != nil {
t.Fatal(err)
}
Expand All @@ -76,7 +76,7 @@ func TestGenerateEbook(t *testing.T) {
tempDir := t.TempDir()
dstDir := t.TempDir()

deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
deps.Domains.Storage = domains.NewStorageDomain(deps, dstDir)

bookmark := model.BookmarkDTO{
ID: 3,
Expand All @@ -89,8 +89,8 @@ func TestGenerateEbook(t *testing.T) {
}
// Create the archive file
archivePath := model.GetArchivePath(&bookmark)
deps.Domains.Storage.FS().MkdirAll(fp.Dir(archivePath), os.ModePerm)
file, err := deps.Domains.Storage.FS().Create(archivePath)
deps.Domains.Storage.MkDirAll(fp.Dir(archivePath), os.ModePerm)
file, err := deps.Domains.Storage.Create(archivePath)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestGenerateEbook(t *testing.T) {
t.Run("ebook exist return HasEbook true", func(t *testing.T) {
dstDir := t.TempDir()

deps.Domains.Storage = domains.NewStorageDomain(deps, afero.NewBasePathFs(afero.NewOsFs(), dstDir))
deps.Domains.Storage = domains.NewStorageDomain(deps, dstDir)

bookmark := model.BookmarkDTO{
ID: 1,
Expand All @@ -137,8 +137,8 @@ func TestGenerateEbook(t *testing.T) {
}
// Create the ebook file
ebookFilePath := model.GetEbookPath(&bookmark)
deps.Domains.Storage.FS().MkdirAll(fp.Dir(ebookFilePath), os.ModePerm)
file, err := deps.Domains.Storage.FS().Create(ebookFilePath)
deps.Domains.Storage.MkDirAll(fp.Dir(ebookFilePath), os.ModePerm)
file, err := deps.Domains.Storage.Create(ebookFilePath)
if err != nil {
t.Fatal(err)
}
Expand Down
13 changes: 6 additions & 7 deletions internal/core/processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"image/color"
"image/draw"
"image/jpeg"
_ "image/png"
"io"
"log"
"math"
Expand All @@ -18,14 +19,12 @@ import (

"github.com/disintegration/imaging"
"github.com/go-shiori/go-readability"
"github.com/go-shiori/shiori/internal/dependencies"
"github.com/go-shiori/shiori/internal/model"
"github.com/go-shiori/warc"
"github.com/pkg/errors"
_ "golang.org/x/image/webp"

// Add support for png
_ "image/png"
"github.com/go-shiori/shiori/internal/dependencies"
"github.com/go-shiori/shiori/internal/model"
)

// ProcessRequest is the request for processing bookmark.
Expand Down Expand Up @@ -108,7 +107,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
if article.Image != "" {
imageURLs = append(imageURLs, article.Image)
} else {
deps.Domains.Storage.FS().Remove(imgPath)
deps.Domains.Storage.Remove(imgPath)
}

if article.Favicon != "" {
Expand All @@ -128,7 +127,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
if err != nil && errors.Is(err, ErrNoSupportedImageType) {
log.Printf("%s: %s", err, imageURL)
if i == len(imageURLs)-1 {
deps.Domains.Storage.FS().Remove(imgPath)
deps.Domains.Storage.Remove(imgPath)
}
}
if err != nil {
Expand Down Expand Up @@ -163,7 +162,7 @@ func ProcessBookmark(deps *dependencies.Dependencies, req ProcessRequest) (book
if err != nil {
return book, false, fmt.Errorf("failed to create temp archive: %v", err)
}
defer deps.Domains.Storage.FS().Remove(tmpFile.Name())
defer deps.Domains.Storage.Remove(tmpFile.Name())

archivalRequest := warc.ArchivalRequest{
URL: book.URL,
Expand Down
14 changes: 6 additions & 8 deletions internal/core/processing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import (
fp "path/filepath"
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"

"github.com/go-shiori/shiori/internal/core"
"github.com/go-shiori/shiori/internal/model"
"github.com/go-shiori/shiori/internal/testutil"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
)

func TestDownloadBookImage(t *testing.T) {
Expand All @@ -24,8 +25,7 @@ func TestDownloadBookImage(t *testing.T) {
t.Run("fails", func(t *testing.T) {
// images is too small with unsupported format with a valid URL
imageURL := "https://github.com/go-shiori/shiori/blob/master/internal/view/assets/res/apple-touch-icon-152x152.png"
tempDir := t.TempDir()
dstPath := fp.Join(tempDir, "1")
dstPath := fp.Join("image", "1")
defer os.Remove(dstPath)

// Act
Expand All @@ -38,8 +38,7 @@ func TestDownloadBookImage(t *testing.T) {
t.Run("successful download image", func(t *testing.T) {
// Arrange
imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/master/docs/readme/cover.png"
tempDir := t.TempDir()
dstPath := fp.Join(tempDir, "1")
dstPath := fp.Join("image", "1")
defer os.Remove(dstPath)

// Act
Expand All @@ -59,8 +58,7 @@ func TestDownloadBookImage(t *testing.T) {

// Arrange
imageURL := server.URL + "/medium_image.png"
tempDir := t.TempDir()
dstPath := fp.Join(tempDir, "1")
dstPath := fp.Join("image", "1")
defer os.Remove(dstPath)

// Act
Expand Down
20 changes: 12 additions & 8 deletions internal/domains/bookmarks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,23 @@ package domains_test

import (
"context"
"os"
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"

"github.com/go-shiori/shiori/internal/config"
"github.com/go-shiori/shiori/internal/database"
"github.com/go-shiori/shiori/internal/dependencies"
"github.com/go-shiori/shiori/internal/domains"
"github.com/go-shiori/shiori/internal/model"
"github.com/go-shiori/shiori/internal/testutil"
"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/stretchr/testify/require"
)

func TestBookmarkDomain(t *testing.T) {
fs := afero.NewMemMapFs()
tmpDir, err := os.MkdirTemp("", "")
require.NoError(t, err)

db, err := database.OpenSQLiteDatabase(context.TODO(), ":memory:")
require.NoError(t, err)
Expand All @@ -28,13 +30,15 @@ func TestBookmarkDomain(t *testing.T) {
Log: logrus.New(),
Domains: &dependencies.Domains{},
}
deps.Domains.Storage = domains.NewStorageDomain(deps, fs)
deps.Domains.Storage = domains.NewStorageDomain(deps, tmpDir)

fs := deps.Domains.Storage

fs.MkdirAll("thumb", 0755)
fs.MkDirAll("thumb", 0755)
fs.Create("thumb/1")
fs.MkdirAll("ebook", 0755)
fs.MkDirAll("ebook", 0755)
fs.Create("ebook/1.epub")
fs.MkdirAll("archive", 0755)
fs.MkDirAll("archive", 0755)
// TODO: write a valid archive file
fs.Create("archive/1")

Expand Down
48 changes: 33 additions & 15 deletions internal/domains/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,47 @@ import (

"github.com/go-shiori/shiori/internal/dependencies"
"github.com/go-shiori/shiori/internal/model"
"github.com/spf13/afero"
)

type StorageDomain struct {
deps *dependencies.Dependencies
fs afero.Fs
path string
}

func NewStorageDomain(deps *dependencies.Dependencies, fs afero.Fs) *StorageDomain {
func NewStorageDomain(deps *dependencies.Dependencies, path string) *StorageDomain {
return &StorageDomain{
deps: deps,
fs: fs,
path: path,
}
}

func (d *StorageDomain) generateFullPath(name string) string {
return filepath.Join(d.path, name)
}

// Stat returns the FileInfo structure describing file.
func (d *StorageDomain) Stat(name string) (fs.FileInfo, error) {
return d.fs.Stat(name)
return os.Stat(d.generateFullPath(name))
}

// Create creates a file in storage.
func (d *StorageDomain) Create(name string) (fs.File, error) {
return os.Create(d.generateFullPath(name))
}

// Open opens a file in storage.
func (d *StorageDomain) Open(name string) (fs.File, error) {
return os.Open(d.generateFullPath(name))
}

// MkDirAll creates a directory in storage.
func (d *StorageDomain) MkDirAll(name string, mode os.FileMode) error {
return os.MkdirAll(d.generateFullPath(name), mode)
}

// FS returns the filesystem used by this domain.
func (d *StorageDomain) FS() afero.Fs {
return d.fs
// Remove removes a file in storage.
func (d *StorageDomain) Remove(name string) error {
return os.Remove(d.generateFullPath(name))

Check warning on line 52 in internal/domains/storage.go

View check run for this annotation

Codecov / codecov/patch

internal/domains/storage.go#L51-L52

Added lines #L51 - L52 were not covered by tests
}

// FileExists checks if a file exists in storage.
Expand All @@ -51,15 +69,14 @@ func (d *StorageDomain) DirExists(name string) bool {
func (d *StorageDomain) WriteData(dst string, data []byte) error {
// Create directory if not exist
dir := filepath.Dir(dst)
if !d.DirExists(dir) {
err := d.fs.MkdirAll(dir, os.ModePerm)
if err != nil {
if dir != "" && !d.DirExists(dir) {
if err := d.MkDirAll(dir, model.DataDirPerm); err != nil {
return err
}
}

// Create file
file, err := d.fs.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.ModePerm)
file, err := os.Create(d.generateFullPath(dst))
if err != nil {
return err
}
Expand All @@ -72,14 +89,15 @@ func (d *StorageDomain) WriteData(dst string, data []byte) error {

// WriteFile writes a file to storage.
func (d *StorageDomain) WriteFile(dst string, tmpFile *os.File) error {
if dst != "" && !d.DirExists(dst) {
err := d.fs.MkdirAll(filepath.Dir(dst), model.DataDirPerm)
dir := filepath.Dir(dst)
if dir != "" && !d.DirExists(dir) {
err := d.MkDirAll(dir, model.DataDirPerm)

Check warning on line 94 in internal/domains/storage.go

View check run for this annotation

Codecov / codecov/patch

internal/domains/storage.go#L92-L94

Added lines #L92 - L94 were not covered by tests
if err != nil {
return fmt.Errorf("failed to create destination dir: %v", err)
}
}

dstFile, err := d.fs.Create(dst)
dstFile, err := os.Create(d.generateFullPath(dst))

Check warning on line 100 in internal/domains/storage.go

View check run for this annotation

Codecov / codecov/patch

internal/domains/storage.go#L100

Added line #L100 was not covered by tests
if err != nil {
return fmt.Errorf("failed to create destination file: %v", err)
}
Expand Down
Loading

0 comments on commit bbfc9ba

Please sign in to comment.