Skip to content

Commit 34f6e72

Browse files
johnstcnjohnstcn
authored
feat(coderd): add lookup task by name in httpmw.TaskParam (#20647)
* Adds a `GetTaskByOwnerIDAndName` query * Updates `httpmw.TaskParam` to fall back to task name if no task by UUID found. * Updates the `TaskByIdentifier` used in `cli/` to use direct lookup instead of searching.
1 parent 46b2f3d commit 34f6e72

File tree

13 files changed

+438
-224
lines changed

13 files changed

+438
-224
lines changed

‎cli/exp_task_delete_test.go‎

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,14 @@ func TestExpTaskDelete(t *testing.T) {
5656
taskID := uuid.MustParse(id1)
5757
return func(w http.ResponseWriter, r *http.Request) {
5858
switch {
59-
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks" && r.URL.Query().Get("q") == "owner:\"me\"":
59+
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks/me/exists":
6060
c.nameResolves.Add(1)
61-
httpapi.Write(r.Context(), w, http.StatusOK, struct {
62-
Tasks []codersdk.Task `json:"tasks"`
63-
Count int `json:"count"`
64-
}{
65-
Tasks: []codersdk.Task{{
61+
httpapi.Write(r.Context(), w, http.StatusOK,
62+
codersdk.Task{
6663
ID: taskID,
6764
Name: "exists",
6865
OwnerName: "me",
69-
}},
70-
Count: 1,
71-
})
66+
})
7267
case r.Method == http.MethodDelete && r.URL.Path == "/api/experimental/tasks/me/"+id1:
7368
c.deleteCalls.Add(1)
7469
w.WriteHeader(http.StatusAccepted)
@@ -107,27 +102,21 @@ func TestExpTaskDelete(t *testing.T) {
107102
name: "Multiple_YesFlag",
108103
args: []string{"--yes", "first", id4},
109104
buildHandler: func(c *testCounters) http.HandlerFunc {
110-
firstID := uuid.MustParse(id3)
111105
return func(w http.ResponseWriter, r *http.Request) {
112106
switch {
113-
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks" && r.URL.Query().Get("q") == "owner:\"me\"":
107+
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks/me/first":
114108
c.nameResolves.Add(1)
115-
httpapi.Write(r.Context(), w, http.StatusOK, struct {
116-
Tasks []codersdk.Task `json:"tasks"`
117-
Count int `json:"count"`
118-
}{
119-
Tasks: []codersdk.Task{{
120-
ID: firstID,
121-
Name: "first",
122-
OwnerName: "me",
123-
}},
124-
Count: 1,
109+
httpapi.Write(r.Context(), w, http.StatusOK, codersdk.Task{
110+
ID: uuid.MustParse(id3),
111+
Name: "first",
112+
OwnerName: "me",
125113
})
126114
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks/me/"+id4:
115+
c.nameResolves.Add(1)
127116
httpapi.Write(r.Context(), w, http.StatusOK, codersdk.Task{
128117
ID: uuid.MustParse(id4),
129118
OwnerName: "me",
130-
Name: "uuid-task-2",
119+
Name: "uuid-task-4",
131120
})
132121
case r.Method == http.MethodDelete && r.URL.Path == "/api/experimental/tasks/me/"+id3:
133122
c.deleteCalls.Add(1)
@@ -141,7 +130,7 @@ func TestExpTaskDelete(t *testing.T) {
141130
}
142131
},
143132
wantDeleteCalls: 2,
144-
wantNameResolves: 1,
133+
wantNameResolves: 2,
145134
wantDeletedMessage: 2,
146135
},
147136
{
@@ -174,20 +163,14 @@ func TestExpTaskDelete(t *testing.T) {
174163
taskID := uuid.MustParse(id5)
175164
return func(w http.ResponseWriter, r *http.Request) {
176165
switch {
177-
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks" && r.URL.Query().Get("q") == "owner:\"me\"":
166+
case r.Method == http.MethodGet && r.URL.Path == "/api/experimental/tasks/me/bad":
178167
c.nameResolves.Add(1)
179-
httpapi.Write(r.Context(), w, http.StatusOK, struct {
180-
Tasks []codersdk.Task `json:"tasks"`
181-
Count int `json:"count"`
182-
}{
183-
Tasks: []codersdk.Task{{
184-
ID: taskID,
185-
Name: "bad",
186-
OwnerName: "me",
187-
}},
188-
Count: 1,
168+
httpapi.Write(r.Context(), w, http.StatusOK, codersdk.Task{
169+
ID: taskID,
170+
Name: "bad",
171+
OwnerName: "me",
189172
})
190-
case r.Method == http.MethodDelete && r.URL.Path == "/api/experimental/tasks/me/"+id5:
173+
case r.Method == http.MethodDelete && r.URL.Path == "/api/experimental/tasks/me/bad":
191174
httpapi.InternalServerError(w, xerrors.New("boom"))
192175
default:
193176
httpapi.InternalServerError(w, xerrors.New("unwanted path: "+r.Method+" "+r.URL.Path))

‎cli/exp_task_status_test.go‎

Lines changed: 30 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,9 @@ func Test_TaskStatus(t *testing.T) {
3636
hf: func(ctx context.Context, _ time.Time) func(w http.ResponseWriter, r *http.Request) {
3737
return func(w http.ResponseWriter, r *http.Request) {
3838
switch r.URL.Path {
39-
case "/api/experimental/tasks":
40-
if r.URL.Query().Get("q") == "owner:\"me\"" {
41-
httpapi.Write(ctx, w, http.StatusOK, struct {
42-
Tasks []codersdk.Task `json:"tasks"`
43-
Count int `json:"count"`
44-
}{
45-
Tasks: []codersdk.Task{},
46-
Count: 0,
47-
})
48-
return
49-
}
39+
case "/api/experimental/tasks/me/doesnotexist":
40+
httpapi.ResourceNotFound(w)
41+
return
5042
default:
5143
t.Errorf("unexpected path: %s", r.URL.Path)
5244
}
@@ -60,35 +52,7 @@ func Test_TaskStatus(t *testing.T) {
6052
hf: func(ctx context.Context, now time.Time) func(w http.ResponseWriter, r *http.Request) {
6153
return func(w http.ResponseWriter, r *http.Request) {
6254
switch r.URL.Path {
63-
case "/api/experimental/tasks":
64-
if r.URL.Query().Get("q") == "owner:\"me\"" {
65-
httpapi.Write(ctx, w, http.StatusOK, struct {
66-
Tasks []codersdk.Task `json:"tasks"`
67-
Count int `json:"count"`
68-
}{
69-
Tasks: []codersdk.Task{{
70-
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
71-
Name: "exists",
72-
OwnerName: "me",
73-
WorkspaceStatus: codersdk.WorkspaceStatusRunning,
74-
CreatedAt: now,
75-
UpdatedAt: now,
76-
CurrentState: &codersdk.TaskStateEntry{
77-
State: codersdk.TaskStateWorking,
78-
Timestamp: now,
79-
Message: "Thinking furiously...",
80-
},
81-
WorkspaceAgentHealth: &codersdk.WorkspaceAgentHealth{
82-
Healthy: true,
83-
},
84-
WorkspaceAgentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleReady),
85-
Status: codersdk.TaskStatusActive,
86-
}},
87-
Count: 1,
88-
})
89-
return
90-
}
91-
case "/api/experimental/tasks/me/11111111-1111-1111-1111-111111111111":
55+
case "/api/experimental/tasks/me/exists":
9256
httpapi.Write(ctx, w, http.StatusOK, codersdk.Task{
9357
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
9458
WorkspaceStatus: codersdk.WorkspaceStatusRunning,
@@ -124,30 +88,21 @@ func Test_TaskStatus(t *testing.T) {
12488
var calls atomic.Int64
12589
return func(w http.ResponseWriter, r *http.Request) {
12690
switch r.URL.Path {
127-
case "/api/experimental/tasks":
128-
if r.URL.Query().Get("q") == "owner:\"me\"" {
129-
// Return initial task state for --watch test
130-
httpapi.Write(ctx, w, http.StatusOK, struct {
131-
Tasks []codersdk.Task `json:"tasks"`
132-
Count int `json:"count"`
133-
}{
134-
Tasks: []codersdk.Task{{
135-
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
136-
Name: "exists",
137-
OwnerName: "me",
138-
WorkspaceStatus: codersdk.WorkspaceStatusPending,
139-
CreatedAt: now.Add(-5 * time.Second),
140-
UpdatedAt: now.Add(-5 * time.Second),
141-
WorkspaceAgentHealth: &codersdk.WorkspaceAgentHealth{
142-
Healthy: true,
143-
},
144-
WorkspaceAgentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleReady),
145-
Status: codersdk.TaskStatusPending,
146-
}},
147-
Count: 1,
148-
})
149-
return
150-
}
91+
case "/api/experimental/tasks/me/exists":
92+
httpapi.Write(ctx, w, http.StatusOK, codersdk.Task{
93+
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
94+
Name: "exists",
95+
OwnerName: "me",
96+
WorkspaceStatus: codersdk.WorkspaceStatusPending,
97+
CreatedAt: now.Add(-5 * time.Second),
98+
UpdatedAt: now.Add(-5 * time.Second),
99+
WorkspaceAgentHealth: &codersdk.WorkspaceAgentHealth{
100+
Healthy: true,
101+
},
102+
WorkspaceAgentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleReady),
103+
Status: codersdk.TaskStatusPending,
104+
})
105+
return
151106
case "/api/experimental/tasks/me/11111111-1111-1111-1111-111111111111":
152107
defer calls.Add(1)
153108
switch calls.Load() {
@@ -263,40 +218,18 @@ func Test_TaskStatus(t *testing.T) {
263218
ts := time.Date(2025, 8, 26, 12, 34, 56, 0, time.UTC)
264219
return func(w http.ResponseWriter, r *http.Request) {
265220
switch r.URL.Path {
266-
case "/api/experimental/tasks":
267-
if r.URL.Query().Get("q") == "owner:\"me\"" {
268-
httpapi.Write(ctx, w, http.StatusOK, struct {
269-
Tasks []codersdk.Task `json:"tasks"`
270-
Count int `json:"count"`
271-
}{
272-
Tasks: []codersdk.Task{{
273-
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
274-
Name: "exists",
275-
OwnerName: "me",
276-
WorkspaceStatus: codersdk.WorkspaceStatusRunning,
277-
CreatedAt: ts,
278-
UpdatedAt: ts,
279-
CurrentState: &codersdk.TaskStateEntry{
280-
State: codersdk.TaskStateWorking,
281-
Timestamp: ts.Add(time.Second),
282-
Message: "Thinking furiously...",
283-
},
284-
WorkspaceAgentHealth: &codersdk.WorkspaceAgentHealth{
285-
Healthy: true,
286-
},
287-
WorkspaceAgentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleReady),
288-
Status: codersdk.TaskStatusActive,
289-
}},
290-
Count: 1,
291-
})
292-
return
293-
}
294-
case "/api/experimental/tasks/me/11111111-1111-1111-1111-111111111111":
221+
case "/api/experimental/tasks/me/exists":
295222
httpapi.Write(ctx, w, http.StatusOK, codersdk.Task{
296-
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
297-
WorkspaceStatus: codersdk.WorkspaceStatusRunning,
298-
CreatedAt: ts,
299-
UpdatedAt: ts,
223+
ID: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
224+
Name: "exists",
225+
OwnerName: "me",
226+
WorkspaceAgentHealth: &codersdk.WorkspaceAgentHealth{
227+
Healthy: true,
228+
},
229+
WorkspaceAgentLifecycle: ptr.Ref(codersdk.WorkspaceAgentLifecycleReady),
230+
WorkspaceStatus: codersdk.WorkspaceStatusRunning,
231+
CreatedAt: ts,
232+
UpdatedAt: ts,
300233
CurrentState: &codersdk.TaskStateEntry{
301234
State: codersdk.TaskStateWorking,
302235
Timestamp: ts.Add(time.Second),

‎coderd/aitasks_test.go‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,13 @@ func TestTasks(t *testing.T) {
156156
t.Parallel()
157157

158158
var (
159-
client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
160-
ctx = testutil.Context(t, testutil.WaitLong)
161-
user = coderdtest.CreateFirstUser(t, client)
162-
template = createAITemplate(t, client, user)
163-
wantPrompt = "review my code"
164-
exp = codersdk.NewExperimentalClient(client)
159+
client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
160+
ctx = testutil.Context(t, testutil.WaitLong)
161+
user = coderdtest.CreateFirstUser(t, client)
162+
anotherUser, _ = coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
163+
template = createAITemplate(t, client, user)
164+
wantPrompt = "review my code"
165+
exp = codersdk.NewExperimentalClient(client)
165166
)
166167

167168
task, err := exp.CreateTask(ctx, "me", codersdk.CreateTaskRequest{
@@ -211,6 +212,24 @@ func TestTasks(t *testing.T) {
211212
assert.Equal(t, taskAppID, updated.WorkspaceAppID.UUID, "workspace app id should match")
212213
assert.NotEmpty(t, updated.WorkspaceStatus, "task status should not be empty")
213214

215+
// Fetch the task by name and verify the same result
216+
byName, err := exp.TaskByOwnerAndName(ctx, codersdk.Me, task.Name)
217+
require.NoError(t, err)
218+
require.Equal(t, byName, updated)
219+
220+
// Another member user should not be able to fetch the task
221+
otherClient := codersdk.NewExperimentalClient(anotherUser)
222+
_, err = otherClient.TaskByID(ctx, task.ID)
223+
require.Error(t, err, "fetching task should fail by ID for another member user")
224+
var sdkErr *codersdk.Error
225+
require.ErrorAs(t, err, &sdkErr)
226+
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
227+
// Also test by name
228+
_, err = otherClient.TaskByOwnerAndName(ctx, task.OwnerName, task.Name)
229+
require.Error(t, err, "fetching task should fail by name for another member user")
230+
require.ErrorAs(t, err, &sdkErr)
231+
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
232+
214233
// Stop the workspace
215234
coderdtest.MustTransitionWorkspace(t, client, task.WorkspaceID.UUID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
216235

@@ -654,7 +673,7 @@ func TestTasks(t *testing.T) {
654673
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
655674

656675
// Fetch the task by ID via experimental API and verify fields.
657-
task, err = exp.TaskByID(ctx, task.ID)
676+
task, err = exp.TaskByIdentifier(ctx, task.ID.String())
658677
require.NoError(t, err)
659678
require.NotZero(t, task.WorkspaceBuildNumber)
660679
require.True(t, task.WorkspaceAgentID.Valid)

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2989,6 +2989,10 @@ func (q *querier) GetTaskByID(ctx context.Context, id uuid.UUID) (database.Task,
29892989
return fetch(q.log, q.auth, q.db.GetTaskByID)(ctx, id)
29902990
}
29912991

2992+
func (q *querier) GetTaskByOwnerIDAndName(ctx context.Context, arg database.GetTaskByOwnerIDAndNameParams) (database.Task, error) {
2993+
return fetch(q.log, q.auth, q.db.GetTaskByOwnerIDAndName)(ctx, arg)
2994+
}
2995+
29922996
func (q *querier) GetTaskByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (database.Task, error) {
29932997
return fetch(q.log, q.auth, q.db.GetTaskByWorkspaceID)(ctx, workspaceID)
29942998
}

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2375,6 +2375,17 @@ func (s *MethodTestSuite) TestTasks() {
23752375
dbm.EXPECT().GetTaskByID(gomock.Any(), task.ID).Return(task, nil).AnyTimes()
23762376
check.Args(task.ID).Asserts(task, policy.ActionRead).Returns(task)
23772377
}))
2378+
s.Run("GetTaskByOwnerIDAndName", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
2379+
task := testutil.Fake(s.T(), faker, database.Task{})
2380+
dbm.EXPECT().GetTaskByOwnerIDAndName(gomock.Any(), database.GetTaskByOwnerIDAndNameParams{
2381+
OwnerID: task.OwnerID,
2382+
Name: task.Name,
2383+
}).Return(task, nil).AnyTimes()
2384+
check.Args(database.GetTaskByOwnerIDAndNameParams{
2385+
OwnerID: task.OwnerID,
2386+
Name: task.Name,
2387+
}).Asserts(task, policy.ActionRead).Returns(task)
2388+
}))
23782389
s.Run("DeleteTask", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
23792390
task := testutil.Fake(s.T(), faker, database.Task{})
23802391
arg := database.DeleteTaskParams{

‎coderd/database/dbmetrics/querymetrics.go‎

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

‎coderd/database/dbmock/dbmock.go‎

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

‎coderd/database/querier.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)