Merge pull request '[REFACTOR] Simplify converting struct to map in admin stats' (#2442) from gusted/forgejo-admin-stats into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2442
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

routers/web/admin/admin.go

@@ -7,8 +7,8 @@ package admin
 import (
 	"fmt"
 	"net/http"
+	"reflect"
 	"runtime"
-	"sort"
 	"time"
 
 	activities_model "code.gitea.io/gitea/models/activities"
@@ -16,7 +16,6 @@ import (
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/graceful"
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/updatechecker"
@@ -225,26 +224,22 @@ func CronTasks(ctx *context.Context) {
 func MonitorStats(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("admin.monitor.stats")
 	ctx.Data["PageIsAdminMonitorStats"] = true
-	bs, err := json.Marshal(activities_model.GetStatistic(ctx).Counter)
+	modelStats := activities_model.GetStatistic(ctx).Counter
-	if err != nil {
+	stats := map[string]any{}
-		ctx.ServerError("MonitorStats", err)
+
-		return
+	// To avoid manually converting the values of the stats struct to an map,
-	}
+	// and to avoid using JSON to do this for us (JSON encoder converts numbers to
-	statsCounter := map[string]any{}
+	// scientific notation). Use reflect to convert the struct to an map.
-	err = json.Unmarshal(bs, &statsCounter)
+	rv := reflect.ValueOf(modelStats)
-	if err != nil {
+	for i := 0; i < rv.NumField(); i++ {
-		ctx.ServerError("MonitorStats", err)
+		field := rv.Field(i)
-		return
+		// Preserve old behavior, do not show arrays that are empty.
-	}
+		if field.Kind() == reflect.Slice && field.Len() == 0 {
-	statsKeys := make([]string, 0, len(statsCounter))
-	for k := range statsCounter {
-		if statsCounter[k] == nil {
 			continue
 		}
-		statsKeys = append(statsKeys, k)
+		stats[rv.Type().Field(i).Name] = field.Interface()
 	}
-	sort.Strings(statsKeys)
+
-	ctx.Data["StatsKeys"] = statsKeys
+	ctx.Data["Stats"] = stats
-	ctx.Data["StatsCounter"] = statsCounter
 	ctx.HTML(http.StatusOK, tplStats)
 }

routers/web/admin/admin_test.go

@@ -6,6 +6,11 @@ package admin
 import (
 	"testing"
 
+	activities_model "code.gitea.io/gitea/models/activities"
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/contexttest"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -66,3 +71,46 @@ func TestShadowPassword(t *testing.T) {
 		assert.EqualValues(t, k.Result, shadowPassword(k.Provider, k.CfgItem))
 	}
 }
+
+func TestMonitorStats(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+
+	t.Run("Normal", func(t *testing.T) {
+		defer test.MockVariableValue(&setting.Metrics.EnabledIssueByLabel, false)()
+		defer test.MockVariableValue(&setting.Metrics.EnabledIssueByRepository, false)()
+
+		ctx, _ := contexttest.MockContext(t, "admin/stats")
+		MonitorStats(ctx)
+
+		// Test some of the stats manually.
+		mappedStats := ctx.Data["Stats"].(map[string]any)
+		stats := activities_model.GetStatistic(ctx).Counter
+
+		assert.EqualValues(t, stats.Comment, mappedStats["Comment"])
+		assert.EqualValues(t, stats.Issue, mappedStats["Issue"])
+		assert.EqualValues(t, stats.User, mappedStats["User"])
+		assert.EqualValues(t, stats.Milestone, mappedStats["Milestone"])
+
+		// Ensure that these aren't set.
+		assert.Empty(t, stats.IssueByLabel)
+		assert.Empty(t, stats.IssueByRepository)
+		assert.Nil(t, mappedStats["IssueByLabel"])
+		assert.Nil(t, mappedStats["IssueByRepository"])
+	})
+
+	t.Run("IssueByX", func(t *testing.T) {
+		defer test.MockVariableValue(&setting.Metrics.EnabledIssueByLabel, true)()
+		defer test.MockVariableValue(&setting.Metrics.EnabledIssueByRepository, true)()
+
+		ctx, _ := contexttest.MockContext(t, "admin/stats")
+		MonitorStats(ctx)
+
+		mappedStats := ctx.Data["Stats"].(map[string]any)
+		stats := activities_model.GetStatistic(ctx).Counter
+
+		assert.NotEmpty(t, stats.IssueByLabel)
+		assert.NotEmpty(t, stats.IssueByRepository)
+		assert.EqualValues(t, stats.IssueByLabel, mappedStats["IssueByLabel"])
+		assert.EqualValues(t, stats.IssueByRepository, mappedStats["IssueByRepository"])
+	})
+}

templates/admin/stats.tmpl

@@ -5,10 +5,10 @@
 	</h4>
 	<div class="ui attached table segment">
 		<table class="ui very basic striped table unstackable">
-			{{range $statsKey := .StatsKeys}}
+			{{range $statsKey, $statsValue := .Stats}}
 			<tr>
 				<td width="200">{{$statsKey}}</td>
-				<td>{{index $.StatsCounter $statsKey}}</td>
+				<td>{{$statsValue}}</td>
 			</tr>
 			{{end}}
 		</table>