Don't store stats if count=0

Now that only visitors are stored, we don't need to store rows in the stats tables if there are only pageviews but no visits.
arp242 · Nov 8, 2022 · 6bc0162 · 6bc0162
1 parent d556673
commit 6bc0162
Show file tree

Hide file tree

Showing 10 changed files with 74 additions and 14 deletions.
diff --git a/cron/browser_stat.go b/cron/browser_stat.go
@@ -63,7 +63,9 @@ func updateBrowserStats(ctx context.Context, hits []goatcounter.Hit) error {
 		}
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, v.browserID, v.count)
+			if v.count > 0 {
+				ins.Values(siteID, v.day, v.pathID, v.browserID, v.count)
+			}
 		}
 		return ins.Finish()
 	}), "cron.updateBrowserStats")

diff --git a/cron/browser_stat_test.go b/cron/browser_stat_test.go
@@ -33,7 +33,7 @@ func TestBrowserStats(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	want := `{false [{ Firefox 1 <nil>} { Chrome 0 <nil>}]}`
+	want := `{false [{ Firefox 1 <nil>}]}`
 	out := fmt.Sprintf("%v", stats)
 	if want != out {
 		t.Errorf("\nwant: %s\nout:  %s", want, out)
@@ -45,6 +45,7 @@ func TestBrowserStats(t *testing.T) {
 		{Site: site.ID, CreatedAt: now, UserAgentHeader: "Firefox/69.0"},
 		{Site: site.ID, CreatedAt: now, UserAgentHeader: "Firefox/70.0"},
 		{Site: site.ID, CreatedAt: now, UserAgentHeader: "Firefox/70.0"},
+		{Site: site.ID, CreatedAt: now, UserAgentHeader: "Chrome/77.0.123.666", FirstVisit: true},
 	}...)
 
 	stats = goatcounter.HitStats{}
@@ -53,7 +54,7 @@ func TestBrowserStats(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	want = `{false [{ Firefox 2 <nil>} { Chrome 0 <nil>}]}`
+	want = `{false [{ Firefox 2 <nil>} { Chrome 1 <nil>}]}`
 	out = fmt.Sprintf("%v", stats)
 	if want != out {
 		t.Errorf("\nwant: %s\nout:  %s", want, out)
@@ -66,7 +67,7 @@ func TestBrowserStats(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	want = `{false [{ Firefox 68 1 <nil>} { Firefox 69 1 <nil>} { Firefox 70 0 <nil>}]}`
+	want = `{false [{ Firefox 68 1 <nil>} { Firefox 69 1 <nil>}]}`
 	out = fmt.Sprintf("%v", stats)
 	if want != out {
 		t.Errorf("\nwant: %s\nout:  %s", want, out)

diff --git a/cron/campaign_stat.go b/cron/campaign_stat.go
@@ -56,7 +56,9 @@ func updateCampaignStats(ctx context.Context, hits []goatcounter.Hit) error {
 		}
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, v.campaignID, v.ref, v.count)
+			if v.count > 0 {
+				ins.Values(siteID, v.day, v.pathID, v.campaignID, v.ref, v.count)
+			}
 		}
 		return ins.Finish()
 	}), "cron.updateCampaignStats")

diff --git a/cron/campaign_stat_test.go b/cron/campaign_stat_test.go
@@ -0,0 +1,43 @@
+package cron_test
+
+import (
+	"testing"
+	"time"
+
+	"zgo.at/goatcounter/v2"
+	"zgo.at/goatcounter/v2/gctest"
+	"zgo.at/zstd/zjson"
+	"zgo.at/zstd/ztest"
+	"zgo.at/zstd/ztime"
+)
+
+func TestCampaignStats(t *testing.T) {
+	ctx := gctest.DB(t)
+
+	site := goatcounter.MustGetSite(ctx)
+	now := time.Date(2019, 8, 31, 14, 42, 0, 0, time.UTC)
+
+	gctest.StoreHits(ctx, t, false, []goatcounter.Hit{
+		{Site: site.ID, CreatedAt: now, Query: "utm_campaign=one", FirstVisit: true},
+		{Site: site.ID, CreatedAt: now, Query: "utm_campaign=one"},
+		{Site: site.ID, CreatedAt: now, Query: "utm_campaign=two"},
+		{Site: site.ID, CreatedAt: now, Query: "utm_campaign=three", FirstVisit: true},
+	}...)
+
+	var have goatcounter.HitStats
+	err := have.ListCampaigns(ctx, ztime.NewRange(now).To(now), nil, 10, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := `{
+		"more": false,
+		"stats": [
+			{"count": 1, "id": "1", "name": "one"},
+			{"count": 1, "id": "3", "name": "three"}
+		]
+	}`
+	if d := ztest.Diff(zjson.MustMarshalString(have), want, ztest.DiffJSON); d != "" {
+		t.Error(d)
+	}
+}
diff --git a/cron/hit_stat.go b/cron/hit_stat.go
@@ -18,6 +18,7 @@ func updateHitStats(ctx context.Context, hits []goatcounter.Hit) error {
 	return errors.Wrap(zdb.TX(ctx, func(ctx context.Context) error {
 		type gt struct {
 			count  []int
+			total  int
 			day    string
 			hour   string
 			pathID int64
@@ -50,6 +51,7 @@ func updateHitStats(ctx context.Context, hits []goatcounter.Hit) error {
 			hour, _ := strconv.ParseInt(h.CreatedAt.Format("15"), 10, 8)
 			if h.FirstVisit {
 				v.count[hour] += 1
+				v.total += 1
 			}
 			grouped[k] = v
 		}
@@ -77,7 +79,9 @@ func updateHitStats(ctx context.Context, hits []goatcounter.Hit) error {
 		// }
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, zjson.MustMarshal(v.count))
+			if v.total > 0 {
+				ins.Values(siteID, v.day, v.pathID, zjson.MustMarshal(v.count))
+			}
 		}
 		return errors.Wrap(ins.Finish(), "updateHitStats hit_stats")
 	}), "cron.updateHitStats")

diff --git a/cron/language_stat.go b/cron/language_stat.go
@@ -54,7 +54,9 @@ func updateLanguageStats(ctx context.Context, hits []goatcounter.Hit) error {
 		}
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, v.language, v.count)
+			if v.count > 0 {
+				ins.Values(siteID, v.day, v.pathID, v.language, v.count)
+			}
 		}
 		return ins.Finish()
 	}), "cron.updateLanguageStats")

