remove lookup by workspace name, refactor and simplify

Author: johnstcn

coderd/httpmw/taskparam.go

@@ -3,6 +3,7 @@ package httpmw
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"net/http"
 
 	"github.com/go-chi/chi/v5"
@@ -28,61 +29,10 @@ func TaskParam(r *http.Request) database.Task {
 	return task
 }
 
-func fetchTaskByUUID(ctx context.Context, db database.Store, taskParam string, _ uuid.UUID) (database.Task, error) {
-	taskID, err := uuid.Parse(taskParam)
-	if err != nil {
-		// Not a valid UUID, skip this strategy
-		return database.Task{}, sql.ErrNoRows
-	}
-	task, err := db.GetTaskByID(ctx, taskID)
-	if err != nil {
-		return database.Task{}, xerrors.Errorf("fetch task by uuid: %w", err)
-	}
-	return task, nil
-}
-
-func fetchTaskByName(ctx context.Context, db database.Store, taskParam string, ownerID uuid.UUID) (database.Task, error) {
-	task, err := db.GetTaskByOwnerIDAndName(ctx, database.GetTaskByOwnerIDAndNameParams{
-		OwnerID: ownerID,
-		Name:    taskParam,
-	})
-	if err != nil {
-		return database.Task{}, xerrors.Errorf("fetch task by name: %w", err)
-	}
-	return task, nil
-}
-
-func fetchTaskByWorkspaceName(ctx context.Context, db database.Store, taskParam string, ownerID uuid.UUID) (database.Task, error) {
-	workspace, err := db.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
-		OwnerID: ownerID,
-		Name:    taskParam,
-	})
-	if err != nil {
-		return database.Task{}, xerrors.Errorf("fetch workspace by name: %w", err)
-	}
-	// Check if workspace has an associated task before querying
-	if !workspace.TaskID.Valid {
-		return database.Task{}, sql.ErrNoRows
-	}
-	task, err := db.GetTaskByWorkspaceID(ctx, workspace.ID)
-	if err != nil {
-		return database.Task{}, xerrors.Errorf("fetch task by workspace id: %w", err)
-	}
-	return task, nil
-}
-
-type taskFetchFunc func(ctx context.Context, db database.Store, taskParam string, ownerID uuid.UUID) (database.Task, error)
-
-var (
-	taskLookupStrategyNames = []string{"uuid", "name", "workspace"}
-	taskLookupStrategyFuncs = []taskFetchFunc{fetchTaskByUUID, fetchTaskByName, fetchTaskByWorkspaceName}
-)
-
 // ExtractTaskParam grabs a task from the "task" URL parameter.
-// It supports three lookup strategies with cascading fallback:
+// It supports two lookup strategies:
 // 1. Task UUID (primary)
 // 2. Task name scoped to owner (secondary)
-// 3. Workspace name scoped to owner (tertiary, for legacy links)
 //
 // This middleware depends on ExtractOrganizationMembersParam being in the chain
 // to provide the owner context for name-based lookups.
