Skip to content

Commit 011b276

Browse files
ethanndicksonethanndickson
committed
review
1 parent 6b9b1be commit 011b276

File tree

7 files changed

+130
-68
lines changed

7 files changed

+130
-68
lines changed

‎coderd/agentapi/connectionlog_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestConnectionLog(t *testing.T) {
105105
t.Run(tt.name, func(t *testing.T) {
106106
t.Parallel()
107107

108-
connLogger := connectionlog.NewMock()
108+
connLogger := connectionlog.NewFake()
109109

110110
mDB := dbmock.NewMockStore(gomock.NewController(t))
111111
mDB.EXPECT().GetWorkspaceByAgentID(gomock.Any(), agent.ID).Return(workspace, nil)

‎coderd/connectionlog/connectionlog.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,28 @@ func (nop) Upsert(context.Context, database.UpsertConnectionLogParams) error {
2424
return nil
2525
}
2626

27-
func NewMock() *MockConnectionLogger {
28-
return &MockConnectionLogger{}
27+
func NewFake() *FakeConnectionLogger {
28+
return &FakeConnectionLogger{}
2929
}
3030

31-
type MockConnectionLogger struct {
31+
type FakeConnectionLogger struct {
3232
mu sync.Mutex
3333
upsertions []database.UpsertConnectionLogParams
3434
}
3535

36-
func (m *MockConnectionLogger) Reset() {
36+
func (m *FakeConnectionLogger) Reset() {
3737
m.mu.Lock()
3838
defer m.mu.Unlock()
3939
m.upsertions = make([]database.UpsertConnectionLogParams, 0)
4040
}
4141

42-
func (m *MockConnectionLogger) ConnectionLogs() []database.UpsertConnectionLogParams {
42+
func (m *FakeConnectionLogger) ConnectionLogs() []database.UpsertConnectionLogParams {
4343
m.mu.Lock()
4444
defer m.mu.Unlock()
4545
return m.upsertions
4646
}
4747

48-
func (m *MockConnectionLogger) Upsert(_ context.Context, clog database.UpsertConnectionLogParams) error {
48+
func (m *FakeConnectionLogger) Upsert(_ context.Context, clog database.UpsertConnectionLogParams) error {
4949
m.mu.Lock()
5050
defer m.mu.Unlock()
5151

@@ -54,7 +54,7 @@ func (m *MockConnectionLogger) Upsert(_ context.Context, clog database.UpsertCon
5454
return nil
5555
}
5656

57-
func (m *MockConnectionLogger) Contains(t testing.TB, expected database.UpsertConnectionLogParams) bool {
57+
func (m *FakeConnectionLogger) Contains(t testing.TB, expected database.UpsertConnectionLogParams) bool {
5858
m.mu.Lock()
5959
defer m.mu.Unlock()
6060
for idx, cl := range m.upsertions {

‎coderd/database/dbmem/dbmem.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12291,6 +12291,11 @@ func (q *FakeQuerier) UpsertConnectionLog(_ context.Context, arg database.Upsert
1229112291
}
1229212292
}
1229312293

12294+
var closeTime sql.NullTime
12295+
if arg.ConnectionStatus == database.ConnectionStatusDisconnected {
12296+
closeTime = sql.NullTime{Valid: true, Time: arg.Time}
12297+
}
12298+
1229412299
log := database.ConnectionLog{
1229512300
ID: arg.ID,
1229612301
Time: arg.Time,
@@ -12307,6 +12312,7 @@ func (q *FakeQuerier) UpsertConnectionLog(_ context.Context, arg database.Upsert
1230712312
SlugOrPort: arg.SlugOrPort,
1230812313
ConnectionID: arg.ConnectionID,
1230912314
CloseReason: arg.CloseReason,
12315+
CloseTime: closeTime,
1231012316
}
1231112317

1231212318
q.connectionLogs = append(q.connectionLogs, log)

‎coderd/database/querier_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2369,6 +2369,46 @@ func TestUpsertConnectionLog(t *testing.T) {
23692369
require.Equal(t, int64(1), rows[0].Count)
23702370
require.Equal(t, log, rows[0].ConnectionLog)
23712371
})
2372+
2373+
t.Run("NoConnect", func(t *testing.T) {
2374+
t.Parallel()
2375+
2376+
db, _ := dbtestutil.NewDB(t)
2377+
ctx := context.Background()
2378+
2379+
ws := createWorkspace(t, db)
2380+
2381+
connectionID := uuid.New()
2382+
agentName := "test-agent"
2383+
2384+
// Insert just a 'disconect' event
2385+
disconnectTime := dbtime.Now()
2386+
connectParams := database.UpsertConnectionLogParams{
2387+
ID: uuid.New(),
2388+
Time: disconnectTime,
2389+
OrganizationID: ws.OrganizationID,
2390+
WorkspaceOwnerID: ws.OwnerID,
2391+
WorkspaceID: ws.ID,
2392+
WorkspaceName: ws.Name,
2393+
AgentName: agentName,
2394+
Type: database.ConnectionTypeSsh,
2395+
ConnectionID: uuid.NullUUID{UUID: connectionID, Valid: true},
2396+
ConnectionStatus: database.ConnectionStatusDisconnected,
2397+
}
2398+
2399+
_, err := db.UpsertConnectionLog(ctx, connectParams)
2400+
require.NoError(t, err)
2401+
2402+
rows, err := db.GetConnectionLogsOffset(ctx, database.GetConnectionLogsOffsetParams{})
2403+
require.NoError(t, err)
2404+
require.Len(t, rows, 1)
2405+
2406+
// We expect the connection event to be marked as closed with the start
2407+
// and close time being the same.
2408+
require.True(t, rows[0].ConnectionLog.CloseTime.Valid)
2409+
require.Equal(t, disconnectTime, rows[0].ConnectionLog.CloseTime.Time.UTC())
2410+
require.Equal(t, rows[0].ConnectionLog.Time.UTC(), rows[0].ConnectionLog.CloseTime.Time.UTC())
2411+
})
23722412
}
23732413

23742414
type tvArgs struct {

‎coderd/database/queries.sql.go

Lines changed: 28 additions & 20 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/connectionlogs.sql

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,25 @@
22
SELECT
33
sqlc.embed(connection_logs),
44
workspaces.deleted AS workspace_deleted,
5-
-- sqlc.embed(users) would be nice but it does not seem to play well with
6-
-- left joins. This user metadata is necessary for parity with the audit logs
5+
-- sqlc.embed(users) would be nice but it does not seem to play well with
6+
-- left joins. This user metadata is necessary for parity with the audit logs
77
-- API.
8-
users.username AS user_username,
9-
users.name AS user_name,
10-
users.email AS user_email,
11-
users.created_at AS user_created_at,
12-
users.updated_at AS user_updated_at,
13-
users.last_seen_at AS user_last_seen_at,
14-
users.status AS user_status,
15-
users.login_type AS user_login_type,
16-
users.rbac_roles AS user_roles,
17-
users.avatar_url AS user_avatar_url,
18-
users.deleted AS user_deleted,
19-
users.quiet_hours_schedule AS user_quiet_hours_schedule,
8+
users.username AS user_username,
9+
users.name AS user_name,
10+
users.email AS user_email,
11+
users.created_at AS user_created_at,
12+
users.updated_at AS user_updated_at,
13+
users.last_seen_at AS user_last_seen_at,
14+
users.status AS user_status,
15+
users.login_type AS user_login_type,
16+
users.rbac_roles AS user_roles,
17+
users.avatar_url AS user_avatar_url,
18+
users.deleted AS user_deleted,
19+
users.quiet_hours_schedule AS user_quiet_hours_schedule,
2020
workspace_owner.username AS workspace_owner_username,
21-
organizations.name as organization_name,
22-
organizations.display_name as organization_display_name,
23-
organizations.icon as organization_icon,
21+
organizations.name AS organization_name,
22+
organizations.display_name AS organization_display_name,
23+
organizations.icon AS organization_icon,
2424
COUNT(connection_logs.*) OVER () AS count
2525
FROM
2626
connection_logs
@@ -63,9 +63,17 @@ INSERT INTO connection_logs (
6363
user_id,
6464
slug_or_port,
6565
connection_id,
66-
close_reason
66+
close_reason,
67+
close_time
6768
) VALUES
68-
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15)
69+
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15,
70+
-- If we've only received a disconnect event, mark the event as immediately
71+
-- closed.
72+
CASE
73+
WHEN @connection_status::connection_status = 'disconnected'
74+
THEN $2 :: timestamp with time zone
75+
ELSE NULL
76+
END)
6977
ON CONFLICT (connection_id, workspace_id, agent_name)
7078
DO UPDATE SET
7179
-- No-op if the connection is still open.
@@ -83,5 +91,5 @@ DO UPDATE SET
8391
WHEN @connection_status::connection_status = 'disconnected'
8492
THEN EXCLUDED.code
8593
ELSE connection_logs.code
86-
END
94+
END
8795
RETURNING *;

0 commit comments

Comments
 (0)