diff --git a/cron/location_stat.go b/cron/location_stat.go
@@ -60,7 +60,9 @@ func updateLocationStats(ctx context.Context, hits []goatcounter.Hit) error {
 		}
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, v.location, v.count)
+			if v.count > 0 {
+				ins.Values(siteID, v.day, v.pathID, v.location, v.count)
+			}
 		}
 		return ins.Finish()
 	}), "cron.updateLocationStats")

diff --git a/cron/location_stat_test.go b/cron/location_stat_test.go
@@ -32,7 +32,7 @@ func TestLocationStats(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	want := `{false [{ET Ethiopia 1 <nil>} {ID Indonesia 0 <nil>}]}`
+	want := `{false [{ET Ethiopia 1 <nil>}]}`
 	out := fmt.Sprintf("%v", stats)
 	if want != out {
 		t.Errorf("\nwant: %s\nout:  %s", want, out)
@@ -41,12 +41,12 @@ func TestLocationStats(t *testing.T) {
 	// Update existing.
 	gctest.StoreHits(ctx, t, false, []goatcounter.Hit{
 		{Site: site.ID, CreatedAt: now, Location: "ID"},
-		{Site: site.ID, CreatedAt: now, Location: "ID"},
+		{Site: site.ID, CreatedAt: now, Location: "ID", FirstVisit: true},
 		{Site: site.ID, CreatedAt: now, Location: "ET"},
 		{Site: site.ID, CreatedAt: now, Location: "ET", FirstVisit: true},
 		{Site: site.ID, CreatedAt: now, Location: "ET", FirstVisit: true},
 		{Site: site.ID, CreatedAt: now, Location: "ET"},
-		{Site: site.ID, CreatedAt: now, Location: "NZ"},
+		{Site: site.ID, CreatedAt: now, Location: "NZ", FirstVisit: true},
 	}...)
 
 	stats = goatcounter.HitStats{}
@@ -55,7 +55,7 @@ func TestLocationStats(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	want = `{false [{ET Ethiopia 3 <nil>} {ID Indonesia 0 <nil>} {NZ New Zealand 0 <nil>}]}`
+	want = `{false [{ET Ethiopia 3 <nil>} {ID Indonesia 1 <nil>} {NZ New Zealand 1 <nil>}]}`
 	out = fmt.Sprintf("%v", stats)
 	if want != out {
 		t.Errorf("\nwant: %s\nout:  %s", want, out)

diff --git a/cron/size_stat.go b/cron/size_stat.go
@@ -59,7 +59,9 @@ func updateSizeStats(ctx context.Context, hits []goatcounter.Hit) error {
 		}
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, v.width, v.count)
+			if v.count > 0 {
+				ins.Values(siteID, v.day, v.pathID, v.width, v.count)
+			}
 		}
 		return ins.Finish()
 	}), "cron.updateSizeStats")

diff --git a/cron/system_stat.go b/cron/system_stat.go
@@ -61,7 +61,9 @@ func updateSystemStats(ctx context.Context, hits []goatcounter.Hit) error {
 		}
 
 		for _, v := range grouped {
-			ins.Values(siteID, v.day, v.pathID, v.systemID, v.count)
+			if v.count > 0 {
+				ins.Values(siteID, v.day, v.pathID, v.systemID, v.count)
+			}
 		}
 		return ins.Finish()
 	}), "cron.updateSystemStats")