Skip to content

Commit 9b79f98

Browse files
ckshitijckshitij
committed
fix: add missing punctuation in comments and improve logging error handling (linting-issues)
1 parent 3a20630 commit 9b79f98

File tree

4 files changed

+94
-42
lines changed

4 files changed

+94
-42
lines changed

‎pkg/gofr/config/remote_config.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package config
22

3-
// RemoteConfigurable represents any component that can be updated at runtime
3+
// RemoteConfigurable represents any component that can be updated at runtime.
44
type RemoteConfigurable interface {
55
UpdateConfig(config map[string]any)
66
}
77

8-
// RemoteConfiguration represents a runtime config provider
8+
// RemoteConfiguration represents a runtime config provider.
99
type RemoteConfiguration interface {
1010
Register(c RemoteConfigurable)
1111
Start()

‎pkg/gofr/config/remote_config_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"time"
77
)
88

9-
// mockRemoteConfigurable is a mock that records updates for verification
9+
// mockRemoteConfigurable is a mock that records updates for verification.
1010
type mockRemoteConfigurable struct {
1111
mu sync.Mutex
1212
updatedCount int
@@ -16,11 +16,12 @@ type mockRemoteConfigurable struct {
1616
func (m *mockRemoteConfigurable) UpdateConfig(cfg map[string]any) {
1717
m.mu.Lock()
1818
defer m.mu.Unlock()
19+
1920
m.updatedCount++
2021
m.lastConfig = cfg
2122
}
2223

23-
// mockRemoteConfiguration simulates a runtime configuration manager
24+
// mockRemoteConfiguration simulates a runtime configuration manager.
2425
type mockRemoteConfiguration struct {
2526
mu sync.Mutex
2627
registered []RemoteConfigurable
@@ -37,6 +38,7 @@ func newMockRemoteConfiguration() *mockRemoteConfiguration {
3738
func (m *mockRemoteConfiguration) Register(c RemoteConfigurable) {
3839
m.mu.Lock()
3940
defer m.mu.Unlock()
41+
4042
m.registered = append(m.registered, c)
4143
}
4244

@@ -48,9 +50,11 @@ func (m *mockRemoteConfiguration) Start() {
4850
go func() {
4951
for cfg := range m.startTrigger {
5052
m.mu.Lock()
53+
5154
for _, r := range m.registered {
5255
r.UpdateConfig(cfg)
5356
}
57+
5458
m.mu.Unlock()
5559
}
5660
}()
@@ -90,14 +94,14 @@ func TestRemoteConfiguration_RegisterAndStart(t *testing.T) {
9094
}
9195
}
9296

93-
func TestRemoteConfiguration_NoRegisteredComponents(t *testing.T) {
97+
func TestRemoteConfiguration_NoRegisteredComponents(_ *testing.T) {
9498
rc := newMockRemoteConfiguration()
9599
rc.Start()
96100

97101
cfg := map[string]any{"feature": "enabled"}
98102
rc.pushConfig(cfg)
99103

100-
// Should not panic even if no components are registered
104+
// Should not panic even if no components are registered.
101105
time.Sleep(20 * time.Millisecond)
102106
}
103107

‎pkg/gofr/logging/remotelogger/dynamic_level_logger.go‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,10 @@ func New(level logging.Level, remoteConfigURL string, loggerFetchInterval time.D
122122
}
123123

124124
if remoteConfigURL != "" {
125-
config := NewHTTPRemoteConfig(remoteConfigURL, loggerFetchInterval, l.Logger)
126-
config.Register(l)
127-
go config.Start()
125+
cfg := NewHTTPRemoteConfig(remoteConfigURL, loggerFetchInterval, l.Logger)
126+
cfg.Register(l)
127+
128+
go cfg.Start()
128129
}
129130

130131
return l
@@ -138,12 +139,13 @@ type remoteLogger struct {
138139
logging.Logger
139140
}
140141

141-
// RemoteConfigurable implementation
142-
func (r *remoteLogger) UpdateConfig(config map[string]any) {
143-
if levelStr, ok := config["logLevel"].(string); ok {
142+
// UpdateConfig implements the config.RemoteConfigurable interface and updates the log level based on the provided configuration.
143+
func (r *remoteLogger) UpdateConfig(cfg map[string]any) {
144+
if levelStr, ok := cfg["logLevel"].(string); ok {
144145
newLevel := logging.GetLevelFromString(levelStr)
145146

146147
r.mu.Lock()
148+
147149
if r.currentLevel != newLevel {
148150
oldLevel := r.currentLevel
149151
r.currentLevel = newLevel
@@ -241,6 +243,7 @@ func fetchAndUpdateLogLevel(remoteService service.HTTP, currentLevel logging.Lev
241243
} else if newLogLevelStr != "" {
242244
return logging.GetLevelFromString(newLogLevelStr), nil
243245
}
246+
244247
return currentLevel, nil
245248
}
246249

@@ -324,8 +327,10 @@ func (h *httpRemoteConfig) Start() {
324327

325328
// then periodically
326329
ticker := time.NewTicker(h.interval)
330+
327331
go func() {
328332
defer ticker.Stop()
333+
329334
for range ticker.C {
330335
checkAndUpdate()
331336
}
@@ -334,11 +339,12 @@ func (h *httpRemoteConfig) Start() {
334339

335340
func fetchRemoteConfig(remoteService service.HTTP) (map[string]any, error) {
336341
if newLogLevelStr, err := fetchLogLevelStr(remoteService); err != nil {
337-
return map[string]any{}, nil
342+
return map[string]any{}, err
338343
} else if newLogLevelStr != "" {
339344
return map[string]any{
340345
"logLevel": newLogLevelStr,
341346
}, nil
342347
}
348+
343349
return map[string]any{}, nil
344350
}

‎pkg/gofr/logging/remotelogger/remotelogger_test.go‎

Lines changed: 71 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package remotelogger
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"net/http"
67
"net/http/httptest"
8+
"strings"
79
"sync"
810
"sync/atomic"
911
"testing"
@@ -12,43 +14,58 @@ import (
1214
"gofr.dev/pkg/gofr/logging"
1315
)
1416

15-
// Mock Logger (implements logging.Logger)
17+
// Mock Logger (implements logging.Logger).
1618
type mockLogger struct {
1719
mu sync.Mutex
1820
messages []string
1921
level logging.Level
2022
changeCnt int32
2123
}
2224

23-
func (m *mockLogger) record(prefix string, _ ...any) {
25+
func (m *mockLogger) record(prefix string, parts ...any) {
2426
m.mu.Lock()
2527
defer m.mu.Unlock()
26-
m.messages = append(m.messages, prefix)
27-
}
28-
29-
func (m *mockLogger) Debug(args ...any) { m.record("Debug", args...) }
30-
func (m *mockLogger) Debugf(format string, args ...any) { m.record("Debugf", args...) }
31-
func (m *mockLogger) Log(args ...any) { m.record("Log", args...) }
32-
func (m *mockLogger) Logf(format string, args ...any) { m.record("Logf", args...) }
33-
func (m *mockLogger) Info(args ...any) { m.record("Info", args...) }
34-
func (m *mockLogger) Infof(format string, args ...any) { m.record("Infof", args...) }
35-
func (m *mockLogger) Notice(args ...any) { m.record("Notice", args...) }
36-
func (m *mockLogger) Noticef(format string, args ...any) { m.record("Noticef", args...) }
37-
func (m *mockLogger) Warn(args ...any) { m.record("Warn", args...) }
38-
func (m *mockLogger) Warnf(format string, args ...any) { m.record("Warnf", args...) }
39-
func (m *mockLogger) Error(args ...any) { m.record("Error", args...) }
40-
func (m *mockLogger) Errorf(format string, args ...any) { m.record("Errorf", args...) }
41-
func (m *mockLogger) Fatal(args ...any) { m.record("Fatal", args...) }
42-
func (m *mockLogger) Fatalf(format string, args ...any) { m.record("Fatalf", args...) }
28+
29+
msg := fmt.Sprint(append([]any{prefix, ":"}, parts...)...)
30+
m.messages = append(m.messages, msg)
31+
}
32+
33+
func (m *mockLogger) Debug(args ...any) { m.record("Debug", args...) }
34+
func (m *mockLogger) Debugf(format string, args ...any) {
35+
m.record("Debugf", fmt.Sprintf(format, args...))
36+
}
37+
func (m *mockLogger) Log(args ...any) { m.record("Log", args...) }
38+
func (m *mockLogger) Logf(format string, args ...any) { m.record("Logf", fmt.Sprintf(format, args...)) }
39+
func (m *mockLogger) Info(args ...any) { m.record("Info", args...) }
40+
func (m *mockLogger) Infof(format string, args ...any) {
41+
m.record("Infof", fmt.Sprintf(format, args...))
42+
}
43+
func (m *mockLogger) Notice(args ...any) { m.record("Notice", args...) }
44+
func (m *mockLogger) Noticef(format string, args ...any) {
45+
m.record("Noticef", fmt.Sprintf(format, args...))
46+
}
47+
func (m *mockLogger) Warn(args ...any) { m.record("Warn", args...) }
48+
func (m *mockLogger) Warnf(format string, args ...any) {
49+
m.record("Warnf", fmt.Sprintf(format, args...))
50+
}
51+
func (m *mockLogger) Error(args ...any) { m.record("Error", args...) }
52+
func (m *mockLogger) Errorf(format string, args ...any) {
53+
m.record("Errorf", fmt.Sprintf(format, args...))
54+
}
55+
func (m *mockLogger) Fatal(args ...any) { m.record("Fatal", args...) }
56+
func (m *mockLogger) Fatalf(format string, args ...any) {
57+
m.record("Fatalf", fmt.Sprintf(format, args...))
58+
}
4359

4460
func (m *mockLogger) ChangeLevel(level logging.Level) {
4561
m.mu.Lock()
4662
defer m.mu.Unlock()
63+
4764
m.level = level
4865
atomic.AddInt32(&m.changeCnt, 1)
4966
}
5067

51-
// Mock RemoteConfigurable Client
68+
// Mock RemoteConfigurable Client.
5269
type mockClient struct {
5370
updateCalled int32
5471
lastConfig map[string]any
@@ -59,10 +76,10 @@ func (m *mockClient) UpdateConfig(cfg map[string]any) {
5976
m.lastConfig = cfg
6077
}
6178

62-
// Tests for httpRemoteConfig
79+
// Tests for httpRemoteConfig.
6380
func TestHttpRemoteConfig_RegisterAndStart(t *testing.T) {
6481
// mock HTTP server returning valid JSON
65-
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
82+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
6683
resp := map[string]any{
6784
"data": map[string]string{
6885
"serviceName": "test-service",
@@ -71,6 +88,7 @@ func TestHttpRemoteConfig_RegisterAndStart(t *testing.T) {
7188
}
7289
_ = json.NewEncoder(w).Encode(resp)
7390
})
91+
7492
server := httptest.NewServer(handler)
7593
defer server.Close()
7694

@@ -94,27 +112,51 @@ func TestHttpRemoteConfig_RegisterAndStart(t *testing.T) {
94112
}
95113

96114
func TestHttpRemoteConfig_InvalidJSON(t *testing.T) {
115+
t.Parallel()
116+
97117
// mock server returning invalid JSON
98-
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
99-
w.Write([]byte(`invalid-json`))
118+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
119+
_, _ = w.Write([]byte(`invalid-json`))
100120
})
121+
101122
server := httptest.NewServer(handler)
102123
defer server.Close()
103124

104125
logger := &mockLogger{}
105126
cfg := NewHTTPRemoteConfig(server.URL, 50*time.Millisecond, logger).(*httpRemoteConfig)
106127
client := &mockClient{}
107-
cfg.Register(client)
108128

129+
cfg.Register(client)
109130
cfg.Start()
110-
time.Sleep(120 * time.Millisecond)
111131

112-
if atomic.LoadInt32(&client.updateCalled) == 0 {
113-
t.Errorf("expected UpdateConfig to be called at least once even if response invalid")
132+
// Wait for a couple of polling intervals
133+
time.Sleep(200 * time.Millisecond)
134+
135+
updateCount := atomic.LoadInt32(&client.updateCalled)
136+
137+
if updateCount != 0 {
138+
t.Errorf("expected UpdateConfig not to be called on invalid JSON, but got %d calls", updateCount)
139+
}
140+
141+
// Check that an error log was recorded for invalid JSON
142+
logger.mu.Lock()
143+
defer logger.mu.Unlock()
144+
145+
foundErrorLog := false
146+
147+
for _, msg := range logger.messages {
148+
if strings.Contains(msg, "invalid") || strings.Contains(strings.ToLower(msg), "error") {
149+
foundErrorLog = true
150+
break
151+
}
152+
}
153+
154+
if !foundErrorLog {
155+
t.Errorf("expected error log message for invalid JSON response, got logs: %v", logger.messages)
114156
}
115157
}
116158

117-
// Tests for remoteLogger (RemoteConfigurable)
159+
// Tests for remoteLogger (RemoteConfigurable).
118160
func TestRemoteLogger_UpdateConfig_ChangesLevel(t *testing.T) {
119161
logger := &mockLogger{}
120162
r := &remoteLogger{

0 commit comments

Comments
 (0)