From 349a07e68c7ebfc31240cc4572ffe062c25af537 Mon Sep 17 00:00:00 2001 From: Swarit Pandey Date: Sun, 29 Sep 2024 09:50:18 +0000 Subject: [PATCH] chore: refactor server code Signed-off-by: Swarit Pandey --- config/config.go | 2 +- internal/db/commands.go | 16 --- internal/db/dicedb.go | 99 ----------------- internal/handlers/handlers.go | 73 +++++++++++++ internal/middleware/ratelimiter.go | 11 +- internal/repository/repository.go | 33 ++++++ internal/repository/request.go | 14 +++ internal/server/http.go | 123 ++++++++++++++++++++++ internal/server/httpServer.go | 96 ----------------- internal/service/cli.go | 53 ++++++++++ internal/service/health.go | 3 + internal/service/search.go | 3 + internal/service/types.go | 57 ++++++++++ main.go | 29 ++--- pkg/{util/helpers.go => common/common.go} | 12 +-- 15 files changed, 378 insertions(+), 246 deletions(-) delete mode 100644 internal/db/commands.go delete mode 100644 internal/db/dicedb.go create mode 100644 internal/handlers/handlers.go create mode 100644 internal/repository/repository.go create mode 100644 internal/repository/request.go create mode 100644 internal/server/http.go delete mode 100644 internal/server/httpServer.go create mode 100644 internal/service/cli.go create mode 100644 internal/service/health.go create mode 100644 internal/service/search.go create mode 100644 internal/service/types.go rename pkg/{util/helpers.go => common/common.go} (68%) diff --git a/config/config.go b/config/config.go index a9991de..0bc5630 100644 --- a/config/config.go +++ b/config/config.go @@ -14,7 +14,7 @@ type Config struct { } // LoadConfig loads the application configuration from environment variables or defaults -func LoadConfig() *Config { +func Load() *Config { return &Config{ DiceAddr: getEnv("DICE_ADDR", "localhost:7379"), // Default Dice address ServerPort: getEnv("SERVER_PORT", ":8080"), // Default server port diff --git a/internal/db/commands.go b/internal/db/commands.go deleted file mode 100644 index f78b616..0000000 --- a/internal/db/commands.go +++ /dev/null @@ -1,16 +0,0 @@ -package db - -func (db *DiceDB) getKey(key string) (string, error) { - val, err := db.Client.Get(db.Ctx, key).Result() - return val, err -} - -func (db *DiceDB) setKey(key, value string) error { - err := db.Client.Set(db.Ctx, key, value, 0).Err() - return err -} - -func (db *DiceDB) deleteKeys(keys []string) error { - err := db.Client.Del(db.Ctx, keys...).Err() - return err -} diff --git a/internal/db/dicedb.go b/internal/db/dicedb.go deleted file mode 100644 index c38776f..0000000 --- a/internal/db/dicedb.go +++ /dev/null @@ -1,99 +0,0 @@ -/* -this will be the DiceDB client -*/ - -package db - -import ( - "context" - "errors" - "fmt" - "log/slog" - "os" - "server/config" - "server/internal/cmds" - "time" - - dice "github.com/dicedb/go-dice" -) - -type DiceDB struct { - Client *dice.Client - Ctx context.Context -} - -func (db *DiceDB) CloseDiceDB() { - err := db.Client.Close() - if err != nil { - slog.Error("error closing DiceDB connection", - slog.Any("error", err)) - os.Exit(1) - } -} - -func InitDiceClient(configValue *config.Config) (*DiceDB, error) { - diceClient := dice.NewClient(&dice.Options{ - Addr: configValue.DiceAddr, - DialTimeout: 10 * time.Second, - MaxRetries: 10, - }) - - // Ping the dice client to verify the connection - err := diceClient.Ping(context.Background()).Err() - if err != nil { - return nil, err - } - - return &DiceDB{ - Client: diceClient, - Ctx: context.Background(), - }, nil -} - -func errorResponse(response string) map[string]string { - return map[string]string{"error": response} -} - -// ExecuteCommand executes a command based on the input -func (db *DiceDB) ExecuteCommand(command *cmds.CommandRequest) interface{} { - switch command.Cmd { - case "get": - if command.Args.Key == "" { - return errorResponse("key is required") - } - - val, err := db.getKey(command.Args.Key) - switch { - case errors.Is(err, dice.Nil): - return errorResponse("key does not exist") - case err != nil: - return errorResponse(fmt.Sprintf("Get failed %v", err)) - } - - return map[string]string{"value": val} - - case "set": - if command.Args.Key == "" || command.Args.Value == "" { - return errorResponse("key and value are required") - } - err := db.setKey(command.Args.Key, command.Args.Value) - if err != nil { - return errorResponse("failed to set key") - } - return map[string]string{"result": "OK"} - - case "del": - if len(command.Args.Keys) == 0 { - return errorResponse("at least one key is required") - } - err := db.deleteKeys(command.Args.Keys) - if err != nil { - return errorResponse("failed to delete keys") - } - - return map[string]string{"result": "OK"} - - default: - return errorResponse("unknown command") - } -} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go new file mode 100644 index 0000000..e9746f0 --- /dev/null +++ b/internal/handlers/handlers.go @@ -0,0 +1,73 @@ +package handlers + +import ( + "net/http" + "server/internal/service" + "server/pkg/common" +) + +const ( + OpGET = "get" + OpSET = "set" + OpDEL = "delete" +) + +type Handler interface { + CLIHandler(w http.ResponseWriter, r *http.Request) + Health(w http.ResponseWriter, r *http.Request) + Search(w http.ResponseWriter, r *http.Request) +} + +type handler struct { + service service.Service +} + +func NewHandler(service service.Service) Handler { + return &handler{service: service} +} + +func (h *handler) CLIHandler(w http.ResponseWriter, r *http.Request) { + command, err := common.ParseHTTPRequest(r) + if err != nil { + common.JSONResponse(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) + } + + var result any + switch command.Cmd { + + case OpGET: + result, err = h.service.Get(r.Context(), &service.GetRequest{Key: command.Args.Key}) + + case OpSET: + result, err = h.service.Set(r.Context(), &service.SetRequest{Key: command.Args.Key, Value: command.Args.Value}) + + case OpDEL: + var keys []string + if command.Args.Key != "" { + keys = []string{command.Args.Key} + } else if len(command.Args.Keys) > 0 { + keys = command.Args.Keys + } else { + common.JSONResponse(w, http.StatusBadRequest, map[string]string{"error": "at least one key is required"}) + return + } + result, err = h.service.Delete(r.Context(), &service.DeleteRequest{Keys: keys}) + } + + if err != nil { + common.JSONResponse(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + + common.JSONResponse(w, http.StatusOK, result) +} + +func (h *handler) Health(w http.ResponseWriter, r *http.Request) { + // TODO: Call service layer for Health check + common.JSONResponse(w, http.StatusOK, map[string]string{"message": "Server is running"}) +} + +func (h *handler) Search(w http.ResponseWriter, r *http.Request) { + // TODO: Call service layer for search over keys or whatever + common.JSONResponse(w, http.StatusOK, map[string]string{"message": "Results..."}) +} diff --git a/internal/middleware/ratelimiter.go b/internal/middleware/ratelimiter.go index c81f60e..dae36aa 100644 --- a/internal/middleware/ratelimiter.go +++ b/internal/middleware/ratelimiter.go @@ -6,7 +6,6 @@ import ( "fmt" "log/slog" "net/http" - "server/internal/db" "strconv" "strings" "time" @@ -14,8 +13,10 @@ import ( dice "github.com/dicedb/go-dice" ) +// Middleware should not depend on db structs + // RateLimiter middleware to limit requests based on a specified limit and duration -func RateLimiter(client *db.DiceDB, next http.Handler, limit, window int) http.Handler { +func RateLimiter(client *dice.Client, next http.Handler, limit, window int) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() @@ -32,7 +33,7 @@ func RateLimiter(client *db.DiceDB, next http.Handler, limit, window int) http.H slog.Info("Created rate limiter key", slog.Any("key", key)) // Fetch the current request count - val, err := client.Client.Get(ctx, key).Result() + val, err := client.Get(ctx, key).Result() if err != nil && !errors.Is(err, dice.Nil) { slog.Error("Error fetching request count", "error", err) http.Error(w, "Internal Server Error", http.StatusInternalServerError) @@ -58,7 +59,7 @@ func RateLimiter(client *db.DiceDB, next http.Handler, limit, window int) http.H } // Increment the request count - if _, err := client.Client.Incr(ctx, key).Result(); err != nil { + if _, err := client.Incr(ctx, key).Result(); err != nil { slog.Error("Error incrementing request count", "error", err) http.Error(w, "Internal Server Error", http.StatusInternalServerError) return @@ -66,7 +67,7 @@ func RateLimiter(client *db.DiceDB, next http.Handler, limit, window int) http.H // Set the key expiry if it's newly created if requestCount == 0 { - if err := client.Client.Expire(ctx, key, time.Duration(window)*time.Second).Err(); err != nil { + if err := client.Expire(ctx, key, time.Duration(window)*time.Second).Err(); err != nil { slog.Error("Error setting expiry for request count", "error", err) } } diff --git a/internal/repository/repository.go b/internal/repository/repository.go new file mode 100644 index 0000000..c2a01d0 --- /dev/null +++ b/internal/repository/repository.go @@ -0,0 +1,33 @@ +package repository + +import ( + "context" + + dice "github.com/dicedb/go-dice" +) + +type Repository interface { + Get(ctx context.Context, req *GetRequest) (string, error) + Set(ctx context.Context, req *SetRequest) error + Delete(ctx context.Context, req *DeleteRequest) error +} + +type repository struct { + client *dice.Client +} + +func NewRepository(client *dice.Client) Repository { + return &repository{client: client} +} + +func (r *repository) Get(ctx context.Context, req *GetRequest) (string, error) { + return r.client.Get(ctx, req.Key).Result() +} + +func (r *repository) Set(ctx context.Context, req *SetRequest) error { + return r.client.Set(ctx, req.Key, req.Value, 0).Err() +} + +func (r *repository) Delete(ctx context.Context, req *DeleteRequest) error { + return r.client.Del(ctx, req.Keys...).Err() +} diff --git a/internal/repository/request.go b/internal/repository/request.go new file mode 100644 index 0000000..e2f0aec --- /dev/null +++ b/internal/repository/request.go @@ -0,0 +1,14 @@ +package repository + +type GetRequest struct { + Key string +} + +type SetRequest struct { + Key string + Value string +} + +type DeleteRequest struct { + Keys []string +} diff --git a/internal/server/http.go b/internal/server/http.go new file mode 100644 index 0000000..b1d04d3 --- /dev/null +++ b/internal/server/http.go @@ -0,0 +1,123 @@ +package server + +import ( + "context" + "fmt" + "log" + "net/http" + "os" + "os/signal" + "strings" + "syscall" + "time" + + "server/config" + "server/internal/handlers" + "server/internal/middleware" + "server/internal/repository" + "server/internal/service" + + dice "github.com/dicedb/go-dice" +) + +type Server struct { + httpServer *http.Server + config *config.Config +} + +func NewServer(config *config.Config) *Server { + return &Server{ + config: config, + } +} + +func (s *Server) Run() error { + // Step1: Initialize DiceDB client + diceClient, err := initDiceClient(s.config) + if err != nil { + return fmt.Errorf("failed to initialize DiceDB client: %w", err) + } + + // Step2: Initialize repository + repo := repository.NewRepository(diceClient) + + // Step3: Initialize service + svc := service.NewService(repo) + + // Step4: Initialize handler + handler := handlers.NewHandler(svc) + + // TODO: We should ideally move this to routes package + // Step5: Set up routes + mux := http.NewServeMux() + mux.HandleFunc("/health", handler.Health) + mux.HandleFunc("/cli/{cmd}", handler.CLIHandler) + mux.HandleFunc("/search", handler.Search) + + handlerMux := &HandlerMux{ + mux: mux, + rateLimiter: func(w http.ResponseWriter, r *http.Request, next http.Handler) { + middleware.RateLimiter(diceClient, next, s.config.RequestLimit, s.config.RequestWindow).ServeHTTP(w, r) + }, + } + + // Create HTTP server + s.httpServer = &http.Server{ + Addr: s.config.ServerPort, + Handler: handlerMux, + } + + // Start server + go func() { + log.Printf("Starting server on %s", s.config.ServerPort) + if err := s.httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Fatalf("Server failed to start: %v", err) + } + }() + + quit := make(chan os.Signal, 1) + signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) + <-quit + log.Println("Shutting down server...") + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + // Doesn't block if no connections, but will otherwise wait until the timeout deadline + if err := s.httpServer.Shutdown(ctx); err != nil { + return fmt.Errorf("server forced to shutdown: %w", err) + } + + log.Println("Server exited properly") + return nil +} + +// HandlerMux wraps ServeMux and forces REST paths to lowercase +// and attaches a rate limiter with the handler +type HandlerMux struct { + mux *http.ServeMux + rateLimiter func(http.ResponseWriter, *http.Request, http.Handler) +} + +func (hm *HandlerMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Convert the path to lowercase before passing to the underlying mux. + r.URL.Path = strings.ToLower(r.URL.Path) + hm.rateLimiter(w, r, hm.mux) +} + +// initDiceClient initializes dice client +func initDiceClient(config *config.Config) (*dice.Client, error) { + diceClient := dice.NewClient(&dice.Options{ + Addr: config.DiceAddr, + DialTimeout: 10 * time.Second, + MaxRetries: 10, + }) + + // Ping the dice client to verify the connection + err := diceClient.Ping(context.Background()).Err() + if err != nil { + return nil, err + } + + return diceClient, nil +} diff --git a/internal/server/httpServer.go b/internal/server/httpServer.go deleted file mode 100644 index be713f8..0000000 --- a/internal/server/httpServer.go +++ /dev/null @@ -1,96 +0,0 @@ -package server - -import ( - "context" - "errors" - "log" - "net/http" - "server/internal/middleware" - "strings" - "sync" - "time" - - "server/internal/db" - util "server/pkg/util" -) - -type HTTPServer struct { - httpServer *http.Server - DiceClient *db.DiceDB -} - -// HandlerMux wraps ServeMux and forces REST paths to lowercase -// and attaches a rate limiter with the handler -type HandlerMux struct { - mux *http.ServeMux - rateLimiter func(http.ResponseWriter, *http.Request, http.Handler) -} - -func (cim *HandlerMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Convert the path to lowercase before passing to the underlying mux. - r.URL.Path = strings.ToLower(r.URL.Path) - // Apply rate limiter - cim.rateLimiter(w, r, cim.mux) -} - -func NewHTTPServer(addr string, mux *http.ServeMux, client *db.DiceDB, limit, window int) *HTTPServer { - handlerMux := &HandlerMux{ - mux: mux, - rateLimiter: func(w http.ResponseWriter, r *http.Request, next http.Handler) { - middleware.RateLimiter(client, next, limit, window).ServeHTTP(w, r) - }, - } - - return &HTTPServer{ - httpServer: &http.Server{ - Addr: addr, - Handler: handlerMux, - ReadHeaderTimeout: 5 * time.Second, - }, - DiceClient: client, - } -} - -func (s *HTTPServer) Run(ctx context.Context) error { - var wg sync.WaitGroup - - wg.Add(1) - go func() { - defer wg.Done() - log.Printf("Starting server at %s\n", s.httpServer.Addr) - if err := s.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { - log.Fatalf("HTTP server error: %v", err) - } - }() - - <-ctx.Done() - log.Println("Shutting down server...") - return s.Shutdown() -} - -func (s *HTTPServer) Shutdown() error { - if err := s.DiceClient.Client.Close(); err != nil { - log.Printf("Failed to close dice client: %v", err) - } - - return s.httpServer.Shutdown(context.Background()) -} - -func (s *HTTPServer) HealthCheck(w http.ResponseWriter, request *http.Request) { - util.JSONResponse(w, http.StatusOK, map[string]string{"message": "Server is running"}) -} - -func (s *HTTPServer) CliHandler(w http.ResponseWriter, r *http.Request) { - diceCmds, err := util.ParseHTTPRequest(r) - if err != nil { - http.Error(w, "Error parsing HTTP request", http.StatusBadRequest) - return - } - - resp := s.DiceClient.ExecuteCommand(diceCmds) - util.JSONResponse(w, http.StatusOK, resp) -} - -func (s *HTTPServer) SearchHandler(w http.ResponseWriter, request *http.Request) { - util.JSONResponse(w, http.StatusOK, map[string]string{"message": "Search results"}) -} diff --git a/internal/service/cli.go b/internal/service/cli.go new file mode 100644 index 0000000..66bae7d --- /dev/null +++ b/internal/service/cli.go @@ -0,0 +1,53 @@ +package service + +import ( + "context" + "errors" + "server/internal/repository" +) + +func (s *service) Get(ctx context.Context, req *GetRequest) (*GetResponse, error) { + if req.Key == "" { + return &GetResponse{}, errors.New("key is required") + } + + value, err := s.repo.Get(ctx, &repository.GetRequest{ + Key: req.Key, + }) + if err != nil { + return &GetResponse{}, err + } + + return &GetResponse{Value: value}, nil +} + +func (s *service) Set(ctx context.Context, req *SetRequest) (*SetResponse, error) { + if req.Key == "" || req.Value == "" { + return nil, errors.New("key and value are required") + } + + err := s.repo.Set(ctx, &repository.SetRequest{ + Key: req.Key, + Value: req.Value, + }) + if err != nil { + return nil, err + } + + return &SetResponse{Success: "OK"}, nil +} + +func (s *service) Delete(ctx context.Context, req *DeleteRequest) (*DeleteResponse, error) { + if len(req.Keys) == 0 { + return nil, errors.New("at least one key is required") + } + + err := s.repo.Delete(ctx, &repository.DeleteRequest{ + Keys: req.Keys, + }) + if err != nil { + return nil, err + } + + return &DeleteResponse{Success: "OK"}, nil +} diff --git a/internal/service/health.go b/internal/service/health.go new file mode 100644 index 0000000..fe1e29a --- /dev/null +++ b/internal/service/health.go @@ -0,0 +1,3 @@ +package service + +// TODO: Implement me diff --git a/internal/service/search.go b/internal/service/search.go new file mode 100644 index 0000000..fe1e29a --- /dev/null +++ b/internal/service/search.go @@ -0,0 +1,3 @@ +package service + +// TODO: Implement me diff --git a/internal/service/types.go b/internal/service/types.go new file mode 100644 index 0000000..e6b28e6 --- /dev/null +++ b/internal/service/types.go @@ -0,0 +1,57 @@ +package service + +import ( + "context" + "server/internal/repository" +) + +// Service is service layer client interface that should have all the methods +// exposed to handlers +type Service interface { + Get(ctx context.Context, req *GetRequest) (*GetResponse, error) + Set(ctx context.Context, req *SetRequest) (*SetResponse, error) + Delete(ctx context.Context, req *DeleteRequest) (*DeleteResponse, error) +} + +// service is private struct and concrete implementation that implements the +// Service client level interface +type service struct { + repo repository.Repository +} + +// NewService instantiates the service layer it takes Repository layer's client +// interface as dependency +func NewService(repo repository.Repository) Service { + return &service{repo: repo} +} + +// Service layer response and request models for GET + +type GetRequest struct { + Key string +} + +type GetResponse struct { + Value string +} + +// Service layer response and request models for SET + +type SetRequest struct { + Key string + Value string +} + +type SetResponse struct { + Success string +} + +// Service layer response and request models for DELETE + +type DeleteRequest struct { + Keys []string +} + +type DeleteResponse struct { + Success string +} diff --git a/main.go b/main.go index 09a082f..bf7c752 100644 --- a/main.go +++ b/main.go @@ -1,34 +1,17 @@ package main import ( - "context" "log" - "net/http" "server/config" - "server/internal/db" - "server/internal/server" // Import the new package for HTTPServer + "server/internal/server" ) func main() { - configValue := config.LoadConfig() - diceClient, err := db.InitDiceClient(configValue) - if err != nil { - log.Fatalf("Failed to initialize dice client: %v", err) - } - - // Create mux and register routes - mux := http.NewServeMux() - httpServer := server.NewHTTPServer(":8080", mux, diceClient, configValue.RequestLimit, configValue.RequestWindow) - mux.HandleFunc("/health", httpServer.HealthCheck) - mux.HandleFunc("/cli/{cmd}", httpServer.CliHandler) - mux.HandleFunc("/search", httpServer.SearchHandler) - - // Graceful shutdown context - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + configValue := config.Load() + srv := server.NewServer(configValue) - // Run the HTTP Server - if err := httpServer.Run(ctx); err != nil { - log.Printf("Server failed: %v\n", err) + if err := srv.Run(); err != nil { + // Crash now + log.Fatalf("Server error: %v", err) } } diff --git a/pkg/util/helpers.go b/pkg/common/common.go similarity index 68% rename from pkg/util/helpers.go rename to pkg/common/common.go index 2c09262..4767796 100644 --- a/pkg/util/helpers.go +++ b/pkg/common/common.go @@ -1,10 +1,10 @@ -package helpers +package common import ( "encoding/json" "errors" + "fmt" "io" - "log" "net/http" "server/internal/cmds" ) @@ -12,18 +12,18 @@ import ( func ParseHTTPRequest(r *http.Request) (*cmds.CommandRequest, error) { command := r.PathValue("cmd") if command == "" { - return nil, errors.New("invalid command") + return &cmds.CommandRequest{}, errors.New("invalid command") } body, err := io.ReadAll(r.Body) if err != nil { - log.Fatalf("error reading body: %v", err) + return &cmds.CommandRequest{}, fmt.Errorf("failed to read request: %w", err) } var commandRequestArgs *cmds.CommandRequestArgs err = json.Unmarshal(body, &commandRequestArgs) if err != nil { - log.Fatalf("error unmarshalling body: %v", err) + return &cmds.CommandRequest{}, fmt.Errorf("failed to unmarshal: %w", err) } return &cmds.CommandRequest{ @@ -32,7 +32,7 @@ func ParseHTTPRequest(r *http.Request) (*cmds.CommandRequest, error) { }, nil } -func JSONResponse(w http.ResponseWriter, status int, data interface{}) { +func JSONResponse(w http.ResponseWriter, status int, data any) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) if err := json.NewEncoder(w).Encode(data); err != nil {