fix: Adjust pagination limit to be zero-based (coder#2663)

kylecarbs · web-flow · commit 01c31b47a320 · 2022-06-26T20:23:25.000Z
There isn't a use-case for querying a limit of zero. Using
-1 led to issues when using default parameters for querying.
diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -396,7 +396,7 @@ func AwaitWorkspaceAgents(t *testing.T, client *codersdk.Client, build uuid.UUID
 			}
 		}
 		return true
-	}, 5*time.Second, 25*time.Millisecond)
+	}, 15*time.Second, 50*time.Millisecond)
 	return resources
 }
 
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql
@@ -29,8 +29,8 @@ ORDER BY
     -- a timestamp. This is to ensure consistent pagination.
 	(created_at, id) ASC OFFSET @offset_opt
 LIMIT
-	-- A null limit means "no limit", so -1 means return all
-	NULLIF(@limit_opt :: int, -1);
+	-- A null limit means "no limit", so 0 means return all
+	NULLIF(@limit_opt :: int, 0);
 
 -- name: GetTemplateVersionByJobID :one
 SELECT
diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql
@@ -126,8 +126,8 @@ ORDER BY
     -- a timestamp. This is to ensure consistent pagination.
 	(created_at, id) ASC OFFSET @offset_opt
 LIMIT
-	-- A null limit means "no limit", so -1 means return all
-	NULLIF(@limit_opt :: int, -1);
+	-- A null limit means "no limit", so 0 means return all
+	NULLIF(@limit_opt :: int, 0);
 
 -- name: UpdateUserStatus :one
 UPDATE
diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql
@@ -68,8 +68,8 @@ END
 ORDER BY
     build_number desc OFFSET @offset_opt
 LIMIT
-    -- A null limit means "no limit", so -1 means return all
-    NULLIF(@limit_opt :: int, -1);
+    -- A null limit means "no limit", so 0 means return all
+    NULLIF(@limit_opt :: int, 0);
 
 -- name: GetLatestWorkspaceBuildByWorkspaceID :one
 SELECT
diff --git a/coderd/pagination.go b/coderd/pagination.go
@@ -17,7 +17,7 @@ func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Paginat
 	params := codersdk.Pagination{
 		AfterID: parser.UUID(queryParams, uuid.Nil, "after_id"),
 		// Limit default to "-1" which returns all results
-		Limit:  parser.Int(queryParams, -1, "limit"),
+		Limit:  parser.Int(queryParams, 0, "limit"),
 		Offset: parser.Int(queryParams, 0, "offset"),
 	}
 	if len(parser.Errors) > 0 {
diff --git a/coderd/pagination_internal_test.go b/coderd/pagination_internal_test.go
@@ -77,15 +77,13 @@ func TestPagination(t *testing.T) {
 			ExpectedParams: codersdk.Pagination{
 				AfterID: uuid.Nil,
 				Offset:  150,
-				Limit:   -1,
 			},
 		},
 		{
 			Name:    "ValidAfterID",
 			AfterID: "5f2005fc-acc4-4e5e-a7fa-be017359c60b",
 			ExpectedParams: codersdk.Pagination{
 				AfterID: uuid.MustParse("5f2005fc-acc4-4e5e-a7fa-be017359c60b"),
-				Limit:   -1,
 			},
 		},
 	}

Original file line number	Diff line number	Diff line change
`@@ -396,7 +396,7 @@ func AwaitWorkspaceAgents(t testing.T, client codersdk.Client, build uuid.UUID`
`396`	`396`	`}`
`397`	`397`	`}`
`398`	`398`	`return true`
`399`		`- }, 5time.Second, 25time.Millisecond)`
	`399`	`+ }, 15time.Second, 50time.Millisecond)`
`400`	`400`	`return resources`
`401`	`401`	`}`
`402`	`402`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Paginat`
`17`	`17`	`params := codersdk.Pagination{`
`18`	`18`	`AfterID: parser.UUID(queryParams, uuid.Nil, "after_id"),`
`19`	`19`	`// Limit default to "-1" which returns all results`
`20`		`- Limit: parser.Int(queryParams, -1, "limit"),`
	`20`	`+ Limit: parser.Int(queryParams, 0, "limit"),`
`21`	`21`	`Offset: parser.Int(queryParams, 0, "offset"),`
`22`	`22`	`}`
`23`	`23`	`if len(parser.Errors) > 0 {`