Skip to content

Commit

Permalink
AIP-84 Allow only positive limit and offset (#44109)
Browse files Browse the repository at this point in the history 
* limit and offset can't be negative

* add tests

* refactor

* use NonNegativeInt
  • Loading branch information
@rawwar
rawwar authored Nov 18, 2024
1 parent 1acb54e commit 49daa6c
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 5 deletions.
10 changes: 5 additions & 5 deletions airflow/api_fastapi/common/parameters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from fastapi import Depends, HTTPException, Query
from pendulum.parsing.exceptions import ParserError
from pydantic import AfterValidator, BaseModel
from pydantic import AfterValidator, BaseModel, NonNegativeInt
from sqlalchemy import Column, case, or_
from sqlalchemy.inspection import inspect

Expand Down Expand Up @@ -66,7 +66,7 @@ def depends(self, *args: Any, **kwargs: Any) -> Self:
pass


class LimitFilter(BaseParam[int]):
class LimitFilter(BaseParam[NonNegativeInt]):
"""Filter on the limit."""

def to_orm(self, select: Select) -> Select:
Expand All @@ -75,19 +75,19 @@ def to_orm(self, select: Select) -> Select:

return select.limit(self.value)

def depends(self, limit: int = 100) -> LimitFilter:
def depends(self, limit: NonNegativeInt = 100) -> LimitFilter:
return self.set_value(limit)


class OffsetFilter(BaseParam[int]):
class OffsetFilter(BaseParam[NonNegativeInt]):
"""Filter on offset."""

def to_orm(self, select: Select) -> Select:
if self.value is None and self.skip_none:
return select
return select.offset(self.value)

def depends(self, offset: int = 0) -> OffsetFilter:
def depends(self, offset: NonNegativeInt = 0) -> OffsetFilter:
return self.set_value(offset)


Expand Down
34 changes: 34 additions & 0 deletions airflow/api_fastapi/core_api/openapi/v1-generated.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: tags
Expand Down Expand Up @@ -183,13 +185,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: uri_pattern
Expand Down Expand Up @@ -259,13 +263,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -558,13 +564,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -1002,13 +1010,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -1574,13 +1584,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -1628,13 +1640,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: tags
Expand Down Expand Up @@ -1746,13 +1760,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: tags
Expand Down Expand Up @@ -1858,13 +1874,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -2275,13 +2293,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -2374,13 +2394,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -2427,13 +2449,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
responses:
Expand Down Expand Up @@ -2631,13 +2655,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -2728,13 +2754,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
responses:
Expand Down Expand Up @@ -2970,13 +2998,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -3358,13 +3388,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down Expand Up @@ -3727,13 +3759,15 @@ paths:
required: false
schema:
type: integer
minimum: 0
default: 100
title: Limit
- name: offset
in: query
required: false
schema:
type: integer
minimum: 0
default: 0
title: Offset
- name: order_by
Expand Down
53 changes: 53 additions & 0 deletions tests/api_fastapi/core_api/routes/public/test_assets.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,59 @@ def test_should_respect_page_size_limit_default(self, test_client):
assert response.status_code == 200
assert len(response.json()["assets"]) == 100

@pytest.mark.parametrize(
"query_params, expected_detail",
[
(
{"limit": 1, "offset": -1},
[
{
"type": "greater_than_equal",
"loc": ["query", "offset"],
"msg": "Input should be greater than or equal to 0",
"input": "-1",
"ctx": {"ge": 0},
}
],
),
(
{"limit": -1, "offset": 1},
[
{
"type": "greater_than_equal",
"loc": ["query", "limit"],
"msg": "Input should be greater than or equal to 0",
"input": "-1",
"ctx": {"ge": 0},
}
],
),
(
{"limit": -1, "offset": -1},
[
{
"type": "greater_than_equal",
"loc": ["query", "limit"],
"msg": "Input should be greater than or equal to 0",
"input": "-1",
"ctx": {"ge": 0},
},
{
"type": "greater_than_equal",
"loc": ["query", "offset"],
"msg": "Input should be greater than or equal to 0",
"input": "-1",
"ctx": {"ge": 0},
},
],
),
],
)
def test_bad_limit_and_offset(self, test_client, query_params, expected_detail):
response = test_client.get("/public/assets", params=query_params)
assert response.status_code == 422
assert response.json()["detail"] == expected_detail


class TestGetAssetEvents(TestAssets):
def test_should_respond_200(self, test_client, session):
Expand Down

0 comments on commit 49daa6c

Please sign in to comment.