Skip to content

Commit bc73e42

Browse files
committed
fix(api): fence offline alerts with heartbeat versions
Revalidate running ownership and history before offline detection and restart. Atomically record online arrivals and guard offline fanout with heartbeat-version CAS. Keep version writes independent of check leases and preserve fail-open heartbeat storage. Cover delayed delivery, lease takeover, concurrent arrivals, and version-name collisions. Narrowly suppress the Semgrep transport warning for the bufconn-only test server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f27d8f52-9620-4ee2-a564-ec4870a5f840
1 parent 9763037 commit bc73e42

15 files changed

Lines changed: 746 additions & 38 deletions

‎api/pkg/di/container.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import (
6464
"github.com/gofiber/fiber/v3/middleware/cors"
6565

6666
"github.com/NdoleStudio/httpsms/pkg/entities"
67+
"github.com/NdoleStudio/httpsms/pkg/events"
6768
"github.com/NdoleStudio/httpsms/pkg/listeners"
6869
"github.com/NdoleStudio/httpsms/pkg/repositories"
6970
"github.com/NdoleStudio/httpsms/pkg/services"
@@ -1501,11 +1502,12 @@ func (container *Container) RegisterNotificationListeners() {
15011502
// RegisterHeartbeatListeners registers event listeners for listeners.HeartbeatListener
15021503
func (container *Container) RegisterHeartbeatListeners() {
15031504
container.logger.Debug(fmt.Sprintf("registering listners for %T", listeners.HeartbeatListener{}))
1504-
_, routes := listeners.NewHeartbeatListener(
1505+
heartbeatListener, routes := listeners.NewHeartbeatListener(
15051506
container.Logger(),
15061507
container.Tracer(),
15071508
container.HeartbeatService(),
15081509
)
1510+
container.EventDispatcher().SubscribeFilter(events.EventTypePhoneHeartbeatOffline, heartbeatListener.FilterOfflineEvent)
15091511

15101512
for event, handler := range routes {
15111513
container.EventDispatcher().Subscribe(event, handler)

‎api/pkg/entities/heartbeat_monitor.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ type HeartbeatMonitor struct {
1616
PhoneID uuid.UUID `json:"phone_id" bson:"phone_id" example:"32343a19-da5e-4b1b-a767-3298a73703cb"`
1717
UserID UserID `json:"user_id" bson:"user_id" example:"WB7DRDWrJZRGbYrv2CKGkqbzvqdC"`
1818
// QueueID is the current check ownership marker, not the external task queue's ID.
19-
QueueID string `json:"queue_id" bson:"queue_id" example:"0360259236613675274"`
20-
Owner string `json:"owner" bson:"owner" example:"+18005550199"`
21-
PhoneOnline bool `json:"phone_online" bson:"phone_online" example:"true" default:"true"`
22-
CreatedAt time.Time `json:"created_at" bson:"created_at" example:"2022-06-05T14:26:02.302718+03:00"`
23-
UpdatedAt time.Time `json:"updated_at" bson:"updated_at" example:"2022-06-05T14:26:10.303278+03:00"`
19+
QueueID string `json:"queue_id" bson:"queue_id" example:"0360259236613675274"`
20+
// LastHeartbeatID fences delayed offline transitions independently of check leases.
21+
LastHeartbeatID uuid.UUID `json:"-" bson:"last_heartbeat_id,omitempty" gorm:"type:uuid"`
22+
Owner string `json:"owner" bson:"owner" example:"+18005550199"`
23+
PhoneOnline bool `json:"phone_online" bson:"phone_online" example:"true" default:"true"`
24+
CreatedAt time.Time `json:"created_at" bson:"created_at" example:"2022-06-05T14:26:02.302718+03:00"`
25+
UpdatedAt time.Time `json:"updated_at" bson:"updated_at" example:"2022-06-05T14:26:10.303278+03:00"`
2426
}
2527

2628
// RequiresCheck returns true if the heartbeat monitor requires a check

‎api/pkg/events/listeners.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,6 @@ import (
88

99
// EventListener is the type for processing events
1010
type EventListener func(ctx context.Context, event cloudevents.Event) error
11+
12+
// EventFilter decides whether an event may reach any subscribers.
13+
type EventFilter func(ctx context.Context, event cloudevents.Event) (bool, error)

‎api/pkg/events/phone_heartbeat_offline_event.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const EventTypePhoneHeartbeatOffline = "phone.heartbeat.offline"
1313
// PhoneHeartbeatOfflinePayload is the payload of the EventTypePhoneHeartbeatOffline event
1414
type PhoneHeartbeatOfflinePayload struct {
1515
PhoneID uuid.UUID `json:"phone_id"`
16+
HeartbeatID uuid.UUID `json:"heartbeat_id,omitempty"`
1617
UserID entities.UserID `json:"user_id"`
1718
LastHeartbeatTimestamp time.Time `json:"last_heartbeat_timestamp"`
1819
Timestamp time.Time `json:"timestamp"`

‎api/pkg/listeners/heartbeat_listener.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,10 @@ func NewHeartbeatListener(
3333
}
3434

3535
return l, map[string]events.EventListener{
36-
events.EventTypePhoneUpdated: l.onPhoneUpdated,
37-
events.EventTypePhoneDeleted: l.onPhoneDeleted,
38-
events.EventTypePhoneHeartbeatCheck: l.onPhoneHeartbeatCheck,
39-
events.EventTypePhoneHeartbeatOffline: l.onPhoneHeartbeatOffline,
40-
events.UserAccountDeleted: l.onUserAccountDeleted,
36+
events.EventTypePhoneUpdated: l.onPhoneUpdated,
37+
events.EventTypePhoneDeleted: l.onPhoneDeleted,
38+
events.EventTypePhoneHeartbeatCheck: l.onPhoneHeartbeatCheck,
39+
events.UserAccountDeleted: l.onUserAccountDeleted,
4140
}
4241
}
4342

@@ -108,21 +107,22 @@ func (listener *HeartbeatListener) onPhoneHeartbeatCheck(ctx context.Context, ev
108107
return nil
109108
}
110109

111-
// onPhoneDeleted handles the events.EventTypePhoneDeleted event
112-
func (listener *HeartbeatListener) onPhoneHeartbeatOffline(ctx context.Context, event cloudevents.Event) error {
110+
// FilterOfflineEvent validates and applies the transition before notification subscribers run.
111+
func (listener *HeartbeatListener) FilterOfflineEvent(ctx context.Context, event cloudevents.Event) (bool, error) {
113112
ctx, span := listener.tracer.Start(ctx)
114113
defer span.End()
115114

116115
var payload events.PhoneHeartbeatOfflinePayload
117116
if err := event.DataAs(&payload); err != nil {
118-
return listener.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot decode [%s] into [%T]", event.Data(), payload))
117+
return false, listener.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot decode [%s] into [%T]", event.Data(), payload))
119118
}
120119

121-
if err := listener.service.UpdatePhoneOnline(ctx, payload.UserID, payload.MonitorID, false); err != nil {
122-
return listener.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot delete heartbeat monitor with userID [%s] and owner [%s] for event with ID [%s]", payload.UserID, payload.Owner, event.ID()))
120+
allowed, err := listener.service.ApplyOfflineHeartbeat(ctx, &payload)
121+
if err != nil {
122+
return false, listener.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot validate offline heartbeat for monitor [%s]", payload.MonitorID))
123123
}
124124

125-
return nil
125+
return allowed, nil
126126
}
127127

128128
func (listener *HeartbeatListener) onUserAccountDeleted(ctx context.Context, event cloudevents.Event) error {
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
package listeners
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"sync/atomic"
7+
"testing"
8+
"time"
9+
10+
"github.com/NdoleStudio/httpsms/pkg/entities"
11+
"github.com/NdoleStudio/httpsms/pkg/events"
12+
"github.com/NdoleStudio/httpsms/pkg/repositories"
13+
"github.com/NdoleStudio/httpsms/pkg/services"
14+
"github.com/NdoleStudio/httpsms/pkg/telemetry"
15+
"github.com/NdoleStudio/stacktrace"
16+
cloudevents "github.com/cloudevents/sdk-go/v2"
17+
"github.com/google/uuid"
18+
"github.com/stretchr/testify/require"
19+
"go.opentelemetry.io/otel/metric/noop"
20+
)
21+
22+
type offlineHeartbeatRepository struct {
23+
repositories.HeartbeatRepository
24+
heartbeat *entities.Heartbeat
25+
err error
26+
}
27+
28+
func (repository *offlineHeartbeatRepository) Last(context.Context, entities.UserID, string) (*entities.Heartbeat, error) {
29+
return repository.heartbeat, repository.err
30+
}
31+
32+
func (repository *offlineHeartbeatRepository) Store(_ context.Context, heartbeat *entities.Heartbeat) error {
33+
repository.heartbeat = heartbeat
34+
return nil
35+
}
36+
37+
type offlineMonitorRepository struct {
38+
repositories.HeartbeatMonitorRepository
39+
monitor *entities.HeartbeatMonitor
40+
lastHeartbeatID uuid.UUID
41+
beforeOffline func()
42+
}
43+
44+
func (repository *offlineMonitorRepository) Load(context.Context, entities.UserID, string) (*entities.HeartbeatMonitor, error) {
45+
monitor := *repository.monitor
46+
monitor.LastHeartbeatID = repository.lastHeartbeatID
47+
return &monitor, nil
48+
}
49+
50+
func (repository *offlineMonitorRepository) CompareAndSwapQueueID(_ context.Context, _ uuid.UUID, expected, next string) (bool, error) {
51+
if repository.monitor.QueueID != expected {
52+
return false, nil
53+
}
54+
repository.monitor.QueueID = next
55+
repository.monitor.UpdatedAt = time.Now().UTC()
56+
return true, nil
57+
}
58+
59+
func (repository *offlineMonitorRepository) RecordHeartbeat(_ context.Context, _ entities.UserID, _ uuid.UUID, expected, heartbeatID uuid.UUID) (*entities.HeartbeatMonitor, error) {
60+
if repository.lastHeartbeatID != expected {
61+
return nil, nil
62+
}
63+
previous := *repository.monitor
64+
previous.LastHeartbeatID = repository.lastHeartbeatID
65+
repository.lastHeartbeatID = heartbeatID
66+
repository.monitor.PhoneOnline = true
67+
return &previous, nil
68+
}
69+
70+
func (repository *offlineMonitorRepository) UpdatePhoneOfflineForHeartbeat(_ context.Context, _ entities.UserID, _ uuid.UUID, expected, heartbeatID uuid.UUID) (bool, error) {
71+
repository.runOfflineHook()
72+
if repository.lastHeartbeatID != expected {
73+
return false, nil
74+
}
75+
repository.monitor.PhoneOnline = false
76+
if heartbeatID != uuid.Nil {
77+
repository.lastHeartbeatID = heartbeatID
78+
}
79+
return true, nil
80+
}
81+
82+
func (repository *offlineMonitorRepository) UpdatePhoneOnline(_ context.Context, _ entities.UserID, _ uuid.UUID, online bool) error {
83+
if !online {
84+
repository.runOfflineHook()
85+
}
86+
repository.monitor.PhoneOnline = online
87+
return nil
88+
}
89+
90+
func (repository *offlineMonitorRepository) runOfflineHook() {
91+
if repository.beforeOffline != nil {
92+
hook := repository.beforeOffline
93+
repository.beforeOffline = nil
94+
hook()
95+
}
96+
}
97+
98+
type offlineEventQueue struct{ events []cloudevents.Event }
99+
100+
func (queue *offlineEventQueue) Enqueue(_ context.Context, task *services.PushQueueTask, _ time.Duration) (string, error) {
101+
var event cloudevents.Event
102+
if err := json.Unmarshal(task.Body, &event); err != nil {
103+
return "", err
104+
}
105+
queue.events = append(queue.events, event)
106+
return event.ID(), nil
107+
}
108+
109+
func TestHeartbeatOfflineDelivery_RejectsNewerHeartbeatBeforeSubscribers(t *testing.T) {
110+
for _, arrival := range []string{"before delivery", "between history and status update", "history read fails", "history expired", "no newer heartbeat", "legacy event"} {
111+
t.Run(arrival, func(t *testing.T) {
112+
logger := &noopListenerLogger{}
113+
tracer := telemetry.NewOtelLogger("test", logger)
114+
meter, err := noop.NewMeterProvider().Meter("test").Float64Histogram("dispatch")
115+
require.NoError(t, err)
116+
queue := &offlineEventQueue{}
117+
dispatcher := services.NewEventDispatcher(logger, tracer, meter, queue, services.PushQueueConfig{})
118+
history := &offlineHeartbeatRepository{heartbeat: &entities.Heartbeat{
119+
ID: uuid.New(), Timestamp: time.Now().UTC().Add(-65 * time.Minute),
120+
}}
121+
monitor := &offlineMonitorRepository{monitor: &entities.HeartbeatMonitor{
122+
ID: uuid.New(), PhoneID: uuid.New(), UserID: "user-1", Owner: "+18005550199",
123+
PhoneOnline: true, QueueID: "check:current-check", UpdatedAt: time.Now().UTC(),
124+
}, lastHeartbeatID: history.heartbeat.ID}
125+
service := services.NewHeartbeatService(logger, tracer, history, monitor, dispatcher)
126+
heartbeatListener, routes := NewHeartbeatListener(logger, tracer, service)
127+
dispatcher.SubscribeFilter(events.EventTypePhoneHeartbeatOffline, heartbeatListener.FilterOfflineEvent)
128+
for eventType, listener := range routes {
129+
dispatcher.Subscribe(eventType, listener)
130+
}
131+
var notifications atomic.Int32
132+
dispatcher.Subscribe(events.EventTypePhoneHeartbeatOffline, func(context.Context, cloudevents.Event) error {
133+
notifications.Add(1)
134+
return nil
135+
})
136+
require.NoError(t, service.Monitor(context.Background(), &services.HeartbeatMonitorParams{
137+
UserID: monitor.monitor.UserID, Owner: monitor.monitor.Owner, MonitorID: monitor.monitor.ID,
138+
Source: "https://example.com", CheckID: "current-check",
139+
}))
140+
require.Len(t, queue.events, 3)
141+
event := queue.events[1]
142+
storeHeartbeat := func() {
143+
_, err := service.Store(context.Background(), services.HeartbeatStoreParams{
144+
UserID: monitor.monitor.UserID, Owner: monitor.monitor.Owner,
145+
Source: "https://example.com", Timestamp: time.Now().UTC(),
146+
})
147+
require.NoError(t, err)
148+
}
149+
switch arrival {
150+
case "before delivery":
151+
storeHeartbeat()
152+
case "between history and status update":
153+
monitor.beforeOffline = storeHeartbeat
154+
case "history read fails":
155+
history.err = stacktrace.NewError("history unavailable")
156+
case "history expired":
157+
history.err = stacktrace.NewErrorWithCode(repositories.ErrCodeNotFound, "history expired")
158+
case "legacy event":
159+
var data map[string]any
160+
require.NoError(t, event.DataAs(&data))
161+
delete(data, "heartbeat_id")
162+
require.NoError(t, event.SetData(cloudevents.ApplicationJSON, data))
163+
monitor.lastHeartbeatID = uuid.Nil
164+
}
165+
err = dispatcher.DispatchSync(context.Background(), event)
166+
if arrival == "history read fails" {
167+
require.Error(t, err)
168+
} else {
169+
require.NoError(t, err)
170+
}
171+
if arrival == "no newer heartbeat" || arrival == "legacy event" {
172+
require.EqualValues(t, 1, notifications.Load())
173+
require.False(t, monitor.monitor.PhoneOnline)
174+
} else {
175+
require.Zero(t, notifications.Load(), "stale or unverifiable events must not reach email/webhook subscribers")
176+
require.True(t, monitor.monitor.PhoneOnline)
177+
}
178+
})
179+
}
180+
}

‎api/pkg/repositories/gorm_heartbeat_monitor_repository.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/NdoleStudio/httpsms/pkg/telemetry"
1414
"github.com/NdoleStudio/stacktrace"
1515
"gorm.io/gorm"
16+
"gorm.io/gorm/clause"
1617
)
1718

1819
// gormHeartbeatRepository is responsible for persisting entities.Heartbeat
@@ -127,6 +128,63 @@ func (repository *gormHeartbeatMonitorRepository) CompareAndSwapQueueID(ctx cont
127128
return result.RowsAffected == 1, nil
128129
}
129130

131+
func (repository *gormHeartbeatMonitorRepository) RecordHeartbeat(ctx context.Context, userID entities.UserID, monitorID, expected, heartbeatID uuid.UUID) (*entities.HeartbeatMonitor, error) {
132+
ctx, span := repository.tracer.Start(ctx)
133+
defer span.End()
134+
ctx, cancel := context.WithTimeout(ctx, dbOperationDuration)
135+
defer cancel()
136+
137+
var previous *entities.HeartbeatMonitor
138+
err := repository.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
139+
monitor := new(entities.HeartbeatMonitor)
140+
query := heartbeatMonitorWithVersion(tx.Where("id = ?", monitorID).Where("user_id = ?", userID), expected)
141+
if err := query.Clauses(clause.Locking{Strength: "UPDATE"}).First(monitor).Error; err != nil {
142+
if errors.Is(err, gorm.ErrRecordNotFound) {
143+
return nil
144+
}
145+
return stacktrace.Propagatef(err, "cannot lock heartbeat version for monitor [%s]", monitorID)
146+
}
147+
before := *monitor
148+
if err := tx.Model(monitor).UpdateColumns(map[string]any{
149+
"last_heartbeat_id": heartbeatID,
150+
"phone_online": true,
151+
}).Error; err != nil {
152+
return stacktrace.Propagatef(err, "cannot record heartbeat version for monitor [%s]", monitorID)
153+
}
154+
previous = &before
155+
return nil
156+
})
157+
if err != nil {
158+
return nil, repository.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot mark heartbeat monitor [%s] online", monitorID))
159+
}
160+
return previous, nil
161+
}
162+
163+
func (repository *gormHeartbeatMonitorRepository) UpdatePhoneOfflineForHeartbeat(ctx context.Context, userID entities.UserID, monitorID, expected, heartbeatID uuid.UUID) (bool, error) {
164+
ctx, span := repository.tracer.Start(ctx)
165+
defer span.End()
166+
ctx, cancel := context.WithTimeout(ctx, dbOperationDuration)
167+
defer cancel()
168+
169+
query := heartbeatMonitorWithVersion(repository.db.WithContext(ctx).
170+
Model(&entities.HeartbeatMonitor{}).Where("id = ?", monitorID).Where("user_id = ?", userID), expected)
171+
result := query.UpdateColumns(map[string]any{
172+
"last_heartbeat_id": heartbeatID,
173+
"phone_online": false,
174+
})
175+
if result.Error != nil {
176+
return false, repository.tracer.WrapErrorSpan(span, stacktrace.Propagatef(result.Error, "cannot apply offline heartbeat version for monitor [%s]", monitorID))
177+
}
178+
return result.RowsAffected == 1, nil
179+
}
180+
181+
func heartbeatMonitorWithVersion(query *gorm.DB, heartbeatID uuid.UUID) *gorm.DB {
182+
if heartbeatID == uuid.Nil {
183+
return query.Where("last_heartbeat_id IS NULL OR last_heartbeat_id = ?", uuid.Nil)
184+
}
185+
return query.Where("last_heartbeat_id = ?", heartbeatID)
186+
}
187+
130188
// Index entities.Message between 2 parties
131189
func (repository *gormHeartbeatMonitorRepository) Index(ctx context.Context, userID entities.UserID, owner string, params IndexParams) (*[]entities.Heartbeat, error) {
132190
ctx, span := repository.tracer.Start(ctx)

0 commit comments

Comments
 (0)