Fixed database migration, imporved logging, imporved hashing function,

introduced connection pooling instead of single conncetion.
2025-08-07 20:45:18 +02:00
parent e178613607
commit f27f1fc3d6
7 changed files with 211 additions and 85 deletions
--- a/internal/shortener/handler.go
+++ b/internal/shortener/handler.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 	"sync"
 	"time"

@@ -12,8 +13,11 @@ import (
 	"github.com/ferdzo/ferurl/internal/db"
 	"github.com/ferdzo/ferurl/utils"
 	"github.com/go-chi/chi/v5"
+	"github.com/sirupsen/logrus"
 )

+var log = logrus.New()
+
 type Service struct {
 	cache    *cache.Cache
 	database *db.Database
@@ -25,12 +29,17 @@ type Handler struct {
 }

 func NewService(redisConfig utils.RedisConfig, databaseConfig utils.DatabaseConfig) (*Service, error) {
+	log.Info("Initializing Redis client")
 	redisClient, err := cache.NewRedisClient(redisConfig)
 	if err != nil {
+		log.WithError(err).Error("Failed to initialize Redis client")
 		return nil, err
 	}
+
+	log.Info("Initializing Database client")
 	databaseClient, err := db.NewDatabaseClient(databaseConfig)
 	if err != nil {
+		log.WithError(err).Error("Failed to initialize Database client")
 		return nil, err
 	}

@@ -63,28 +72,34 @@ func (h *Handler) CreateShortURL(w http.ResponseWriter, r *http.Request) {

 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&input); err != nil {
+		log.WithError(err).Warn("Invalid JSON input received")
 		http.Error(w, "Invalid JSON input", http.StatusBadRequest)
 		return
 	}

 	longUrl := input.URL
 	if longUrl == "" {
+		log.Warn("Empty URL received")
 		http.Error(w, "Long URL is required", http.StatusBadRequest)
 		return
 	}
 	if !utils.IsValidUrl(longUrl) {
+		log.WithField("url", longUrl).Warn("Invalid URL format received")
 		http.Error(w, "Invalid URL", http.StatusBadRequest)
 		return
 	}
 	expiresAt := input.ExpiresAt

 	shortUrl := generateShortUrl(longUrl)
-	fetchedShortUrl, _ := h.fetchUrl(shortUrl)
-	if fetchedShortUrl != "" {
+	fetchedShortUrl, err := h.fetchUrl(shortUrl)
+	if err == nil && fetchedShortUrl != "" {
 		fullShortUrl := fmt.Sprintf("%s%s", h.baseURL, shortUrl)
 		json.NewEncoder(w).Encode(map[string]string{"short_url": fullShortUrl})
 		return
 	}
+	if err != nil && isNotFoundError(err) {
+		log.WithField("short_url", shortUrl).Debug("Creating new short URL")
+	}

 	newUrl := db.URL{
 		URL:       longUrl,
@@ -93,7 +108,7 @@ func (h *Handler) CreateShortURL(w http.ResponseWriter, r *http.Request) {
 	}

 	if err := h.service.storeUrl(newUrl); err != nil {
-		fmt.Println(err)
+		log.Error("Failed to store URL", "error", err)
 		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
 		return
 	}
@@ -113,19 +128,28 @@ func (h *Handler) GetUrl(w http.ResponseWriter, r *http.Request) {
 	}
 	shortKey := chi.URLParam(r, "key")
 	if shortKey == "" {
+		log.Warn("Empty short URL key received")
 		http.Error(w, "ID is required", http.StatusBadRequest)
 		return
 	}

 	if !utils.IsValidShortUrl(shortKey) {
+		log.WithField("key", shortKey).Warn("Invalid short URL format received")
 		http.Error(w, "Invalid short URL", http.StatusBadRequest)
 		return
 	}

 	longUrl, err := h.fetchUrl(shortKey)
 	if err != nil {
-		fmt.Println(err)
-		http.Error(w, "URL not found", http.StatusInternalServerError)
+		if isNotFoundError(err) {
+			http.Error(w, "URL not found", http.StatusNotFound)
+		} else {
+			log.WithFields(logrus.Fields{
+				"short_url": shortKey,
+				"error":     err.Error(),
+			}).Error("Failed to fetch URL")
+			http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		}
 		return
 	}

@@ -137,12 +161,16 @@ func (h *Handler) GetUrl(w http.ResponseWriter, r *http.Request) {
 		CreatedAt:  time.Now(),
 	}

-	err = h.service.database.InsertAnalytics(pv)
-	if err != nil {
-		fmt.Println(err)
-		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
-		return
-	}
+	go func(pageVisit db.PageVisit) {
+		defer func() {
+			if r := recover(); r != nil {
+				log.WithField("error", r).Error("Panic in analytics insertion")
+			}
+		}()
+		if err := h.service.database.InsertAnalytics(pageVisit); err != nil {
+			logrus.Error("Failed to store analytics", "error", err)
+		}
+	}(pv)

 	http.Redirect(w, r, longUrl, http.StatusSeeOther)
 }
@@ -161,9 +189,13 @@ func (h *Handler) fetchUrl(id string) (string, error) {
 	}
 	resultChan := make(chan result, 2)

-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
+	var wg sync.WaitGroup
+	wg.Add(2)
+
 	go func() {
+		defer wg.Done()
 		url, err := h.service.fetchUrlFromCache(id)
 		select {
 		case resultChan <- result{url, err, "cache"}:
@@ -173,6 +205,7 @@ func (h *Handler) fetchUrl(id string) (string, error) {
 	}()

 	go func() {
+		defer wg.Done()
 		url, err := h.service.fetchUrlFromDatabase(id)
 		select {
 		case resultChan <- result{url, err, "db"}:
@@ -181,65 +214,118 @@ func (h *Handler) fetchUrl(id string) (string, error) {
 		}
 	}()

+	go func() {
+		wg.Wait()
+		close(resultChan)
+	}()
+
 	var foundInDB string
 	var cacheHit bool
 	var lastErr error

 	for range 2 {
-		res := <-resultChan
-		if res.err == nil {
-			if res.source == "db" {
-				foundInDB = res.url
-				if !cacheHit {
-					continue
-				}
-			} else if res.source == "cache" {
-				cacheHit = true
-				if foundInDB != "" {
-					return res.url, nil
-				}
+		select {
+		case res, ok := <-resultChan:
+			if !ok {
+				break
 			}
-
-			return res.url, nil
+			if res.err == nil {
+				if res.source == "db" {
+					foundInDB = res.url
+					if !cacheHit {
+						continue
+					}
+				} else if res.source == "cache" {
+					cacheHit = true
+					if foundInDB != "" {
+						return res.url, nil
+					}
+				}
+				return res.url, nil
+			}
+			lastErr = res.err
+		case <-ctx.Done():
+			return "", fmt.Errorf("timeout while fetching URL: %w", ctx.Err())
 		}
-		lastErr = res.err
 	}

 	if foundInDB != "" {
-		go func() {
-			_ = h.service.cache.Set(id, foundInDB)
-			fmt.Println("URL updated in Redis")
-		}()
+		go func(cacheKey, url string) {
+			defer func() {
+				if r := recover(); r != nil {
+					log.Error("Panic in cache update", "error", r)
+				}
+			}()
+			if err := h.service.cache.Set(cacheKey, url); err != nil {
+				log.Error("Failed to update URL in cache", "error", err)
+			}
+		}(id, foundInDB)
+
 		return foundInDB, nil
 	}

+	if isNotFoundError(lastErr) {
+		log.Warn("URL not found")
+		return "", nil
+	}
 	return "", lastErr
 }

 func (s *Service) fetchUrlFromCache(shortUrl string) (string, error) {
-	if url, err := s.cache.Get(shortUrl); err == nil {
-		return url, nil
+	url, err := s.cache.Get(shortUrl)
+	if err != nil {
+		log.WithFields(logrus.Fields{
+			"short_url": shortUrl,
+			"error":     err,
+		}).Debug("Cache miss")
+		return "", err
 	}
-	return "", fmt.Errorf("URL not found")
+	return url, nil
+
 }

 func (s *Service) fetchUrlFromDatabase(shortUrl string) (string, error) {
-	if url, err := s.database.GetURL(shortUrl); err == nil {
-		return url, nil
+	url, err := s.database.GetURL(shortUrl)
+	if err != nil {
+		log.WithFields(logrus.Fields{
+			"short_url": shortUrl,
+			"error":     err,
+		}).Debug("Database lookup failed")
+		return "", err
 	}
+	log.WithField("short_url", shortUrl).Debug("Database hit")
+	return url, nil
+}

-	return "", fmt.Errorf("URL not found")
+func isNotFoundError(err error) bool {
+	if err == nil {
+		return false
+	}
+	errMsg := err.Error()
+	return strings.Contains(errMsg, "not found") ||
+		strings.Contains(errMsg, "redis: nil") ||
+		strings.Contains(errMsg, "no rows in result set")
 }

 func (s *Service) storeUrl(u db.URL) error {
 	var wg sync.WaitGroup
 	errChan := make(chan error, 2)

-	wg.Add(1)
+	wg.Add(2)
+
 	go func() {
 		defer wg.Done()
 		if err := s.cache.Set(u.ShortURL, u.URL); err != nil {
-			errChan <- err
+			log.WithFields(logrus.Fields{
+				"short_url": u.ShortURL,
+				"error":     err,
+			}).Error("Failed to store URL in cache")
+			select {
+			case errChan <- err:
+			default:
+			}
+		} else {
+			log.WithField("short_url", u.ShortURL).Debug("Stored URL in cache")
 		}
 	}()

@@ -247,18 +333,26 @@ func (s *Service) storeUrl(u db.URL) error {
 	go func() {
 		defer wg.Done()
 		if err := s.database.InsertNewURL(u); err != nil {
-			errChan <- err
+			log.WithFields(logrus.Fields{
+				"short_url": u.ShortURL,
+				"error":     err,
+			}).Error("Failed to store URL in database")
+			select {
+			case errChan <- err:
+			default:
+			}
+		} else {
+			log.WithField("short_url", u.ShortURL).Debug("Stored URL in database")
 		}
 	}()

-	go func() {
-		wg.Wait()
-		close(errChan)
-	}()
+	wg.Wait()
+	close(errChan)

-	for err := range errChan {
+	select {
+	case err := <-errChan:
 		return err
+	default:
+		return nil
 	}
-
-	return nil
 }