@@ -92,7 +42,7 @@ func ExtractTaskParam(db database.Store) func(http.Handler) http.Handler {
 			ctx := r.Context()
 
 			// Get the task parameter value. We can't use ParseUUIDParam here because
-			// we need to support non-UUID values (task names and workspace names) and
+			// we need to support non-UUID values (task names) and
 			// attempt all lookup strategies.
 			taskParam := chi.URLParam(r, "task")
 			if taskParam == "" {
@@ -106,29 +56,15 @@ func ExtractTaskParam(db database.Store) func(http.Handler) http.Handler {
 			members := OrganizationMembersParam(r)
 			ownerID := members.UserID()
 
-			// Try each strategy in order until one succeeds
-			var task database.Task
-			var foundBy string
-			for i, fetch := range taskLookupStrategyFuncs {
-				t, err := fetch(ctx, db, taskParam, ownerID)
-				if err == nil {
-					task = t
-					foundBy = taskLookupStrategyNames[i]
-					break
-				}
+			task, err := fetchTaskWithFallback(ctx, db, taskParam, ownerID)
+			if err != nil {
 				if !httpapi.Is404Error(err) {
-					// Real error (not just "not found")
 					httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 						Message: "Internal error fetching task.",
 						Detail:  err.Error(),
 					})
 					return
 				}
-				// Continue to next strategy on 404
-			}
-
-			// If no strategy succeeded, return 404
-			if foundBy == "" {
 				httpapi.ResourceNotFound(rw)
 				return
 			}
@@ -139,11 +75,35 @@ func ExtractTaskParam(db database.Store) func(http.Handler) http.Handler {
 				rlogger.WithFields(
 					slog.F("task_id", task.ID),
 					slog.F("task_name", task.Name),
-					slog.F("task_lookup_strategy", foundBy),
 				)
 			}
 
 			next.ServeHTTP(rw, r.WithContext(ctx))
 		})
 	}
 }
+
+func fetchTaskWithFallback(ctx context.Context, db database.Store, taskParam string, ownerID uuid.UUID) (database.Task, error) {
+	// Attempt to first lookup the task by UUID.
+	taskID, err := uuid.Parse(taskParam)
+	if err == nil {
+		task, err := db.GetTaskByID(ctx, taskID)
+		if err == nil {
+			return task, nil
+		}
+		// There may be a task named with a valid UUID. Fall back to name lookup in this case.
+		if !errors.Is(err, sql.ErrNoRows) {
+			return database.Task{}, xerrors.Errorf("fetch task by uuid: %w", err)
+		}
+	}
+
+	// taskParam not a valid UUID, OR valid UUID but not found, so attempt lookup by name.
+	task, err := db.GetTaskByOwnerIDAndName(ctx, database.GetTaskByOwnerIDAndNameParams{
+		OwnerID: ownerID,
+		Name:    taskParam,
+	})
+	if err != nil {
+		return database.Task{}, xerrors.Errorf("fetch task by name: %w", err)
+	}
+	return task, nil
+}

coderd/httpmw/taskparam_test.go

@@ -184,7 +184,7 @@ func TestTaskParam(t *testing.T) {
 		require.Equal(t, http.StatusOK, res.StatusCode)
 	})
 
-	t.Run("FoundByWorkspaceName", func(t *testing.T) {
+	t.Run("NotFoundByWorkspaceName", func(t *testing.T) {
 		t.Parallel()
 		rtr := makeRouter()
 		r := makeRequest(user.ID, token)
@@ -194,7 +194,7 @@ func TestTaskParam(t *testing.T) {
 
 		res := rw.Result()
 		defer res.Body.Close()
-		require.Equal(t, http.StatusOK, res.StatusCode)
+		require.Equal(t, http.StatusNotFound, res.StatusCode)
 	})
 
 	t.Run("CaseInsensitiveTaskName", func(t *testing.T) {
@@ -229,20 +229,6 @@ func TestTaskParam(t *testing.T) {
 		require.Equal(t, "found-by-uuid", taskFoundByUUID.Name)
 	})
 
-	t.Run("TaskNameNotFoundFallsBackToWorkspace", func(t *testing.T) {
-		t.Parallel()
-		rtr := makeRouter()
-		r := makeRequest(user.ID, token)
-		// Look up by workspace name (which is not a task name) - should fallback
-		chi.RouteContext(r.Context()).URLParams.Add("task", "shared-name")
-		rw := httptest.NewRecorder()
-		rtr.ServeHTTP(rw, r)
-
-		res := rw.Result()
-		defer res.Body.Close()
-		require.Equal(t, http.StatusOK, res.StatusCode)
-	})
-
 	t.Run("NotFoundWhenNoMatch", func(t *testing.T) {
 		t.Parallel()
 		rtr := makeRouter()