From 4ea24f07d12cf02a2a984d4767c4820d872bde8b Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sat, 12 Jul 2025 12:34:17 +0200 Subject: [PATCH] Refactor file routes and update API endpoints for consistency --- server/internal/app/routes.go | 16 +- server/internal/handlers/file_handlers.go | 141 +++++++++--------- .../file_handlers_integration_test.go | 46 +++--- 3 files changed, 100 insertions(+), 103 deletions(-) diff --git a/server/internal/app/routes.go b/server/internal/app/routes.go index 1dd17da..6bb2946 100644 --- a/server/internal/app/routes.go +++ b/server/internal/app/routes.go @@ -129,16 +129,16 @@ func setupRouter(o Options) *chi.Mux { // File routes r.Route("/files", func(r chi.Router) { r.Get("/", handler.ListFiles()) - r.Get("/_op/last", handler.GetLastOpenedFile()) - r.Put("/_op/last", handler.UpdateLastOpenedFile()) - r.Get("/_op/lookup", handler.LookupFileByName()) + r.Get("/last", handler.GetLastOpenedFile()) + r.Put("/last", handler.UpdateLastOpenedFile()) + r.Get("/lookup", handler.LookupFileByName()) - r.Post("/_op/upload/*", handler.UploadFile()) - r.Put("/_op/move", handler.MoveFile()) + r.Post("/upload", handler.UploadFile()) + r.Put("/move", handler.MoveFile()) - r.Post("/*", handler.SaveFile()) - r.Get("/*", handler.GetFileContent()) - r.Delete("/*", handler.DeleteFile()) + r.Post("/", handler.SaveFile()) + r.Get("/content", handler.GetFileContent()) + r.Delete("/", handler.DeleteFile()) }) // Git routes diff --git a/server/internal/handlers/file_handlers.go b/server/internal/handlers/file_handlers.go index ee419a5..16e3613 100644 --- a/server/internal/handlers/file_handlers.go +++ b/server/internal/handlers/file_handlers.go @@ -1,7 +1,6 @@ package handlers import ( - "encoding/json" "io" "net/http" "net/url" @@ -11,8 +10,6 @@ import ( "lemma/internal/context" "lemma/internal/logging" "lemma/internal/storage" - - "github.com/go-chi/chi/v5" ) // LookupResponse represents a response to a file lookup request @@ -32,11 +29,6 @@ type LastOpenedFileResponse struct { LastOpenedFilePath string `json:"lastOpenedFilePath"` } -// UpdateLastOpenedFileRequest represents a request to update the last opened file -type UpdateLastOpenedFileRequest struct { - FilePath string `json:"filePath"` -} - func getFilesLogger() logging.Logger { return getHandlersLogger().WithGroup("files") } @@ -90,7 +82,7 @@ func (h *Handler) ListFiles() http.HandlerFunc { // @Success 200 {object} LookupResponse // @Failure 400 {object} ErrorResponse "Filename is required" // @Failure 404 {object} ErrorResponse "File not found" -// @Router /workspaces/{workspace_name}/files/_op/lookup [get] +// @Router /workspaces/{workspace_name}/files/lookup [get] func (h *Handler) LookupFileByName() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -150,13 +142,13 @@ func (h *Handler) LookupFileByName() http.HandlerFunc { // @Security CookieAuth // @Produce plain // @Param workspace_name path string true "Workspace name" -// @Param file_path path string true "File path" +// @Param file_path query string true "File path" // @Success 200 {string} string "Raw file content" // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 404 {object} ErrorResponse "File not found" // @Failure 500 {object} ErrorResponse "Failed to read file" // @Failure 500 {object} ErrorResponse "Failed to write response" -// @Router /workspaces/{workspace_name}/files/{file_path} [get] +// @Router /workspaces/{workspace_name}/files/content [get] func (h *Handler) GetFileContent() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -170,8 +162,7 @@ func (h *Handler) GetFileContent() http.HandlerFunc { "clientIP", r.RemoteAddr, ) - filePath := chi.URLParam(r, "*") - // URL-decode the file path + filePath := r.URL.Query().Get("file_path") decodedPath, err := url.PathUnescape(filePath) if err != nil { log.Error("failed to decode file path", @@ -231,12 +222,12 @@ func (h *Handler) GetFileContent() http.HandlerFunc { // @Accept plain // @Produce json // @Param workspace_name path string true "Workspace name" -// @Param file_path path string true "File path" +// @Param file_path query string true "File path" // @Success 200 {object} SaveFileResponse // @Failure 400 {object} ErrorResponse "Failed to read request body" // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 500 {object} ErrorResponse "Failed to save file" -// @Router /workspaces/{workspace_name}/files/{file_path} [post] +// @Router /workspaces/{workspace_name}/files/ [post] func (h *Handler) SaveFile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -250,7 +241,7 @@ func (h *Handler) SaveFile() http.HandlerFunc { "clientIP", r.RemoteAddr, ) - filePath := chi.URLParam(r, "*") + filePath := r.URL.Query().Get("file_path") // URL-decode the file path decodedPath, err := url.PathUnescape(filePath) if err != nil { @@ -311,14 +302,14 @@ func (h *Handler) SaveFile() http.HandlerFunc { // @Accept multipart/form-data // @Produce json // @Param workspace_name path string true "Workspace name" -// @Param path path string true "Directory path" +// @Param file_path query string true "Directory path" // @Param file formData file true "File to upload" // @Success 200 {object} SaveFileResponse // @Failure 400 {object} ErrorResponse "Failed to get file from form" // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 500 {object} ErrorResponse "Failed to read uploaded file" // @Failure 500 {object} ErrorResponse "Failed to save file" -// @Router /workspaces/{workspace_name}/files/_op/upload/{path} [post] +// @Router /workspaces/{workspace_name}/files/upload/ [post] func (h *Handler) UploadFile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -348,33 +339,40 @@ func (h *Handler) UploadFile() http.HandlerFunc { } }() - decodedPath, err := url.PathUnescape(chi.URLParam(r, "*")) + filePath := r.URL.Query().Get("file_path") + if filePath == "" { + log.Debug("missing file_path parameter") + respondError(w, "file_path is required", http.StatusBadRequest) + return + } + + decodedPath, err := url.PathUnescape(filePath) if err != nil { log.Error("failed to decode file path", - "filePath", decodedPath, + "filePath", filePath, "error", err.Error(), ) respondError(w, "Invalid file path", http.StatusBadRequest) return } - filePath := decodedPath + "/" + header.Filename + decodedPath = decodedPath + "/" + header.Filename content := make([]byte, header.Size) _, err = file.Read(content) if err != nil && err != io.EOF { log.Error("failed to read uploaded file", - "filePath", filePath, + "filePath", decodedPath, "error", err.Error(), ) respondError(w, "Failed to read uploaded file", http.StatusInternalServerError) return } - err = h.Storage.SaveFile(ctx.UserID, ctx.Workspace.ID, filePath, content) + err = h.Storage.SaveFile(ctx.UserID, ctx.Workspace.ID, decodedPath, content) if err != nil { if storage.IsPathValidationError(err) { log.Error("invalid file path attempted", - "filePath", filePath, + "filePath", decodedPath, "error", err.Error(), ) respondError(w, "Invalid file path", http.StatusBadRequest) @@ -382,7 +380,7 @@ func (h *Handler) UploadFile() http.HandlerFunc { } log.Error("failed to save file", - "filePath", filePath, + "filePath", decodedPath, "contentSize", len(content), "error", err.Error(), ) @@ -391,7 +389,7 @@ func (h *Handler) UploadFile() http.HandlerFunc { } response := SaveFileResponse{ - FilePath: filePath, + FilePath: decodedPath, Size: int64(len(content)), UpdatedAt: time.Now().UTC(), } @@ -412,7 +410,7 @@ func (h *Handler) UploadFile() http.HandlerFunc { // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 404 {object} ErrorResponse "File not found" // @Failure 500 {object} ErrorResponse "Failed to move file" -// @Router /workspaces/{workspace_name}/files/_op/move [post] +// @Router /workspaces/{workspace_name}/files/move [post] func (h *Handler) MoveFile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -498,12 +496,12 @@ func (h *Handler) MoveFile() http.HandlerFunc { // @ID deleteFile // @Security CookieAuth // @Param workspace_name path string true "Workspace name" -// @Param file_path path string true "File path" +// @Param file_path query string true "File path" // @Success 204 "No Content - File deleted successfully" // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 404 {object} ErrorResponse "File not found" // @Failure 500 {object} ErrorResponse "Failed to delete file" -// @Router /workspaces/{workspace_name}/files/{file_path} [delete] +// @Router /workspaces/{workspace_name}/files/ [delete] func (h *Handler) DeleteFile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -517,7 +515,13 @@ func (h *Handler) DeleteFile() http.HandlerFunc { "clientIP", r.RemoteAddr, ) - filePath := chi.URLParam(r, "*") + filePath := r.URL.Query().Get("file_path") + if filePath == "" { + log.Debug("missing file_path parameter") + respondError(w, "file_path is required", http.StatusBadRequest) + return + } + // URL-decode the file path decodedPath, err := url.PathUnescape(filePath) if err != nil { @@ -571,7 +575,7 @@ func (h *Handler) DeleteFile() http.HandlerFunc { // @Success 200 {object} LastOpenedFileResponse // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 500 {object} ErrorResponse "Failed to get last opened file" -// @Router /workspaces/{workspace_name}/files/_op/last [get] +// @Router /workspaces/{workspace_name}/files/last [get] func (h *Handler) GetLastOpenedFile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -616,13 +620,13 @@ func (h *Handler) GetLastOpenedFile() http.HandlerFunc { // @Accept json // @Produce json // @Param workspace_name path string true "Workspace name" -// @Param body body UpdateLastOpenedFileRequest true "Update last opened file request" +// @Param file_path query string true "File path" // @Success 204 "No Content - Last opened file updated successfully" // @Failure 400 {object} ErrorResponse "Invalid request body" // @Failure 400 {object} ErrorResponse "Invalid file path" // @Failure 404 {object} ErrorResponse "File not found" // @Failure 500 {object} ErrorResponse "Failed to update file" -// @Router /workspaces/{workspace_name}/files/_op/last [put] +// @Router /workspaces/{workspace_name}/files/last [put] func (h *Handler) UpdateLastOpenedFile() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx, ok := context.GetRequestContext(w, r) @@ -636,60 +640,53 @@ func (h *Handler) UpdateLastOpenedFile() http.HandlerFunc { "clientIP", r.RemoteAddr, ) - var requestBody UpdateLastOpenedFileRequest - if err := json.NewDecoder(r.Body).Decode(&requestBody); err != nil { - log.Error("failed to decode request body", - "error", err.Error(), - ) - respondError(w, "Invalid request body", http.StatusBadRequest) + filePath := r.URL.Query().Get("file_path") + if filePath == "" { + log.Debug("missing file_path parameter") + respondError(w, "file_path is required", http.StatusBadRequest) return } - // Validate the file path in the workspace - if requestBody.FilePath != "" { - // URL-decode the file path - decodedPath, err := url.PathUnescape(requestBody.FilePath) - if err != nil { - log.Error("failed to decode file path", - "filePath", requestBody.FilePath, + decodedPath, err := url.PathUnescape(filePath) + if err != nil { + log.Error("failed to decode file path", + "filePath", filePath, + "error", err.Error(), + ) + respondError(w, "Invalid file path", http.StatusBadRequest) + return + } + + _, err = h.Storage.GetFileContent(ctx.UserID, ctx.Workspace.ID, decodedPath) + if err != nil { + if storage.IsPathValidationError(err) { + log.Error("invalid file path attempted", + "filePath", decodedPath, "error", err.Error(), ) respondError(w, "Invalid file path", http.StatusBadRequest) return } - requestBody.FilePath = decodedPath - _, err = h.Storage.GetFileContent(ctx.UserID, ctx.Workspace.ID, requestBody.FilePath) - if err != nil { - if storage.IsPathValidationError(err) { - log.Error("invalid file path attempted", - "filePath", requestBody.FilePath, - "error", err.Error(), - ) - respondError(w, "Invalid file path", http.StatusBadRequest) - return - } - - if os.IsNotExist(err) { - log.Debug("file not found", - "filePath", requestBody.FilePath, - ) - respondError(w, "File not found", http.StatusNotFound) - return - } - - log.Error("failed to validate file path", - "filePath", requestBody.FilePath, - "error", err.Error(), + if os.IsNotExist(err) { + log.Debug("file not found", + "filePath", decodedPath, ) - respondError(w, "Failed to update last opened file", http.StatusInternalServerError) + respondError(w, "File not found", http.StatusNotFound) return } + + log.Error("failed to validate file path", + "filePath", decodedPath, + "error", err.Error(), + ) + respondError(w, "Failed to update last opened file", http.StatusInternalServerError) + return } - if err := h.DB.UpdateLastOpenedFile(ctx.Workspace.ID, requestBody.FilePath); err != nil { + if err := h.DB.UpdateLastOpenedFile(ctx.Workspace.ID, decodedPath); err != nil { log.Error("failed to update last opened file in database", - "filePath", requestBody.FilePath, + "filePath", decodedPath, "error", err.Error(), ) respondError(w, "Failed to update last opened file", http.StatusInternalServerError) diff --git a/server/internal/handlers/file_handlers_integration_test.go b/server/internal/handlers/file_handlers_integration_test.go index edef78b..3d64e40 100644 --- a/server/internal/handlers/file_handlers_integration_test.go +++ b/server/internal/handlers/file_handlers_integration_test.go @@ -55,11 +55,11 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { filePath := "test.md" // Save file - rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularTestUser) + rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"?file_path="+url.QueryEscape(filePath), strings.NewReader(content), h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Get file content - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/content?file_path="+url.QueryEscape(filePath), nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, content, rr.Body.String()) @@ -84,7 +84,7 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { // Create all files for path, content := range files { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularTestUser) + rr := h.makeRequest(t, http.MethodPost, baseURL+"?file_path="+url.QueryEscape(path), content, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) } @@ -120,11 +120,11 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { // Look up a file that exists in multiple locations filename := "readme.md" dupContent := "Another readme" - rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularTestUser) + rr := h.makeRequest(t, http.MethodPost, baseURL+"?file_path="+url.QueryEscape("projects/"+filename), dupContent, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Search for the file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/_op/lookup?filename="+filename, nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+url.QueryEscape(filename), nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -135,7 +135,7 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { assert.Len(t, response.Paths, 2) // Search for non-existent file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/_op/lookup?filename=nonexistent.md", nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+url.QueryEscape("nonexistent.md"), nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -144,21 +144,21 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { content := "This file will be deleted" // Create file - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularTestUser) + rr := h.makeRequest(t, http.MethodPost, baseURL+"?file_path="+url.QueryEscape(filePath), content, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Delete file - rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodDelete, baseURL+"?file_path="+url.QueryEscape(filePath), nil, h.RegularTestUser) require.Equal(t, http.StatusNoContent, rr.Code) // Verify file is gone - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/content?file_path="+url.QueryEscape(filePath), nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("last opened file", func(t *testing.T) { // Initially should be empty - rr := h.makeRequest(t, http.MethodGet, baseURL+"/_op/last", nil, h.RegularTestUser) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -174,11 +174,11 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { }{ FilePath: "docs/readme.md", } - rr = h.makeRequest(t, http.MethodPut, baseURL+"/_op/last", updateReq, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last?file_path="+url.QueryEscape(updateReq.FilePath), nil, h.RegularTestUser) require.Equal(t, http.StatusNoContent, rr.Code) // Verify update - rr = h.makeRequest(t, http.MethodGet, baseURL+"/_op/last", nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(&response) @@ -187,7 +187,7 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { // Test invalid file path updateReq.FilePath = "nonexistent.md" - rr = h.makeRequest(t, http.MethodPut, baseURL+"/_op/last", updateReq, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last?file_path="+url.QueryEscape(updateReq.FilePath), nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -199,11 +199,11 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { body any }{ {"list files", http.MethodGet, baseURL, nil}, - {"get file", http.MethodGet, baseURL + "/test.md", nil}, - {"save file", http.MethodPost, baseURL + "/test.md", "content"}, - {"delete file", http.MethodDelete, baseURL + "/test.md", nil}, - {"get last file", http.MethodGet, baseURL + "/_op/last", nil}, - {"update last file", http.MethodPut, baseURL + "/_op/last", struct{ FilePath string }{"test.md"}}, + {"get file", http.MethodGet, baseURL + "/content?file_path=" + url.QueryEscape("test.md"), nil}, + {"save file", http.MethodPost, baseURL + "?file_path=" + url.QueryEscape("test.md"), "content"}, + {"delete file", http.MethodDelete, baseURL + "?file_path=" + url.QueryEscape("test.md"), nil}, + {"get last file", http.MethodGet, baseURL + "/last", nil}, + {"update last file", http.MethodPut, baseURL + "/last?file_path=" + url.QueryEscape("test.md"), nil}, } for _, tc := range tests { @@ -230,11 +230,11 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { for _, path := range maliciousPaths { t.Run(path, func(t *testing.T) { // Try to read - rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularTestUser) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/content?file_path="+url.QueryEscape(path), nil, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) // Try to write - rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularTestUser) + rr = h.makeRequest(t, http.MethodPost, baseURL+"?file_path="+url.QueryEscape(path), "malicious content", h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) } @@ -245,7 +245,7 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { fileName := "uploaded-test.txt" fileContent := "This is an uploaded file" - rr := h.makeUploadRequest(t, baseURL+"/_op/upload/uploads", fileName, fileContent, h.RegularTestUser) + rr := h.makeUploadRequest(t, baseURL+"/upload?file_path="+url.QueryEscape("uploads"), fileName, fileContent, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Verify response structure @@ -260,13 +260,13 @@ func testFileHandlers(t *testing.T, dbConfig DatabaseConfig) { assert.Equal(t, int64(len(fileContent)), response.Size) // Verify file was saved - rr = h.makeRequest(t, http.MethodGet, baseURL+"/uploads/"+fileName, nil, h.RegularTestUser) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/content?file_path="+url.QueryEscape("uploads/"+fileName), nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, fileContent, rr.Body.String()) }) t.Run("upload without file", func(t *testing.T) { - rr := h.makeUploadRequest(t, baseURL+"/_op/upload/test", "", "", h.RegularTestUser) + rr := h.makeUploadRequest(t, baseURL+"/upload?file_path="+url.QueryEscape("test"), "", "", h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) })