Merge pull request #23 from Beebeeoii/hotfix-v1.1.4

Hotfix v1.1.4

### Changelog:
- [Bugfix] App crashes when attempting to sync files in folders with restricted/reserved characters
- [Optimisation] Removed GET_FILES mode in DocumentRequest
- [Optimisation] Renamed GET_FOLDERS mode in DocumentRequest to GET_ALL_FOLDERS
- [Optimisation] GetAllFiles now returns files in the current folder only.
- [Optimisation] Code cleanup for GetRootFiles

FyneApp.toml

@@ -4,5 +4,5 @@ Website = "https://github.com/beebeeoii/lominus"
   Icon = "./assets/app-icon.png"
   Name = "Lominus"
   ID = "com.beebeeoii.lominus"
-  Version = "1.1.3"
-  Build = 60
+  Version = "1.1.4"
+  Build = 77

README.md

@@ -1,4 +1,4 @@
-![GitHub Lominus version](https://img.shields.io/badge/Lominus-v1.1.3-blueviolet)
+![GitHub Lominus version](https://img.shields.io/badge/Lominus-v1.1.4-blueviolet)
 ![GitHub go.mod Go version](https://img.shields.io/github/go-mod/go-version/beebeeoii/lominus)
 [![Go Reference](https://pkg.go.dev/badge/github.com/beebeeoii/lominus.svg)](https://pkg.go.dev/github.com/beebeeoii/lominus)
 # Table of Contents

internal/cron/cron.go

@@ -3,13 +3,13 @@ package cron
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 	"time"
 
 	appApp "github.com/beebeeoii/lominus/internal/app"
 	intTelegram "github.com/beebeeoii/lominus/internal/app/integrations/telegram"
 	appPref "github.com/beebeeoii/lominus/internal/app/pref"
+	files "github.com/beebeeoii/lominus/internal/file"
 	"github.com/beebeeoii/lominus/internal/indexing"
 	logs "github.com/beebeeoii/lominus/internal/log"
 	"github.com/beebeeoii/lominus/internal/notifications"
@@ -120,7 +120,7 @@ func createJob(frequency int) (*gocron.Job, error) {
 					continue
 				}
 
-				files, fileErr := fileRequest.GetAllFiles()
+				files, fileErr := fileRequest.GetRootFiles()
 				if fileErr != nil {
 					notifications.NotificationChannel <- notifications.Notification{Title: "Sync", Content: "Unable to retrieve files"}
 					logs.WarningLogger.Println(fileErr)
@@ -221,7 +221,7 @@ func createJob(frequency int) (*gocron.Job, error) {
 // directory based on the File's Ancestors.
 func downloadFile(baseDir string, file api.File) error {
 	fileDirSlice := append([]string{baseDir}, file.Ancestors...)
-	ensureDir(filepath.Join(append(fileDirSlice, file.Name)...))
+	files.EnsureDir(filepath.Join(append(fileDirSlice, file.Name)...))
 
 	downloadReq, dlReqErr := api.BuildDocumentRequest(file, api.DOWNLOAD_FILE)
 	if dlReqErr != nil {
@@ -230,16 +230,3 @@ func downloadFile(baseDir string, file api.File) error {
 
 	return downloadReq.Download(filepath.Join(fileDirSlice...))
 }
-
-// ensureDir is a helper function that ensures that the directory exists by creating them
-// if they do not already exist.
-func ensureDir(dir string) {
-	dirName := filepath.Dir(dir)
-	if _, serr := os.Stat(dirName); serr != nil {
-		merr := os.MkdirAll(dirName, os.ModePerm)
-		if merr != nil {
-			logs.ErrorLogger.Println(merr)
-			panic(merr)
-		}
-	}
-}

internal/file/file.go

@@ -5,6 +5,10 @@ import (
 	"encoding/gob"
 	"fmt"
 	"os"
+	"path/filepath"
+	"strings"
+
+	logs "github.com/beebeeoii/lominus/internal/log"
 )
 
 // EncodeStructToFile takes in any struct and encodes it into a file specified by fileName.
@@ -46,6 +50,41 @@ func Exists(name string) bool {
 	return err == nil
 }
 
+// EnsureDir is a helper function that ensures that the directory exists by creating them
+// if they do not already exist.
+func EnsureDir(dir string) {
+	dirName := filepath.Dir(dir)
+	if _, serr := os.Stat(dirName); serr != nil {
+		merr := os.MkdirAll(dirName, os.ModePerm)
+		if merr != nil {
+			logs.ErrorLogger.Println(merr)
+			panic(merr)
+		}
+	}
+}
+
+// CleanseFolderFileName is a helper function that ensures folders' and files' names are valid,
+// that they do not contain prohibited characters. However, some are still not caught for
+// unlikeliness and simplicity reasons.
+// The following are reserved file names for Windows that are uncaught:
+// CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, LPT9.
+// The following are non-printable characters that are uncaught:
+// ASCII 0-31.
+func CleanseFolderFileName(name string) string {
+	name = strings.Replace(name, "/", " ", -1)
+	name = strings.Replace(name, "\\", " ", -1)
+	name = strings.Replace(name, "<", " ", -1)
+	name = strings.Replace(name, ">", " ", -1)
+	name = strings.Replace(name, ":", " ", -1)
+	name = strings.Replace(name, "\"", " ", -1)
+	name = strings.Replace(name, "|", " ", -1)
+	name = strings.Replace(name, "?", " ", -1)
+	name = strings.Replace(name, "*", " ", -1)
+	name = strings.TrimSpace(name)
+
+	return name
+}
+
 // FileNotFoundError struct is an error struct that contains the custom error that will be thrown when file is not found.
 type FileNotFoundError struct {
 	FileName string

internal/lominus/lominus.go

@@ -3,7 +3,7 @@ package lominus
 
 const APP_NAME = "Lominus"
 const APP_ID = "com.lominus.beebeeoii"
-const APP_VERSION = "1.1.3"
+const APP_VERSION = "1.1.4"
 
 const LOCK_FILE_NAME = "lominus.lock"
 

pkg/api/files.go

@@ -4,12 +4,12 @@ package api
 import (
 	"errors"
 	"io"
-	"log"
 	"net/http"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
+
+	"github.com/beebeeoii/lominus/internal/file"
 )
 
 // Folder struct is the datapack for containing details about a Folder
@@ -38,81 +38,89 @@ const FILE_URL_ENDPOINT = "https://luminus.nus.edu.sg/v2/api/files/%s/file?popul
 const DOWNLOAD_URL_ENDPOINT = "https://luminus.nus.edu.sg/v2/api/files/file/%s/downloadurl"
 
 // GetAllFolders returns a slice of Folder objects from a DocumentRequest.
-// Ensure DocumentRequest mode is GET_FOLDERS (0).
+// It will only return folders in the current folder.
+// Nested folders will not be returned.
+// Ensure that DocumentRequest mode is GET_ALL_FOLDERS (0).
 // Find out more about DocumentRequests under request.go.
 func (req DocumentRequest) GetAllFolders() ([]Folder, error) {
-	folder := []Folder{}
-	if req.Mode != GET_FOLDERS {
-		return folder, errors.New("mode mismatched: ensure DocumentRequest mode is GET_FOLDERS (0)")
+	folders := []Folder{}
+	if req.Mode != GET_ALL_FOLDERS {
+		return folders, errors.New("mode mismatched: ensure DocumentRequest mode is GET_ALL_FOLDERS (0)")
 	}
 
 	rawResponse := RawResponse{}
 	err := req.Request.GetRawResponse(&rawResponse)
 	if err != nil {
-		return folder, err
+		return folders, err
 	}
 
 	for _, content := range rawResponse.Data {
 		if _, exists := content["access"]; exists { // only folder that can be accessed will be placed in folders slice
 			newFolder := Folder{
 				Id:           content["id"].(string),
-				Name:         content["name"].(string),
+				Name:         file.CleanseFolderFileName(content["name"].(string)),
 				Downloadable: content["isActive"].(bool) && !content["allowUpload"].(bool), // downloadable = active folder + does not allow uploads
 				HasSubFolder: int(content["subFolderCount"].(float64)) > 0,
-				Ancestors:    []string{strings.TrimSpace(req.Module.ModuleCode)},
+				Ancestors:    append(req.Folder.Ancestors, req.Folder.Name),
 			}
-			folder = append(folder, newFolder)
+			folders = append(folders, newFolder)
 		}
 	}
-	return folder, nil
+	return folders, nil
 }
 
-// Deprecated - build DocumentRequest with a Folder instead of a module instead, and call getRootFiles() directly.
-// GetAllFiles returns a slice of File objects that are in a Folder using a DocumentRequest.
-// Ensure DocumentRequest mode is GET_ALL_FILES (1).
+// GetAllFiles returns a slice of File objects that are in a Folder from a DocumentRequest.
+// It will only return files in the current folder.
+// To return nested files, use GetRootFiles() instead.
+// Ensure that DocumentRequest mode is GET_ALL_FILES (1).
 // Find out more about DocumentRequests under request.go.
 func (req DocumentRequest) GetAllFiles() ([]File, error) {
 	files := []File{}
 	if req.Mode != GET_ALL_FILES {
 		return files, errors.New("mode mismatched: ensure DocumentRequest mode is GET_ALL_FILES (1)")
 	}
 
-	rootFilesReq, rootFilesBuildErr := BuildDocumentRequest(Folder{
-		Id:           req.Module.Id,
-		Name:         req.Module.ModuleCode,
-		Downloadable: true,
-		Ancestors:    []string{strings.TrimSpace(req.Module.ModuleCode)},
-		HasSubFolder: true,
-	}, GET_FILES)
-	if rootFilesBuildErr != nil {
-		return files, rootFilesBuildErr
-	}
-
-	baseFiles, err := rootFilesReq.getRootFiles()
-	log.Println(baseFiles)
+	rawResponse := RawResponse{}
+	err := req.Request.GetRawResponse(&rawResponse)
 	if err != nil {
 		return files, err
 	}
-	files = append(files, baseFiles...)
+
+	for _, content := range rawResponse.Data {
+		lastUpdated, timeParseErr := time.Parse(time.RFC3339, content["lastUpdatedDate"].(string))
+
+		if timeParseErr != nil {
+			return files, timeParseErr
+		}
+
+		file := File{
+			Id:          content["id"].(string),
+			Name:        file.CleanseFolderFileName(content["name"].(string)),
+			LastUpdated: lastUpdated,
+			Ancestors:   append(req.Folder.Ancestors, req.Folder.Name),
+		}
+		files = append(files, file)
+	}
 
 	return files, nil
 }
 
-// getRootFiles returns a slice of File objects and nested File objects that are in a Folder or nested Folder from a DocumentRequest.
-// Ensure DocumentRequest mode is GET_FILES (3).
+// GetRootFiles returns a slice of File objects and nested File objects that are in a Folder from a DocumentRequest.
+// It will traverse all nested folders and return all nested files.
+// Ensure that DocumentRequest mode is GET_ALL_FILES (1).
 // Find out more about DocumentRequests under request.go.
-func (req DocumentRequest) getRootFiles() ([]File, error) {
+func (req DocumentRequest) GetRootFiles() ([]File, error) {
 	files := []File{}
-	if req.Mode != GET_FILES {
-		return files, errors.New("mode mismatched: ensure DocumentRequest mode is GET_FILES (3)")
+	if req.Mode != GET_ALL_FILES {
+		return files, errors.New("mode mismatched: ensure DocumentRequest mode is GET_ALL_FILES (1)")
 	}
 
 	if !req.Folder.Downloadable {
 		return files, nil
 	}
 
 	if req.Folder.HasSubFolder {
-		subFolderReq, subFolderReqBuildErr := BuildDocumentRequest(req.Folder, GET_FOLDERS)
+		subFolderReq, subFolderReqBuildErr := BuildDocumentRequest(req.Folder, GET_ALL_FOLDERS)
 		if subFolderReqBuildErr != nil {
 			return files, subFolderReqBuildErr
 		}
@@ -123,14 +131,12 @@ func (req DocumentRequest) getRootFiles() ([]File, error) {
 		}
 
 		for _, subFolder := range subFolders {
-			subFolder.Ancestors = append(subFolder.Ancestors, req.Folder.Ancestors...)
-			subFolder.Ancestors = append(subFolder.Ancestors, strings.TrimSpace(subFolder.Name))
-			rootFilesReq, rootFilesBuildErr := BuildDocumentRequest(subFolder, GET_FILES)
+			rootFilesReq, rootFilesBuildErr := BuildDocumentRequest(subFolder, GET_ALL_FILES)
 			if rootFilesBuildErr != nil {
 				return files, rootFilesBuildErr
 			}
 
-			subFiles, err := rootFilesReq.getRootFiles()
+			subFiles, err := rootFilesReq.GetRootFiles()
 			if err != nil {
 				return files, err
 			}
@@ -139,32 +145,18 @@ func (req DocumentRequest) getRootFiles() ([]File, error) {
 		}
 	}
 
-	rawResponse := RawResponse{}
-	err := req.Request.GetRawResponse(&rawResponse)
+	subFiles, err := req.GetAllFiles()
 	if err != nil {
 		return files, err
 	}
 
-	for _, content := range rawResponse.Data {
-		lastUpdated, timeParseErr := time.Parse(time.RFC3339, content["lastUpdatedDate"].(string))
-
-		if timeParseErr != nil {
-			return files, timeParseErr
-		}
-		newFile := File{
-			Id:          content["id"].(string),
-			Name:        content["name"].(string),
-			LastUpdated: lastUpdated,
-			Ancestors:   req.Folder.Ancestors,
-		}
-		files = append(files, newFile)
-	}
+	files = append(files, subFiles...)
 
 	return files, nil
 }
 
-// Download downloads the specified file in a DocumentRequest into local storage.
-// Ensure DocumentRequest mode is DOWNLOAD_FILE (2).
+// Download downloads the specified File in a DocumentRequest into local storage.
+// Ensure that DocumentRequest mode is DOWNLOAD_FILE (2).
 // Find out more about DocumentRequests under request.go.
 func (req DocumentRequest) Download(filePath string) error {
 	if req.Mode != DOWNLOAD_FILE {

pkg/api/request.go

@@ -37,10 +37,9 @@ type ModuleRequest struct {
 }
 
 const (
-	GET_FOLDERS   = 0
-	GET_ALL_FILES = 1
-	DOWNLOAD_FILE = 2
-	GET_FILES     = 3
+	GET_ALL_FOLDERS = 0
+	GET_ALL_FILES   = 1
+	DOWNLOAD_FILE   = 2
 )
 
 const USER_AGENT = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:94.0) Gecko/20100101 Firefox/94.0"
@@ -101,26 +100,40 @@ func BuildDocumentRequest(builder interface{}, mode int) (DocumentRequest, error
 	var urlEndpoint string
 
 	switch mode {
-	case GET_FOLDERS:
+	case GET_ALL_FOLDERS:
+		_, isModule := builder.(Module)
+		_, isFolder := builder.(Folder)
+		if !isModule && !isFolder {
+			return DocumentRequest{}, errors.New("invalid mode: DocumentRequest must be built using Module or Folder to have mode=GET_ALL_FOLDERS")
+		}
 		urlEndpoint = FOLDER_URL_ENDPOINT
 	case GET_ALL_FILES:
-		urlEndpoint = FOLDER_URL_ENDPOINT
+		_, isModule := builder.(Module)
+		_, isFolder := builder.(Folder)
+		if !isModule && !isFolder {
+			return DocumentRequest{}, errors.New("invalid mode: DocumentRequest must be built using Module or Folder to have mode=GET_ALL_FILES")
+		}
+		urlEndpoint = FILE_URL_ENDPOINT
 	case DOWNLOAD_FILE:
 		_, isFile := builder.(File)
 		if !isFile {
-			return DocumentRequest{}, errors.New("invalid arguments: DocumentRequest must be built using File to download")
+			return DocumentRequest{}, errors.New("invalid mode: DocumentRequest must be built using File to download")
 		}
 		urlEndpoint = DOWNLOAD_URL_ENDPOINT
-	case GET_FILES:
-		urlEndpoint = FILE_URL_ENDPOINT
 	default:
-		return DocumentRequest{}, errors.New("invalid arguments: mode provided is not a valid mode")
+		return DocumentRequest{}, errors.New("invalid mode: mode provided is invalid. Valid modes are GET_ALL_FOLDERS (0), GET_ALL_FILES (1), DOWNLOAD_FILE (2)")
 	}
 
 	switch builder := builder.(type) {
 	case Module:
 		return DocumentRequest{
-			Module: builder,
+			Folder: Folder{
+				Id:           builder.Id,
+				Name:         builder.ModuleCode,
+				Downloadable: true,
+				Ancestors:    []string{},
+				HasSubFolder: true,
+			},
 			Request: Request{
 				Url:       fmt.Sprintf(urlEndpoint, builder.Id),
 				JwtToken:  jwtToken,
@@ -149,7 +162,7 @@ func BuildDocumentRequest(builder interface{}, mode int) (DocumentRequest, error
 			Mode: mode,
 		}, nil
 	default:
-		return DocumentRequest{}, errors.New("invalid arguments: DocumentRequest must be built using Module or Folder only")
+		return DocumentRequest{}, errors.New("invalid builder: DocumentRequest must be built using Module, Folder or File only")
 	}
 }