diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 5edc286613..287f289451 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2426,11 +2426,11 @@ func (q *querier) GetLatestCryptoKeyByFeature(ctx context.Context, feature datab return q.db.GetLatestCryptoKeyByFeature(ctx, feature) } -func (q *querier) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) { +func (q *querier) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (database.WorkspaceAppStatus, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err + return database.WorkspaceAppStatus{}, err } - return q.db.GetLatestWorkspaceAppStatusesByAppID(ctx, appID) + return q.db.GetLatestWorkspaceAppStatusByAppID(ctx, appID) } func (q *querier) GetLatestWorkspaceAppStatusesByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceAppStatus, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 3304155af8..d14429a105 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2864,9 +2864,9 @@ func (s *MethodTestSuite) TestSystemFunctions() { dbm.EXPECT().UpdateUserLinkedID(gomock.Any(), arg).Return(l, nil).AnyTimes() check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns(l) })) - s.Run("GetLatestWorkspaceAppStatusesByAppID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + s.Run("GetLatestWorkspaceAppStatusByAppID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { appID := uuid.New() - dbm.EXPECT().GetLatestWorkspaceAppStatusesByAppID(gomock.Any(), appID).Return([]database.WorkspaceAppStatus{}, nil).AnyTimes() + dbm.EXPECT().GetLatestWorkspaceAppStatusByAppID(gomock.Any(), appID).Return(database.WorkspaceAppStatus{}, nil).AnyTimes() check.Args(appID).Asserts(rbac.ResourceSystem, policy.ActionRead) })) s.Run("GetLatestWorkspaceAppStatusesByWorkspaceIDs", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 8b3190ee17..89269b295c 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1033,10 +1033,10 @@ func (m queryMetricsStore) GetLatestCryptoKeyByFeature(ctx context.Context, feat return r0, r1 } -func (m queryMetricsStore) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) { +func (m queryMetricsStore) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (database.WorkspaceAppStatus, error) { start := time.Now() - r0, r1 := m.s.GetLatestWorkspaceAppStatusesByAppID(ctx, appID) - m.queryLatencies.WithLabelValues("GetLatestWorkspaceAppStatusesByAppID").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.GetLatestWorkspaceAppStatusByAppID(ctx, appID) + m.queryLatencies.WithLabelValues("GetLatestWorkspaceAppStatusByAppID").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 4970a53701..ce50bf87d2 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2172,19 +2172,19 @@ func (mr *MockStoreMockRecorder) GetLatestCryptoKeyByFeature(ctx, feature any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestCryptoKeyByFeature", reflect.TypeOf((*MockStore)(nil).GetLatestCryptoKeyByFeature), ctx, feature) } -// GetLatestWorkspaceAppStatusesByAppID mocks base method. -func (m *MockStore) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) { +// GetLatestWorkspaceAppStatusByAppID mocks base method. +func (m *MockStore) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (database.WorkspaceAppStatus, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetLatestWorkspaceAppStatusesByAppID", ctx, appID) - ret0, _ := ret[0].([]database.WorkspaceAppStatus) + ret := m.ctrl.Call(m, "GetLatestWorkspaceAppStatusByAppID", ctx, appID) + ret0, _ := ret[0].(database.WorkspaceAppStatus) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetLatestWorkspaceAppStatusesByAppID indicates an expected call of GetLatestWorkspaceAppStatusesByAppID. -func (mr *MockStoreMockRecorder) GetLatestWorkspaceAppStatusesByAppID(ctx, appID any) *gomock.Call { +// GetLatestWorkspaceAppStatusByAppID indicates an expected call of GetLatestWorkspaceAppStatusByAppID. +func (mr *MockStoreMockRecorder) GetLatestWorkspaceAppStatusByAppID(ctx, appID any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestWorkspaceAppStatusesByAppID", reflect.TypeOf((*MockStore)(nil).GetLatestWorkspaceAppStatusesByAppID), ctx, appID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestWorkspaceAppStatusByAppID", reflect.TypeOf((*MockStore)(nil).GetLatestWorkspaceAppStatusByAppID), ctx, appID) } // GetLatestWorkspaceAppStatusesByWorkspaceIDs mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 0f8570127b..5c249f3bdf 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -238,7 +238,7 @@ type sqlcQuerier interface { GetInboxNotificationsByUserID(ctx context.Context, arg GetInboxNotificationsByUserIDParams) ([]InboxNotification, error) GetLastUpdateCheck(ctx context.Context) (string, error) GetLatestCryptoKeyByFeature(ctx context.Context, feature CryptoKeyFeature) (CryptoKey, error) - GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]WorkspaceAppStatus, error) + GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (WorkspaceAppStatus, error) GetLatestWorkspaceAppStatusesByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAppStatus, error) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d592a0f4b5..d99155ba99 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -20041,43 +20041,28 @@ func (q *sqlQuerier) UpsertWorkspaceAppAuditSession(ctx context.Context, arg Ups return new_or_stale, err } -const getLatestWorkspaceAppStatusesByAppID = `-- name: GetLatestWorkspaceAppStatusesByAppID :many +const getLatestWorkspaceAppStatusByAppID = `-- name: GetLatestWorkspaceAppStatusByAppID :one SELECT id, created_at, agent_id, app_id, workspace_id, state, message, uri FROM workspace_app_statuses WHERE app_id = $1::uuid ORDER BY created_at DESC, id DESC +LIMIT 1 ` -func (q *sqlQuerier) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]WorkspaceAppStatus, error) { - rows, err := q.db.QueryContext(ctx, getLatestWorkspaceAppStatusesByAppID, appID) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAppStatus - for rows.Next() { - var i WorkspaceAppStatus - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.AgentID, - &i.AppID, - &i.WorkspaceID, - &i.State, - &i.Message, - &i.Uri, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil +func (q *sqlQuerier) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (WorkspaceAppStatus, error) { + row := q.db.QueryRowContext(ctx, getLatestWorkspaceAppStatusByAppID, appID) + var i WorkspaceAppStatus + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.AgentID, + &i.AppID, + &i.WorkspaceID, + &i.State, + &i.Message, + &i.Uri, + ) + return i, err } const getLatestWorkspaceAppStatusesByWorkspaceIDs = `-- name: GetLatestWorkspaceAppStatusesByWorkspaceIDs :many diff --git a/coderd/database/queries/workspaceapps.sql b/coderd/database/queries/workspaceapps.sql index d76e789f19..b3694fb275 100644 --- a/coderd/database/queries/workspaceapps.sql +++ b/coderd/database/queries/workspaceapps.sql @@ -73,11 +73,12 @@ RETURNING *; -- name: GetWorkspaceAppStatusesByAppIDs :many SELECT * FROM workspace_app_statuses WHERE app_id = ANY(@ids :: uuid [ ]); --- name: GetLatestWorkspaceAppStatusesByAppID :many +-- name: GetLatestWorkspaceAppStatusByAppID :one SELECT * FROM workspace_app_statuses WHERE app_id = @app_id::uuid -ORDER BY created_at DESC, id DESC; +ORDER BY created_at DESC, id DESC +LIMIT 1; -- name: GetLatestWorkspaceAppStatusesByWorkspaceIDs :many SELECT DISTINCT ON (workspace_id) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index eced6ff6d3..d3cca07066 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -388,16 +388,17 @@ func (api *API) patchWorkspaceAgentAppStatus(rw http.ResponseWriter, r *http.Req // Treat the message as untrusted input. cleaned := strutil.UISanitize(req.Message) - // Get the latest statuses for the workspace app to detect no-op updates + // Get the latest status for the workspace app to detect no-op updates // nolint:gocritic // This is a system restricted operation. - latestAppStatus, err := api.Database.GetLatestWorkspaceAppStatusesByAppID(dbauthz.AsSystemRestricted(ctx), app.ID) - if err != nil { + latestAppStatus, err := api.Database.GetLatestWorkspaceAppStatusByAppID(dbauthz.AsSystemRestricted(ctx), app.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to get latest workspace app statuses.", + Message: "Failed to get latest workspace app status.", Detail: err.Error(), }) return } + // If no rows found, latestAppStatus will be a zero-value struct (ID == uuid.Nil) // nolint:gocritic // This is a system restricted operation. _, err = api.Database.InsertWorkspaceAppStatus(dbauthz.AsSystemRestricted(ctx), database.InsertWorkspaceAppStatusParams{ @@ -442,7 +443,7 @@ func (api *API) patchWorkspaceAgentAppStatus(rw http.ResponseWriter, r *http.Req func (api *API) enqueueAITaskStateNotification( ctx context.Context, appID uuid.UUID, - latestAppStatus []database.WorkspaceAppStatus, + latestAppStatus database.WorkspaceAppStatus, newAppStatus codersdk.WorkspaceAppStatusState, workspace database.Workspace, agent database.WorkspaceAgent, @@ -492,14 +493,16 @@ func (api *API) enqueueAITaskStateNotification( } // Skip if the latest persisted state equals the new state (no new transition) - if len(latestAppStatus) > 0 && latestAppStatus[0].State == database.WorkspaceAppStatusState(newAppStatus) { + // Note: uuid.Nil check is valid here. If no previous status exists, + // GetLatestWorkspaceAppStatusByAppID returns sql.ErrNoRows and we get a zero-value struct. + if latestAppStatus.ID != uuid.Nil && latestAppStatus.State == database.WorkspaceAppStatusState(newAppStatus) { return } // Skip the initial "Working" notification when task first starts. // This is obvious to the user since they just created the task. // We still notify on first "Idle" status and all subsequent transitions. - if len(latestAppStatus) == 0 && newAppStatus == codersdk.WorkspaceAppStatusStateWorking { + if latestAppStatus.ID == uuid.Nil && newAppStatus == codersdk.WorkspaceAppStatusStateWorking { return }