docs/docs/api/management-open-api.yaml (1)

1022-1041: Document pagination request parameters on list-projects

The endpoint advertises a paginated response but does not declare pageToken/pageSize query parameters. Please add them for parity with list-users/list-roles.

Apply this diff under /management/v1/project-list -> get:

   /management/v1/project-list:
     get:
       tags:
         - project
       summary: List Projects
       description: Lists all projects that the requesting user has access to.
       operationId: list_projects
+      parameters:
+        - name: pageToken
+          in: query
+          description: Next page token
+          required: false
+          schema:
+            type:
+              - string
+              - 'null'
+        - name: pageSize
+          in: query
+          description: |
+            Signals an upper bound of the number of results that a client will receive.
+            Default: 100
+          required: false
+          schema:
+            type: integer
+            format: int64
       responses:
         '200':
           description: List of projects
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/ListProjectsResponse'

crates/lakekeeper/src/api/management/v1/project.rs (1)

50-61: Consistent pagination query type (PageToken) — good

Using PageToken with serde/utoipa annotations aligns with other paginated endpoints. Defaulting page_size via management::v1::default_page_size is consistent.

crates/lakekeeper/src/implementations/postgres/catalog.rs (1)

442-444: Import ListProjectsResponse to shorten the return type

Same as earlier feedback: prefer importing the type over fully qualifying.

-    ) -> Result<crate::api::management::v1::project::ListProjectsResponse> {
+    ) -> Result<ListProjectsResponse> {
         list_projects(project_ids, pagination, &mut **transaction).await
     }

Add this import near the top:

use crate::api::management::v1::project::ListProjectsResponse;

.sqlx/query-ac395ee12ac90d563f6ed2b5afe52c2b2e8f6f0bce9792aae0a253a729d3fe16.json (1)

3-3: Keyset pagination query looks correct

The WHERE clause and ORDER BY match the cursor fields; semantics are good for stable keyset pagination and first-page behavior with null token. Two small recommendations:

• Explicitly use ORDER BY created_at ASC, project_id ASC for readability.
• Ensure an index exists to support this cursor: e.g. CREATE INDEX ON project (created_at ASC, project_id ASC).

crates/lakekeeper/src/service/catalog.rs (1)

598-602: Shorten the return type by importing ListProjectsResponse

The long fully qualified path is harder to scan. Import the type and use it directly.

-    ) -> Result<crate::api::management::v1::project::ListProjectsResponse>;
+    ) -> Result<ListProjectsResponse>;

Add this import near the other use lines:

use crate::api::management::v1::project::ListProjectsResponse;

crates/lakekeeper/src/implementations/postgres/warehouse.rs (3)

455-457: Consider more robust architecture handling.

While the expect message is descriptive, consider using a more graceful approach:

-    let page_as_usize: usize = page_size
-        .try_into()
-        .expect("should be running on at least 32 bit architecture");
+    let page_as_usize: usize = page_size
+        .try_into()
+        .map_err(|_| ErrorModel::internal(
+            "Page size exceeds platform limits",
+            "PageSizeConversionError",
+            None,
+        ))?;

---

416-416: Fix formatting issues.

The static analysis tool detected formatting inconsistencies.

Run cargo fmt to fix the formatting issues at lines 416 and 475.

Also applies to: 475-475

---

1283-1353: Comprehensive pagination test coverage!

The test thoroughly validates:

• Multi-page pagination with page_size=2
• Correct ordering of results across pages (addressing previous review feedback)
• Next page token generation and consumption
• Completeness of paginated results

Note: Fix the formatting issue at line 1335 by running cargo fmt.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

crates/lakekeeper/src/implementations/postgres/catalog.rs (1)

441-444: Import the return type; avoid fully-qualified path in signature

Prefer importing the type at the top for readability and consistency (echoing earlier feedback).

Apply this diff to the signature:

