From bafbc347060ec212bcf5d0ef8526689110a8ac33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ferdinand=20M=C3=BCtsch?= <ferdinand@muetsch.io>
Date: Thu, 29 Dec 2022 11:54:14 +0100
Subject: [PATCH] refactor: minor code refactorings

---
 config/config.go              | 41 --------------------------------
 config/sentry.go              | 22 +++++++++++-------
 migrations/migrations.go      | 44 ++++++++++++++++++++++++++++++++++-
 models/shared.go              | 22 ------------------
 models/user.go                | 14 +++++++++++
 models/view/leaderboard.go    |  3 ++-
 routes/settings.go            |  2 +-
 routes/summary.go             |  9 ++++---
 routes/utils/summary_utils.go | 11 +++++++++
 services/housekeeping.go      | 12 +++++-----
 services/leaderboard.go       |  7 +++---
 services/services.go          |  7 +++---
 utils/color.go                | 16 -------------
 utils/http.go                 | 26 +++++++++++++++++----
 14 files changed, 124 insertions(+), 112 deletions(-)
 delete mode 100644 utils/color.go

diff --git a/config/config.go b/config/config.go
index df56dd9..5ab8e91 100644
--- a/config/config.go
+++ b/config/config.go
@@ -18,9 +18,7 @@ import (
 	"github.com/gorilla/securecookie"
 	"github.com/jinzhu/configor"
 	"github.com/muety/wakapi/data"
-	"github.com/muety/wakapi/models"
 	uuid "github.com/satori/go.uuid"
-	"gorm.io/gorm"
 )
 
 const (
@@ -205,45 +203,6 @@ func (c *Config) UseTLS() bool {
 	return c.Server.TlsCertPath != "" && c.Server.TlsKeyPath != ""
 }
 
-func (c *Config) GetMigrationFunc(dbDialect string) models.MigrationFunc {
-	switch dbDialect {
-	default:
-		return func(db *gorm.DB) error {
-			if err := db.AutoMigrate(&models.User{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.KeyStringValue{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.Alias{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.Heartbeat{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.Summary{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.SummaryItem{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.LanguageMapping{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.ProjectLabel{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.Diagnostics{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			if err := db.AutoMigrate(&models.LeaderboardItem{}); err != nil && !c.Db.AutoMigrateFailSilently {
-				return err
-			}
-			return nil
-		}
-	}
-}
-
 func (c *appConfig) GetCustomLanguages() map[string]string {
 	return utils.CloneStringMap(c.CustomLanguages, false)
 }
diff --git a/config/sentry.go b/config/sentry.go
index d0df388..252fd78 100644
--- a/config/sentry.go
+++ b/config/sentry.go
@@ -3,7 +3,6 @@ package config
 import (
 	"github.com/emvi/logbuch"
 	"github.com/getsentry/sentry-go"
-	"github.com/muety/wakapi/models"
 	"io"
 	"net/http"
 	"os"
@@ -89,8 +88,8 @@ func (l *SentryWrapperLogger) log(msg string, level sentry.Level) {
 		if h := l.req.Context().Value(sentry.HubContextKey); h != nil {
 			hub := h.(*sentry.Hub)
 			hub.Scope().SetRequest(l.req)
-			if u := getPrincipal(l.req); u != nil {
-				hub.Scope().SetUser(sentry.User{ID: u.ID})
+			if uid := getPrincipal(l.req); uid != "" {
+				hub.Scope().SetUser(sentry.User{ID: uid})
 			}
 			hub.CaptureEvent(event)
 			return
@@ -133,8 +132,8 @@ func initSentry(config sentryConfig, debug bool) {
 		BeforeSend: func(event *sentry.Event, hint *sentry.EventHint) *sentry.Event {
 			if hint.Context != nil {
 				if req, ok := hint.Context.Value(sentry.RequestContextKey).(*http.Request); ok {
-					if u := getPrincipal(req); u != nil {
-						event.User.ID = u.ID
+					if uid := getPrincipal(req); uid != "" {
+						event.User.ID = uid
 					}
 				}
 			}
@@ -145,12 +144,17 @@ func initSentry(config sentryConfig, debug bool) {
 	}
 }
 
-func getPrincipal(r *http.Request) *models.User {
+// returns a user id
+func getPrincipal(r *http.Request) string {
+	type identifiable interface {
+		Identity() string
+	}
 	type principalGetter interface {
-		GetPrincipal() *models.User
+		GetPrincipal() *identifiable
 	}
+
 	if p := r.Context().Value("principal"); p != nil {
-		return p.(principalGetter).GetPrincipal()
+		return (*p.(principalGetter).GetPrincipal()).Identity()
 	}
-	return nil
+	return ""
 }
diff --git a/migrations/migrations.go b/migrations/migrations.go
index e08705f..bcc0c1a 100644
--- a/migrations/migrations.go
+++ b/migrations/migrations.go
@@ -3,11 +3,14 @@ package migrations
 import (
 	"github.com/emvi/logbuch"
 	"github.com/muety/wakapi/config"
+	"github.com/muety/wakapi/models"
 	"gorm.io/gorm"
 	"sort"
 	"strings"
 )
 
+type gormMigrationFunc func(db *gorm.DB) error
+
 type migrationFunc struct {
 	f    func(db *gorm.DB, cfg *config.Config) error
 	name string
@@ -20,6 +23,45 @@ var (
 	postMigrations migrationFuncs
 )
 
+func GetMigrationFunc(cfg *config.Config) gormMigrationFunc {
+	switch cfg.Db.Dialect {
+	default:
+		return func(db *gorm.DB) error {
+			if err := db.AutoMigrate(&models.User{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.KeyStringValue{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.Alias{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.Heartbeat{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.Summary{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.SummaryItem{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.LanguageMapping{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.ProjectLabel{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.Diagnostics{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			if err := db.AutoMigrate(&models.LeaderboardItem{}); err != nil && !cfg.Db.AutoMigrateFailSilently {
+				return err
+			}
+			return nil
+		}
+	}
+}
+
 func registerPreMigration(f migrationFunc) {
 	preMigrations = append(preMigrations, f)
 }
@@ -35,7 +77,7 @@ func Run(db *gorm.DB, cfg *config.Config) {
 }
 
 func RunSchemaMigrations(db *gorm.DB, cfg *config.Config) {
-	if err := cfg.GetMigrationFunc(cfg.Db.Dialect)(db); err != nil {
+	if err := GetMigrationFunc(cfg)(db); err != nil {
 		logbuch.Fatal(err.Error())
 	}
 }
diff --git a/models/shared.go b/models/shared.go
index 920e29d..36f5aff 100644
--- a/models/shared.go
+++ b/models/shared.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"gorm.io/gorm"
 	"strconv"
 	"strings"
 	"time"
@@ -18,8 +17,6 @@ const (
 	PersistentIntervalKey = "wakapi_summary_interval"
 )
 
-type MigrationFunc func(db *gorm.DB) error
-
 type KeyStringValue struct {
 	Key   string `gorm:"primary_key"`
 	Value string `gorm:"type:text"`
@@ -35,11 +32,6 @@ type KeyedInterval struct {
 	Key *IntervalKey
 }
 
-type PageParams struct {
-	Page     int `json:"page"`
-	PageSize int `json:"page_size"`
-}
-
 // CustomTime is a wrapper type around time.Time, mainly used for the purpose of transparently unmarshalling Python timestamps in the format <sec>.<nsec> (e.g. 1619335137.3324468)
 type CustomTime time.Time
 
@@ -105,17 +97,3 @@ func (j CustomTime) T() time.Time {
 func (j CustomTime) Valid() bool {
 	return j.T().Unix() >= 0
 }
-
-func (p *PageParams) Limit() int {
-	if p.PageSize < 0 {
-		return 0
-	}
-	return p.PageSize
-}
-
-func (p *PageParams) Offset() int {
-	if p.PageSize <= 0 {
-		return 0
-	}
-	return (p.Page - 1) * p.PageSize
-}
diff --git a/models/user.go b/models/user.go
index 3b2f858..8d16d5d 100644
--- a/models/user.go
+++ b/models/user.go
@@ -3,6 +3,7 @@ package models
 import (
 	"crypto/md5"
 	"fmt"
+	conf "github.com/muety/wakapi/config"
 	"regexp"
 	"strings"
 	"time"
@@ -83,6 +84,10 @@ type CountByUser struct {
 	Count int64
 }
 
+func (u *User) Identity() string {
+	return u.ID
+}
+
 func (u *User) TZ() *time.Location {
 	if u.Location == "" {
 		u.Location = "Local"
@@ -125,6 +130,15 @@ func (u *User) HasActiveSubscription() bool {
 	return u.SubscribedUntil != nil && u.SubscribedUntil.T().After(time.Now())
 }
 
+func (u *User) MinDataAge() time.Time {
+	retentionMonths := conf.Get().App.DataRetentionMonths
+	if retentionMonths <= 0 || u.HasActiveSubscription() {
+		return time.Time{}
+	}
+	// this is not exactly precise, because of summer / winter time, etc.
+	return time.Now().AddDate(0, -retentionMonths, 0)
+}
+
 func (c *CredentialsReset) IsValid() bool {
 	return ValidatePassword(c.PasswordNew) &&
 		c.PasswordNew == c.PasswordRepeat
diff --git a/models/view/leaderboard.go b/models/view/leaderboard.go
index 143aca9..5ef5554 100644
--- a/models/view/leaderboard.go
+++ b/models/view/leaderboard.go
@@ -2,6 +2,7 @@ package view
 
 import (
 	"github.com/muety/wakapi/models"
+	"github.com/muety/wakapi/utils"
 	"strings"
 	"time"
 )
@@ -14,7 +15,7 @@ type LeaderboardViewModel struct {
 	TopKeys       []string
 	UserLanguages map[string][]string
 	ApiKey        string
-	PageParams    *models.PageParams
+	PageParams    *utils.PageParams
 	Success       string
 	Error         string
 }
diff --git a/routes/settings.go b/routes/settings.go
index b99ae2d..0b54a9a 100644
--- a/routes/settings.go
+++ b/routes/settings.go
@@ -181,7 +181,7 @@ func (h *SettingsHandler) actionUpdateUser(w http.ResponseWriter, r *http.Reques
 		return http.StatusBadRequest, "", "invalid parameters"
 	}
 
-	if user.Email == "" && user.HasActiveSubscription() {
+	if user.Email == "" && h.config.Subscriptions.Enabled && user.HasActiveSubscription() {
 		return http.StatusBadRequest, "", "cannot unset email while subscription is active"
 	}
 
diff --git a/routes/summary.go b/routes/summary.go
index 5924131..3b53a18 100644
--- a/routes/summary.go
+++ b/routes/summary.go
@@ -1,6 +1,7 @@
 package routes
 
 import (
+	"fmt"
 	"github.com/gorilla/mux"
 	conf "github.com/muety/wakapi/config"
 	"github.com/muety/wakapi/helpers"
@@ -9,8 +10,6 @@ import (
 	"github.com/muety/wakapi/models/view"
 	su "github.com/muety/wakapi/routes/utils"
 	"github.com/muety/wakapi/services"
-	"github.com/muety/wakapi/utils"
-	"fmt"
 	"net/http"
 )
 
@@ -80,9 +79,9 @@ func (h *SummaryHandler) GetIndex(w http.ResponseWriter, r *http.Request) {
 		Summary:        summary,
 		SummaryParams:  summaryParams,
 		User:           user,
-		EditorColors:   utils.FilterColors(h.config.App.GetEditorColors(), summary.Editors),
-		LanguageColors: utils.FilterColors(h.config.App.GetLanguageColors(), summary.Languages),
-		OSColors:       utils.FilterColors(h.config.App.GetOSColors(), summary.OperatingSystems),
+		EditorColors:   su.FilterColors(h.config.App.GetEditorColors(), summary.Editors),
+		LanguageColors: su.FilterColors(h.config.App.GetLanguageColors(), summary.Languages),
+		OSColors:       su.FilterColors(h.config.App.GetOSColors(), summary.OperatingSystems),
 		ApiKey:         user.ApiKey,
 		RawQuery:       rawQuery,
 	}
diff --git a/routes/utils/summary_utils.go b/routes/utils/summary_utils.go
index dfee8bf..9afcea1 100644
--- a/routes/utils/summary_utils.go
+++ b/routes/utils/summary_utils.go
@@ -5,6 +5,7 @@ import (
 	"github.com/muety/wakapi/models"
 	"github.com/muety/wakapi/services"
 	"net/http"
+	"strings"
 )
 
 func LoadUserSummary(ss services.ISummaryService, r *http.Request) (*models.Summary, error, int) {
@@ -38,3 +39,13 @@ func LoadUserSummaryByParams(ss services.ISummaryService, params *models.Summary
 
 	return summary, nil, http.StatusOK
 }
+
+func FilterColors(all map[string]string, haystack models.SummaryItems) map[string]string {
+	subset := make(map[string]string)
+	for _, item := range haystack {
+		if c, ok := all[strings.ToLower(item.Key)]; ok {
+			subset[strings.ToLower(item.Key)] = c
+		}
+	}
+	return subset
+}
diff --git a/services/housekeeping.go b/services/housekeeping.go
index f18af41..92b6ffb 100644
--- a/services/housekeeping.go
+++ b/services/housekeeping.go
@@ -35,9 +35,6 @@ func (s *HousekeepingService) Schedule() {
 
 	logbuch.Info("scheduling data cleanup")
 
-	// this is not exactly precise, because of summer / winter time, etc.
-	retentionDuration := time.Now().Sub(time.Now().AddDate(0, -s.config.App.DataRetentionMonths, 0))
-
 	_, err := s.queueDefault.DispatchCron(func() {
 		// fetch all users
 		users, err := s.userSrvc.GetAll()
@@ -53,9 +50,13 @@ func (s *HousekeepingService) Schedule() {
 				continue
 			}
 
+			if u.MinDataAge().IsZero() {
+				continue
+			}
+
 			user := *u
 			s.queueWorkers.Dispatch(func() {
-				if err := s.ClearOldUserData(&user, retentionDuration); err != nil {
+				if err := s.CleanUserDataBefore(&user, u.MinDataAge()); err != nil {
 					config.Log().Error("failed to clear old user data for '%s'", user.ID)
 				}
 			})
@@ -67,8 +68,7 @@ func (s *HousekeepingService) Schedule() {
 	}
 }
 
-func (s *HousekeepingService) ClearOldUserData(user *models.User, maxAge time.Duration) error {
-	before := time.Now().Add(-maxAge)
+func (s *HousekeepingService) CleanUserDataBefore(user *models.User, before time.Time) error {
 	logbuch.Warn("cleaning up user data for '%s' older than %v", user.ID, before)
 
 	// clear old heartbeats
diff --git a/services/leaderboard.go b/services/leaderboard.go
index ec74c58..abb94a9 100644
--- a/services/leaderboard.go
+++ b/services/leaderboard.go
@@ -8,6 +8,7 @@ import (
 	"github.com/muety/wakapi/helpers"
 	"github.com/muety/wakapi/models"
 	"github.com/muety/wakapi/repositories"
+	"github.com/muety/wakapi/utils"
 	"github.com/patrickmn/go-cache"
 	"reflect"
 	"strconv"
@@ -147,7 +148,7 @@ func (srv *LeaderboardService) CountUsers() (int64, error) {
 	return count, err
 }
 
-func (srv *LeaderboardService) GetByInterval(interval *models.IntervalKey, pageParams *models.PageParams, resolveUsers bool) (models.Leaderboard, error) {
+func (srv *LeaderboardService) GetByInterval(interval *models.IntervalKey, pageParams *utils.PageParams, resolveUsers bool) (models.Leaderboard, error) {
 	return srv.GetAggregatedByInterval(interval, nil, pageParams, resolveUsers)
 }
 
@@ -155,7 +156,7 @@ func (srv *LeaderboardService) GetByIntervalAndUser(interval *models.IntervalKey
 	return srv.GetAggregatedByIntervalAndUser(interval, userId, nil, resolveUser)
 }
 
-func (srv *LeaderboardService) GetAggregatedByInterval(interval *models.IntervalKey, by *uint8, pageParams *models.PageParams, resolveUsers bool) (models.Leaderboard, error) {
+func (srv *LeaderboardService) GetAggregatedByInterval(interval *models.IntervalKey, by *uint8, pageParams *utils.PageParams, resolveUsers bool) (models.Leaderboard, error) {
 	// check cache
 	cacheKey := srv.getHash(interval, by, "", pageParams)
 	if cacheResult, ok := srv.cache.Get(cacheKey); ok {
@@ -259,7 +260,7 @@ func (srv *LeaderboardService) GenerateAggregatedByUser(user *models.User, inter
 	return items, nil
 }
 
-func (srv *LeaderboardService) getHash(interval *models.IntervalKey, by *uint8, user string, pageParams *models.PageParams) string {
+func (srv *LeaderboardService) getHash(interval *models.IntervalKey, by *uint8, user string, pageParams *utils.PageParams) string {
 	k := strings.Join(*interval, "__") + "__" + user
 	if by != nil && !reflect.ValueOf(by).IsNil() {
 		k += "__" + models.GetEntityColumn(*by)
diff --git a/services/services.go b/services/services.go
index d1d6908..3135a0d 100644
--- a/services/services.go
+++ b/services/services.go
@@ -3,6 +3,7 @@ package services
 import (
 	datastructure "github.com/duke-git/lancet/v2/datastructure/set"
 	"github.com/muety/wakapi/models"
+	"github.com/muety/wakapi/utils"
 	"time"
 )
 
@@ -102,7 +103,7 @@ type IReportService interface {
 
 type IHousekeepingService interface {
 	Schedule()
-	ClearOldUserData(*models.User, time.Duration) error
+	CleanUserDataBefore(*models.User, time.Time) error
 }
 
 type ILeaderboardService interface {
@@ -110,9 +111,9 @@ type ILeaderboardService interface {
 	ComputeLeaderboard([]*models.User, *models.IntervalKey, []uint8) error
 	ExistsAnyByUser(string) (bool, error)
 	CountUsers() (int64, error)
-	GetByInterval(*models.IntervalKey, *models.PageParams, bool) (models.Leaderboard, error)
+	GetByInterval(*models.IntervalKey, *utils.PageParams, bool) (models.Leaderboard, error)
 	GetByIntervalAndUser(*models.IntervalKey, string, bool) (models.Leaderboard, error)
-	GetAggregatedByInterval(*models.IntervalKey, *uint8, *models.PageParams, bool) (models.Leaderboard, error)
+	GetAggregatedByInterval(*models.IntervalKey, *uint8, *utils.PageParams, bool) (models.Leaderboard, error)
 	GetAggregatedByIntervalAndUser(*models.IntervalKey, string, *uint8, bool) (models.Leaderboard, error)
 	GenerateByUser(*models.User, *models.IntervalKey) (*models.LeaderboardItem, error)
 	GenerateAggregatedByUser(*models.User, *models.IntervalKey, uint8) ([]*models.LeaderboardItem, error)
diff --git a/utils/color.go b/utils/color.go
deleted file mode 100644
index e3fef76..0000000
--- a/utils/color.go
+++ /dev/null
@@ -1,16 +0,0 @@
-package utils
-
-import (
-	"github.com/muety/wakapi/models"
-	"strings"
-)
-
-func FilterColors(all map[string]string, haystack models.SummaryItems) map[string]string {
-	subset := make(map[string]string)
-	for _, item := range haystack {
-		if c, ok := all[strings.ToLower(item.Key)]; ok {
-			subset[strings.ToLower(item.Key)] = c
-		}
-	}
-	return subset
-}
diff --git a/utils/http.go b/utils/http.go
index 5ee9b29..282ab82 100644
--- a/utils/http.go
+++ b/utils/http.go
@@ -2,7 +2,6 @@ package utils
 
 import (
 	"errors"
-	"github.com/muety/wakapi/models"
 	"net/http"
 	"regexp"
 	"strconv"
@@ -22,6 +21,25 @@ func init() {
 	cacheMaxAgeRe = regexp.MustCompile(cacheMaxAgePattern)
 }
 
+type PageParams struct {
+	Page     int `json:"page"`
+	PageSize int `json:"page_size"`
+}
+
+func (p *PageParams) Limit() int {
+	if p.PageSize < 0 {
+		return 0
+	}
+	return p.PageSize
+}
+
+func (p *PageParams) Offset() int {
+	if p.PageSize <= 0 {
+		return 0
+	}
+	return (p.Page - 1) * p.PageSize
+}
+
 func IsNoCache(r *http.Request, cacheTtl time.Duration) bool {
 	cacheControl := r.Header.Get("cache-control")
 	if strings.Contains(cacheControl, "no-cache") {
@@ -35,8 +53,8 @@ func IsNoCache(r *http.Request, cacheTtl time.Duration) bool {
 	return false
 }
 
-func ParsePageParams(r *http.Request) *models.PageParams {
-	pageParams := &models.PageParams{}
+func ParsePageParams(r *http.Request) *PageParams {
+	pageParams := &PageParams{}
 	page := r.URL.Query().Get("page")
 	pageSize := r.URL.Query().Get("page_size")
 	if p, err := strconv.Atoi(page); err == nil {
@@ -48,7 +66,7 @@ func ParsePageParams(r *http.Request) *models.PageParams {
 	return pageParams
 }
 
-func ParsePageParamsWithDefault(r *http.Request, page, size int) *models.PageParams {
+func ParsePageParamsWithDefault(r *http.Request, page, size int) *PageParams {
 	pageParams := ParsePageParams(r)
 	if pageParams.Page == 0 {
 		pageParams.Page = page