fixup! Rate limiting by default

cmd/configwrite/configwrite_test.go

@@ -132,7 +132,7 @@ func TestWriteErr(t *testing.T) {
 
 func TestWriteSaveErr(t *testing.T) {
 	v := viper.New()
-	w := &writerMock{
+	w := &mockWriter{
 		WriteFn: func(section string, keyValue map[string]string) error {
 			assert.Equal(t, "settings", section)
 			assert.Equal(t, map[string]string{"debug": "false"}, keyValue)
@@ -148,10 +148,10 @@ func TestWriteSaveErr(t *testing.T) {
 	assert.Error(t, err)
 }
 
-type writerMock struct {
+type mockWriter struct {
 	WriteFn func(section string, keyValue map[string]string) error
 }
 
-func (m *writerMock) Write(section string, keyValue map[string]string) error {
+func (m *mockWriter) Write(section string, keyValue map[string]string) error {
 	return m.WriteFn(section, keyValue)
 }

cmd/heartbeat/heartbeat.go

@@ -79,9 +79,9 @@ func SendHeartbeats(v *viper.Viper, queueFilepath string) error {
 	log.Debugf("params: %s", params)
 
 	if RateLimited(RateLimitParams{
-		Disabled:         params.Offline.Disabled,
-		LastSentAt:       params.Offline.LastSentAt,
-		RateLimitTimeout: params.Offline.RateLimit,
+		Disabled:   params.Offline.Disabled,
+		LastSentAt: params.Offline.LastSentAt,
+		Timeout:    params.Offline.RateLimit,
 	}) {
 		if err = offlinecmd.SaveHeartbeats(v, nil, queueFilepath); err == nil {
 			return nil
@@ -158,9 +158,8 @@ func SendHeartbeats(v *viper.Viper, queueFilepath string) error {
 		}
 	}
 
-	err = ResetRateLimit(v)
-	if err != nil {
-		log.Errorln(err)
+	if err := ResetRateLimit(v); err != nil {
+		log.Errorf("failed to reset rate limit: %s", err)
 	}
 
 	return nil
@@ -192,9 +191,9 @@ func LoadParams(v *viper.Viper) (paramscmd.Params, error) {
 
 // RateLimitParams contains params for the RateLimited function.
 type RateLimitParams struct {
-	Disabled         bool
-	LastSentAt       time.Time
-	RateLimitTimeout time.Duration
+	Disabled   bool
+	LastSentAt time.Time
+	Timeout    time.Duration
 }
 
 // RateLimited determines if we should send heartbeats to the API or save to the offline db.
@@ -203,15 +202,15 @@ func RateLimited(params RateLimitParams) bool {
 		return false
 	}
 
-	if params.RateLimitTimeout == 0 {
+	if params.Timeout == 0 {
 		return false
 	}
 
 	if params.LastSentAt.IsZero() {
 		return false
 	}
 
-	return time.Since(params.LastSentAt) < params.RateLimitTimeout
+	return time.Since(params.LastSentAt) < params.Timeout
 }
 
 // ResetRateLimit updates the internal.heartbeats_last_sent_at timestamp.

cmd/heartbeat/heartbeat_test.go

@@ -47,6 +47,8 @@ func TestSendHeartbeats(t *testing.T) {
 	subfolders := project.CountSlashesInProjectFolder(projectFolder)
 
 	router.HandleFunc("/users/current/heartbeats.bulk", func(w http.ResponseWriter, req *http.Request) {
+		numCalls++
+
 		// check request
 		assert.Equal(t, http.MethodPost, req.Method)
 		assert.Equal(t, []string{"application/json"}, req.Header["Accept"])
@@ -85,7 +87,47 @@ func TestSendHeartbeats(t *testing.T) {
 
 		_, err = io.Copy(w, f)
 		require.NoError(t, err)
+	})
+
+	v := viper.New()
+	v.SetDefault("sync-offline-activity", 1000)
+	v.Set("api-url", testServerURL)
+	v.Set("category", "debugging")
+	v.Set("cursorpos", 42)
+	v.Set("entity", "testdata/main.go")
+	v.Set("entity-type", "file")
+	v.Set("key", "00000000-0000-4000-8000-000000000000")
+	v.Set("language", "Go")
+	v.Set("alternate-language", "Golang")
+	v.Set("hide-branch-names", true)
+	v.Set("project", "wakatime-cli")
+	v.Set("lineno", 13)
+	v.Set("local-file", "testdata/localfile.go")
+	v.Set("plugin", plugin)
+	v.Set("time", 1585598059.1)
+	v.Set("timeout", 5)
+	v.Set("write", true)
+
+	offlineQueueFile, err := os.CreateTemp(t.TempDir(), "")
+	require.NoError(t, err)
+
+	err = cmdheartbeat.SendHeartbeats(v, offlineQueueFile.Name())
+	require.NoError(t, err)
+
+	assert.Eventually(t, func() bool { return numCalls == 1 }, time.Second, 50*time.Millisecond)
+}
+
+func TestSendHeartbeats_RateLimited(t *testing.T) {
+	testServerURL, router, tearDown := setupTestServer()
+	defer tearDown()
+
+	var (
+		plugin   = "plugin/0.0.1"
+		numCalls int
+	)
 
+	router.HandleFunc("/users/current/heartbeats.bulk", func(_ http.ResponseWriter, _ *http.Request) {
+		// Should not be called
 		numCalls++
 	})
 
@@ -107,14 +149,16 @@ func TestSendHeartbeats(t *testing.T) {
 	v.Set("time", 1585598059.1)
 	v.Set("timeout", 5)
 	v.Set("write", true)
+	v.Set("heartbeat-rate-limit-seconds", 500)
+	v.Set("internal.heartbeats_last_sent_at", time.Now().Add(-time.Minute).Format(time.RFC3339))
 
 	offlineQueueFile, err := os.CreateTemp(t.TempDir(), "")
 	require.NoError(t, err)
 
 	err = cmdheartbeat.SendHeartbeats(v, offlineQueueFile.Name())
 	require.NoError(t, err)
 
-	assert.Eventually(t, func() bool { return numCalls == 1 }, time.Second, 50*time.Millisecond)
+	assert.Zero(t, numCalls)
 }
 
 func TestSendHeartbeats_WithFiltering_Exclude(t *testing.T) {
@@ -1053,22 +1097,72 @@ func TestSendHeartbeats_ObfuscateProjectNotBranch(t *testing.T) {
 }
 
 func TestRateLimited(t *testing.T) {
-	p := cmdheartbeat.RateLimitParams{Disabled: true, LastSentAt: time.Now(), RateLimitTimeout: 1}
+	p := cmdheartbeat.RateLimitParams{
+		Timeout:    time.Duration(offline.RateLimitDefaultSeconds) * time.Second,
+		LastSentAt: time.Now(),
+	}
+
+	assert.True(t, cmdheartbeat.RateLimited(p))
+}
+
+func TestRateLimited_NotLimited(t *testing.T) {
+	p := cmdheartbeat.RateLimitParams{
+		LastSentAt: time.Now().Add(time.Duration(-offline.RateLimitDefaultSeconds*2) * time.Second),
+		Timeout:    time.Duration(offline.RateLimitDefaultSeconds) * time.Second,
+	}
+
 	assert.False(t, cmdheartbeat.RateLimited(p))
+}
+
+func TestRateLimited_Disabled(t *testing.T) {
+	p := cmdheartbeat.RateLimitParams{
+		Disabled: true,
+	}
 
-	p = cmdheartbeat.RateLimitParams{RateLimitTimeout: 0, LastSentAt: time.Now()}
 	assert.False(t, cmdheartbeat.RateLimited(p))
+}
+
+func TestRateLimited_TimeoutZero(t *testing.T) {
+	p := cmdheartbeat.RateLimitParams{
+		LastSentAt: time.Time{},
+	}
 
-	limit := time.Duration(offline.RateLimitDefaultSeconds) * time.Second
+	assert.False(t, cmdheartbeat.RateLimited(p))
+}
 
-	p = cmdheartbeat.RateLimitParams{
-		LastSentAt:       time.Now().Add(time.Duration(-offline.RateLimitDefaultSeconds*2) * time.Second),
-		RateLimitTimeout: limit,
+func TestRateLimited_LastSentAtZero(t *testing.T) {
+	p := cmdheartbeat.RateLimitParams{
+		Timeout: 0,
 	}
+
 	assert.False(t, cmdheartbeat.RateLimited(p))
+}
 
-	p = cmdheartbeat.RateLimitParams{RateLimitTimeout: limit, LastSentAt: time.Now()}
-	assert.True(t, cmdheartbeat.RateLimited(p))
+func TestResetRateLimit(t *testing.T) {
+	tmpFile, err := os.CreateTemp(t.TempDir(), "wakatime")
+	require.NoError(t, err)
+
+	defer tmpFile.Close()
+
+	v := viper.New()
+	v.Set("config", tmpFile.Name())
+	v.Set("internal-config", tmpFile.Name())
+
+	writer, err := ini.NewWriter(v, func(vp *viper.Viper) (string, error) {
+		assert.Equal(t, v, vp)
+		return tmpFile.Name(), nil
+	})
+	require.NoError(t, err)
+
+	err = cmdheartbeat.ResetRateLimit(v)
+	require.NoError(t, err)
+
+	err = writer.File.Reload()
+	require.NoError(t, err)
+
+	lastSentAt := writer.File.Section("internal").Key("heartbeats_last_sent_at").MustTimeFormat(ini.DateFormat)
+
+	assert.WithinDuration(t, time.Now(), lastSentAt, 1*time.Second)
 }
 
 func setupTestServer() (string, *http.ServeMux, func()) {

cmd/offlinesync/offlinesync.go

@@ -27,9 +27,9 @@ func RunWithRateLimiting(v *viper.Viper) (int, error) {
 	paramOffline := params.LoadOfflineParams(v)
 
 	if cmdheartbeat.RateLimited(cmdheartbeat.RateLimitParams{
-		Disabled:         paramOffline.Disabled,
-		LastSentAt:       paramOffline.LastSentAt,
-		RateLimitTimeout: paramOffline.RateLimit,
+		Disabled:   paramOffline.Disabled,
+		LastSentAt: paramOffline.LastSentAt,
+		Timeout:    paramOffline.RateLimit,
 	}) {
 		log.Debugln("skip syncing offline activity to respect rate limit")
 		return exitcode.Success, nil
@@ -152,9 +152,8 @@ func SyncOfflineActivity(v *viper.Viper, queueFilepath string) error {
 		return err
 	}
 
-	err = cmdheartbeat.ResetRateLimit(v)
-	if err != nil {
-		log.Errorln(err)
+	if err := cmdheartbeat.ResetRateLimit(v); err != nil {
+		log.Errorf("failed to reset rate limit: %s", err)
 	}
 
 	return nil