-    ) -> Result<crate::api::management::v1::project::ListProjectsResponse> {
+    ) -> Result<ListProjectsResponse> {

And add this import near the other crate::api imports:

use crate::api::management::v1::project::ListProjectsResponse;

crates/lakekeeper/src/implementations/postgres/warehouse.rs (1)

454-457: Simplify has_more calculation and avoid lossy conversion

No need to convert page_size to usize. Compare in i64 space to avoid the expect and make intent clearer.

Apply this diff:

-    let page_as_usize: usize = page_size
-        .try_into()
-        .expect("should be running on at least 32 bit architecture");
-    let has_more = projects.len() > page_as_usize;
+    let has_more = (projects.len() as i64) > page_size;

crates/lakekeeper/src/implementations/postgres/warehouse.rs (3)

435-441: Add composite index to support the pagination ORDER BY

To keep this query efficient at scale, ensure there is a composite index on (created_at, project_id) on the project table. Without it, cursor scans will degrade to sequential scans under load.

Would you like me to open a follow-up issue or migration snippet to add this index?

---

886-896: LGTM: tests updated for paginated return shape

Tests now unwrap the ListProjectsResponse correctly and validate membership. Consider extracting a small helper for constructing a default PaginationQuery to reduce repetition.

Also applies to: 910-920, 932-942

---

1281-1362: Add tests for filtered pagination and uneven last page

Great coverage for the unfiltered, even-split case. Two additions would harden this:

• Filtered pagination: pass Some(HashSet) and paginate to verify the WHERE clause and ordering behave with a filter.
• Uneven page: e.g., 5 projects with page-size=2 to validate three pages (2,2,1) and a final None token.

I can draft these tests if helpful.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

crates/lakekeeper/src/implementations/postgres/catalog.rs (1)

443-447: Nit: align param naming with other paginated methods

Other methods in this impl (e.g., list_tables, list_views, list_tabulars) use pagination_query for the parameter name. Consider the same here for consistency.

Apply this diff:

-    async fn list_projects(
-        project_ids: Option<HashSet<ProjectId>>,
-        pagination: PaginationQuery,
+    async fn list_projects(
+        project_ids: Option<HashSet<ProjectId>>,
+        pagination_query: PaginationQuery,
         transaction: <Self::Transaction as Transaction<Self::State>>::Transaction<'_>,
     ) -> Result<ListProjectsResponse> {
-        list_projects(project_ids, pagination, &mut **transaction).await
+        list_projects(project_ids, pagination_query, &mut **transaction).await
     }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

crates/lakekeeper/src/api/management/mod.rs (1)

26-28: Use the imported type instead of a fully-qualified path in the handler.

You already import ListProjectsQuery; use it directly for consistency/nit.

-        Query(query): Query<crate::api::management::v1::project::ListProjectsQuery>,
+        Query(query): Query<ListProjectsQuery>,

Also applies to: 568-571

crates/lakekeeper/src/implementations/postgres/warehouse.rs (1)

454-457: Avoid lossy i64→usize conversion; compare in i64 directly.

Converting page_size: i64 to usize is unnecessary and adds an infallibility assumption. Compare in i64 to keep types aligned.

-    let page_as_usize: usize = page_size
-        .try_into()
-        .expect("should be running on at least 32 bit architecture");
-    let has_more = projects.len() > page_as_usize;
+    let has_more = (projects.len() as i64) > page_size;

crates/lakekeeper/src/api/management/mod.rs (1)

553-564: Document query params in OpenAPI for list_projects.

The endpoint is now query-driven but the OpenAPI annotation doesn’t declare params. Add params(ListProjectsQuery) to expose pageToken/pageSize in the spec (consistent with other list endpoints like users/roles).

 #[utoipa::path(
     get,
     tag = "project",
     path = ManagementV1Endpoint::ListProjects.path(),
+    params(ListProjectsQuery),
     responses(
         (status = 200, description = "List of projects", body = ListProjectsResponse),
         (status = "4XX", body = IcebergErrorResponse),
     )
 )]

crates/lakekeeper/src/api/management/v1/project.rs (1)

49-59: Harden query deserialization and keep it consistent with other endpoints.

• Consider defaulting page_token to avoid deserialization errors when omitted.
• Optionally skip serializing page_size when None to match other list queries.

 #[derive(Debug, Clone, Deserialize, utoipa::IntoParams)]
 #[serde(rename_all = "camelCase")]
 pub struct ListProjectsQuery {
     /// Next page token for pagination
-    #[serde(skip_serializing_if = "PageToken::skip_serialize")]
+    #[serde(skip_serializing_if = "PageToken::skip_serialize")]
+    #[serde(default)]
     #[param(value_type=String)]
     pub page_token: PageToken,
     /// Signal an upper bound of the number of results that a client will receive
     /// Default: 100
-    pub page_size: Option<i64>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub page_size: Option<i64>,
 }

crates/lakekeeper/src/implementations/postgres/warehouse.rs (2)

442-449: Optional: bind UUIDs directly instead of strings for ANY($1).

You currently pass Vec<String> of UUIDs. Binding as Vec<uuid::Uuid> (and optionally casting ANY($1::uuid[])) avoids string conversions and can help the planner. Only do this if ProjectId exposes the inner UUID cheaply.

---

1279-1325: Tests cover happy-path pagination and ordering; slight flakiness risk remains.

Order is (created_at, project_id). If two rows share the same created_at at DB precision, ordering falls back to project_id which may not match insertion order. This is unlikely but possible. If this ever flakes, consider ensuring distinct created_at (e.g., tiny sleep) or relaxing the exact index-based assertions.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

crates/lakekeeper/src/api/management/mod.rs (1)

570-571: Forwarding query to ApiServer looks correct.

The new call signature ApiServer::<C, A, S>::list_projects(query, api_context, metadata) aligns with the trait change. LGTM.

crates/lakekeeper/src/service/catalog.rs (1)

28-32: Catalog list_projects signature and usage verified

• Confirmed the Catalog trait in service/catalog.rs defines async fn list_projects(project_ids: Option<HashSet<ProjectId>>, pagination: PaginationQuery, …) and that the Postgres implementation in implementations/postgres/catalog.rs matches this updated signature.
• Inspected all direct invocations of C::list_projects (e.g., in src/api/management/v1/project.rs) and PostgresCatalog::list_projects (in implementations/postgres/warehouse.rs) to ensure the new pagination parameter (and transaction argument where required) are passed.
• No other Catalog implementors or mocks were found in the codebase; alternate backends or in-memory mocks are not present.

All backends and call sites have been updated correctly.

crates/lakekeeper/src/api/management/v1/project.rs (2)

74-76: Response shape with next_page_token is clear and matches the SQL-layer behavior.

No issues spotted.

---

245-249: Plumbing pagination through to Catalog and returning ListProjectsResponse is correct.

Transaction handling and authZ filter look good.

crates/lakekeeper/src/implementations/postgres/warehouse.rs (1)

414-483: Cursor-based pagination implementation looks solid.

• Correct keyset condition on (created_at, project_id) with LIMIT page_size + 1.
• Properly trims the sentinel and constructs the next-page token from the last visible row.