From 7aa37d2e11d2d77600dcf406eeb23fc74b60ce61 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Fri, 28 Jul 2023 20:28:47 +0330 Subject: [PATCH 01/36] generate ebook get dstPath --- internal/core/ebook.go | 20 +++++++------------- internal/core/processing.go | 7 ++++--- internal/webserver/handler-api.go | 4 +++- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index 522094185..8ba8c322d 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -16,7 +16,7 @@ import ( "github.com/pkg/errors" ) -func GenerateEbook(req ProcessRequest) (book model.Bookmark, err error) { +func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err error) { // variable for store generated html code var html string @@ -40,34 +40,28 @@ func GenerateEbook(req ProcessRequest) (book model.Bookmark, err error) { if _, err := os.Stat(archivePath); err == nil { book.HasArchive = true } - ebookfile := fp.Join(req.DataDir, "ebook", fmt.Sprintf("%d.epub", book.ID)) - // if epub exist finish prosess else continue - if _, err := os.Stat(ebookfile); err == nil { - book.HasEbook = true - return book, nil - } + contentType := req.ContentType if strings.Contains(contentType, "application/pdf") { return book, errors.New("can't create ebook for pdf") } - ebookDir := fp.Join(req.DataDir, "ebook") // check if directory not exsist create that - if _, err := os.Stat(ebookDir); os.IsNotExist(err) { - err := os.MkdirAll(ebookDir, model.DataDirPerm) + if _, err := os.Stat(dstPath); os.IsNotExist(err) { + err := os.MkdirAll(dstPath, model.DataDirPerm) if err != nil { return book, errors.Wrap(err, "can't create ebook directory") } } // create epub file - epubFile, err := os.Create(ebookfile) + dstFile, err := os.Create(fp.Join(dstPath, fmt.Sprintf("%d.epub", book.ID))) if err != nil { return book, errors.Wrap(err, "can't create ebook") } - defer epubFile.Close() + defer dstFile.Close() // Create zip archive - epubWriter := zip.NewWriter(epubFile) + epubWriter := zip.NewWriter(dstFile) defer epubWriter.Close() // Create the mimetype file diff --git a/internal/core/processing.go b/internal/core/processing.go index 12a2c1050..2b90605b0 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -128,13 +128,14 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // If needed, create ebook as well if book.CreateEbook { - ebookPath := fp.Join(req.DataDir, "ebook", fmt.Sprintf("%d.epub", book.ID)) - os.Remove(ebookPath) + // ebookPath := fp.Join(req.DataDir, "ebook", fmt.Sprintf("%d.epub", book.ID)) + ebookPath := fp.Join(req.DataDir, "ebook") + // os.Remove(ebookPath) if strings.Contains(contentType, "application/pdf") { return book, false, errors.Wrap(err, "can't create ebook from pdf") } else { - _, err = GenerateEbook(req) + _, err = GenerateEbook(req, ebookPath) if err != nil { return book, true, errors.Wrap(err, "failed to create ebook") } diff --git a/internal/webserver/handler-api.go b/internal/webserver/handler-api.go index 8362a835f..c84a874f1 100644 --- a/internal/webserver/handler-api.go +++ b/internal/webserver/handler-api.go @@ -434,7 +434,9 @@ func (h *Handler) ApiDownloadEbook(w http.ResponseWriter, r *http.Request, ps ht ContentType: contentType, } - book, err = core.GenerateEbook(request) + //TODO: if file exist book return avilable file + ebookPath := fp.Join(request.DataDir, "ebook") + book, err = core.GenerateEbook(request, ebookPath) content.Close() if err != nil { From 91937b3e252eb6872a6cd588ff86e728a11b0ce2 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Fri, 28 Jul 2023 21:39:57 +0330 Subject: [PATCH 02/36] Archive and ebook can recover if download faild --- internal/core/processing.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index 2b90605b0..c148c6410 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -128,17 +128,20 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // If needed, create ebook as well if book.CreateEbook { - // ebookPath := fp.Join(req.DataDir, "ebook", fmt.Sprintf("%d.epub", book.ID)) - ebookPath := fp.Join(req.DataDir, "ebook") - // os.Remove(ebookPath) + ebookFile := fp.Join(req.DataDir, "ebook", fmt.Sprintf("%d.epub", book.ID)) + tmpebookfile := fp.Join(req.DataDir, "tmp/ebook", fmt.Sprintf("%d.epub", book.ID)) + tmpEbookPath := fp.Join(req.DataDir, "tmp/ebook") if strings.Contains(contentType, "application/pdf") { return book, false, errors.Wrap(err, "can't create ebook from pdf") } else { - _, err = GenerateEbook(req, ebookPath) + _, err = GenerateEbook(req, tmpEbookPath) if err != nil { + os.Remove(tmpebookfile) return book, true, errors.Wrap(err, "failed to create ebook") } + os.Remove(ebookFile) + os.Rename(tmpebookfile, ebookFile) book.HasEbook = true } } @@ -146,7 +149,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // If needed, create offline archive as well if book.CreateArchive { archivePath := fp.Join(req.DataDir, "archive", fmt.Sprintf("%d", book.ID)) - os.Remove(archivePath) + tmpArchivePath := fp.Join(req.DataDir, "tmp/archive", fmt.Sprintf("%d", book.ID)) archivalRequest := warc.ArchivalRequest{ URL: book.URL, @@ -156,11 +159,13 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, LogEnabled: req.LogArchival, } - err = warc.NewArchive(archivalRequest, archivePath) + err = warc.NewArchive(archivalRequest, tmpArchivePath) if err != nil { + os.Remove(tmpArchivePath) return book, false, fmt.Errorf("failed to create archive: %v", err) } - + os.Remove(archivePath) + os.Rename(tmpArchivePath, archivePath) book.HasArchive = true } From c12e3f213c0162abe2cd8b3748b681c233880816 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 5 Aug 2023 11:57:33 +0330 Subject: [PATCH 03/36] recover thumb if download faild --- internal/core/processing.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index c148c6410..500ddae64 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -72,7 +72,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, nurl, err := url.Parse(book.URL) if err != nil { - return book, true, fmt.Errorf("Failed to parse url: %v", err) + return book, true, fmt.Errorf("failed to parse url: %v", err) } article, err := readability.FromReader(readabilityInput, nurl) @@ -117,13 +117,17 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Save article image to local disk strID := strconv.Itoa(book.ID) imgPath := fp.Join(req.DataDir, "thumb", strID) + tmpImgPath := fp.Join(req.DataDir, "tmp/thumb", strID) for _, imageURL := range imageURLs { - err = downloadBookImage(imageURL, imgPath) + err = downloadBookImage(imageURL, tmpImgPath) if err == nil { book.ImageURL = path.Join("/", "bookmark", strID, "thumb") + os.Remove(tmpImgPath) break } + os.Remove(imgPath) + os.Rename(tmpImgPath, imgPath) } // If needed, create ebook as well From 379a3ba476189863cc932f0adb6d629f3ba4a387 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 5 Aug 2023 17:09:51 +0330 Subject: [PATCH 04/36] thumb image create just if image processing is sucssesful --- internal/core/processing.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index 500ddae64..ab734bbd1 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -117,17 +117,13 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Save article image to local disk strID := strconv.Itoa(book.ID) imgPath := fp.Join(req.DataDir, "thumb", strID) - tmpImgPath := fp.Join(req.DataDir, "tmp/thumb", strID) for _, imageURL := range imageURLs { - err = downloadBookImage(imageURL, tmpImgPath) + err = downloadBookImage(imageURL, imgPath) if err == nil { book.ImageURL = path.Join("/", "bookmark", strID, "thumb") - os.Remove(tmpImgPath) break } - os.Remove(imgPath) - os.Rename(tmpImgPath, imgPath) } // If needed, create ebook as well @@ -200,6 +196,11 @@ func downloadBookImage(url, dstPath string) error { if err != nil { return fmt.Errorf("failed to create image dir: %v", err) } + tmpFile, err := os.CreateTemp("", "image") + if err != nil { + return fmt.Errorf("failed to create temporary image file: %v", err) + } + defer os.Remove(tmpFile.Name()) dstFile, err := os.Create(dstPath) if err != nil { @@ -221,7 +222,7 @@ func downloadBookImage(url, dstPath string) error { imgRatio := float64(imgWidth) / float64(imgHeight) if imgWidth >= 600 && imgHeight >= 400 && imgRatio > 1.3 { - err = jpeg.Encode(dstFile, img, nil) + err = jpeg.Encode(tmpFile, img, nil) } else { // Create background bg := image.NewNRGBA(imgRect) @@ -246,12 +247,23 @@ func downloadBookImage(url, dstPath string) error { draw.Draw(bg, bgRect, fg, fgPosition, draw.Over) // Save to file - err = jpeg.Encode(dstFile, bg, nil) + err = jpeg.Encode(tmpFile, bg, nil) } if err != nil { return fmt.Errorf("failed to save image %s: %v", url, err) } + // Copy temporary file to destination + _, err = tmpFile.Seek(0, io.SeekStart) + if err != nil { + return fmt.Errorf("failed to rewind temporary image file: %v", err) + } + + _, err = io.Copy(dstFile, tmpFile) + if err != nil { + return fmt.Errorf("failed to copy image to the destination") + } + return nil } From 42afd233b4cca947020b7f4cd936180cca7869f6 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 5 Aug 2023 18:57:08 +0330 Subject: [PATCH 05/36] create epub in tmp if it sucssesful copy to destination --- internal/core/ebook.go | 55 +++++++++++++++++++++++++++++-------- internal/core/processing.go | 32 ++++++++++----------- 2 files changed, 57 insertions(+), 30 deletions(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index 8ba8c322d..eba3fc6b4 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -46,22 +46,16 @@ func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err return book, errors.New("can't create ebook for pdf") } - // check if directory not exsist create that - if _, err := os.Stat(dstPath); os.IsNotExist(err) { - err := os.MkdirAll(dstPath, model.DataDirPerm) - if err != nil { - return book, errors.Wrap(err, "can't create ebook directory") - } - } - // create epub file - dstFile, err := os.Create(fp.Join(dstPath, fmt.Sprintf("%d.epub", book.ID))) + // create temporary epub file + tmpFile, err := os.CreateTemp("", "ebook*.epub") if err != nil { - return book, errors.Wrap(err, "can't create ebook") + return book, errors.Wrap(err, "can't create temporary EPUB file") } - defer dstFile.Close() + defer tmpFile.Close() + defer os.Remove(tmpFile.Name()) // Create zip archive - epubWriter := zip.NewWriter(dstFile) + epubWriter := zip.NewWriter(tmpFile) defer epubWriter.Close() // Create the mimetype file @@ -217,6 +211,43 @@ img { if err != nil { return book, errors.Wrap(err, "can't write into content.html") } + // close epub and tmpFile + err = epubWriter.Close() + if err != nil { + return book, errors.Wrap(err, "failed to close EPUB writer") + } + err = tmpFile.Close() + if err != nil { + return book, errors.Wrap(err, "failed to close temporary EPUB file") + } + // open temporary file again + tmpFile, err = os.Open(tmpFile.Name()) + if err != nil { + return book, errors.Wrap(err, "can't open temporary EPUB file") + } + defer tmpFile.Close() + + // create ebook directory if it need + err = os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) + + // create dstFile + dstFile, err := os.Create(dstPath + ".epub") + if err != nil { + return book, errors.Wrap(err, "can't create ebook in dstPath") + } + defer dstFile.Close() + + // Copy the content from the temporary file to the destination file + _, err = tmpFile.Seek(0, io.SeekStart) + if err != nil { + return book, errors.Wrap(err, "failed to rewind temporary ebook file") + } + + _, err = io.Copy(dstFile, tmpFile) + if err != nil { + return book, errors.Wrap(err, "failed to copy image to the destination") + } + book.HasEbook = true return book, nil } diff --git a/internal/core/processing.go b/internal/core/processing.go index ab734bbd1..5b7da0a82 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -128,20 +128,15 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // If needed, create ebook as well if book.CreateEbook { - ebookFile := fp.Join(req.DataDir, "ebook", fmt.Sprintf("%d.epub", book.ID)) - tmpebookfile := fp.Join(req.DataDir, "tmp/ebook", fmt.Sprintf("%d.epub", book.ID)) - tmpEbookPath := fp.Join(req.DataDir, "tmp/ebook") + ebookPath := fp.Join(req.DataDir, "ebook", strID) if strings.Contains(contentType, "application/pdf") { return book, false, errors.Wrap(err, "can't create ebook from pdf") } else { - _, err = GenerateEbook(req, tmpEbookPath) + _, err = GenerateEbook(req, ebookPath) if err != nil { - os.Remove(tmpebookfile) return book, true, errors.Wrap(err, "failed to create ebook") } - os.Remove(ebookFile) - os.Rename(tmpebookfile, ebookFile) book.HasEbook = true } } @@ -191,23 +186,13 @@ func downloadBookImage(url, dstPath string) error { } // At this point, the download has finished successfully. - // Prepare destination file. - err = os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) - if err != nil { - return fmt.Errorf("failed to create image dir: %v", err) - } + // Create tmpFile tmpFile, err := os.CreateTemp("", "image") if err != nil { return fmt.Errorf("failed to create temporary image file: %v", err) } defer os.Remove(tmpFile.Name()) - dstFile, err := os.Create(dstPath) - if err != nil { - return fmt.Errorf("failed to create image file: %v", err) - } - defer dstFile.Close() - // Parse image and process it. // If image is smaller than 600x400 or its ratio is less than 4:3, resize. // Else, save it as it is. @@ -254,6 +239,17 @@ func downloadBookImage(url, dstPath string) error { return fmt.Errorf("failed to save image %s: %v", url, err) } + // Prepare destination file. + err = os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) + if err != nil { + return fmt.Errorf("failed to create image dir: %v", err) + } + + dstFile, err := os.Create(dstPath) + if err != nil { + return fmt.Errorf("failed to create image file: %v", err) + } + defer dstFile.Close() // Copy temporary file to destination _, err = tmpFile.Seek(0, io.SeekStart) if err != nil { From 624a74638a00e38b0f82e3432988358f3b4bdf49 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 5 Aug 2023 20:03:45 +0330 Subject: [PATCH 06/36] archive file create in tmp if it successful move to destination --- internal/core/processing.go | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index 5b7da0a82..c7e59ced3 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -143,8 +143,11 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // If needed, create offline archive as well if book.CreateArchive { - archivePath := fp.Join(req.DataDir, "archive", fmt.Sprintf("%d", book.ID)) - tmpArchivePath := fp.Join(req.DataDir, "tmp/archive", fmt.Sprintf("%d", book.ID)) + tmpFile, err := os.CreateTemp("", "archive") + if err != nil { + return book, false, fmt.Errorf("failed to create temp archive: %v", err) + } + defer os.Remove(tmpFile.Name()) archivalRequest := warc.ArchivalRequest{ URL: book.URL, @@ -154,13 +157,34 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, LogEnabled: req.LogArchival, } - err = warc.NewArchive(archivalRequest, tmpArchivePath) + err = warc.NewArchive(archivalRequest, tmpFile.Name()) if err != nil { - os.Remove(tmpArchivePath) + defer os.Remove(tmpFile.Name()) return book, false, fmt.Errorf("failed to create archive: %v", err) } - os.Remove(archivePath) - os.Rename(tmpArchivePath, archivePath) + + // Prepare destination file. + archivePath := fp.Join(req.DataDir, "archive", fmt.Sprintf("%d", book.ID)) + err = os.MkdirAll(fp.Dir(archivePath), model.DataDirPerm) + if err != nil { + return book, false, fmt.Errorf("failed to create destination directory archive: %v", err) + } + dstFile, err := os.Create(archivePath) + if err != nil { + return book, false, fmt.Errorf("failed to create destination archive: %v", err) + } + defer dstFile.Close() + // Copy temporary file to destination + _, err = tmpFile.Seek(0, io.SeekStart) + if err != nil { + return book, false, fmt.Errorf("failed to rewind temporary archive file: %v", err) + } + + _, err = io.Copy(dstFile, tmpFile) + if err != nil { + return book, false, fmt.Errorf("failed to copy archive to the destination %v", err) + } + book.HasArchive = true } From 2ec8e5ebdcd468f324e112a7630d4489907f7813 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 5 Aug 2023 20:46:08 +0330 Subject: [PATCH 07/36] move to destination as function --- internal/core/ebook.go | 20 ++----------------- internal/core/processing.go | 39 ++++++++++++++++--------------------- 2 files changed, 19 insertions(+), 40 deletions(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index eba3fc6b4..7f4281daf 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -227,25 +227,9 @@ img { } defer tmpFile.Close() - // create ebook directory if it need - err = os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) - - // create dstFile - dstFile, err := os.Create(dstPath + ".epub") - if err != nil { - return book, errors.Wrap(err, "can't create ebook in dstPath") - } - defer dstFile.Close() - - // Copy the content from the temporary file to the destination file - _, err = tmpFile.Seek(0, io.SeekStart) - if err != nil { - return book, errors.Wrap(err, "failed to rewind temporary ebook file") - } - - _, err = io.Copy(dstFile, tmpFile) + err = MoveToDestination(dstPath, tmpFile) if err != nil { - return book, errors.Wrap(err, "failed to copy image to the destination") + return book, errors.Wrap(err, "failed move ebook to destination") } book.HasEbook = true diff --git a/internal/core/processing.go b/internal/core/processing.go index c7e59ced3..1da0654f3 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -164,25 +164,11 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, } // Prepare destination file. - archivePath := fp.Join(req.DataDir, "archive", fmt.Sprintf("%d", book.ID)) - err = os.MkdirAll(fp.Dir(archivePath), model.DataDirPerm) - if err != nil { - return book, false, fmt.Errorf("failed to create destination directory archive: %v", err) - } - dstFile, err := os.Create(archivePath) - if err != nil { - return book, false, fmt.Errorf("failed to create destination archive: %v", err) - } - defer dstFile.Close() - // Copy temporary file to destination - _, err = tmpFile.Seek(0, io.SeekStart) - if err != nil { - return book, false, fmt.Errorf("failed to rewind temporary archive file: %v", err) - } + dstPath := fp.Join(req.DataDir, "archive", fmt.Sprintf("%d", book.ID)) - _, err = io.Copy(dstFile, tmpFile) + err = MoveToDestination(dstPath, tmpFile) if err != nil { - return book, false, fmt.Errorf("failed to copy archive to the destination %v", err) + return book, false, fmt.Errorf("failed move archive to destination `: %v", err) } book.HasArchive = true @@ -263,26 +249,35 @@ func downloadBookImage(url, dstPath string) error { return fmt.Errorf("failed to save image %s: %v", url, err) } + err = MoveToDestination(dstPath, tmpFile) + if err != nil { + return err + } + + return nil +} + +func MoveToDestination(dstPath string, tmpFile *os.File) error { // Prepare destination file. - err = os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) + err := os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) if err != nil { - return fmt.Errorf("failed to create image dir: %v", err) + return fmt.Errorf("failed to create destination dir: %v", err) } dstFile, err := os.Create(dstPath) if err != nil { - return fmt.Errorf("failed to create image file: %v", err) + return fmt.Errorf("failed to create destination file: %v", err) } defer dstFile.Close() // Copy temporary file to destination _, err = tmpFile.Seek(0, io.SeekStart) if err != nil { - return fmt.Errorf("failed to rewind temporary image file: %v", err) + return fmt.Errorf("failed to rewind temporary file: %v", err) } _, err = io.Copy(dstFile, tmpFile) if err != nil { - return fmt.Errorf("failed to copy image to the destination") + return fmt.Errorf("failed to copy file to the destination") } return nil From 44b9a13c048f15aebf56f351e045b25137c6b488 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 5 Aug 2023 21:33:12 +0330 Subject: [PATCH 08/36] update ebook download api and remove .epub from file name --- internal/webserver/handler-api.go | 36 +++++++++++++++++++++++-------- internal/webserver/handler-ui.go | 6 +++--- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/internal/webserver/handler-api.go b/internal/webserver/handler-api.go index c84a874f1..cbbeb81b0 100644 --- a/internal/webserver/handler-api.go +++ b/internal/webserver/handler-api.go @@ -110,7 +110,7 @@ func (h *Handler) ApiGetBookmarks(w http.ResponseWriter, r *http.Request, ps htt strID := strconv.Itoa(bookmarks[i].ID) imgPath := fp.Join(h.DataDir, "thumb", strID) archivePath := fp.Join(h.DataDir, "archive", strID) - ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") + ebookPath := fp.Join(h.DataDir, "ebook", strID) if fileExists(imgPath) { bookmarks[i].ImageURL = path.Join(h.RootPath, "bookmark", strID, "thumb") @@ -285,7 +285,7 @@ func (h *Handler) ApiDeleteBookmark(w http.ResponseWriter, r *http.Request, ps h strID := strconv.Itoa(id) imgPath := fp.Join(h.DataDir, "thumb", strID) archivePath := fp.Join(h.DataDir, "archive", strID) - ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") + ebookPath := fp.Join(h.DataDir, "ebook", strID) os.Remove(imgPath) os.Remove(archivePath) @@ -434,14 +434,32 @@ func (h *Handler) ApiDownloadEbook(w http.ResponseWriter, r *http.Request, ps ht ContentType: contentType, } - //TODO: if file exist book return avilable file - ebookPath := fp.Join(request.DataDir, "ebook") - book, err = core.GenerateEbook(request, ebookPath) - content.Close() + // if file exist book return avilable file + strID := strconv.Itoa(book.ID) + ebookPath := fp.Join(request.DataDir, "ebook", strID) + _, err = os.Stat(ebookPath) + if err == nil { + // file already exists, return the existing file + imagePath := fp.Join(request.DataDir, "thumb", fmt.Sprintf("%d", book.ID)) + archivePath := fp.Join(request.DataDir, "archive", fmt.Sprintf("%d", book.ID)) + + if _, err := os.Stat(imagePath); err == nil { + book.ImageURL = fp.Join("/", "bookmark", strID, "thumb") + } - if err != nil { - chProblem <- book.ID - return + if _, err := os.Stat(archivePath); err == nil { + book.HasArchive = true + } + book.HasEbook = true + } else { + // generate ebook file + book, err = core.GenerateEbook(request, ebookPath) + content.Close() + + if err != nil { + chProblem <- book.ID + return + } } // Update list of bookmarks diff --git a/internal/webserver/handler-ui.go b/internal/webserver/handler-ui.go index 2f300aea4..17800d169 100644 --- a/internal/webserver/handler-ui.go +++ b/internal/webserver/handler-ui.go @@ -48,8 +48,8 @@ func (h *Handler) ServeBookmarkContent(w http.ResponseWriter, r *http.Request, p } } - // Check if it has archive. - ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") + // Check if it has ebook. + ebookPath := fp.Join(h.DataDir, "ebook", strID) if fileExists(ebookPath) { bookmark.HasEbook = true } @@ -320,7 +320,7 @@ func (h *Handler) ServeBookmarkEbook(w http.ResponseWriter, r *http.Request, ps } // Check if it has ebook. - ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") + ebookPath := fp.Join(h.DataDir, "ebook", strID) if !fileExists(ebookPath) { http.Error(w, "ebook not found", http.StatusNotFound) return From 622ff9d275cfe8730127df3a152da26a189ada37 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 00:28:10 +0330 Subject: [PATCH 09/36] report faild item to user --- internal/view/assets/js/page/home.js | 38 +++++++++++++++++++++------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/internal/view/assets/js/page/home.js b/internal/view/assets/js/page/home.js index 1ff6feddd..9cf5f63fa 100644 --- a/internal/view/assets/js/page/home.js +++ b/internal/view/assets/js/page/home.js @@ -702,15 +702,35 @@ export default { this.editMode = false; this.dialog.loading = false; this.dialog.visible = false; - - json.forEach(book => { - var item = items.find(el => el.id === book.id); - this.bookmarks.splice(item.index, 1, book); - }); - }).catch(err => { - this.selection = []; - this.editMode = false; - this.dialog.loading = false; + + let faildUpdateArchives = []; + let faildCreateEbook = []; + json.forEach(book => { + var item = items.find(el => el.id === book.id); + this.bookmarks.splice(item.index, 1, book); + + if (data.createArchive && !book.hasArchive){ + faildUpdateArchives.push(book.id); + } + if (data.createEbook && !book.hasEbook){ + faildCreateEbook.push(book.id); + } + }), + + this.showDialog({ + title: `Update Archive Error`, + content: `Bookmarks Update Archive Faild : ${faildUpdateArchives.join(", ")} + Bookmarks Create Ebook Faild: ${faildCreateEbook.join(",")} + We recovered the last available version.`, + mainText: "OK", + mainClick: () => { + this.dialog.visible = false; + }, + }) + }).catch(err => { + this.selection = []; + this.editMode = false; + this.dialog.loading = false; this.getErrorMessage(err).then(msg => { this.showErrorDialog(msg); From bf31c3b4284a9caa23802e2337a142cda9015d7f Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 01:24:49 +0330 Subject: [PATCH 10/36] not show dialog if error not happen --- internal/view/assets/js/page/home.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/view/assets/js/page/home.js b/internal/view/assets/js/page/home.js index 9cf5f63fa..347e55f4f 100644 --- a/internal/view/assets/js/page/home.js +++ b/internal/view/assets/js/page/home.js @@ -715,8 +715,8 @@ export default { if (data.createEbook && !book.hasEbook){ faildCreateEbook.push(book.id); } - }), - + }); + if(faildCreateEbook.length > 0 || faildUpdateArchives.length > 0){ this.showDialog({ title: `Update Archive Error`, content: `Bookmarks Update Archive Faild : ${faildUpdateArchives.join(", ")} @@ -727,6 +727,7 @@ export default { this.dialog.visible = false; }, }) + } }).catch(err => { this.selection = []; this.editMode = false; From 658c80fee749a4226e4f8244e2d7f3642aaaa0ad Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 02:05:53 +0330 Subject: [PATCH 11/36] update thumbnail based on last status of bookmark fix #524 --- internal/core/processing.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index 1da0654f3..dd9759586 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -66,6 +66,8 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, } // If this is HTML, parse for readable content + strID := strconv.Itoa(book.ID) + imgPath := fp.Join(req.DataDir, "thumb", strID) var imageURLs []string if strings.Contains(contentType, "text/html") { isReadable := readability.Check(readabilityCheckInput) @@ -101,6 +103,8 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Get image URL if article.Image != "" { imageURLs = append(imageURLs, article.Image) + } else { + os.Remove(imgPath) } if article.Favicon != "" { @@ -115,9 +119,6 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, } // Save article image to local disk - strID := strconv.Itoa(book.ID) - imgPath := fp.Join(req.DataDir, "thumb", strID) - for _, imageURL := range imageURLs { err = downloadBookImage(imageURL, imgPath) if err == nil { From 87b6861a81dc50f78631f37611cd0b0716812890 Mon Sep 17 00:00:00 2001 From: Monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 13:11:40 +0330 Subject: [PATCH 12/36] better warning massage Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com> --- internal/view/assets/js/page/home.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/view/assets/js/page/home.js b/internal/view/assets/js/page/home.js index 347e55f4f..e44cf69f4 100644 --- a/internal/view/assets/js/page/home.js +++ b/internal/view/assets/js/page/home.js @@ -720,8 +720,8 @@ export default { this.showDialog({ title: `Update Archive Error`, content: `Bookmarks Update Archive Faild : ${faildUpdateArchives.join(", ")} - Bookmarks Create Ebook Faild: ${faildCreateEbook.join(",")} - We recovered the last available version.`, + Bookmarks Create Ebook Failed: ${faildCreateEbook.join(",")} + Files that failed retrieval were not overwritten.`, mainText: "OK", mainClick: () => { this.dialog.visible = false; From c54da2637aa12681bca10ace3a8e13db7efee54f Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 13:27:42 +0330 Subject: [PATCH 13/36] tmpFile without .epub --- internal/core/ebook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index 7f4281daf..d767783dd 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -47,7 +47,7 @@ func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err } // create temporary epub file - tmpFile, err := os.CreateTemp("", "ebook*.epub") + tmpFile, err := os.CreateTemp("", "ebook") if err != nil { return book, errors.Wrap(err, "can't create temporary EPUB file") } From 0a752e182e47ff24b21075a23c38960ad2f38c34 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 13:34:13 +0330 Subject: [PATCH 14/36] MoveToDestination change to MoveFileToDestination --- internal/core/ebook.go | 2 +- internal/core/processing.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index d767783dd..6df619d47 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -227,7 +227,7 @@ img { } defer tmpFile.Close() - err = MoveToDestination(dstPath, tmpFile) + err = MoveFileToDestination(dstPath, tmpFile) if err != nil { return book, errors.Wrap(err, "failed move ebook to destination") } diff --git a/internal/core/processing.go b/internal/core/processing.go index dd9759586..aaaf1f726 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -167,7 +167,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Prepare destination file. dstPath := fp.Join(req.DataDir, "archive", fmt.Sprintf("%d", book.ID)) - err = MoveToDestination(dstPath, tmpFile) + err = MoveFileToDestination(dstPath, tmpFile) if err != nil { return book, false, fmt.Errorf("failed move archive to destination `: %v", err) } @@ -250,7 +250,7 @@ func downloadBookImage(url, dstPath string) error { return fmt.Errorf("failed to save image %s: %v", url, err) } - err = MoveToDestination(dstPath, tmpFile) + err = MoveFileToDestination(dstPath, tmpFile) if err != nil { return err } @@ -258,7 +258,8 @@ func downloadBookImage(url, dstPath string) error { return nil } -func MoveToDestination(dstPath string, tmpFile *os.File) error { +// dstPath requires the filename +func MoveFileToDestination(dstPath string, tmpFile *os.File) error { // Prepare destination file. err := os.MkdirAll(fp.Dir(dstPath), model.DataDirPerm) if err != nil { From 89c04d69390d328fb22b1714dc580b8b027afe59 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 14:01:50 +0330 Subject: [PATCH 15/36] return .epub --- internal/core/processing.go | 2 +- internal/webserver/handler-api.go | 6 +++--- internal/webserver/handler-ui.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index aaaf1f726..ab866fe12 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -129,7 +129,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // If needed, create ebook as well if book.CreateEbook { - ebookPath := fp.Join(req.DataDir, "ebook", strID) + ebookPath := fp.Join(req.DataDir, "ebook", strID+".epub") if strings.Contains(contentType, "application/pdf") { return book, false, errors.Wrap(err, "can't create ebook from pdf") diff --git a/internal/webserver/handler-api.go b/internal/webserver/handler-api.go index cbbeb81b0..e4f6278a8 100644 --- a/internal/webserver/handler-api.go +++ b/internal/webserver/handler-api.go @@ -110,7 +110,7 @@ func (h *Handler) ApiGetBookmarks(w http.ResponseWriter, r *http.Request, ps htt strID := strconv.Itoa(bookmarks[i].ID) imgPath := fp.Join(h.DataDir, "thumb", strID) archivePath := fp.Join(h.DataDir, "archive", strID) - ebookPath := fp.Join(h.DataDir, "ebook", strID) + ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") if fileExists(imgPath) { bookmarks[i].ImageURL = path.Join(h.RootPath, "bookmark", strID, "thumb") @@ -285,7 +285,7 @@ func (h *Handler) ApiDeleteBookmark(w http.ResponseWriter, r *http.Request, ps h strID := strconv.Itoa(id) imgPath := fp.Join(h.DataDir, "thumb", strID) archivePath := fp.Join(h.DataDir, "archive", strID) - ebookPath := fp.Join(h.DataDir, "ebook", strID) + ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") os.Remove(imgPath) os.Remove(archivePath) @@ -436,7 +436,7 @@ func (h *Handler) ApiDownloadEbook(w http.ResponseWriter, r *http.Request, ps ht // if file exist book return avilable file strID := strconv.Itoa(book.ID) - ebookPath := fp.Join(request.DataDir, "ebook", strID) + ebookPath := fp.Join(request.DataDir, "ebook", strID+".epub") _, err = os.Stat(ebookPath) if err == nil { // file already exists, return the existing file diff --git a/internal/webserver/handler-ui.go b/internal/webserver/handler-ui.go index 17800d169..712744f51 100644 --- a/internal/webserver/handler-ui.go +++ b/internal/webserver/handler-ui.go @@ -49,7 +49,7 @@ func (h *Handler) ServeBookmarkContent(w http.ResponseWriter, r *http.Request, p } // Check if it has ebook. - ebookPath := fp.Join(h.DataDir, "ebook", strID) + ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") if fileExists(ebookPath) { bookmark.HasEbook = true } @@ -320,7 +320,7 @@ func (h *Handler) ServeBookmarkEbook(w http.ResponseWriter, r *http.Request, ps } // Check if it has ebook. - ebookPath := fp.Join(h.DataDir, "ebook", strID) + ebookPath := fp.Join(h.DataDir, "ebook", strID+".epub") if !fileExists(ebookPath) { http.Error(w, "ebook not found", http.StatusNotFound) return From 2b489ea7585d83afe4166d56dfbbebe5081dd114 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 14:37:34 +0330 Subject: [PATCH 16/36] log if downloadBookImage return error --- internal/core/processing.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index ab866fe12..de5aef868 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -8,6 +8,7 @@ import ( "image/draw" "image/jpeg" "io" + "log" "math" "net/url" "os" @@ -121,6 +122,14 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Save article image to local disk for _, imageURL := range imageURLs { err = downloadBookImage(imageURL, imgPath) + if err != nil { + if err.Error() == fmt.Sprintf("%s is not a supported image", imageURL) { + log.Printf("Not found image for URL: %s", imageURL) + } else { + log.Printf("File download not successful for image URL: %s", imageURL) + } + continue + } if err == nil { book.ImageURL = path.Join("/", "bookmark", strID, "thumb") break @@ -192,7 +201,7 @@ func downloadBookImage(url, dstPath string) error { !strings.Contains(cp, "image/pjpeg") && !strings.Contains(cp, "image/jpg") && !strings.Contains(cp, "image/png") { - + os.Remove(dstPath) return fmt.Errorf("%s is not a supported image", url) } From 358811946a1ea756cfbe21b56264b66b63c90b17 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 15:10:56 +0330 Subject: [PATCH 17/36] fix bug remove imgPath just if download last image be unsuccessful --- internal/core/processing.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index de5aef868..8350a4f9e 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -120,11 +120,14 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, } // Save article image to local disk - for _, imageURL := range imageURLs { + for i, imageURL := range imageURLs { err = downloadBookImage(imageURL, imgPath) if err != nil { if err.Error() == fmt.Sprintf("%s is not a supported image", imageURL) { log.Printf("Not found image for URL: %s", imageURL) + if i == len(imageURLs)-1 { + os.Remove(imgPath) + } } else { log.Printf("File download not successful for image URL: %s", imageURL) } @@ -201,7 +204,6 @@ func downloadBookImage(url, dstPath string) error { !strings.Contains(cp, "image/pjpeg") && !strings.Contains(cp, "image/jpg") && !strings.Contains(cp, "image/png") { - os.Remove(dstPath) return fmt.Errorf("%s is not a supported image", url) } From 81703cb3f68af824d6d5d69a02bfd538c2f40353 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 6 Aug 2023 20:34:29 +0330 Subject: [PATCH 18/36] update old unit test --- internal/core/ebook_test.go | 73 +++++++++---------------------------- 1 file changed, 18 insertions(+), 55 deletions(-) diff --git a/internal/core/ebook_test.go b/internal/core/ebook_test.go index 77c7bf15f..d4523b14c 100644 --- a/internal/core/ebook_test.go +++ b/internal/core/ebook_test.go @@ -1,7 +1,6 @@ package core_test import ( - "errors" "fmt" "os" fp "path/filepath" @@ -14,8 +13,9 @@ import ( func TestGenerateEbook_ValidBookmarkID_ReturnsBookmarkWithHasEbookTrue(t *testing.T) { tempDir := t.TempDir() - defer os.RemoveAll(tempDir) + parentDir := t.TempDir() + defer os.RemoveAll(parentDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ @@ -24,11 +24,11 @@ func TestGenerateEbook_ValidBookmarkID_ReturnsBookmarkWithHasEbookTrue(t *testin HTML: "Example HTML", HasEbook: false, }, - DataDir: tempDir, + DataDir: parentDir, ContentType: "text/html", } - bookmark, err := core.GenerateEbook(mockRequest) + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) assert.True(t, bookmark.HasEbook) assert.NoError(t, err) @@ -46,7 +46,7 @@ func TestGenerateEbook_InvalidBookmarkID_ReturnsError(t *testing.T) { ContentType: "text/html", } - bookmark, err := core.GenerateEbook(mockRequest) + bookmark, err := core.GenerateEbook(mockRequest, tempDir) assert.Equal(t, model.Bookmark{ ID: 0, @@ -58,13 +58,15 @@ func TestGenerateEbook_InvalidBookmarkID_ReturnsError(t *testing.T) { func TestGenerateEbook_ValidBookmarkID_EbookExist_EbookExist_ReturnWithHasEbookTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) + parentDir := t.TempDir() + defer os.RemoveAll(parentDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 1, HasEbook: false, }, - DataDir: tempDir, + DataDir: parentDir, ContentType: "text/html", } // Create the ebook directory @@ -81,7 +83,7 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_EbookExist_ReturnWithHasEbookT } defer file.Close() - bookmark, err := core.GenerateEbook(mockRequest) + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) assert.True(t, bookmark.HasEbook) assert.NoError(t, err) @@ -90,13 +92,15 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_EbookExist_ReturnWithHasEbookT func TestGenerateEbook_ValidBookmarkID_EbookExist_ImagePathExist_ReturnWithHasEbookTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) + parentDir := t.TempDir() + defer os.RemoveAll(parentDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 1, HasEbook: false, }, - DataDir: tempDir, + DataDir: parentDir, ContentType: "text/html", } // Create the image directory @@ -113,7 +117,7 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_ImagePathExist_ReturnWithHasEb } defer file.Close() - bookmark, err := core.GenerateEbook(mockRequest) + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) expectedimagePath := "/bookmark/1/thumb" if expectedimagePath != bookmark.ImageURL { t.Errorf("Expected imageURL %s, but got %s", bookmark.ImageURL, expectedimagePath) @@ -125,13 +129,15 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_ImagePathExist_ReturnWithHasEb func TestGenerateEbook_ValidBookmarkID_EbookExist_ReturnWithHasArchiveTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) + parentDir := t.TempDir() + defer os.RemoveAll(parentDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 1, HasEbook: false, }, - DataDir: tempDir, + DataDir: parentDir, ContentType: "text/html", } // Create the archive directory @@ -148,7 +154,7 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_ReturnWithHasArchiveTrue(t *te } defer file.Close() - bookmark, err := core.GenerateEbook(mockRequest) + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) assert.True(t, bookmark.HasArchive) assert.NoError(t, err) } @@ -166,56 +172,13 @@ func TestGenerateEbook_ValidBookmarkID_RetuenError_PDF(t *testing.T) { ContentType: "application/pdf", } - bookmark, err := core.GenerateEbook(mockRequest) + bookmark, err := core.GenerateEbook(mockRequest, tempDir) assert.False(t, bookmark.HasEbook) assert.Error(t, err) assert.Contains(t, err.Error(), "can't create ebook for pdf") } -func TestGenerateEbook_CreateEbookDirectoryNotWritable(t *testing.T) { - // Create a temporary directory to use as the parent directory - parentDir := t.TempDir() - - // Create a child directory with read-only permissions - ebookDir := fp.Join(parentDir, "ebook") - err := os.Mkdir(ebookDir, 0444) - if err != nil { - t.Fatalf("could not create ebook directory: %s", err) - } - - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 1, - HasEbook: false, - }, - DataDir: ebookDir, - ContentType: "text/html", - } - - // Call GenerateEbook to create the ebook directory - bookmark, err := core.GenerateEbook(mockRequest) - if err == nil { - t.Fatal("GenerateEbook succeeded even though MkdirAll should have failed") - } - if !errors.Is(err, os.ErrPermission) { - t.Fatalf("unexpected error: expected os.ErrPermission, got %v", err) - } - - // Check if the ebook directory still exists and has read-only permissions - info, err := os.Stat(ebookDir) - if err != nil { - t.Fatalf("could not retrieve ebook directory info: %s", err) - } - if !info.IsDir() { - t.Errorf("ebook directory is not a directory") - } - if info.Mode().Perm() != 0444 { - t.Errorf("ebook directory has incorrect permissions: expected 0444, got %o", info.Mode().Perm()) - } - assert.False(t, bookmark.HasEbook) -} - // Add more unit tests for other scenarios that missing specialy // can't create ebook directory and can't write situatuin // writing inside zip file From e56ef8fa0a981cf802db96a88a04c39eb4404687 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 7 Aug 2023 00:52:41 +0330 Subject: [PATCH 19/36] add processing.go unit test --- internal/core/processing.go | 4 +- internal/core/processing_test.go | 281 +++++++++++++++++++++++++++++++ 2 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 internal/core/processing_test.go diff --git a/internal/core/processing.go b/internal/core/processing.go index 8350a4f9e..b359a1ce8 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -121,7 +121,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Save article image to local disk for i, imageURL := range imageURLs { - err = downloadBookImage(imageURL, imgPath) + err = DownloadBookImage(imageURL, imgPath) if err != nil { if err.Error() == fmt.Sprintf("%s is not a supported image", imageURL) { log.Printf("Not found image for URL: %s", imageURL) @@ -190,7 +190,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, return book, false, nil } -func downloadBookImage(url, dstPath string) error { +func DownloadBookImage(url, dstPath string) error { // Fetch data from URL resp, err := httpClient.Get(url) if err != nil { diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go new file mode 100644 index 000000000..0c6001dd7 --- /dev/null +++ b/internal/core/processing_test.go @@ -0,0 +1,281 @@ +package core_test + +import ( + "bytes" + "os" + fp "path/filepath" + "testing" + + "github.com/go-shiori/shiori/internal/core" + "github.com/go-shiori/shiori/internal/model" + "github.com/stretchr/testify/assert" +) + +func TestMoveFileToDestination_CreateDir_Fails(t *testing.T) { + tmpFile, err := os.CreateTemp("", "image") + + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + err = core.MoveFileToDestination("/destination/test", tmpFile) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create destination dir") +} + +func TestMoveFileToDestination_CreateFile_Fails(t *testing.T) { + tmpFile, err := os.CreateTemp("", "image") + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + // Create a destination directory + dstDir := t.TempDir() + assert.NoError(t, err) + defer os.Remove(dstDir) + + // Set destination path to an invalid file name to force os.Create to fail + dstPath := fp.Join(dstDir, "\000invalid\000") + + err = core.MoveFileToDestination(dstPath, tmpFile) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create destination file") +} + +// TestDownloadBookImage_Success tests the DownloadBookImage function with a valid image URL. +func TestDownloadBookImage_notSuccess(t *testing.T) { + // Arrange + imageURL := "https://github.com/go-shiori/shiori/blob/master/internal/view/assets/res/apple-touch-icon-152x152.png" + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstPath := fp.Join(tempDir, "1") + defer os.Remove(dstPath) + + // Act + err := core.DownloadBookImage(imageURL, dstPath) + + // Assert + //assert.NoError(t, err) + assert.EqualError(t, err, "https://github.com/go-shiori/shiori/blob/master/internal/view/assets/res/apple-touch-icon-152x152.png is not a supported image") + assert.NoFileExists(t, dstPath) +} + +func TestDownloadBookImage_Success(t *testing.T) { + // Arrange + imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/master/docs/readme/cover.png" + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstPath := fp.Join(tempDir, "1") + defer os.Remove(dstPath) + + // Act + err := core.DownloadBookImage(imageURL, dstPath) + + // Assert + assert.NoError(t, err) + assert.FileExists(t, dstPath) +} + +func TestDownloadBookImage_SuccessSmallSize(t *testing.T) { + // Arrange + imageURL := "https://www.google.com/images/branding/googlelogo/2x/googlelogo_light_color_272x92dp.png" + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstPath := fp.Join(tempDir, "1") + defer os.Remove(dstPath) + + // Act + err := core.DownloadBookImage(imageURL, dstPath) + + // Assert + assert.NoError(t, err) + assert.FileExists(t, dstPath) +} + +func TestProcessBookmark(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.Title { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } +} + +func TestProcessBookmarkMultipleImage(t *testing.T) { + html := `html + + + + + + +

This is an example article

+ +` + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString(html) + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.Title { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } +} + +func TestProcessBookmarkMultipleImagefaveiconAndThumb(t *testing.T) { + html := `html + + + + + + +

This is an example article

+ +` + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString(html) + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.Title { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } +} + +func TestProcessBookmarkEmptyTitle(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.URL { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } +} + +func TestProcessBookmarkKeepExcerptEmpty(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: false, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.URL { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } +} From 947eefa80dd544daecdd656001fbcd1447f99803 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 7 Aug 2023 01:18:22 +0330 Subject: [PATCH 20/36] small massage for report failded item to the user --- internal/view/assets/js/page/home.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/view/assets/js/page/home.js b/internal/view/assets/js/page/home.js index e44cf69f4..4b0d5acc0 100644 --- a/internal/view/assets/js/page/home.js +++ b/internal/view/assets/js/page/home.js @@ -703,24 +703,24 @@ export default { this.dialog.loading = false; this.dialog.visible = false; - let faildUpdateArchives = []; - let faildCreateEbook = []; + let faildedUpdateArchives = []; + let faildedCreateEbook = []; json.forEach(book => { var item = items.find(el => el.id === book.id); this.bookmarks.splice(item.index, 1, book); if (data.createArchive && !book.hasArchive){ - faildUpdateArchives.push(book.id); + faildedUpdateArchives.push(book.id); } if (data.createEbook && !book.hasEbook){ - faildCreateEbook.push(book.id); + faildedCreateEbook.push(book.id); } }); if(faildCreateEbook.length > 0 || faildUpdateArchives.length > 0){ this.showDialog({ - title: `Update Archive Error`, - content: `Bookmarks Update Archive Faild : ${faildUpdateArchives.join(", ")} - Bookmarks Create Ebook Failed: ${faildCreateEbook.join(",")} + title: `Bookmarks Id that Update Action Faild`, + content: `Archive:[ ${faildedUpdateArchives.join(", ")} ] + Ebook: [ ${faildedCreateEbook.join(",")} ] Files that failed retrieval were not overwritten.`, mainText: "OK", mainClick: () => { From 757599fcfc729bffc28e007adf71400248885d0b Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 7 Aug 2023 13:28:54 +0330 Subject: [PATCH 21/36] add some more unit test and samplefile --- internal/core/processing_test.go | 48 +++++++++++++++++++++++++++++-- testdata/big_image.png | Bin 0 -> 16174 bytes testdata/favicon.png | Bin 0 -> 534 bytes testdata/favicon.svg | 6 ++++ testdata/medium_image.png | Bin 0 -> 3340 bytes 5 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 testdata/big_image.png create mode 100644 testdata/favicon.png create mode 100644 testdata/favicon.svg create mode 100644 testdata/medium_image.png diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go index 0c6001dd7..e029f2b02 100644 --- a/internal/core/processing_test.go +++ b/internal/core/processing_test.go @@ -53,7 +53,6 @@ func TestDownloadBookImage_notSuccess(t *testing.T) { err := core.DownloadBookImage(imageURL, dstPath) // Assert - //assert.NoError(t, err) assert.EqualError(t, err, "https://github.com/go-shiori/shiori/blob/master/internal/view/assets/res/apple-touch-icon-152x152.png is not a supported image") assert.NoFileExists(t, dstPath) } @@ -173,7 +172,7 @@ func TestProcessBookmarkMultipleImagefaveiconAndThumb(t *testing.T) { - +

This is an example article

@@ -279,3 +278,48 @@ func TestProcessBookmarkKeepExcerptEmpty(t *testing.T) { t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) } } + +func TestProcessBookmarkIDZero(t *testing.T) { + bookmark := model.Bookmark{ + ID: 0, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + _, isFatal, err := core.ProcessBookmark(request) + assert.Error(t, err) + assert.Contains(t, err.Error(), "bookmark ID is not valid") + assert.True(t, isFatal) +} +func TestProcessBookmarkContentTypeNotTextHtml(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "application/pdf", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + _, _, err := core.ProcessBookmark(request) + assert.NoError(t, err) +} diff --git a/testdata/big_image.png b/testdata/big_image.png new file mode 100644 index 0000000000000000000000000000000000000000..e1a2372628f0fecf017f025e3bd5ccf5198a0ea3 GIT binary patch literal 16174 zcmeHtX;@QNyLQ?hO9zp+wH1*<-nTNer3wgw3`t5=h9Xe4N*SYqfDI5CL&#*cm7+ju zQbDGmQlXWppb#J=ky1&w3=vVLkOW9%3Q0&9GAC!p@4de3oFC`k`E@SVpS`k^wf0)i zx}W=go@b}<>+m4UclNvkfj}%nf{&blKrF67AaAt(<3GSVnv3vh@bb@u;1gjGNWK#U za_I*ML<`=!v8-VlsWu;64O?vd9hzJ8H4; zmc`p{j*2@jlMo2}{g5L8-{eiL@UcDZSxl2cHSd+=+duf(&3_$z^7XIZe`5GL>8s#3 zj(_SCViQp}mss@VXNtms^6^dg6E5Znb( z2I9V4)y*1Lb<1WIojCq@^x;0Ry#_7=Y`2SS-emr}4{<8}di(#o{dLh+Z_5S{$d(PZ zY+z=DEgWpwU<(IZ+PD=JY%$>$6K*l#787nU;T98aG2#CeCR`s%lQ`>;)W$=}2UNYl z)30^V>&#oJnnLkUD2nr#-FmaRMzUV_#v^D+V-_KEst)L( z@6eX~mQprKsqLuB$2Vaq*G;~Q{IK}Lhpz~FO`6SUfa6Nb*9S#9+qO&@tJ00o8#q}+ zX>97s!W$6CvC}7C)nd*}WzWn5W!yz!K0EK^0VL&9or6hpKT$#6q22L&;*G0ZUUcff zNK@a;=h^*>{nm(9M$Q81>{HB}5QrQ6hjS3fnQ!7J{#ma1N&FZdI9HZW6eborNQS9; zd|!ZY->E@-Sf9H8iok|np^=89*W}~tx_D5;0G{9EH!8cr@&LD|nDx8%8st)QTx-~Y zm(IZTmOUm^+#e%i?YA*?S;%yR|9X1>x+vWeI_B1zpqst3w>vhkGC*hpfjm;L2D(Ea zPj5?vc@CCzo_DWrj-)?IQxJZ8?&6jJWA&A{@eI){86Ob;LJ-*&0a33>7k&cT16hd^sEBw60* zEcPLXu(~ml9%0eyRfRC(7@RN*PVuw}OG_~bXn#Xv2#tn#T5PL9oe*lN{S?H&_EK-N zT}p=Whp^ub%xn5+!MW8B=%_~HHwUT`HBt(q`nmB|!dO_z=6lcG!qoDvXWB5>+n+O! zxAoowUYAGIrB-YwxoLPp>Uj07L}q||2e`6}=0m$(JG3j6=9gy@>{W#Fv(G7~4pI@` zJvN>FL%_>{9oj<3C?&F~qAKfg=$q+#)=NM@I$1zrmJmo0;QzppLAqy>2v~%*=?@1i zwXg76XZ5A|Bo-fqRV)x$GIgBl@A}Gx5)yxgW&w_VwEm^pJbS9U4zEq$PbLd<6-y3~P4mW3I23HDcG`d2H~YaXfxeBIUFo63&o8utLss|LX(rT1>muVo=Yy7Ma?| zwb437JyFO;`LCbJe8i)E7#}>(^0L|Uohe}mMDeFvYD<;c^2=A-kwB5=HNQ8o>3z?%aLs+3=!sneYI$q61r!PLBLg$N{Yd6A^CX$B!}>3_ zCXzLQ@oP0SE|MBLZ)0G^FF&oP^5fk%)SD6q4swDw6_TkAqH>FsSZCl#ki$aN&!6X~ z$#J`_uIN4P#zgX~8d=j5D)sa>U{F#lgvH~~%HA$2BS>R)JU)Y%IcG}=s~;h`nPEwL z14ipV8SayJuA_>p7adBI~sAvL}GQ3N_tp zZ@B_TXtOxHeWWyL+UGY8g&0@M6I&&e565lRtB)~uP5%mFqE=5hXfL>US~>3+2rZ1A_9nL>Ax@}s>yuggQq!y12sI)@^rUYsZ{MSRw!ctoYDT9&1yrN~}AdA5&Z71>YZ)D=;?Z7>+h2XB&8L5+X-wpO$q% zYtC;seR2mmjQDk94Le$Ah>~wuIO6uZcj`bJ*M+_ErW8Y#-=HD;DMK=x))F?~c~(DW z8n_I!HIx{8?znn#`NV*>0Sny&_J=Gz1+}Rt<~<(X{kRwB9Rbsc65DIF^p*v5SbWA8 zrZL+FjDu#RUdxtUi*!mJ_FZIDc5IhEbrW!qnA(ix^MKYgWy-PY^o|m#d#6E*xuv~t zW}iSPF>FB-E{yPv>M?TpcJ#&TKwza4l;N*gJPtf@c6uP^@w@1;)v*u%{>0ns9!RU= z97|zAx8yF+W#!~8zp2+=OJwh)MhW1`dtQe;v3yvFk!jFJ&W92^5;E7^0oU~4T1jM*X$@( zRh|mfHjs5HuU;)ED6n9@4I)Ij?DQrn^RajWWx*W%Jq>d)^I4fMbWJ35wr%!TM*9(A zKDnxSP1OEyV{(&Bwb|E1X^N1_Cf3Tr1giRZ&0bO&K54saJuTmtm7Kzo`68zbU1-OyMI|j@3IQn;fG|9@24!Oqs~zP$b?fc>dvz5SE%LqdmS8 z;mly3TUJQNLrU0x=#ulNnLvG@`mLFhNnds3cVLaf<<$0wFk=>VSaR$tdkCXlJ9}Wo zHAKdDh?+yx*^{JNAZ?)-Z_KiG@bK1)ibn*ZU=od(TPv%Uh?>-~ zmzXP{Yvf>ScS9hHDV2`{;q#r6=+_d5xQ>y~1B-U1n(!NW;rn}1@G_SR&_WE-1Dkt&KDf%krTIoI zJwQ1S=hwp40V2A}srq9@(5@7iDjb|Pej_!L(+`}&qn&BUK68J!&-zoLgwl{ zKGY=Z3nT`*5Fe0qwgA2cMe|$f5sTSH>C97Q$7pe#SR;w}b1eR^zxcwG!8Lw>{7j~1 ziUzzJL9dB9TPrTS7sQ@ly{L4hdvVd7M^q@#p%1?O7!<*u?4fAePNqX=qgJABk=LO z0UKt^SC`(_rTHx~z^K8=upnZP&Y zoa*B^q7_Sm@%YBseZzP5>K>@FPTT3aT=cD2S!06s0lO@Jwv5@LxbDtM)19ccH^h_I zO)RABvd7r5&`r(Rwnx297kfqrnJt>WDC)_v@L^*>=I{Z4TpAj#4rnulF+hninYCs5UU_H}&px+_j*RzS zsAU-GG}Jaxd*p829$Q*z>c;iN+9s-HdN@4qWE+1B_~kH8D73U3@yyy}ZQ>cKT;rGM z3kcRc0~dqot(%z?aeUFLZ1e*AIWf|cYY?9_Fop-JM^*#nr-M`~Uq3JWU#rx2$wKBx zWhn8vzxPXo4pTi8d4a}|CtM+dFSH*eI+|q{HYx5AcMn95sP8bTg#F>WmPa{Kqq6_B z8+_w_fbb>4uW89IpdcTqJ1Cb(_15(dp`?u^ShFs+?+BUHFpZfM43oMhkwT(=lmUh%|$Zn*ia&fO_E(3Z3h+seKnJ&(CyqA&?G| z)Yk=OmMW_JFg&CrYOjs)3zy~C)eZ4sdU0A4R9KOE$aF>~62K^gx`%>vqwKN>-|Cp} zZ<@TSh{})2RhMV58{_8VKe@WVuM@2@ zpEcka26Z)WhxSu2UZ%eIMNkG_7l(0}P(h`GDR1ZMtuX50)4?(UgR#aXc>3s00v2Sc zNnjs^(C|slk;6A)AAF_MH)N>r%^!CBg=VuJd&ksq-eouX!Lz?P#IwJ*#wGB#MIX<5jBzgD zmQNcv0m2;H(MPK0&4DjNBUU!9LYGVxDj7K8Q*rsdH<@oZH6-IpKLRtBFU)y~(@l9C zRiFC#LfHsxza!0Eq9 zk8|zH01}&3&K~i|s; zmOc~6e7yLTH=oqZR{*WZ5T&$iJF#U0+PCwAYgF;CpbQH}iL$x| zzZyS`HF_kiJy`kjCiNwk0KkZ7@fyu9d5+e*zT)al?z>wVu>$*CS+FVc9y-{n1EG3p zL^(2Q-l4#=S7^r*H_6j!I_?fqWfyu$To~!im1m!j6thmsNFfgPKhD}Co>Y{g$cv_m zGNe0*k|$VOr*PT?!L7I}>8UL^-#^U3tJf~g1n4GhKFMh`-i&A{D{_CuM|@Gt$YC(W zmR(RnRiN#6F%q6)1|KfoZ?QBVA>>u$-TxWSo6B$4-NuY7=O-%{n$y(Exd=m0+Z2I` z#HDlu2o38ZmPpGGkaJTItU33cBy_~}zh9=c<>?gHg@+ca!Bcw+)ch}OTdHcY>ExF1 zX}G=d*xK|v= zavVAGn^nfV%&}NtiQ^#b^3f=2MHuvY61AL1hGIiaEjfdSfeSdtSK9RJ7_s zQ=f0oK9=HCh<4p-TYNgTf#Sj|l6OTv<65sq8gnvrOCg&k&ZBx$h5<({?q3*N9I3N% zig{>5ntA9@zbW?>8d1unNLJ#E!Xc)^2&RZnYsSH2eaNvj8zTmnqYj?Q*;RcD_7U#n z+Vr26!lO8TpOjOue^JOPiAgvoV_yp`UE=y8zwN=watexsMuc#+TEKNJjm7-AIJPX_ z9OaSaFBZu1I#N>JeQbJV1E@I>!#pBh$FMMf>GmbynAxSLrc^tJvaDf0>-D1|oT1tm zuit34dwn3*r5y^5rz3RTNwW^g2jlSg8eo07n!d^Er*XZwX*njfuyCs3OCzT1g0o2Gs=`Pcs!Dgn_wIxjDT1qW0AM1sYE14s;o` zgEM7a8WceH?J2}eyd27E(L7hfsW<&Vq4?R`kCQrnk3*E1Rw_Pn3tiI}M&`1|z9qCA zE+TR`xENijA7xaP0tRvx{U*WUkUh?eJGAunvobS5n_0c*X6jf@unFl?!QluXj ztp>!mk(;UtYvhg3RX67~`^Jy8q1N;Jk{lj!^{Ow_BRh~(bJS7Tv)4y7 z+gk0%^R8T&pYklniWEXqzMa5{y}|&DnBp{ka&mB~^dBQ!HJ}!Pkw;JgBD749vLeaV zdMqtYS`f^qn&@m+cRPZvXLt?Q$iMdC0=`y-VrppOxV?Cj+{)EXr$w?oG;73Kg#6(s zHHyAl)2V|l8En4k&|C&s9=Rkv+RN5QfXL+hB{kh|@#0E1?u(m(iqo06^zbB`bQ+Ry zndMR?h&vFmHf^_1q?dG7A9Y>!XOEubH-XQ7{)&<82zZ0o1p!W8%$m4nhqAj>^#?{d zIJCDWYhw_IObWglo?eZUQmV;pwJTqWEfacjnYnWDQ(+E$SS|~>+7a5Z^6~r5K;gR> z%tu3;Sod<#Z{kR*8=-a>b< zSLHwhG^{kXc`bw8g`SfY3#t)+Vs2SkI>rbp^oE6MGB2G!)@s!_iXRLM+M!Jc+3CXf zui}FgQxmnCG{$c&Bl>72h(O~&`?YJY7}6J^i$E} zrd5Byn{_i*Ss*U4rCb-#@H!*~Q!`b8JP$ZHhY^fC;b_mMlI#=_H~!eE2Kq9OUUw%) zsO;VpMMx<<$#rAgUcAal9_18;zLz+AJvw2axlAwH*xN!{Okt@yBUGw^s33R}*Dprp zuCt|{@(#|`f6=BG0WKGG^w;CzU0KFqRP3aLo9yW7S%erDX=>M59O{ce_3ri2(vTz) zQZnAFVNIoT+yu6GAEN4WdKLouPta?a6$chUL5GZ9-8&vi^M(0j ztQE313xk|v20gp8!UeF&`Jab5Rm!}~C*f&M9pAwhewd_6_kU;nZ2~b=GpL}LVAHE3 zy?q5v7gT{O)>M6vl@&^ORl%gRN1Akqv8ALJ zq!m=BE`jP`;o10LVNs6C>tvAMH-ZxS!rLLz8HivYP1D!G9Vy=9zpx~E17uzi#`a`{ zOc>wn?}A+@poWIqXeJN@gy!TkEt7XoMf^-NOV`qopjvC^i_Jw=lyhq7bk_1;@oOxu z6Hn%;U4v&{Y!fLB`~fM$k!KL$5Z3A#GzvEU4-##%gPv1Es5{HI$d6H#^Y_SV3%sr6 zJVey&Uf_boMhCsjHpd}cxl(D{&)F^Iv=yZ5^kRcVjGPmtlTl{Lp;Ac!HWQ z9N2|MNzX5p^}ui=qs%7FjFXfj=l@2}k)zeOrgagLG(OH&a0$QqxV1$?UyC+%Y1mGx z98K$7HU4~ItHqSX9+`+`Z#?_g>cz~z1DV1|${^`9ikZ_Q;pX4eziiwTsnW;!8gfw| zkB1O=cjcRmO_+h>z^4z^Ni`s!tf!^q<@Fe=J;30(_s0f@f~L~gi-~en6CpKIjHaFs zUzFPySceJLjwmI^S=;nuev}3z-=8ho1GGjKT}&g1p3st=EKvk_2tG4#_N+C}Y+Y)*Jl;-u4l!L)?jBL+>mw zR%5enXNKly=vN60R{jFul2f*g6d)?_IehV9juU!vf4Pb)<(8k9ClF?x90y*)4HMK^Mlc*8Zd*6?c=T3AdPViwU=waEl4Im~e{; zx0rB?3AdQ={}vO@nCQtCVD$z9xvX!p=<=+14}352s(IY~cQNElpt+!BZi7FW%RXSc zR1WF&^zsb0IpP9-a?C#ZZFb_>?DM_}ndiX^k4zl0AZa85pY67#JE_7#My5g&JNk zFq9fFFuY1&V6d9Oz#v{QXIG#NP=YDR+ueoXe|!I#{XiajiKnkC`*RLY^OdhE_itDe{gikWA4YB zGMWn|J6G%33a+VouV?YOq##!Po9gxQ z5(Xy^CLYiUH#jTIa)0MJ;hN%!*AKbBixTA3JI8CYl= z7+4t?=xHqSLD7(#pOTqYiCaT_sniFc1`W6kC7HRY#U+Wk1-SKaBv|VM^)Pt4`njxg HN@xNAclf_o literal 0 HcmV?d00001 diff --git a/testdata/favicon.svg b/testdata/favicon.svg new file mode 100644 index 000000000..bd36e52d7 --- /dev/null +++ b/testdata/favicon.svg @@ -0,0 +1,6 @@ + + + + + diff --git a/testdata/medium_image.png b/testdata/medium_image.png new file mode 100644 index 0000000000000000000000000000000000000000..bce2a0cce907ed34907335b31d5a8534fa60f259 GIT binary patch literal 3340 zcmeH~TU6579>@QgWmei;&XlF8jd>e$LLDOoFI|;Uw6Ze0s0C_B^`t9}G zYp?a)KOPGUve>v|BLDyvu;9Zd0KmlFIDhe(sd4OZTl3l|_K4sUZ~(}50)T?+0H850 z70d!a1_%J=V*$YT9sq2+TGf6WYTPin6cTjUVE9X;$DlDXONYUan29!+nR{7VOX6Ap zVACS(@PU(AJc%IZc6D5?*`jQ{l|%H#b<}lMAeXweyIc=aeGY!!SUb>LcPuc(tk2Ir z^gvgC?a0~9CpN?Du33f_)wX^;61#)4iv&X730Xp9s&exJE5$eWA0NqF>tZTWcf%76T(q$Jc=If zuav|0Hc5VY`S1Lz^ItMwR>f<7b0(Z_)QVOOC?3r>phEGA7cCL;-*3&z zs@2Tj5z#u`>gp)d(i)!B+@vKiEW_8=gvBR8Tc`z}($#vxzudP@XMzFb;~yD#JC$EI>!nPD;8CzPE{gq`}|wDEm|wCX*$ ztgOjN@z_%|{SnqLec%qsDd9YK4ubAM63ify4*&0PGzV!SkCq=SzyEW)h(Ei`jPm2~ zv_49K z4+#s`e|RkXOqWAdp(>nXE}6aYPnm6R zqj_M(?~$=bJQ2Ygus9vE@hb;*29NG&xcYKdW$~!S%N}Iz|Ma=5U2dcz2(2SKiXLkjx<7$*M4w{Zp4c<|-U7i=wsH(B9MUV6W#~^TqZn$#s2c2s*<*UfdHxv5NYRN-u z@0r*YtE?8UcxH8VUjB>W4d_ed(()EMMPJx*axou03YU z&C){+7_z#OK-6Gmp<8n?a^;qV7h6HD&D0bjJ1!|Rt})~F@lM9hr|4nPn0rp!7FLUV zWn!6bYlR*7vB3hJqm762tI(Kv7E!QWv3R2~h9U2{IokO+r$PE43NG*nSny*zD^qx2 zE^di|NhjqdKC92Cs-FK^?|yk+>X+DC$x4~WbUaY$$+SnR^DkF?)|wE64|;s_HyY+{ zAvu}8$sxBw!k$xdNw3`n(_6M0C~1MG#3=~p-!-EQ9{wxmjQ73L$qd9pl(rKbRmJXn z?uE=Dx|2aWC2UK>b7o+ZkC5}`%zDK0V#KNbw$OcNB92)%=^(_tPxr!iJ~<#?{e{`8 zN6uK|X|I{6nh=VW9M%wI8BU?5;5onoXpL!f92)9K-Oqg8#DbUQw`RzvcjdCG2m?Ez zip4q%_DZWvOW>Q?-FpUNTFHo)mz{Hqn@+SP;tep*j9|fvmr1Il2U9BjH7doTj%kU2m^#eoq~V=A7Mi1OBM+3a z*y;Q1_V41x|LaVi#wT`deLyo-SA>%C4JRWe6fI$buKZDRJ5dZC3c8=>Dx#pB$bQcg zQ6O1YG9;je06-t#NzQIAJ)sKyG zvg?Cp`le3hO3WU(uTJ%*>H5X>;|+{7=vlk|d*h#agh81l)4D2erH8|gYcZav!eL_>jd+159}O$V6NOR<H6!#~-T}33Atm1xJI0o8OFYDrTn%z9~N=Rl`&f z&=_d0&Ceb2S*I4z{25X4NlSG%C6Vu_N*==Mgaa&J3=PUnV%w%!Qq6?*`AIU(Hm2Fu zOqF&XFYGP_^9>Wz@f`1VoDJgih-H401LTx~#}1<|aD@`Xh(LGOk1)*xKm1lx3L6W! zZR`T<-4#b`imYwrRV3bGf~VWBsE7&67>Q=jc@PZz68VQi!^D$ah1?@3nhg^||8{BON0zR6sSC zQ~NQ@?2t;zYXvcNv2?a_bv~nK;cm*vozg8nYg^@<`f=HOe2I*De|X>WknpMXX@dnV zT<}(>sdcE-o9&rvo{Vb~a{2h-NQD?bg8i`asbt|SN?TzY-kj=_Fgi^1wrjN?WZdOZ zAU;v4@t&%xA7$3Fix)tFEUpW6mMe@%aJDn7UT;`?#vviF;3K}r`x?{U zKIJeha&u^sZNa-Q0t;5-N9)g1*t}@-$IXi_7<(^F$UdkCSDlN>aeUd{4S!&?b>*>t znz;X`kJSZi+kg-{JgBKNeg_=WkDN(IUQCbkMWn_V1@HuUdbxx4yMw?dK|a3T9=@Jn zHxS4d1d8$JIsO-rj7BELWB(ZV7ICS<2)O*GLV99yTw40Y Date: Mon, 7 Aug 2023 13:53:58 +0330 Subject: [PATCH 22/36] use sample image in unit test --- internal/core/processing_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go index e029f2b02..0284b7211 100644 --- a/internal/core/processing_test.go +++ b/internal/core/processing_test.go @@ -75,7 +75,7 @@ func TestDownloadBookImage_Success(t *testing.T) { func TestDownloadBookImage_SuccessSmallSize(t *testing.T) { // Arrange - imageURL := "https://www.google.com/images/branding/googlelogo/2x/googlelogo_light_color_272x92dp.png" + imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/757599fcfc729bffc28e007adf71400248885d0b/testdata/medium_image.png" tempDir := t.TempDir() defer os.RemoveAll(tempDir) dstPath := fp.Join(tempDir, "1") @@ -171,7 +171,7 @@ func TestProcessBookmarkMultipleImagefaveiconAndThumb(t *testing.T) { html := `html - + From d1a641270c43efcbf899bd587ac669817d0685c8 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 7 Aug 2023 20:50:37 +0330 Subject: [PATCH 23/36] use local sample file and unit test not need internet connection anymore --- internal/core/processing.go | 3 +-- internal/core/processing_test.go | 36 +++++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index b359a1ce8..1b03cb8cb 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -12,7 +12,6 @@ import ( "math" "net/url" "os" - "path" fp "path/filepath" "strconv" "strings" @@ -134,7 +133,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, continue } if err == nil { - book.ImageURL = path.Join("/", "bookmark", strID, "thumb") + book.ImageURL = fp.Join("/", "bookmark", strID, "thumb") break } } diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go index 0284b7211..758cced91 100644 --- a/internal/core/processing_test.go +++ b/internal/core/processing_test.go @@ -2,6 +2,8 @@ package core_test import ( "bytes" + "net/http" + "net/http/httptest" "os" fp "path/filepath" "testing" @@ -73,9 +75,16 @@ func TestDownloadBookImage_Success(t *testing.T) { assert.FileExists(t, dstPath) } -func TestDownloadBookImage_SuccessSmallSize(t *testing.T) { +func TestDownloadBookImage_SuccessMediumSize(t *testing.T) { + // create a file server handler for the 'testdata' directory + fs := http.FileServer(http.Dir("../../testdata/")) + + // start a test server with the file server handler + server := httptest.NewServer(fs) + defer server.Close() + // Arrange - imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/757599fcfc729bffc28e007adf71400248885d0b/testdata/medium_image.png" + imageURL := server.URL + "/medium_image.png" tempDir := t.TempDir() defer os.RemoveAll(tempDir) dstPath := fp.Join(tempDir, "1") @@ -168,16 +177,23 @@ func TestProcessBookmarkMultipleImage(t *testing.T) { } func TestProcessBookmarkMultipleImagefaveiconAndThumb(t *testing.T) { + // create a file server handler for the 'testdata' directory + fs := http.FileServer(http.Dir("../../testdata/")) + + // start a test server with the file server handler + server := httptest.NewServer(fs) + defer server.Close() + html := `html - + - - - - -

This is an example article

- -` + + + + +

This is an example article

+ + ` bookmark := model.Bookmark{ ID: 1, URL: "https://example.com", From a44caad057b491246139654a7450a93e444dd06d Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 12 Aug 2023 16:10:35 +0330 Subject: [PATCH 24/36] update error to user and log that too --- internal/view/assets/js/page/home.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/view/assets/js/page/home.js b/internal/view/assets/js/page/home.js index 4b0d5acc0..70b589af0 100644 --- a/internal/view/assets/js/page/home.js +++ b/internal/view/assets/js/page/home.js @@ -711,17 +711,17 @@ export default { if (data.createArchive && !book.hasArchive){ faildedUpdateArchives.push(book.id); + console.error("can't update archive for bookmark id", book.id) } if (data.createEbook && !book.hasEbook){ faildedCreateEbook.push(book.id); + console.error("can't update ebook for bookmark id:", book.id) } }); - if(faildCreateEbook.length > 0 || faildUpdateArchives.length > 0){ + if(faildedCreateEbook.length > 0 || faildedUpdateArchives.length > 0){ this.showDialog({ title: `Bookmarks Id that Update Action Faild`, - content: `Archive:[ ${faildedUpdateArchives.join(", ")} ] - Ebook: [ ${faildedCreateEbook.join(",")} ] - Files that failed retrieval were not overwritten.`, + content: `Not all bookmarks could have their contents updated, but no files were overwritten.`, mainText: "OK", mainClick: () => { this.dialog.visible = false; From 612c4391af286a91a41efd165fa6efe2d39352f8 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 12 Aug 2023 16:22:08 +0330 Subject: [PATCH 25/36] add more comment --- internal/core/ebook.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index 6df619d47..b7e00a0f6 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -16,6 +16,9 @@ import ( "github.com/pkg/errors" ) +// this function get request and a destination path and will create an epub file in dstPath from that request +// it will return a bookmark model and err +// bookmark model later use for update UI shiori based on this function be sucssesful or not func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err error) { // variable for store generated html code var html string @@ -27,6 +30,7 @@ func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err return book, errors.New("bookmark ID is not valid") } + // get current state of bookmark // cheak archive and thumb strID := strconv.Itoa(book.ID) @@ -41,6 +45,8 @@ func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err book.HasArchive = true } + // this function create ebook from reader mode of bookmark so + // we can't create ebook from PDF so we return error here if bookmark is a pdf contentType := req.ContentType if strings.Contains(contentType, "application/pdf") { return book, errors.New("can't create ebook for pdf") @@ -226,7 +232,7 @@ img { return book, errors.Wrap(err, "can't open temporary EPUB file") } defer tmpFile.Close() - + // if everitings go well we start move ebook to dstPath err = MoveFileToDestination(dstPath, tmpFile) if err != nil { return book, errors.Wrap(err, "failed move ebook to destination") From 9a42103a3cf105e829ae28e94baa9466d5aa53db Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sat, 12 Aug 2023 16:24:15 +0330 Subject: [PATCH 26/36] update comment --- internal/core/ebook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index b7e00a0f6..2dd0fc91f 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -17,6 +17,7 @@ import ( ) // this function get request and a destination path and will create an epub file in dstPath from that request +// dstPath should be incouded file name with '.epub' // it will return a bookmark model and err // bookmark model later use for update UI shiori based on this function be sucssesful or not func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err error) { From 55bb07130920cd7473e79c70517b7ca13214f2cb Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 13:28:33 +0330 Subject: [PATCH 27/36] change variable name parentDir to dstDir --- internal/core/ebook_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/core/ebook_test.go b/internal/core/ebook_test.go index d4523b14c..46f8dd69c 100644 --- a/internal/core/ebook_test.go +++ b/internal/core/ebook_test.go @@ -14,8 +14,8 @@ import ( func TestGenerateEbook_ValidBookmarkID_ReturnsBookmarkWithHasEbookTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) - parentDir := t.TempDir() - defer os.RemoveAll(parentDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ @@ -24,7 +24,7 @@ func TestGenerateEbook_ValidBookmarkID_ReturnsBookmarkWithHasEbookTrue(t *testin HTML: "Example HTML", HasEbook: false, }, - DataDir: parentDir, + DataDir: dstDir, ContentType: "text/html", } @@ -58,15 +58,15 @@ func TestGenerateEbook_InvalidBookmarkID_ReturnsError(t *testing.T) { func TestGenerateEbook_ValidBookmarkID_EbookExist_EbookExist_ReturnWithHasEbookTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) - parentDir := t.TempDir() - defer os.RemoveAll(parentDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 1, HasEbook: false, }, - DataDir: parentDir, + DataDir: dstDir, ContentType: "text/html", } // Create the ebook directory @@ -92,15 +92,15 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_EbookExist_ReturnWithHasEbookT func TestGenerateEbook_ValidBookmarkID_EbookExist_ImagePathExist_ReturnWithHasEbookTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) - parentDir := t.TempDir() - defer os.RemoveAll(parentDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 1, HasEbook: false, }, - DataDir: parentDir, + DataDir: dstDir, ContentType: "text/html", } // Create the image directory @@ -129,15 +129,15 @@ func TestGenerateEbook_ValidBookmarkID_EbookExist_ImagePathExist_ReturnWithHasEb func TestGenerateEbook_ValidBookmarkID_EbookExist_ReturnWithHasArchiveTrue(t *testing.T) { tempDir := t.TempDir() defer os.RemoveAll(tempDir) - parentDir := t.TempDir() - defer os.RemoveAll(parentDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 1, HasEbook: false, }, - DataDir: parentDir, + DataDir: dstDir, ContentType: "text/html", } // Create the archive directory From 3da62067c84d1d7c36285819c52612203288af69 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:05:06 +0330 Subject: [PATCH 28/36] more simpler error handling --- internal/core/processing.go | 19 ++++++++++--------- internal/core/processing_test.go | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index 1b03cb8cb..bcdcb7124 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -37,6 +37,8 @@ type ProcessRequest struct { LogArchival bool } +var ErrNoSupportedImageType = errors.New("unsupported image type") + // ProcessBookmark process the bookmark and archive it if needed. // Return three values, is error fatal, and error value. func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, err error) { @@ -121,15 +123,14 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, // Save article image to local disk for i, imageURL := range imageURLs { err = DownloadBookImage(imageURL, imgPath) - if err != nil { - if err.Error() == fmt.Sprintf("%s is not a supported image", imageURL) { - log.Printf("Not found image for URL: %s", imageURL) - if i == len(imageURLs)-1 { - os.Remove(imgPath) - } - } else { - log.Printf("File download not successful for image URL: %s", imageURL) + if err != nil && errors.Is(err, ErrNoSupportedImageType) { + log.Printf("Not found image for URL: %s", imageURL) + if i == len(imageURLs)-1 { + os.Remove(imgPath) } + } + if err != nil { + log.Printf("File download not successful for image URL: %s", imageURL) continue } if err == nil { @@ -203,7 +204,7 @@ func DownloadBookImage(url, dstPath string) error { !strings.Contains(cp, "image/pjpeg") && !strings.Contains(cp, "image/jpg") && !strings.Contains(cp, "image/png") { - return fmt.Errorf("%s is not a supported image", url) + return ErrNoSupportedImageType } // At this point, the download has finished successfully. diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go index 758cced91..29fc7855e 100644 --- a/internal/core/processing_test.go +++ b/internal/core/processing_test.go @@ -55,7 +55,7 @@ func TestDownloadBookImage_notSuccess(t *testing.T) { err := core.DownloadBookImage(imageURL, dstPath) // Assert - assert.EqualError(t, err, "https://github.com/go-shiori/shiori/blob/master/internal/view/assets/res/apple-touch-icon-152x152.png is not a supported image") + assert.EqualError(t, err, "unsupported image type") assert.NoFileExists(t, dstPath) } From 1630092c0cb8edf749cad7eeead128a8b8855e57 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:16:12 +0330 Subject: [PATCH 29/36] remove unneeded defer --- internal/core/ebook.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index 2dd0fc91f..9e6747106 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -58,7 +58,6 @@ func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err if err != nil { return book, errors.Wrap(err, "can't create temporary EPUB file") } - defer tmpFile.Close() defer os.Remove(tmpFile.Name()) // Create zip archive From afba6ff5f6a45038912bea3181b15c49c7fd63ea Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:22:46 +0330 Subject: [PATCH 30/36] remvoe unneeded epubWriter.Close() --- internal/core/ebook.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index 9e6747106..ff3b25425 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -62,7 +62,6 @@ func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err // Create zip archive epubWriter := zip.NewWriter(tmpFile) - defer epubWriter.Close() // Create the mimetype file mimetypeWriter, err := epubWriter.Create("mimetype") From 17f984da5ada25273769a3de1f6dc16fa098ad3d Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 15:38:01 +0330 Subject: [PATCH 31/36] more readable unit test in processing --- internal/core/processing_test.go | 653 ++++++++++++++++--------------- 1 file changed, 332 insertions(+), 321 deletions(-) diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go index 29fc7855e..d3e37cccb 100644 --- a/internal/core/processing_test.go +++ b/internal/core/processing_test.go @@ -13,329 +13,340 @@ import ( "github.com/stretchr/testify/assert" ) -func TestMoveFileToDestination_CreateDir_Fails(t *testing.T) { - tmpFile, err := os.CreateTemp("", "image") - - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - - err = core.MoveFileToDestination("/destination/test", tmpFile) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to create destination dir") -} - -func TestMoveFileToDestination_CreateFile_Fails(t *testing.T) { - tmpFile, err := os.CreateTemp("", "image") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - - // Create a destination directory - dstDir := t.TempDir() - assert.NoError(t, err) - defer os.Remove(dstDir) - - // Set destination path to an invalid file name to force os.Create to fail - dstPath := fp.Join(dstDir, "\000invalid\000") - - err = core.MoveFileToDestination(dstPath, tmpFile) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to create destination file") +func TestMoveFileToDestination(t *testing.T) { + t.Run("create fails", func(t *testing.T) { + t.Run("directory create fails", func(t *testing.T) { + // test if create dir fails + tmpFile, err := os.CreateTemp("", "image") + + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + err = core.MoveFileToDestination("/destination/test", tmpFile) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create destination dir") + }) + t.Run("file create fails", func(t *testing.T) { + // if create file failed + tmpFile, err := os.CreateTemp("", "image") + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + // Create a destination directory + dstDir := t.TempDir() + assert.NoError(t, err) + defer os.Remove(dstDir) + + // Set destination path to an invalid file name to force os.Create to fail + dstPath := fp.Join(dstDir, "\000invalid\000") + + err = core.MoveFileToDestination(dstPath, tmpFile) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create destination file") + }) + }) } - -// TestDownloadBookImage_Success tests the DownloadBookImage function with a valid image URL. -func TestDownloadBookImage_notSuccess(t *testing.T) { - // Arrange - imageURL := "https://github.com/go-shiori/shiori/blob/master/internal/view/assets/res/apple-touch-icon-152x152.png" - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstPath := fp.Join(tempDir, "1") - defer os.Remove(dstPath) - - // Act - err := core.DownloadBookImage(imageURL, dstPath) - - // Assert - assert.EqualError(t, err, "unsupported image type") - assert.NoFileExists(t, dstPath) -} - -func TestDownloadBookImage_Success(t *testing.T) { - // Arrange - imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/master/docs/readme/cover.png" - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstPath := fp.Join(tempDir, "1") - defer os.Remove(dstPath) - - // Act - err := core.DownloadBookImage(imageURL, dstPath) - - // Assert - assert.NoError(t, err) - assert.FileExists(t, dstPath) -} - -func TestDownloadBookImage_SuccessMediumSize(t *testing.T) { - // create a file server handler for the 'testdata' directory - fs := http.FileServer(http.Dir("../../testdata/")) - - // start a test server with the file server handler - server := httptest.NewServer(fs) - defer server.Close() - - // Arrange - imageURL := server.URL + "/medium_image.png" - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstPath := fp.Join(tempDir, "1") - defer os.Remove(dstPath) - - // Act - err := core.DownloadBookImage(imageURL, dstPath) - - // Assert - assert.NoError(t, err) - assert.FileExists(t, dstPath) +func TestDownloadBookImage(t *testing.T) { + t.Run("Download Images", func(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() + defer os.RemoveAll(tempDir) + dstPath := fp.Join(tempDir, "1") + defer os.Remove(dstPath) + + // Act + err := core.DownloadBookImage(imageURL, dstPath) + + // Assert + assert.EqualError(t, err, "unsupported image type") + assert.NoFileExists(t, dstPath) + }) + t.Run("sucssesful downlosd image", func(t *testing.T) { + // Arrange + imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/master/docs/readme/cover.png" + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstPath := fp.Join(tempDir, "1") + defer os.Remove(dstPath) + + // Act + err := core.DownloadBookImage(imageURL, dstPath) + + // Assert + assert.NoError(t, err) + assert.FileExists(t, dstPath) + }) + t.Run("sucssesful downlosd medium size image", func(t *testing.T) { + // create a file server handler for the 'testdata' directory + fs := http.FileServer(http.Dir("../../testdata/")) + + // start a test server with the file server handler + server := httptest.NewServer(fs) + defer server.Close() + + // Arrange + imageURL := server.URL + "/medium_image.png" + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstPath := fp.Join(tempDir, "1") + defer os.Remove(dstPath) + + // Act + err := core.DownloadBookImage(imageURL, dstPath) + + // Assert + assert.NoError(t, err) + assert.FileExists(t, dstPath) + + }) + }) } func TestProcessBookmark(t *testing.T) { - bookmark := model.Bookmark{ - ID: 1, - URL: "https://example.com", - Title: "Example", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString("

This is an example article

") - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "text/html", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: true, - } - expected, _, _ := core.ProcessBookmark(request) - - if expected.ID != bookmark.ID { - t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) - } - if expected.URL != bookmark.URL { - t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) - } - if expected.Title != bookmark.Title { - t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) - } - if expected.Excerpt != bookmark.Excerpt { - t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) - } -} - -func TestProcessBookmarkMultipleImage(t *testing.T) { - html := `html - - - - - - -

This is an example article

- -` - bookmark := model.Bookmark{ - ID: 1, - URL: "https://example.com", - Title: "Example", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString(html) - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "text/html", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: true, - } - expected, _, _ := core.ProcessBookmark(request) - - if expected.ID != bookmark.ID { - t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) - } - if expected.URL != bookmark.URL { - t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) - } - if expected.Title != bookmark.Title { - t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) - } - if expected.Excerpt != bookmark.Excerpt { - t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) - } -} - -func TestProcessBookmarkMultipleImagefaveiconAndThumb(t *testing.T) { - // create a file server handler for the 'testdata' directory - fs := http.FileServer(http.Dir("../../testdata/")) - - // start a test server with the file server handler - server := httptest.NewServer(fs) - defer server.Close() - - html := `html - - - - - - -

This is an example article

- - ` - bookmark := model.Bookmark{ - ID: 1, - URL: "https://example.com", - Title: "Example", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString(html) - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "text/html", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: true, - } - expected, _, _ := core.ProcessBookmark(request) - - if expected.ID != bookmark.ID { - t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) - } - if expected.URL != bookmark.URL { - t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) - } - if expected.Title != bookmark.Title { - t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) - } - if expected.Excerpt != bookmark.Excerpt { - t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) - } -} - -func TestProcessBookmarkEmptyTitle(t *testing.T) { - bookmark := model.Bookmark{ - ID: 1, - URL: "https://example.com", - Title: "", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString("

This is an example article

") - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "text/html", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: true, - } - expected, _, _ := core.ProcessBookmark(request) - - if expected.ID != bookmark.ID { - t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) - } - if expected.URL != bookmark.URL { - t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) - } - if expected.Title != bookmark.URL { - t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) - } - if expected.Excerpt != bookmark.Excerpt { - t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) - } -} - -func TestProcessBookmarkKeepExcerptEmpty(t *testing.T) { - bookmark := model.Bookmark{ - ID: 1, - URL: "https://example.com", - Title: "", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString("

This is an example article

") - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "text/html", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: false, - } - expected, _, _ := core.ProcessBookmark(request) - - if expected.ID != bookmark.ID { - t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) - } - if expected.URL != bookmark.URL { - t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) - } - if expected.Title != bookmark.URL { - t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) - } - if expected.Excerpt != bookmark.Excerpt { - t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) - } -} - -func TestProcessBookmarkIDZero(t *testing.T) { - bookmark := model.Bookmark{ - ID: 0, - URL: "https://example.com", - Title: "Example", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString("

This is an example article

") - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "text/html", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: true, - } - _, isFatal, err := core.ProcessBookmark(request) - assert.Error(t, err) - assert.Contains(t, err.Error(), "bookmark ID is not valid") - assert.True(t, isFatal) -} -func TestProcessBookmarkContentTypeNotTextHtml(t *testing.T) { - bookmark := model.Bookmark{ - ID: 1, - URL: "https://example.com", - Title: "Example", - Excerpt: "This is an example article", - CreateEbook: true, - CreateArchive: true, - } - content := bytes.NewBufferString("

This is an example article

") - request := core.ProcessRequest{ - Bookmark: bookmark, - Content: content, - ContentType: "application/pdf", - DataDir: "/tmp", - KeepTitle: true, - KeepExcerpt: true, - } - _, _, err := core.ProcessBookmark(request) - assert.NoError(t, err) + t.Run("ProcessRequest with sucssesful result", func(t *testing.T) { + t.Run("Normal without image", func(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.Title { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } + }) + t.Run("Normal with multipleimage", func(t *testing.T) { + + html := `html + + + + + + +

This is an example article

+ + ` + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString(html) + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.Title { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } + }) + t.Run("ProcessRequest sucssesful with multipleimage included favicon and Thumbnail ", func(t *testing.T) { + // create a file server handler for the 'testdata' directory + fs := http.FileServer(http.Dir("../../testdata/")) + + // start a test server with the file server handler + server := httptest.NewServer(fs) + defer server.Close() + + html := `html + + + + + + +

This is an example article

+ + ` + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString(html) + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.Title { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } + }) + t.Run("ProcessRequest sucssesful with empty title ", func(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.URL { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } + }) + t.Run("ProcessRequest sucssesful with empty Excerpt", func(t *testing.T) { + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: false, + } + expected, _, _ := core.ProcessBookmark(request) + + if expected.ID != bookmark.ID { + t.Errorf("Unexpected ID: got %v, want %v", expected.ID, bookmark.ID) + } + if expected.URL != bookmark.URL { + t.Errorf("Unexpected URL: got %v, want %v", expected.URL, bookmark.URL) + } + if expected.Title != bookmark.URL { + t.Errorf("Unexpected Title: got %v, want %v", expected.Title, bookmark.Title) + } + if expected.Excerpt != bookmark.Excerpt { + t.Errorf("Unexpected Excerpt: got %v, want %v", expected.Excerpt, bookmark.Excerpt) + } + }) + t.Run("Specific case", func(t *testing.T) { + t.Run("ProcessRequest with ID zero", func(t *testing.T) { + + bookmark := model.Bookmark{ + ID: 0, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "text/html", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + _, isFatal, err := core.ProcessBookmark(request) + assert.Error(t, err) + assert.Contains(t, err.Error(), "bookmark ID is not valid") + assert.True(t, isFatal) + }) + + t.Run("ProcessRequest that content type not zero", func(t *testing.T) { + + bookmark := model.Bookmark{ + ID: 1, + URL: "https://example.com", + Title: "Example", + Excerpt: "This is an example article", + CreateEbook: true, + CreateArchive: true, + } + content := bytes.NewBufferString("

This is an example article

") + request := core.ProcessRequest{ + Bookmark: bookmark, + Content: content, + ContentType: "application/pdf", + DataDir: "/tmp", + KeepTitle: true, + KeepExcerpt: true, + } + _, _, err := core.ProcessBookmark(request) + assert.NoError(t, err) + }) + }) + }) } From bf953215580eb4e765ca536b9d7b656e4b8f4f8a Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 15:56:12 +0330 Subject: [PATCH 32/36] more readable unit test for ebooks --- internal/core/ebook_test.go | 335 ++++++++++++++++++------------------ 1 file changed, 169 insertions(+), 166 deletions(-) diff --git a/internal/core/ebook_test.go b/internal/core/ebook_test.go index 46f8dd69c..fd8fd316b 100644 --- a/internal/core/ebook_test.go +++ b/internal/core/ebook_test.go @@ -11,172 +11,175 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGenerateEbook_ValidBookmarkID_ReturnsBookmarkWithHasEbookTrue(t *testing.T) { - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstDir := t.TempDir() - defer os.RemoveAll(dstDir) - - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 1, - Title: "Example Bookmark", - HTML: "Example HTML", - HasEbook: false, - }, - DataDir: dstDir, - ContentType: "text/html", - } - - bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) - - assert.True(t, bookmark.HasEbook) - assert.NoError(t, err) -} - -func TestGenerateEbook_InvalidBookmarkID_ReturnsError(t *testing.T) { - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 0, - HasEbook: false, - }, - DataDir: tempDir, - ContentType: "text/html", - } - - bookmark, err := core.GenerateEbook(mockRequest, tempDir) - - assert.Equal(t, model.Bookmark{ - ID: 0, - HasEbook: false, - }, bookmark) - assert.Error(t, err) -} - -func TestGenerateEbook_ValidBookmarkID_EbookExist_EbookExist_ReturnWithHasEbookTrue(t *testing.T) { - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstDir := t.TempDir() - defer os.RemoveAll(dstDir) - - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 1, - HasEbook: false, - }, - DataDir: dstDir, - ContentType: "text/html", - } - // Create the ebook directory - ebookDir := fp.Join(mockRequest.DataDir, "ebook") - err := os.MkdirAll(ebookDir, os.ModePerm) - if err != nil { - t.Fatal(err) - } - // Create the ebook file - ebookfile := fp.Join(mockRequest.DataDir, "ebook", fmt.Sprintf("%d.epub", mockRequest.Bookmark.ID)) - file, err := os.Create(ebookfile) - if err != nil { - t.Fatal(err) - } - defer file.Close() - - bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) - - assert.True(t, bookmark.HasEbook) - assert.NoError(t, err) -} - -func TestGenerateEbook_ValidBookmarkID_EbookExist_ImagePathExist_ReturnWithHasEbookTrue(t *testing.T) { - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstDir := t.TempDir() - defer os.RemoveAll(dstDir) - - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 1, - HasEbook: false, - }, - DataDir: dstDir, - ContentType: "text/html", - } - // Create the image directory - imageDir := fp.Join(mockRequest.DataDir, "thumb") - err := os.MkdirAll(imageDir, os.ModePerm) - if err != nil { - t.Fatal(err) - } - // Create the image file - imagePath := fp.Join(mockRequest.DataDir, "thumb", fmt.Sprintf("%d", mockRequest.Bookmark.ID)) - file, err := os.Create(imagePath) - if err != nil { - t.Fatal(err) - } - defer file.Close() - - bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) - expectedimagePath := "/bookmark/1/thumb" - if expectedimagePath != bookmark.ImageURL { - t.Errorf("Expected imageURL %s, but got %s", bookmark.ImageURL, expectedimagePath) - } - assert.True(t, bookmark.HasEbook) - assert.NoError(t, err) -} - -func TestGenerateEbook_ValidBookmarkID_EbookExist_ReturnWithHasArchiveTrue(t *testing.T) { - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - dstDir := t.TempDir() - defer os.RemoveAll(dstDir) - - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 1, - HasEbook: false, - }, - DataDir: dstDir, - ContentType: "text/html", - } - // Create the archive directory - archiveDir := fp.Join(mockRequest.DataDir, "archive") - err := os.MkdirAll(archiveDir, os.ModePerm) - if err != nil { - t.Fatal(err) - } - // Create the archive file - archivePath := fp.Join(mockRequest.DataDir, "archive", fmt.Sprintf("%d", mockRequest.Bookmark.ID)) - file, err := os.Create(archivePath) - if err != nil { - t.Fatal(err) - } - defer file.Close() - - bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) - assert.True(t, bookmark.HasArchive) - assert.NoError(t, err) -} - -func TestGenerateEbook_ValidBookmarkID_RetuenError_PDF(t *testing.T) { - tempDir := t.TempDir() - defer os.RemoveAll(tempDir) - - mockRequest := core.ProcessRequest{ - Bookmark: model.Bookmark{ - ID: 1, - HasEbook: false, - }, - DataDir: tempDir, - ContentType: "application/pdf", - } - - bookmark, err := core.GenerateEbook(mockRequest, tempDir) - - assert.False(t, bookmark.HasEbook) - assert.Error(t, err) - assert.Contains(t, err.Error(), "can't create ebook for pdf") +func TestGenerateEbook(t *testing.T) { + t.Run("Successful ebook generate", func(t *testing.T) { + t.Run("valid bookmarkId that return HasEbook true", func(t *testing.T) { + // test cae + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) + + mockRequest := core.ProcessRequest{ + Bookmark: model.Bookmark{ + ID: 1, + Title: "Example Bookmark", + HTML: "Example HTML", + HasEbook: false, + }, + DataDir: dstDir, + ContentType: "text/html", + } + + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) + + assert.True(t, bookmark.HasEbook) + assert.NoError(t, err) + }) + t.Run("ebook generate with valid BookmarkID EbookExist ImagePathExist ReturnWithHasEbookTrue", func(t *testing.T) { + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) + + mockRequest := core.ProcessRequest{ + Bookmark: model.Bookmark{ + ID: 1, + HasEbook: false, + }, + DataDir: dstDir, + ContentType: "text/html", + } + // Create the image directory + imageDir := fp.Join(mockRequest.DataDir, "thumb") + err := os.MkdirAll(imageDir, os.ModePerm) + if err != nil { + t.Fatal(err) + } + // Create the image file + imagePath := fp.Join(mockRequest.DataDir, "thumb", fmt.Sprintf("%d", mockRequest.Bookmark.ID)) + file, err := os.Create(imagePath) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) + expectedimagePath := "/bookmark/1/thumb" + if expectedimagePath != bookmark.ImageURL { + t.Errorf("Expected imageURL %s, but got %s", bookmark.ImageURL, expectedimagePath) + } + assert.True(t, bookmark.HasEbook) + assert.NoError(t, err) + }) + t.Run("generate ebook valid BookmarkID EbookExist Returnh HasArchive True", func(t *testing.T) { + + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) + + mockRequest := core.ProcessRequest{ + Bookmark: model.Bookmark{ + ID: 1, + HasEbook: false, + }, + DataDir: dstDir, + ContentType: "text/html", + } + // Create the archive directory + archiveDir := fp.Join(mockRequest.DataDir, "archive") + err := os.MkdirAll(archiveDir, os.ModePerm) + if err != nil { + t.Fatal(err) + } + // Create the archive file + archivePath := fp.Join(mockRequest.DataDir, "archive", fmt.Sprintf("%d", mockRequest.Bookmark.ID)) + file, err := os.Create(archivePath) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) + assert.True(t, bookmark.HasArchive) + assert.NoError(t, err) + }) + }) + t.Run("specific ebook generate case", func(t *testing.T) { + t.Run("unvalid bookmarkId that return Error", func(t *testing.T) { + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + mockRequest := core.ProcessRequest{ + Bookmark: model.Bookmark{ + ID: 0, + HasEbook: false, + }, + DataDir: tempDir, + ContentType: "text/html", + } + + bookmark, err := core.GenerateEbook(mockRequest, tempDir) + + assert.Equal(t, model.Bookmark{ + ID: 0, + HasEbook: false, + }, bookmark) + assert.Error(t, err) + }) + t.Run("ebook exist return HasEbook true", func(t *testing.T) { + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + dstDir := t.TempDir() + defer os.RemoveAll(dstDir) + + mockRequest := core.ProcessRequest{ + Bookmark: model.Bookmark{ + ID: 1, + HasEbook: false, + }, + DataDir: dstDir, + ContentType: "text/html", + } + // Create the ebook directory + ebookDir := fp.Join(mockRequest.DataDir, "ebook") + err := os.MkdirAll(ebookDir, os.ModePerm) + if err != nil { + t.Fatal(err) + } + // Create the ebook file + ebookfile := fp.Join(mockRequest.DataDir, "ebook", fmt.Sprintf("%d.epub", mockRequest.Bookmark.ID)) + file, err := os.Create(ebookfile) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + bookmark, err := core.GenerateEbook(mockRequest, fp.Join(tempDir, "1")) + + assert.True(t, bookmark.HasEbook) + assert.NoError(t, err) + }) + t.Run("generate ebook valid BookmarkID RetuenError for PDF file", func(t *testing.T) { + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + + mockRequest := core.ProcessRequest{ + Bookmark: model.Bookmark{ + ID: 1, + HasEbook: false, + }, + DataDir: tempDir, + ContentType: "application/pdf", + } + + bookmark, err := core.GenerateEbook(mockRequest, tempDir) + + assert.False(t, bookmark.HasEbook) + assert.Error(t, err) + assert.Contains(t, err.Error(), "can't create ebook for pdf") + }) + }) } // Add more unit tests for other scenarios that missing specialy From 7f26e3fdb959ad5eb4065254b19a68a41919ac0e Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Mon, 14 Aug 2023 16:02:57 +0330 Subject: [PATCH 33/36] delete all defer os.RemoveAll from unit tests --- internal/core/ebook_test.go | 10 ---------- internal/core/processing_test.go | 3 --- 2 files changed, 13 deletions(-) diff --git a/internal/core/ebook_test.go b/internal/core/ebook_test.go index fd8fd316b..25b0eca0d 100644 --- a/internal/core/ebook_test.go +++ b/internal/core/ebook_test.go @@ -16,9 +16,7 @@ func TestGenerateEbook(t *testing.T) { t.Run("valid bookmarkId that return HasEbook true", func(t *testing.T) { // test cae tempDir := t.TempDir() - defer os.RemoveAll(tempDir) dstDir := t.TempDir() - defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ @@ -38,9 +36,7 @@ func TestGenerateEbook(t *testing.T) { }) t.Run("ebook generate with valid BookmarkID EbookExist ImagePathExist ReturnWithHasEbookTrue", func(t *testing.T) { tempDir := t.TempDir() - defer os.RemoveAll(tempDir) dstDir := t.TempDir() - defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ @@ -75,9 +71,7 @@ func TestGenerateEbook(t *testing.T) { t.Run("generate ebook valid BookmarkID EbookExist Returnh HasArchive True", func(t *testing.T) { tempDir := t.TempDir() - defer os.RemoveAll(tempDir) dstDir := t.TempDir() - defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ @@ -109,7 +103,6 @@ func TestGenerateEbook(t *testing.T) { t.Run("specific ebook generate case", func(t *testing.T) { t.Run("unvalid bookmarkId that return Error", func(t *testing.T) { tempDir := t.TempDir() - defer os.RemoveAll(tempDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ ID: 0, @@ -129,9 +122,7 @@ func TestGenerateEbook(t *testing.T) { }) t.Run("ebook exist return HasEbook true", func(t *testing.T) { tempDir := t.TempDir() - defer os.RemoveAll(tempDir) dstDir := t.TempDir() - defer os.RemoveAll(dstDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ @@ -162,7 +153,6 @@ func TestGenerateEbook(t *testing.T) { }) t.Run("generate ebook valid BookmarkID RetuenError for PDF file", func(t *testing.T) { tempDir := t.TempDir() - defer os.RemoveAll(tempDir) mockRequest := core.ProcessRequest{ Bookmark: model.Bookmark{ diff --git a/internal/core/processing_test.go b/internal/core/processing_test.go index d3e37cccb..11e07af3f 100644 --- a/internal/core/processing_test.go +++ b/internal/core/processing_test.go @@ -52,7 +52,6 @@ func TestDownloadBookImage(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() - defer os.RemoveAll(tempDir) dstPath := fp.Join(tempDir, "1") defer os.Remove(dstPath) @@ -67,7 +66,6 @@ func TestDownloadBookImage(t *testing.T) { // Arrange imageURL := "https://raw.githubusercontent.com/go-shiori/shiori/master/docs/readme/cover.png" tempDir := t.TempDir() - defer os.RemoveAll(tempDir) dstPath := fp.Join(tempDir, "1") defer os.Remove(dstPath) @@ -89,7 +87,6 @@ func TestDownloadBookImage(t *testing.T) { // Arrange imageURL := server.URL + "/medium_image.png" tempDir := t.TempDir() - defer os.RemoveAll(tempDir) dstPath := fp.Join(tempDir, "1") defer os.Remove(dstPath) From cdf6fab449abdf55d184a0ed5b1d7f080821e62e Mon Sep 17 00:00:00 2001 From: Monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 20 Aug 2023 15:28:06 +0330 Subject: [PATCH 34/36] Better comment Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com> --- internal/core/ebook.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/core/ebook.go b/internal/core/ebook.go index ff3b25425..6fd45464d 100644 --- a/internal/core/ebook.go +++ b/internal/core/ebook.go @@ -16,10 +16,9 @@ import ( "github.com/pkg/errors" ) -// this function get request and a destination path and will create an epub file in dstPath from that request -// dstPath should be incouded file name with '.epub' -// it will return a bookmark model and err -// bookmark model later use for update UI shiori based on this function be sucssesful or not +// GenerateEbook receives a `ProcessRequest` and generates an ebook file in the destination path specified. +// The destination path `dstPath` should include file name with ".epub" extension +// The bookmark model will be used to update the UI based on whether this function is successful or not. func GenerateEbook(req ProcessRequest, dstPath string) (book model.Bookmark, err error) { // variable for store generated html code var html string From 587340b16692d9d259fc90e8b99507313eb27374 Mon Sep 17 00:00:00 2001 From: Monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 20 Aug 2023 15:29:52 +0330 Subject: [PATCH 35/36] Better Error output Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com> --- internal/core/processing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index bcdcb7124..20675c261 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -124,7 +124,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, for i, imageURL := range imageURLs { err = DownloadBookImage(imageURL, imgPath) if err != nil && errors.Is(err, ErrNoSupportedImageType) { - log.Printf("Not found image for URL: %s", imageURL) + log.Printf("%s: %s", err.String(), imageURL) if i == len(imageURLs)-1 { os.Remove(imgPath) } From 3003099b9d4791cde17153efc97365763f3504c1 Mon Sep 17 00:00:00 2001 From: monirzadeh <25131576+Monirzadeh@users.noreply.github.com> Date: Sun, 20 Aug 2023 15:44:14 +0330 Subject: [PATCH 36/36] fix err.String() method --- internal/core/processing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/processing.go b/internal/core/processing.go index 20675c261..25df3d316 100644 --- a/internal/core/processing.go +++ b/internal/core/processing.go @@ -124,7 +124,7 @@ func ProcessBookmark(req ProcessRequest) (book model.Bookmark, isFatalErr bool, for i, imageURL := range imageURLs { err = DownloadBookImage(imageURL, imgPath) if err != nil && errors.Is(err, ErrNoSupportedImageType) { - log.Printf("%s: %s", err.String(), imageURL) + log.Printf("%s: %s", err, imageURL) if i == len(imageURLs)-1 { os.Remove(imgPath) }